[Bf-blender-cvs] [4c3e91e5f56] blender-v3.2-release: Fix T96520 Node editor: Tweak fails with unselected nodes

Campbell Barton noreply at git.blender.org
Tue May 10 15:02:41 CEST 2022


Commit: 4c3e91e5f565b81dd79b5d42f55be5b93662d410
Author: Campbell Barton
Date:   Tue May 10 22:57:00 2022 +1000
Branches: blender-v3.2-release
https://developer.blender.org/rB4c3e91e5f565b81dd79b5d42f55be5b93662d410

Fix T96520 Node editor: Tweak fails with unselected nodes

Use the same method for node selection and dragging that is used
in the 3D viewport and UV editor. Instead of relying on a modal
operator - use the keymap to handle click/drag events.

Details:

Failure to transform unselected nodes was caused by [0] & [1] however
detecting drag relied on specific behavior which I don't think we should
be depending on.

This error happened when selection was defined both in the key-map for
the tool and for the node-editor.

- The left mouse button would activate selection in both the tool
  and "Node Editor" keymap.

- The first selection would return `FINISHED | PASS_THROUGH` when
  selecting a previously unselected node.

- The same PRESS would trigger a second selection would return
  `RUNNING_MODAL | PASS_THROUGH`,
  (starting a NODE_OT_select as a modal operator).

  - In 3.1 (with tweak events) the modal operator would then exit and
    fall-back to the tweak event which would transform the selected
    nodes.

  - In 3.2 (as of [0]) the PRESS that starts the modal operator is
    considered "handled" and prevents drag event from being detected.

The correct behavior in this case isn't obvious:
If a modal operator starts on pressing a button, using that same the
release to generate drag/click events is disputable.

Even in the case or 3.1 it was inconsistent as tweak events were
generated but click events weren't.

Note: after investigating this bug it turns out a similar issue already
existed in 2.91 and all releases afterwards. While the bug is more
obscure, it's also caused by the tweak event being interrupted as
described here, this commit resolves T81824 as well.

[0]: 4d0f846b936c9101ecb76a6db962aac2d74a460a
[1]: 4986f718482b061082936f1f6aa13929741093a2

Reviewed By: Severin

Ref D14499

===================================================================

M	release/scripts/presets/keyconfig/keymap_data/blender_default.py
M	source/blender/editors/space_node/node_select.cc

===================================================================

diff --git a/release/scripts/presets/keyconfig/keymap_data/blender_default.py b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
index 78620c41d1e..64e0917da65 100644
--- a/release/scripts/presets/keyconfig/keymap_data/blender_default.py
+++ b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
@@ -2019,37 +2019,20 @@ def km_node_editor(params):
         {"items": items},
     )
 
-    def node_select_ops(select_mouse):
-        return [
-            ("node.select", {"type": select_mouse, "value": 'PRESS'},
-             {"properties": [("deselect_all", True)]}),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "ctrl": True}, None),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "alt": True}, None),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "ctrl": True, "alt": True}, None),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "shift": True},
-             {"properties": [("extend", True)]}),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "shift": True, "ctrl": True},
-             {"properties": [("extend", True)]}),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "shift": True, "alt": True},
-             {"properties": [("extend", True)]}),
-            ("node.select", {"type": select_mouse, "value": 'PRESS', "shift": True, "ctrl": True, "alt": True},
-             {"properties": [("extend", True)]}),
-        ]
-
-    # Allow node selection with both for RMB select
     if not params.legacy:
+        items.extend(_template_node_select(type=params.select_mouse,
+                     value=params.select_mouse_value, select_passthrough=True))
+        # Allow node selection with both for RMB select.
         if params.select_mouse == 'RIGHTMOUSE':
-            items.extend(node_select_ops('LEFTMOUSE'))
-            items.extend(node_select_ops('RIGHTMOUSE'))
-        else:
-            items.extend(node_select_ops('LEFTMOUSE'))
+            items.extend(_template_node_select(type='LEFTMOUSE', value='PRESS', select_passthrough=True))
     else:
-        items.extend(node_select_ops('RIGHTMOUSE'))
+        items.extend(_template_node_select(
+            type='RIGHTMOUSE', value=params.select_mouse_value, select_passthrough=False))
         items.extend([
             ("node.select", {"type": 'LEFTMOUSE', "value": 'PRESS'},
              {"properties": [("deselect_all", False)]}),
             ("node.select", {"type": 'LEFTMOUSE', "value": 'PRESS', "shift": True},
-             {"properties": [("extend", True)]}),
+             {"properties": [("toggle", True)]}),
         ])
 
     items.extend([
@@ -4782,6 +4765,35 @@ def _template_view3d_gpencil_select(*, type, value, legacy, use_select_mouse=Tru
     ]
 
 
+def _template_node_select(*, type, value, select_passthrough):
+    items = [
+        ("node.select", {"type": type, "value": value},
+         {"properties": [("deselect_all", True), ("select_passthrough", True)]}),
+        ("node.select", {"type": type, "value": value, "ctrl": True}, None),
+        ("node.select", {"type": type, "value": value, "alt": True}, None),
+        ("node.select", {"type": type, "value": value, "ctrl": True, "alt": True}, None),
+        ("node.select", {"type": type, "value": value, "shift": True},
+         {"properties": [("toggle", True)]}),
+        ("node.select", {"type": type, "value": value, "shift": True, "ctrl": True},
+         {"properties": [("toggle", True)]}),
+        ("node.select", {"type": type, "value": value, "shift": True, "alt": True},
+         {"properties": [("toggle", True)]}),
+        ("node.select", {"type": type, "value": value, "shift": True, "ctrl": True, "alt": True},
+         {"properties": [("toggle", True)]}),
+    ]
+
+    if select_passthrough and (value == 'PRESS'):
+        # Add an additional click item to de-select all other items,
+        # needed so pass-through is able to de-select other items.
+        items.append((
+            "node.select",
+            {"type": type, "value": 'CLICK'},
+            {"properties": [("deselect_all", True)]},
+        ))
+
+    return items
+
+
 def _template_uv_select(*, type, value, select_passthrough, legacy):
 
     # See: `use_tweak_select_passthrough` doc-string.
@@ -6543,10 +6555,8 @@ def km_node_editor_tool_select(params, *, fallback):
         _fallback_id("Node Tool: Tweak", fallback),
         {"space_type": 'NODE_EDITOR', "region_type": 'WINDOW'},
         {"items": [
-            *([] if (fallback and (params.select_mouse == 'RIGHTMOUSE')) else [
-                ("node.select", {"type": params.select_mouse, "value": 'PRESS'},
-                 {"properties": [("deselect_all", not params.legacy)]}),
-            ]),
+            *([] if (fallback and (params.select_mouse == 'RIGHTMOUSE')) else
+              _template_node_select(type=params.select_mouse, value='PRESS', select_passthrough=True)),
         ]},
     )
 
@@ -6563,6 +6573,8 @@ def km_node_editor_tool_select_box(params, *, fallback):
                    params.tool_tweak_event),
                 properties=[("tweak", True)],
             )),
+            *([] if (params.select_mouse == 'RIGHTMOUSE') else
+              _template_node_select(type='LEFTMOUSE', value='PRESS', select_passthrough=True)),
         ]},
     )
 
diff --git a/source/blender/editors/space_node/node_select.cc b/source/blender/editors/space_node/node_select.cc
index 1d0097068f1..147cbc5a05c 100644
--- a/source/blender/editors/space_node/node_select.cc
+++ b/source/blender/editors/space_node/node_select.cc
@@ -193,11 +193,6 @@ static bool is_event_over_node_or_socket(bContext *C, const wmEvent *event)
   return is_position_over_node_or_socket(*snode, mouse);
 }
 
-static void node_toggle(bNode *node)
-{
-  nodeSetSelected(node, !(node->flag & SELECT));
-}
-
 void node_socket_select(bNode *node, bNodeSocket &sock)
 {
   sock.flag |= SELECT;
@@ -510,10 +505,10 @@ void node_select_single(bContext &C, bNode &node)
   WM_event_add_notifier(&C, NC_NODE | NA_SELECTED, nullptr);
 }
 
-static int node_mouse_select(bContext *C,
-                             wmOperator *op,
-                             const int mval[2],
-                             bool wait_to_deselect_others)
+static bool node_mouse_select(bContext *C,
+                              wmOperator *op,
+                              const int mval[2],
+                              struct SelectPick_Params *params)
 {
   Main &bmain = *CTX_data_main(C);
   SpaceNode &snode = *CTX_wm_space_node(C);
@@ -525,36 +520,38 @@ static int node_mouse_select(bContext *C,
   bNodeSocket *sock = nullptr;
   bNodeSocket *tsock;
   float cursor[2];
-  int ret_value = OPERATOR_CANCELLED;
 
-  const bool extend = RNA_boolean_get(op->ptr, "extend");
   /* always do socket_select when extending selection. */
-  const bool socket_select = extend || RNA_boolean_get(op->ptr, "socket_select");
-  const bool deselect_all = RNA_boolean_get(op->ptr, "deselect_all");
-
-  /* These cases are never modal. */
-  if (extend || socket_select) {
-    wait_to_deselect_others = false;
-  }
+  const bool socket_select = (params->sel_op == SEL_OP_XOR) ||
+                             RNA_boolean_get(op->ptr, "socket_select");
+  bool changed = false;
+  bool found = false;
+  bool node_was_selected = false;
 
   /* get mouse coordinates in view2d space */
   UI_view2d_region_to_view(&region.v2d, mval[0], mval[1], &cursor[0], &cursor[1]);
 
   /* first do socket selection, these generally overlap with nodes. */
   if (socket_select) {
+    /* NOTE: unlike nodes #SelectPick_Params isn't fully supported. */
+    const bool extend = (params->sel_op == SEL_OP_XOR);
     if (node_find_indicated_socket(snode, &node, &sock, cursor, SOCK_IN)) {
+      found = true;
+      node_was_selected = node->flag & SELECT;
+
       /* NOTE: SOCK_IN does not take into account the extend case...
        * This feature is not really used anyway currently? */
       node_socket_toggle(node, *sock, true);
-      ret_value = OPERATOR_FINISHED;
+      changed = true;
     }
     else if (node_find_indicated_socket(snode, &node, &sock, cursor, SOCK_OUT)) {
+      found = true;
+      node_was_selected = node->flag & SELECT;
+
       if (sock->flag & SELECT) {
         if (extend) {
           node_socket_deselect(node, *sock, true);
-        }
-        else {
-          ret_value = OPERATOR_FINISHED;
+          changed = true;
         }
       }
       else {
@@ -566,6 +563,7 @@ static int node_mouse_select(bContext *C,
               continue;
             }
             node_socket_deselect(node, *tsock, true);
+            changed = true;
           }
         }
         if (!extend) {
@@ -575,69 +573,71 @@ static int node_mouse_select(bContext *C,
             }
             for (tsock = (bNodeSocket *)tnode->outputs.first; tsock; tsock = tsock->next) {
               node_socket_deselect(tnode, *tsock, true);
+              changed = true;
             }
           }
         }
         node_socket_select(node, *sock);
-        ret_value = OPERATOR_FINISHED;
+        changed = true;
       }
     }
   }
 
   if (!sock) {
+
     /* find the closest visible node */
     node = node_under_mouse_select(*snode.edittree, (int)cursor[0], (int)cursor[1]);
+    found = (node != nullptr);
+    node_was_selected = node && (node->flag & SELECT);
 
-    if (extend) {
-      if (node != nullptr) {
-        /* If node is selected but not active, we want to make it active,
-         * but not toggle (deselect) it. */
-        if (!((node->flag & SELECT) && (node->flag & NODE_ACTIVE) == 0)) {
-          node_toggle(node);
-        }
-        ret_value = OPERATOR_FINISHED;
+    if (params->sel_op == SEL_OP_SET) {
+      if ((found && params->select_passthrough) && (node->flag & SELECT)) {
+        found = false;
       }
-    }
-    else if (deselect_all && node == nullptr) {
-      /* Rather than deselecting others, users may want to drag to box-select (drag from empty
-       * space) or tweak-translate an already selected item. If these cases may apply, delay
-       * deselection. */
-      if (wait_to_deselect_others) {
-        ret

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list