Paths

Table of Contentst

Differential D10689

Nodes: Fix drag link from output to already linked Multi-Input Socket
ClosedPublic
Actions

Authored by Fabian Schempp (fabian_schempp) on Mar 10 2021, 10:09 PM.

Tags

None

Subscribers

Hans Goudey (HooglyBoogly)

Details

Reviewers

Jacques Lucke (JacquesLucke)
Hans Goudey (HooglyBoogly)

Commits

rBafa30f1a9d21: Nodes: Fix drag link from output to already linked Multi-Input Socket

Summary

before fix:

after fix:

Diff Detail

Repository: rB Blender

Event Timeline

Fabian Schempp (fabian_schempp) requested review of this revision.Mar 10 2021, 10:09 PM

Fabian Schempp (fabian_schempp) created this revision.

Harbormaster completed remote builds in B13419: Diff 35046.Mar 10 2021, 10:09 PM

Fabian Schempp (fabian_schempp) edited the summary of this revision. (Show Details)Mar 10 2021, 10:11 PM

Fabian Schempp (fabian_schempp) added a reviewer: Jacques Lucke (JacquesLucke).

Jacques Lucke (JacquesLucke) added inline comments.Mar 10 2021, 10:18 PM

source/blender/editors/space_node/node_draw.cc
1763	Can you just use the `contains` method?

Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).Mar 10 2021, 10:20 PM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/editors/space_node/node_draw.cc
1751	Why not use a set? Then you just add the relevant links and use the resulting size.

Changes based on review by Hans Goudey and Jacques Lucke.

Harbormaster completed remote builds in B13420: Diff 35047.Mar 10 2021, 10:36 PM

Fabian Schempp (fabian_schempp) marked an inline comment as done.Mar 10 2021, 10:36 PM

I'd go a bit further here. I suggest:

Removing the incrementation of socket->total_inputs
Using add instead of contains when counting temporary links
Set total_inputs to visited_from_sockets.size() at the end of each loop.

Fixed unneeded whitespace change.

Harbormaster completed remote builds in B13421: Diff 35048.Mar 10 2021, 10:40 PM

Change based on Review by Hans Goudey.

Harbormaster completed remote builds in B13422: Diff 35049.Mar 10 2021, 10:44 PM

In D10689#270582, @Hans Goudey (HooglyBoogly) wrote:

I'd go a bit further here. I suggest:

Removing the incrementation of socket->total_inputs

Using add instead of contains when counting temporary links

Set total_inputs to visited_from_sockets.size() at the end of each loop.

beautiful.

Jacques Lucke (JacquesLucke) accepted this revision.Mar 11 2021, 11:38 AM

Jacques Lucke (JacquesLucke) added inline comments.

source/blender/editors/space_node/node_relationships.c
897	I guess you can add a `break` here?

This revision is now accepted and ready to land.Mar 11 2021, 11:38 AM

Jacques Lucke (JacquesLucke) mentioned this in T86412: Crash with relinking join node.Mar 11 2021, 1:59 PM

Also, don't forget to add some explanation / background in the commit message.

Hans Goudey (HooglyBoogly) accepted this revision.Mar 11 2021, 2:06 PM

Fabian Schempp (fabian_schempp) marked an inline comment as done.Mar 11 2021, 6:08 PM

Closed by commit rBafa30f1a9d21: Nodes: Fix drag link from output to already linked Multi-Input Socket (authored by Fabian Schempp (fabian_schempp)). · Explain WhyMar 11 2021, 6:53 PM

This revision was automatically updated to reflect the committed changes.

Fabian Schempp (fabian_schempp) added a commit: rBafa30f1a9d21: Nodes: Fix drag link from output to already linked Multi-Input Socket.

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

space_node/

8 lines

node_relationships.c

14 lines

Diff 35070

source/blender/editors/space_node/node_draw.cc

Loading...

source/blender/editors/space_node/node_relationships.c

Loading...