After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720560 - The background should be drawn explicitly
The background should be drawn explicitly
Status: RESOLVED FIXED
Product: system-monitor
Classification: Core
Component: resources
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: System-monitor maintainers
System-monitor maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-16 21:16 UTC by Soren Sandmann Pedersen
Modified: 2013-12-20 23:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The patch (1.98 KB, patch)
2013-12-16 21:16 UTC, Soren Sandmann Pedersen
committed Details | Review
New patch (2.00 KB, patch)
2013-12-20 20:51 UTC, Soren Sandmann Pedersen
none Details | Review
Patch (2.00 KB, patch)
2013-12-20 20:55 UTC, Soren Sandmann Pedersen
none Details | Review
Another version (4.14 KB, patch)
2013-12-20 20:57 UTC, Soren Sandmann Pedersen
committed Details | Review

Description Soren Sandmann Pedersen 2013-12-16 21:16:50 UTC
Created attachment 264329 [details] [review]
The patch

Details in the commit log of the upcoming patch.

The gdk_window_set_background_pattern() should basically not be used outside of core GTK+.
Comment 1 Robert Roth 2013-12-19 22:34:44 UTC
Review of attachment 264329 [details] [review]:

Thanks for the patch, looks nice. My only objection is that in your version the pattern is created in each iteration, which might be expensive, and unnecessary, as far as I see. We could add the pattern as a member of loadgraph, and we wouldn't have to create it in each iteration, paint it, and destroy it, but create it only after the graph->background has changed (inside the if) and destroy the pattern with the loadgraph or on graph change. What do you think?
Comment 2 Soren Sandmann Pedersen 2013-12-20 20:49:38 UTC
(In reply to comment #1)
> Review of attachment 264329 [details] [review]:
> 
> Thanks for the patch, looks nice. My only objection is that in your version the
> pattern is created in each iteration, which might be expensive, and
> unnecessary, as far as I see. We could add the pattern as a member of
> loadgraph, and we wouldn't have to create it in each iteration, paint it, and
> destroy it, but create it only after the graph->background has changed (inside
> the if) and destroy the pattern with the loadgraph or on graph change. What do
> you think?

Well, creating a pattern is cheap in cairo. It happens internally in cairo every time cairo_set_source_rgba() is called after all.

However, caching the pattern should be harmless I think, so I'm fine with this. New patch coming up.

(An alternative would be to use cairo_set_source_surface() instead of creating the pattern explicitly, though this causes cairo to create a pattern internally).
Comment 3 Soren Sandmann Pedersen 2013-12-20 20:51:09 UTC
Created attachment 264646 [details] [review]
New patch
Comment 4 Soren Sandmann Pedersen 2013-12-20 20:55:11 UTC
Created attachment 264647 [details] [review]
Patch

New version that includes the changes to load-graph.h
Comment 5 Soren Sandmann Pedersen 2013-12-20 20:57:00 UTC
Created attachment 264648 [details] [review]
Another version

Apparently I had two identically named files with different contents
Comment 6 Robert Roth 2013-12-20 23:52:20 UTC
Review of attachment 264648 [details] [review]:

Thanks for the fix, looks ok, applied to trunk, committed.
Comment 7 Robert Roth 2013-12-20 23:54:59 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.