Fix T94415: Nodes: poor selection behavior inside frame nodes

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 pratically 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).
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.

source/blender/editors/space_node/drawnode.cc

@@ -231,7 +231,6 @@ static void node_buts_math(uiLayout *layout, bContext *UNUSED(C), PointerRNA *pt
 NodeResizeDirection node_get_resize_direction(const bNode *node, const int x, const int y)
 {
   if (node->type == NODE_FRAME) {
-    const float size = 10.0f;
     NodeFrame *data = (NodeFrame *)node->storage;
 
     /* shrinking frame size is determined by child nodes */
@@ -242,7 +241,9 @@ NodeResizeDirection node_get_resize_direction(const bNode *node, const int x, co
     NodeResizeDirection dir = NODE_RESIZE_NONE;
 
     const rctf &totr = node->totr;
-    if (x >= totr.xmax - size && x < totr.xmax && y >= totr.ymin && y < totr.ymax) {
+    const float size = NODE_RESIZE_MARGIN;
+
+    if (x > totr.xmax - size && x <= totr.xmax && y >= totr.ymin && y < totr.ymax) {
       dir |= NODE_RESIZE_RIGHT;
     }
     if (x >= totr.xmin && x < totr.xmin + size && y >= totr.ymin && y < totr.ymax) {

source/blender/editors/space_node/node_draw.cc

@@ -2270,6 +2270,13 @@ void node_set_cursor(wmWindow &win, SpaceNode &snode, const float2 &cursor)
   if (node) {
     NodeResizeDirection dir = node_get_resize_direction(node, cursor[0], cursor[1]);
     wmcursor = node_get_resize_cursor(dir);
+    /* We want to indicate that Frame nodes can be moved/selected on their borders. */
+    if (node->type == NODE_FRAME && dir == NODE_RESIZE_NONE) {
+      const rctf frame_inside = node_frame_rect_inside(*node);
+      if (!BLI_rctf_isect_pt(&frame_inside, cursor[0], cursor[1])) {
+        wmcursor = WM_CURSOR_NSEW_SCROLL;
+      }
+    }
   }
 
   WM_cursor_set(&win, wmcursor);

source/blender/editors/space_node/node_intern.hh

@@ -119,6 +119,8 @@ float2 node_link_calculate_multi_input_position(const float2 &socket_position,
                                                 int index,
                                                 int total_inputs);
 
+rctf node_frame_rect_inside(const bNode &node);
+
 int node_get_resize_cursor(NodeResizeDirection directions);
 NodeResizeDirection node_get_resize_direction(const bNode *node, int x, int y);
 /**

source/blender/editors/space_node/node_select.cc

@@ -99,11 +99,51 @@ static bool has_workbench_in_texture_color(const wmWindowManager *wm,
 /** \name Public Node Selection API
  * \{ */
 
+rctf node_frame_rect_inside(const bNode &node)
+{
+  const float margin = 1.5f * U.widget_unit;
+  rctf frame_inside = {
+      node.totr.xmin,
+      node.totr.xmax,
+      node.totr.ymin,
+      node.totr.ymax,
+  };
+
+  BLI_rctf_pad(&frame_inside, -margin, -margin);
+
+  return frame_inside;
+}
+
+static bool node_frame_select_isect_mouse(bNode *node, const float2 &mouse)
+{
+  /* Frame nodes are selectable by their borders (including their whole rect - as for other nodes -
+   * would prevent e.g. box selection of nodes inside that frame). */
+  const rctf frame_inside = node_frame_rect_inside(*node);
+  if (BLI_rctf_isect_pt(&node->totr, mouse.x, mouse.y) &&
+      !BLI_rctf_isect_pt(&frame_inside, mouse.x, mouse.y)) {
+    return true;
+  }
+
+  return false;
+}
+
 static bNode *node_under_mouse_select(bNodeTree &ntree, int mx, int my)
 {
   LISTBASE_FOREACH_BACKWARD (bNode *, node, &ntree.nodes) {
-    if (BLI_rctf_isect_pt(&node->totr, mx, my)) {
-      return node;
+    switch (node->type) {
+      case NODE_FRAME: {
+        const float2 mouse{(float)mx, (float)my};
+        if (node_frame_select_isect_mouse(node, mouse)) {
+          return node;
+        }
+        break;
+      }
+      default: {
+        if (BLI_rctf_isect_pt(&node->totr, mx, my)) {
+          return node;
+        }
+        break;
+      }
     }
   }
   return nullptr;
@@ -114,15 +154,27 @@ static bNode *node_under_mouse_tweak(bNodeTree &ntree, const float2 &mouse)
   using namespace blender::math;
 
   LISTBASE_FOREACH_BACKWARD (bNode *, node, &ntree.nodes) {
-    if (node->type == NODE_REROUTE) {
-      bNodeSocket *socket = (bNodeSocket *)node->inputs.first;
-      const float2 location{socket->locx, socket->locy};
-      if (distance(mouse, location) < 24.0f) {
-        return node;
+    switch (node->type) {
+      case NODE_REROUTE: {
+        bNodeSocket *socket = (bNodeSocket *)node->inputs.first;
+        const float2 location{socket->locx, socket->locy};
+        if (distance(mouse, location) < 24.0f) {
+          return node;
+        }
+        break;
+      }
+      case NODE_FRAME: {
+        if (node_frame_select_isect_mouse(node, mouse)) {
+          return node;
+        }
+        break;
+      }
+      default: {
+        if (BLI_rctf_isect_pt(&node->totr, mouse.x, mouse.y)) {
+          return node;
+        }
+        break;
       }
-    }
-    if (BLI_rctf_isect_pt(&node->totr, mouse.x, mouse.y)) {
-      return node;
     }
   }
   return nullptr;
@@ -687,12 +739,24 @@ static int node_box_select_exec(bContext *C, wmOperator *op)
   }
 
   LISTBASE_FOREACH (bNode *, node, &node_tree.nodes) {
-    bool is_inside;
-    if (node->type == NODE_FRAME) {
-      is_inside = BLI_rctf_inside_rctf(&rectf, &node->totr);
-    }
-    else {
-      is_inside = BLI_rctf_isect(&rectf, &node->totr, nullptr);
+    bool is_inside = false;
+
+    switch (node->type) {
+      case NODE_FRAME: {
+        /* Frame nodes are selectable by their borders (including their whole rect - as for other
+         * nodes - would prevent selection of other nodes inside that frame. */
+        const rctf frame_inside = node_frame_rect_inside(*node);
+        if (BLI_rctf_isect(&rectf, &node->totr, NULL) &&
+            !BLI_rctf_inside_rctf(&frame_inside, &rectf)) {
+          nodeSetSelected(node, select);
+          is_inside = true;
+        }
+        break;
+      }
+      default: {
+        is_inside = BLI_rctf_isect(&rectf, &node->totr, nullptr);
+        break;
+      }
     }
 
     if (is_inside) {
@@ -781,8 +845,25 @@ static int node_circleselect_exec(bContext *C, wmOperator *op)
   UI_view2d_region_to_view(&region->v2d, x, y, &offset[0], &offset[1]);
 
   for (node = (bNode *)snode->edittree->nodes.first; node; node = node->next) {
-    if (BLI_rctf_isect_circle(&node->totr, offset, radius / zoom)) {
-      nodeSetSelected(node, select);
+    switch (node->type) {
+      case NODE_FRAME: {
+        /* Frame nodes are selectable by their borders (including their whole rect - as for other
+         * nodes - would prevent selection of _only_ other nodes inside that frame. */
+        rctf frame_inside = node_frame_rect_inside(*node);
+        const float radius_adjusted = (float)radius / zoom;
+        BLI_rctf_pad(&frame_inside, -2.0f * radius_adjusted, -2.0f * radius_adjusted);
+        if (BLI_rctf_isect_circle(&node->totr, offset, radius_adjusted) &&
+            !BLI_rctf_isect_circle(&frame_inside, offset, radius_adjusted)) {
+          nodeSetSelected(node, select);
+        }
+        break;
+      }
+      default: {
+        if (BLI_rctf_isect_circle(&node->totr, offset, radius / zoom)) {
+          nodeSetSelected(node, select);
+        }
+        break;
+      }
     }
   }
 
@@ -859,16 +940,35 @@ static bool do_lasso_select_node(bContext *C,
       continue;
     }
 
-    int screen_co[2];
-    const float cent[2] = {BLI_rctf_cent_x(&node->totr), BLI_rctf_cent_y(&node->totr)};
+    switch (node->type) {
+      case NODE_FRAME: {
+        /* Frame nodes are selectable by their borders (including their whole rect - as for other
+         * nodes - would prevent selection of other nodes inside that frame. */
+        rctf rectf;
+        BLI_rctf_rcti_copy(&rectf, &rect);
+        UI_view2d_region_to_view_rctf(&region->v2d, &rectf, &rectf);
+        const rctf frame_inside = node_frame_rect_inside(*node);
+        if (BLI_rctf_isect(&rectf, &node->totr, NULL) &&
+            !BLI_rctf_inside_rctf(&frame_inside, &rectf)) {
+          nodeSetSelected(node, select);
+          changed = true;
+        }
+        break;
+      }
+      default: {
+        int screen_co[2];
+        const float cent[2] = {BLI_rctf_cent_x(&node->totr), BLI_rctf_cent_y(&node->totr)};
 
-    /* marker in screen coords */
-    if (UI_view2d_view_to_region_clip(
-            &region->v2d, cent[0], cent[1], &screen_co[0], &screen_co[1]) &&
-        BLI_rcti_isect_pt(&rect, screen_co[0], screen_co[1]) &&
-        BLI_lasso_is_point_inside(mcoords, mcoords_len, screen_co[0], screen_co[1], INT_MAX)) {
-      nodeSetSelected(node, select);
-      changed = true;
+        /* marker in screen coords */
+        if (UI_view2d_view_to_region_clip(
+                &region->v2d, cent[0], cent[1], &screen_co[0], &screen_co[1]) &&
+            BLI_rcti_isect_pt(&rect, screen_co[0], screen_co[1]) &&
+            BLI_lasso_is_point_inside(mcoords, mcoords_len, screen_co[0], screen_co[1], INT_MAX)) {
+          nodeSetSelected(node, select);
+          changed = true;
+        }
+        break;
+      }
     }
   }