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 777487 - GeglNode: Don't emit invalidated before actually setting the passthrough
GeglNode: Don't emit invalidated before actually setting the passthrough
Status: RESOLVED FIXED
Product: GEGL
Classification: Other
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: Default Gegl Component Owner
Default Gegl Component Owner
Depends on:
Blocks:
 
 
Reported: 2017-01-19 10:57 UTC by Debarshi Ray
Modified: 2017-01-19 12:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reproducer (2.50 KB, text/x-csrc)
2017-01-19 10:57 UTC, Debarshi Ray
  Details
GeglNode: Emit invalidated only after setting the passthrough (811 bytes, patch)
2017-01-19 11:00 UTC, Debarshi Ray
committed Details | Review
tests: Bounding box shouldn't be cropped after setting passthrough (4.65 KB, patch)
2017-01-19 12:41 UTC, Debarshi Ray
none Details | Review
tests: Bounding box shouldn't be cropped after setting passthrough (4.66 KB, patch)
2017-01-19 12:48 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2017-01-19 10:57:36 UTC
Created attachment 343788 [details]
Reproducer

Emitting invalidated before the passthrough has been set can cause problems if someone touches the graph in the signal handler. eg., if you set passthrough on a gegl:crop, and call gegl_node_get_bounding_box in the invalidated handler, then the bounding box will still be cropped. It will affect subsequent attempts to get the bounding box, which in turn will also affect attempts to process the graph. See the attached reproducer.

In practice it affects the double-buffered rendering in gnome-photos. Instead of directly rendering the main graph, we copy the output into a separate buffer and render from it. If the graph subsequently changes, we wait for the whole thing to be processed before updating the render buffer.

This "waiting" has two parts. We query the bounding box of the graph in the invalidated handler, and when it starts emitting "computed", we track the chunks using Cairo regions (cairo_region_equal and cairo_region_union_rectangle). Due to this bug, we get a cropped bounding box in the invalidated handler, and the subsequent processing is thrown off.
Comment 1 Debarshi Ray 2017-01-19 11:00:53 UTC
Created attachment 343789 [details] [review]
GeglNode: Emit invalidated only after setting the passthrough
Comment 2 Debarshi Ray 2017-01-19 12:41:59 UTC
Created attachment 343795 [details] [review]
tests: Bounding box shouldn't be cropped after setting passthrough
Comment 3 Debarshi Ray 2017-01-19 12:48:12 UTC
Created attachment 343796 [details] [review]
tests: Bounding box shouldn't be cropped after setting passthrough

Minor style fix.
Comment 4 Debarshi Ray 2017-01-19 12:52:31 UTC
From #gegl on GIMPNet:

12:49 <rishi> pippin: https://bugzilla.gnome.org/show_bug.cgi?id=777487
12:49 <Wilber> Title: Bug 777487 GeglNode: Don't emit invalidated before        
      actually setting the passthrough (at bugzilla.gnome.org)
12:51 <pippin> rishi: I've tested the fix with gimp (not expecting it to make   
      a difference there, but nevertheless) looks good to me, please push and   
      close :)
Comment 5 Debarshi Ray 2017-01-19 12:56:50 UTC
Pushed to master.