From ca731e2cdd71e671f6d1b2b1596d545178e2d429 Mon Sep 17 00:00:00 2001 From: Sahan Fernando Date: Sun, 14 Feb 2021 20:04:38 +1100 Subject: [PATCH] SystemMonitor: Define graphs by ColorRole, not by Color Currently, graphs are defined in terms of graph color. This means that when the system palette is changed, the old colors are still used. We switch to storing the color roles and looking up the palette colors on paint events. We also define the graph line background color as the graph color at half-transparency. --- .../SystemMonitor/GraphWidget.cpp | 21 ++++++++++++------- .../Applications/SystemMonitor/GraphWidget.h | 4 ++-- Userland/Applications/SystemMonitor/main.cpp | 12 +++++------ 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Userland/Applications/SystemMonitor/GraphWidget.cpp b/Userland/Applications/SystemMonitor/GraphWidget.cpp index 3452ece2d5..e1ad40afe1 100644 --- a/Userland/Applications/SystemMonitor/GraphWidget.cpp +++ b/Userland/Applications/SystemMonitor/GraphWidget.cpp @@ -25,6 +25,7 @@ */ #include "GraphWidget.h" +#include #include #include #include @@ -47,6 +48,8 @@ void GraphWidget::add_value(Vector&& value) void GraphWidget::paint_event(GUI::PaintEvent& event) { + const auto& system_palette = GUI::Application::the()->palette(); + GUI::Frame::paint_event(event); GUI::Painter painter(*this); painter.add_clip_rect(event.rect()); @@ -60,8 +63,11 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) // Draw one set of values at a time for (size_t k = 0; k < m_value_format.size(); k++) { const auto& format = m_value_format[k]; - if (format.line_color == Color::Transparent && format.background_color == Color::Transparent) + if (format.graph_color_role == ColorRole::Base) { continue; + } + const auto& line_color = system_palette.color(format.graph_color_role); + const auto& background_color = line_color.with_alpha(0x7f); m_calculated_points.clear_with_capacity(); for (size_t i = 0; i < m_values.size(); i++) { int x = inner_rect.right() - (i * 2) + 1; @@ -83,7 +89,7 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) m_calculated_points.append(current_point); } ASSERT(m_calculated_points.size() <= m_values.size()); - if (format.background_color != Color::Transparent) { + if (format.graph_color_role != ColorRole::Base) { // Fill the background for the area we have values for Gfx::Path path; size_t points_in_path = 0; @@ -99,11 +105,11 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) path.line_to({ current_point->x() - 1, inner_rect.bottom() + 1 }); path.line_to({ first_point->x() + 1, inner_rect.bottom() + 1 }); path.close(); - painter.fill_path(path, format.background_color, Gfx::Painter::WindingRule::EvenOdd); + painter.fill_path(path, background_color, Gfx::Painter::WindingRule::EvenOdd); } else if (points_in_path == 1 && current_point) { // Can't fill any area, we only have one data point. // Just draw a vertical line as a "fill"... - painter.draw_line(*current_point, { current_point->x(), inner_rect.bottom() }, format.background_color); + painter.draw_line(*current_point, { current_point->x(), inner_rect.bottom() }, background_color); } path = {}; points_in_path = 0; @@ -128,7 +134,7 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) } check_fill_area(); } - if (format.line_color != Color::Transparent) { + if (format.graph_color_role != ColorRole::Base) { // Draw the line for the data points we have const Gfx::IntPoint* previous_point = nullptr; for (size_t i = 0; i < m_calculated_points.size(); i++) { @@ -138,7 +144,7 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) continue; } if (previous_point) - painter.draw_line(*previous_point, current_point, format.line_color); + painter.draw_line(*previous_point, current_point, line_color); previous_point = ¤t_point; } } @@ -150,6 +156,7 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) int y = 0; for (size_t i = 0; i < min(m_value_format.size(), current_values.size()); i++) { const auto& format = m_value_format[i]; + const auto& graph_color = system_palette.color(format.graph_color_role); if (!format.text_formatter) continue; auto constrain_rect = inner_rect.shrunken(8, 8); @@ -158,7 +165,7 @@ void GraphWidget::paint_event(GUI::PaintEvent& event) auto text = format.text_formatter(current_values[i]); if (format.text_shadow_color != Color::Transparent) painter.draw_text(text_rect.translated(1, 1), text.characters(), Gfx::TextAlignment::CenterRight, format.text_shadow_color); - painter.draw_text(text_rect, text.characters(), Gfx::TextAlignment::CenterRight, format.line_color); + painter.draw_text(text_rect, text.characters(), Gfx::TextAlignment::CenterRight, graph_color); y += text_rect.height() + 4; } } diff --git a/Userland/Applications/SystemMonitor/GraphWidget.h b/Userland/Applications/SystemMonitor/GraphWidget.h index 67fd453efe..bb0e4fd692 100644 --- a/Userland/Applications/SystemMonitor/GraphWidget.h +++ b/Userland/Applications/SystemMonitor/GraphWidget.h @@ -28,6 +28,7 @@ #include #include +#include class GraphWidget final : public GUI::Frame { C_OBJECT(GraphWidget) @@ -40,8 +41,7 @@ public: void add_value(Vector&&); struct ValueFormat { - Color line_color { Color::Transparent }; - Color background_color { Color::Transparent }; + Gfx::ColorRole graph_color_role { Gfx::ColorRole::Base }; Color text_shadow_color { Color::Transparent }; Function text_formatter; }; diff --git a/Userland/Applications/SystemMonitor/main.cpp b/Userland/Applications/SystemMonitor/main.cpp index 50e5fd9363..93e10b4476 100644 --- a/Userland/Applications/SystemMonitor/main.cpp +++ b/Userland/Applications/SystemMonitor/main.cpp @@ -573,8 +573,6 @@ NonnullRefPtr build_graphs_tab() auto graphs_container = GUI::LazyWidget::construct(); graphs_container->on_first_show = [](GUI::LazyWidget& self) { - const auto system_palette = GUI::Application::the()->palette(); - self.set_fill_with_background_color(true); self.set_background_role(ColorRole::Button); self.set_layout(); @@ -589,13 +587,13 @@ NonnullRefPtr build_graphs_tab() auto& cpu_graph = cpu_graph_group_box.add(); cpu_graph.set_max(100); cpu_graph.set_value_format(0, { - .line_color = system_palette.syntax_preprocessor_statement(), + .graph_color_role = ColorRole::SyntaxPreprocessorStatement, .text_formatter = [](int value) { return String::formatted("Total: {}%", value); }, }); cpu_graph.set_value_format(1, { - .line_color = system_palette.syntax_preprocessor_value(), + .graph_color_role = ColorRole::SyntaxPreprocessorValue, .text_formatter = [](int value) { return String::formatted("Kernel: {}%", value); }, @@ -614,19 +612,19 @@ NonnullRefPtr build_graphs_tab() auto& memory_graph = memory_graph_group_box.add(); memory_graph.set_stack_values(true); memory_graph.set_value_format(0, { - .line_color = system_palette.syntax_comment(), + .graph_color_role = ColorRole::SyntaxComment, .text_formatter = [&memory_graph](int value) { return String::formatted("Committed: {} KiB", value); }, }); memory_graph.set_value_format(1, { - .line_color = system_palette.syntax_preprocessor_statement(), + .graph_color_role = ColorRole::SyntaxPreprocessorStatement, .text_formatter = [&memory_graph](int value) { return String::formatted("Allocated: {} KiB", value); }, }); memory_graph.set_value_format(2, { - .line_color = system_palette.syntax_preprocessor_value(), + .graph_color_role = ColorRole::SyntaxPreprocessorValue, .text_formatter = [&memory_graph](int value) { return String::formatted("Kernel heap: {} KiB", value); },