Page MenuHome

Nodes: A More Predictable/Responsive Method for Detecting Active Link During Translate
AbandonedPublic

Authored by Julian Eisel (Severin) on Jul 6 2015, 7:48 PM.

Details

Summary

Nodes: A More Predictable/Responsive Method for Detecting Active Link During Translate

The Old Method

The "old" method used the node dimensions to get a number of lines and checked if they intersect with the node link. Issue with this is that only a small part of the actual node surface is checked, making the method a bit unpredictable or unresponsive.

The New Method

The new method checks for intersections within the entire node surface. If multiple links are intersected, the one with the most intersections is chosen.

Demos

Old behavior:


New behavior:

But better try it by yourself of course ;)

Gimme Feedback!

Would be good to get some feedback, there are many other possible methods for this, but I find this one was working the best from the ones I tried (also sounds like the way users would expect it to work I think).
In case this needs more design discussion, please ask me to open a design task so we can keep things organized.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D1393

Event Timeline

Julian Eisel (Severin) retitled this revision from to Nodes: A More Predictable/Responsive Method for Detecting Active Link During Translate.Jul 6 2015, 7:48 PM
Julian Eisel (Severin) updated this object.
Julian Eisel (Severin) updated this revision to Diff 4600.

This looks like a really nice improvement @Julian Eisel (Severin). I know I struggled a lot to always get it to splice how I wanted.

I'm not so sure about choosing the noodle with the most intersections, though. This seems like a very strange criteria to me. My first thought was that it should resolve conflicts by the noodle nearest the mouse cursor.

@Jonathan Williamson (carter2422), Using cursor position wouldn't work, keep in mind that the cursor can be at a completely different point than the node.
Simple scenario where this wouldn't work: Imagine two links, one higher than the other. If the cursor is placed a bit higher than the moved node, the upper link line will always be active. If you wanted to activate the lower link line, you had to move the mouse cursor close to the lower line but this again would mean the node is moved downwards, probably so far that it doesn't intersect with the link line anymore. Result would be that you couldn't select the lower link line at all.
Only using one axis for this wouldn't work either afaics.

So to avoid such scenarios, the method needs to be based on the node position.

Using the link with the most intersections just was the method that worked best for me, I tried many others (distance from node center, distance from node header, old method, first/last created link, ...).

I'm impressed. This works SO much better than the old behavior!

Campbell Barton (campbellbarton) requested changes to this revision.

Disagree with the logic here, though its obviously making a real improvement too.

Problem is distribution of points will influence, the result - so if a node is far away - its possible no tessellated points intersect (or more likely a strange bias is given to one link).

Correct logic would be to do closest-point-on-line test with every segment that intersects (BLI_rctf_isect_segment), then just use the closest (dist_squared_to_line_segment_v2), it shouldn't be much extra work.

To be really correct you would first clip the segment by the rect before finding the closest point on the segment, but this may be overkill.

This revision now requires changes to proceed.Jul 8 2015, 10:05 AM
Julian Eisel (Severin) edited edge metadata.EditedJul 23 2015, 7:32 PM
Julian Eisel (Severin) updated this revision to Diff 4739.
NOTE: Position of node center is now decisive for determining link to highlight. This might take a moment to get used to, but think it's worth it ;)

(For the records, was aware that distribution of points influenced result, just thought doing more fancy things would be overkill, but indeed, it's even better like this)

Tried the latest changes and it still is amazing. Probably even more. It is no problem to insert new nodes into even the most narrow, crowded, crammed spaghetti nodes. :)

Julian Eisel (Severin) updated this revision to Diff 4793.
  • Use upper left node edge to compare distances, seems more intuitive than node center
  • Update comments for latest changes
  • Minor cleanup: Add comment to explain missing clipping, naming

The only open question I see here is which point we should use for measuring distance.
E.g. upper-left corner (as currently in this patch), upper-middle-of-header, node center (seems logical, but node link might be completely overlapped by node, making it hard/impossible to see)

This is something that should be tested by and discussed with users, so feedback/testing welcome!

The only open question I see here is which point we should use for measuring distance.
E.g. upper-left corner (as currently in this patch), upper-middle-of-header, node center (seems logical, but node link might be completely overlapped by node, making it hard/impossible to see)
This is something that should be tested by and discussed with users, so feedback/testing welcome!

I find the upper left corner to work precise, reliable and indeed better than the center in cases of narrow and crowded node arrangements. I was going to ask why not to use the right corner, but because nodes collapse from right to left when hidden ("H") I suppose upper left is the best solution.
+1 from me!
:)

Upper left corner seems very intuitive for me. For some reason I automatically try to 'cram in nodes' with their upper left corner, collapsed or open. So also +1 from me!

+1 top left. It's definitely much more intuitive and allows sneaking nodes in the smallest places, this combined with auto-offset makes node editing a dream!

Thank you so much for looking into these usability issues, they're gold.

Lapineige added a comment.EditedAug 4 2015, 3:02 PM

For me too it's seems a lot more natural to detect it on 1 side (and not on the center of this side).
The upper left corner is a good choice, for the same reasons as @Sebastian Koenig (sebastian_k) and @Jasper van Nieuwenhuizen (jasperge).
Another +1 :)
And thank you for you work :)

This revision is now accepted and ready to land.Aug 4 2015, 3:04 PM
Campbell Barton (campbellbarton) requested changes to this revision.
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/space_node/node_relationships.c
1341

dist_this, is a bit of an odd name, we call dist_best in many other places.

1349

also a bit of an odd name cmp_ is sometimes used for comparison callbacks for eg.
maybe just node_location, node_xy.

Also, normally spaces in {} arent added unless it helps make code more readable.

This revision now requires changes to proceed.Aug 4 2015, 3:08 PM
source/blender/editors/space_node/node_relationships.c
1341

Correction to own comment, normally have dist and dist_best to track the smallest distance. see: ./source/blender/editors/mask/mask_add.c:75 for eg

Julian Eisel (Severin) updated this revision to Diff 4812.

Committed rB4041e654cd145cc0, thanks all for the feedback! :)