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 761495 - Don't invalidate the source when connecting two nodes
Don't invalidate the source when connecting two nodes
Status: RESOLVED FIXED
Product: GEGL
Classification: Other
Component: core
git master
Other All
: Normal normal
: ---
Assigned To: Default Gegl Component Owner
Default Gegl Component Owner
Depends on:
Blocks:
 
 
Reported: 2016-02-03 12:38 UTC by Debarshi Ray
Modified: 2016-03-05 17:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GeglNode: Don't invalidate the source when connecting two nodes (948 bytes, patch)
2016-02-03 12:40 UTC, Debarshi Ray
committed Details | Review
tests: Sources shouldn't be invalidated when connecting & disconnecting (1.74 KB, patch)
2016-03-04 18:19 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2016-02-03 12:38:54 UTC
Right now, connecting a source to a sink will invalidate both the source and sink. I can understand the sink being invalidated, but don't see why the source should be affected.

My understanding of the code is that, while the intention was to only invalidate the sink, the source is also getting affected as an unforeseen side-effect. It uses gegl_node_source_invalidated to handle real_source::invalidated, whose job is to invalidate the sink when the source gets invalidated. Then, it calls gegl_node_property_changed to force the source to be invalidated so that it triggers gegl_node_source_invalidated to invalidate the sink.

We can just directly calling gegl_node_source_invalidated, instead of doing this in a roundabout way.

Also, I noticed that the use of gegl_node_property_changed doesn't match the comments in its definition. Supposedly the arg1==NULL code path is meant to handle operation changes. However, gegl_node_set_operation_object doesn't pass NULL. The only time NULL was being passed was during connection (see above), which, according to me, should be removed.
Comment 1 Debarshi Ray 2016-02-03 12:40:30 UTC
Created attachment 320331 [details] [review]
GeglNode: Don't invalidate the source when connecting two nodes
Comment 2 Debarshi Ray 2016-02-03 16:04:22 UTC
From #gegl on GIMPNet:

15:28 <pippin> rishi: going to build gegl and gimp and do some minimal tests    
      with your 761495 change :)
15:30 <rishi> Ok.
15:51 <rishi> pippin: I needed that fix to implement crossfades for point       
      filters.
15:51 <rishi> Working on a rubber band animation for cropping now.
15:52 <pippin> sounds like it might fix the dragging preview curtain in gimp    
      as well :)
15:52 <rishi> I don't know what "dragging preview curtain" is, but sounds       
      good.
15:54 <pippin> the ability to have a split-view, with and without applied       
      filter, and drag it back and forth to reposition it
15:56 <pippin> rishi: looks good to me, please push to master so it gets wider  
      test coverage in gimp for different scenarios :)
Comment 3 Debarshi Ray 2016-02-03 16:05:15 UTC
commit 38ec4ac246039a7b870fd7e121b57d22d274ba97
Author: Debarshi Ray <debarshir@gnome.org>
Date:   Wed Feb 3 13:38:35 2016 +0100

    GeglNode: Don't invalidate the source when connecting two nodes
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761495
Comment 4 Debarshi Ray 2016-03-04 18:19:13 UTC
Created attachment 323120 [details] [review]
tests: Sources shouldn't be invalidated when connecting & disconnecting
Comment 5 Debarshi Ray 2016-03-05 17:25:27 UTC
From #gegl on GIMPNet:
<pippin> rishi: feel free to push the test to master as well :)
Comment 6 Debarshi Ray 2016-03-05 17:25:52 UTC
commit 889d0fd8f8c8bea47c93a1c24cd4964483c98e79
Author: Debarshi Ray <debarshir@gnome.org>
Date:   Fri Mar 4 19:14:20 2016 +0100

    tests: Sources shouldn't be invalidated when connecting & disconnecting
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761495