Fix T94415: Nodes: poor selection behavior inside frame nodes
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Jan 5 2022, 1:12 PM.

Details

Reviewers

Hans Goudey (HooglyBoogly)
Jacques Lucke (JacquesLucke)
Campbell Barton (campbellbarton)

Maniphest Tasks

T46540: In-frame node selection issue
T94415: It is not possible to box select nodes inside of frame

Commits

rB1995aae6e3bf: Fix T94415: Nodes: poor selection behavior inside frame nodes

Summary

Previously, node selection made no distinction between a frame node and
other nodes. So a frame node would be selected by their whole rect or
center (depending on box/lasso/circle select). As a consequence of this,
box and lasso could not practically be started inside a frame node (with
the intention to select a subset of contained child nodes) because the
frame would be selected immediately and tweak-transforming started.
Circle selecting would always contain the frame node as well (making
transforming a subset of nodes without also transforming the whole frame
impossible).

Now change selection behavior so that for all selection modes only the
border [the margin area that is automatically added around all nodes,
see note below] of a frame node is considered in selection. This makes
for a much more intuitive experience when arranging nodes inside frames.

note: to make the area of interest for selection/moving more obvious,
the cursor changes when hovering over (as is done for resizing).
(this is not shown in the video since this was made from an older version of the patch)
note: this also makes the resize margin consistent with other nodes.
note: this also fixes right resize border (was exclusive instead of
inclusive as every other border)

Also fixes T46540.

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Jan 5 2022, 1:12 PM

Philipp Oeser (lichtwerk) created this revision.

Philipp Oeser (lichtwerk) added a task: T94415: It is not possible to box select nodes inside of frame.

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Jan 5 2022, 1:17 PM

Ludvik Koutny (rawalanche) awarded a token.Jan 5 2022, 1:20 PM

While I like the new behavior overall, I don't like that it becomes very non-obvious how to move a frame. That could be improved by changing the mouse cursor when hovering over the dragging area of a frame (as is done for resizing). Not sure if we do something similar in other places when something can be moved.

source/blender/editors/space_node/node_select.cc
105	Think it's not necessary to set every element separately.
843	I don't think it's necessarily counter-intuitive. The issue is really just that box selection doesn't work.

Charlie Jolly (charlie) awarded a token.Jan 5 2022, 9:31 PM

Pinging @Erindale (Erindale) just to make sure this is actually what people want/expect. Is there anything that could be improved here @Erindale (Erindale)?

In D13737#363828, @Jacques Lucke (JacquesLucke) wrote:

While I like the new behavior overall, I don't like that it becomes very non-obvious how to move a frame. That could be improved by changing the mouse cursor when hovering over the dragging area of a frame (as is done for resizing). Not sure if we do something similar in other places when something can be moved.

Hard to tell (if you already know that it can be moved by click-dragging the border), thought it was pretty obvious, will try the cursor thingie though and see how that feels

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Jan 7 2022, 7:16 PM

Iliya Katueshenock (Moder) added a subscriber: Iliya Katueshenock (Moder).Jan 22 2022, 12:29 AM

Philipp Oeser (lichtwerk) added a task: T46540: In-frame node selection issue.Jan 28 2022, 6:03 PM

make box selection use the same behavior
add transform cursor hovering over the border area

Will add @Campbell Barton (campbellbarton) as reviewer (since he was involved with T46540: In-frame node selection issue)

Juan (jc4d) awarded a token.Jan 29 2022, 8:34 AM

The behavior is so much better already! Resizing the frame is a bit weird now.

source/blender/editors/space_node/node_draw.cc
2271 ↗	(On Diff #47694)	That's only true when the "Shrink" property of the frame node is checked. Currently it is a bit weird, because it shows the move cursor when hovering exactly on the frame border, but starts resizing when dragging.

This revision now requires changes to proceed.Jan 31 2022, 12:29 PM

Leon Schittek (lone_noel) awarded a token.Jan 31 2022, 12:45 PM

fix resize cursor not showing for non "auto-shrinking" frame nodes
make the resize margin consistent with other nodes.
small fix for right resize border (was exclusive instead of inclusive as every other border)

Philipp Oeser (lichtwerk) marked 2 inline comments as done.Feb 1 2022, 4:40 PM

This comment was removed by Philipp Oeser (lichtwerk).

Philipp Oeser (lichtwerk) mentioned this in T95395: Crash making a node group with a frame.Feb 1 2022, 4:40 PM

Jacques Lucke (JacquesLucke) accepted this revision.Feb 1 2022, 4:47 PM

Jacques Lucke (JacquesLucke) added 1 blocking reviewer(s): Hans Goudey (HooglyBoogly).

Lukas Tönne (lukastoenne) awarded a token.Feb 1 2022, 4:58 PM

Other than a couple inline comments, this looks good to me.

source/blender/editors/space_node/node_intern.hh
122 ↗	(On Diff #47765)	New functions can use const and a reference, since the node is expected not to be null: `const bNode &node`
source/blender/editors/space_node/node_select.cc
850–851	I'm not sure this comment is exactly right. The behavior I observe without this patch is that circle select just also selected the frame node in the background, not that it prevents selecting nodes in the foreground.