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 762987 - Short circuit gegl_node_connect_from if the nodes are already connected
Short circuit gegl_node_connect_from if the nodes are already connected
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-03-02 13:32 UTC by Debarshi Ray
Modified: 2016-03-05 17:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GeglNode: Clean up (2.02 KB, patch)
2016-03-02 14:01 UTC, Debarshi Ray
committed Details | Review
Short circuit gegl_node_connect_from if the nodes are already connected (1.25 KB, patch)
2016-03-02 14:01 UTC, Debarshi Ray
none Details | Review
Short circuit gegl_node_connect_from if the nodes are already connected (1.25 KB, patch)
2016-03-02 14:16 UTC, Debarshi Ray
none Details | Review
Short circuit gegl_node_connect_from if the nodes are already connected (1.25 KB, patch)
2016-03-04 08:56 UTC, Debarshi Ray
committed Details | Review
tests: Re-connecting connected nodes should not cause invalidation (1.85 KB, patch)
2016-03-04 18:18 UTC, Debarshi Ray
committed Details | Review

Description Debarshi Ray 2016-03-02 13:32:59 UTC
Reading the gegl_node_connect_from implementation, I see that we needlessly disconnect and invalidate the nodes even if they were already connected as desired. Would be nice if we were smart about this instead of expecting applications to be more careful.
Comment 1 Debarshi Ray 2016-03-02 14:01:09 UTC
Created attachment 322860 [details] [review]
GeglNode: Clean up
Comment 2 Debarshi Ray 2016-03-02 14:01:33 UTC
Created attachment 322861 [details] [review]
Short circuit gegl_node_connect_from if the nodes are already connected
Comment 3 Debarshi Ray 2016-03-02 14:16:55 UTC
Created attachment 322865 [details] [review]
Short circuit gegl_node_connect_from if the nodes are already connected
Comment 4 Debarshi Ray 2016-03-02 14:17:18 UTC
I have also pushed the patches to gegl:wip/rishi/node-connect
Comment 5 Debarshi Ray 2016-03-04 08:56:54 UTC
Created attachment 323071 [details] [review]
Short circuit gegl_node_connect_from if the nodes are already connected

Compare other_pad with source_pad, not sink_pad.
Comment 6 Øyvind Kolås (pippin) 2016-03-04 17:02:53 UTC
Definetely desirable - and doesn't seem to cause any regression - pushed to master.
Comment 7 Debarshi Ray 2016-03-04 18:18:18 UTC
Created attachment 323119 [details] [review]
tests: Re-connecting connected nodes should not cause invalidation
Comment 8 Debarshi Ray 2016-03-04 18:19:59 UTC
commit bb8fe84f2801cf20f91a0c4c849022c9f4125808
Author: Debarshi Ray <debarshir@gnome.org>
Date:   Wed Mar 2 14:25:12 2016 +0100

    Short circuit gegl_node_connect_from if the nodes are already connected
    
    This avoids a needless invalidation when the parameters are already
    connected as desired.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=762987

commit 6c3efef409e09325cfb4d21c5f50e82582c95bc0
Author: Debarshi Ray <debarshir@gnome.org>
Date:   Wed Mar 2 14:22:16 2016 +0100

    GeglNode: Clean up
    
    Since gegl_node_pads_exist already logs a WARNING when the pads can't
    be found, there is no need to log another one in the caller. This will
    also help us short gegl_node_connect_from when its parameters are
    already connected, and avoid a needless invalidation.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=762987
Comment 9 Debarshi Ray 2016-03-05 17:25:30 UTC
From #gegl on GIMPNet:
<pippin> rishi: feel free to push the test to master as well :)
Comment 10 Debarshi Ray 2016-03-05 17:26:04 UTC
commit 77a34a29e8e6db876cbc9b1d169ba2ceeefaf43c
Author: Debarshi Ray <debarshir@gnome.org>
Date:   Fri Mar 4 18:55:20 2016 +0100

    tests: Re-connecting connected nodes should not cause invalidation
    
    https://bugzilla.gnome.org/show_bug.cgi?id=762987