GNOME Bugzilla – Bug 792360
Too bright fixed the background color on charts.
Last modified: 2018-02-03 12:38:45 UTC
Created attachment 366537 [details] [review] Patch that fixes background color of graphs. if the user is using the dark theme, the background of the charts on the "resources" tab, look too bright. And it can not be fixed by settings or the CSS of the theme. I propose to give the user the ability to fix this trouble. For this I attach the patch.
Created attachment 366538 [details] So, as of now
Created attachment 366539 [details] Themed background, with patch
Comment on attachment 366537 [details] [review] Patch that fixes background color of graphs. >diff --git a/src/load-graph.cpp b/src/load-graph.cpp >index 649fa11..9bd0e97 100644 >--- a/a/src/load-graph.cpp >+++ b/b/src/load-graph.cpp >@@ -64,6 +64,7 @@ void draw_background(LoadGraph *graph) { > PangoRectangle extents; > cairo_surface_t *surface; > GdkRGBA fg; >+ GdkRGBA fg_grid; > > num_bars = graph->num_bars(); > graph->graph_dely = (graph->draw_height - 15) / num_bars; /* round to int to avoid AA blur */ >@@ -93,13 +94,37 @@ void draw_background(LoadGraph *graph) { > cairo_translate (cr, FRAME_WIDTH, FRAME_WIDTH); > > /* Draw background rectangle */ >- cairo_set_source_rgb (cr, 1.0, 1.0, 1.0); >- cairo_rectangle (cr, graph->indent, 0, >- graph->draw_width - graph->rmargin - graph->indent, graph->real_draw_height); >- cairo_fill(cr); >- >+ /* When a user uses a dark theme, the hard-coded >+ * white background in GSM is a lone white on the >+ * display, which makes the user unhappy. To fix >+ * this, here we offer the user a chance to set >+ * his favorite background color. */ >+ gtk_style_context_save (context); >+ >+ /* Here we specify the name of the class. Now in >+ * the theme's CSS we can specify the own colors >+ * for this class. */ >+ gtk_style_context_add_class (context, "loadgraph"); >+ >+ /* And in case the user does not care, we add >+ * classes that usually have a white background. */ >+ gtk_style_context_add_class (context, GTK_STYLE_CLASS_PAPER); >+ gtk_style_context_add_class (context, GTK_STYLE_CLASS_ENTRY); >+ >+ /* And, as a bonus, the user can choose the color of the grid. */ >+ gtk_style_context_get_color (context, GTK_STATE_FLAG_NORMAL, &fg_grid); >+ >+ /* Why not use the new features of the >+ * GTK instead of cairo_rectangle ?! */ >+ gtk_render_background (context, cr, graph->indent, 0.0, >+ graph->draw_width - graph->rmargin - graph->indent, >+ graph->real_draw_height); >+ >+ gtk_style_context_restore (context); >+ >+ /* Draw the grid a bit less contrast. */ >+ fg_grid.alpha = 0.6; > cairo_set_line_width (cr, 1.0); >- cairo_set_source_rgb (cr, 0.89, 0.89, 0.89); > > for (i = 0; i <= num_bars; ++i) { > double y; >@@ -129,10 +154,7 @@ void draw_background(LoadGraph *graph) { > pango_cairo_show_layout (cr, layout); > g_free(caption); > >- if (i==0 || i==num_bars) >- cairo_set_source_rgb (cr, 0.70, 0.71, 0.70); >- else >- cairo_set_source_rgb (cr, 0.89, 0.89, 0.89); >+ gdk_cairo_set_source_rgba (cr, &fg_grid); > cairo_move_to (cr, graph->indent, i * graph->graph_dely + 0.5); > cairo_line_to (cr, graph->draw_width - graph->rmargin + 0.5 + 4, i * graph->graph_dely + 0.5); > cairo_stroke (cr); >@@ -143,11 +165,8 @@ void draw_background(LoadGraph *graph) { > > for (unsigned int i = 0; i < 7; i++) { > double x = (i) * (graph->draw_width - graph->rmargin - graph->indent) / 6; >- if (i==0 || i==6) >- cairo_set_source_rgb (cr, 0.70, 0.71, 0.70); >- else >- cairo_set_source_rgb (cr, 0.89, 0.89, 0.89); > >+ gdk_cairo_set_source_rgba (cr, &fg_grid); > cairo_move_to (cr, (ceil(x) + 0.5) + graph->indent, 0.5); > cairo_line_to (cr, (ceil(x) + 0.5) + graph->indent, graph->real_draw_height + 4.5); > cairo_stroke(cr);
Review of attachment 366537 [details] [review]: Looks great, tested it, and it works mostly fine, thanks for the contribution. I have a few minor nitpicks though, it would be great if you could give some feedback/update the patch based on my comments. ::: a/src/load-graph.cpp @@ +99,3 @@ + * display, which makes the user unhappy. To fix + * this, here we offer the user a chance to set + * his favorite background color. */ Agreed, I also saw this while using dark themes. @@ +104,3 @@ + /* Here we specify the name of the class. Now in + * the theme's CSS we can specify the own colors + * for this class. */ Great, if a theme wants to override it, it can. @@ +116,3 @@ + + /* Why not use the new features of the + * GTK instead of cairo_rectangle ?! */ Well, legacy code :) @@ +123,3 @@ + gtk_style_context_restore (context); + + /* Draw the grid a bit less contrast. */ A bit less contrast then what? This seems to me a bit too much contrast (too dark on Adwaita, Too bright on Adwaita dark) on the inner lines of the loadgraph, and visually it is more pleasing to have the outer grid lines with more contrast, and the inner lines with less contrast, so we shouldn't use the same value for all the grid lines. @@ -130,3 @@ g_free(caption); - if (i==0 || i==num_bars) See above the details about why this is required. I did experiment a bit, and with your changes we can simply change the alpha of the fg_grid in the if/else, and do a gdk_cairo_set_source_rgba. I have tested and something like 0.5/0.2 (probably a ratio of somewhere around 50% - having the inner lines with half the alpha of the outer lines) looks fairly ok, maybe even 0.6/0.3 would work. @@ -144,3 @@ for (unsigned int i = 0; i < 7; i++) { double x = (i) * (graph->draw_width - graph->rmargin - graph->indent) / 6; - if (i==0 || i==6) The same thing here as for the other bars, the same alpha changes as above.
Robert Roth, I am glad that you are interested in my proposal. I took note of your comments and experimented a little with the alpha values. And I must agree with you that different alpha values look better. I guess I was too hasty to see this. :) I agree that the 50% ratio is optimal, and so far I have chosen 0.7 for the border and 0.7/2 for the grid. Perhaps the value of 0.7 should be discussed. And a short example of what style we can apply now using gtk-widgets.css. .loadgraph { background: linear-gradient(to bottom, @theme_base_color, @theme_bg_color, @theme_base_color ); color: #fff; }
Created attachment 366699 [details] [review] Patch that fixes background color of graphs.
Comment on attachment 366699 [details] [review] Patch that fixes background color of graphs. For now pushing your changes without the CSS requirements, as even without that it is a lot better then the current state of master (with dark themes) obsolete #366537 - Patch that fixes background color of graphs. - reviewed
Added a subtle gradient to the background (via built-in css) and the borders, using theme colors, checked with several themes, both dark and light themes, and overall, this is the best I could come up with. For more tweaking (if required), we'll use a different bug, marking this as fixed. This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.