Fix T87356 & T87358: Small multi-input socket issues
ClosedPublic
Actions

Authored by Fabian Schempp (fabian_schempp) on Apr 11 2021, 2:38 AM.

Details

Reviewers

Maniphest Tasks

T87356: Bad offset for node link in multi-input socket after disconnecting
T87358: Assert for boolean node multi-input socket

Commits

rB2c3a9caffee6: Fix T87356 & T87358: Small multi-input socket issues

Summary

T87356 occured because last_node_hovered_while_dragging_a_link was not set on node_link_init.
The assert in T87358 failed because the sorting operation was called even if the drag link contained a link to another socket.
A little one frame jump was caused because frame was refreshed after picking a link and before sorting happened.

Diff Detail

Repository: rB Blender

Event Timeline

Fabian Schempp (fabian_schempp) requested review of this revision.Apr 11 2021, 2:38 AM

Fabian Schempp (fabian_schempp) created this revision.

Harbormaster completed remote builds in B13986: Diff 36049.Apr 11 2021, 2:38 AM

Fabian Schempp (fabian_schempp) added tasks: T87358: Assert for boolean node multi-input socket, T87356: Bad offset for node link in multi-input socket after disconnecting.Apr 11 2021, 2:40 AM

Fabian Schempp (fabian_schempp) retitled this revision from Fixes T87356 & T87356 to Fixes T87356 & T87358.Apr 11 2021, 2:43 AM

Fabian Schempp (fabian_schempp) edited the summary of this revision. (Show Details)

Fabian Schempp (fabian_schempp) added projects: Nodes & Physics, Geometry Nodes.

Fabian Schempp (fabian_schempp) added a reviewer: Hans Goudey (HooglyBoogly).

Fabian Schempp (fabian_schempp) mentioned this in D10934: Fix T87356: Bad offset for node link in multi-input socket.Apr 11 2021, 2:46 AM

Fixed one frame jump mentioned in comment by Hans Goudey.

Harbormaster completed remote builds in B13999: Diff 36065.Apr 11 2021, 8:33 PM

reverted unneeded change.

Harbormaster completed remote builds in B14000: Diff 36066.Apr 11 2021, 8:37 PM

Fabian Schempp (fabian_schempp) edited the summary of this revision. (Show Details)Apr 11 2021, 8:38 PM

Looks good, and it fixes the issues for me. Thanks for addressing it!

Side note-- last_node_hovered_while_dragging_a_link is the kind of variable where it's really nice to have its general purpose documented briefly in a comment.
Otherwise in 4 years someone will be wondering "what does this even do?", "is this still necessary?"
If you felt inclined, it might be nice to add a comment there in a separate commit.

source/blender/editors/space_node/node_relationships.c
228	Is this going to run needlessly in the non-multi-input-socket case? I'm not sure. Obviously it's not a big problem if it did, but if the check to avoid that was simple I don't see why we wouldn't include it. Only if it's simple though.

This revision is now accepted and ready to land.Apr 14 2021, 6:38 AM

Hans Goudey (HooglyBoogly) retitled this revision from Fixes T87356 & T87358 to Fix T87356 & T87358: Small multi-input socket issues.Apr 14 2021, 6:38 AM

Closed by commit rB2c3a9caffee6: Fix T87356 & T87358: Small multi-input socket issues (authored by Fabian Schempp (fabian_schempp)). · Explain WhyApr 15 2021, 10:00 AM

This revision was automatically updated to reflect the committed changes.

Fabian Schempp (fabian_schempp) marked an inline comment as done.

Fabian Schempp (fabian_schempp) added a commit: rB2c3a9caffee6: Fix T87356 & T87358: Small multi-input socket issues.