Page MenuHome
Paste P966

(An Untitled Masterwork)
ActivePublic
Actions

Authored by Bastien Montagne (mont29) on May 7 2019, 4:00 PM.
Tags
None
Subscribers
William Reynish (billreynish)
Tokens
"Love" token, awarded by billreynish.
diff --git a/source/blender/editors/space_node/node_select.c b/source/blender/editors/space_node/node_select.c
index 07b01e576bc..681bf693246 100644
--- a/source/blender/editors/space_node/node_select.c
+++ b/source/blender/editors/space_node/node_select.c
@@ -435,13 +435,21 @@ static int node_mouse_select(Main *bmain,
const int mval[2],
const bool extend,
const bool socket_select,
- const bool deselect_all)
+ const bool deselect_all,
+ const bool do_first_step,
+ const bool do_second_step)
{
bNode *node, *tnode;
bNodeSocket *sock = NULL;
bNodeSocket *tsock;
float cursor[2];
- bool selected = false;
+ int ret_value = OPERATOR_CANCELLED;
+
+ /* Handle two-steps selection (with deselection of already selected items as second step). */
+ const bool use_two_steps = do_first_step || do_second_step;
+
+ /* Two-steps modal selection is only allowed for basic selection. */
+ BLI_assert(!(extend || socket_select) || !use_two_steps);
/* get mouse coordinates in view2d space */
UI_view2d_region_to_view(&ar->v2d, mval[0], mval[1], &cursor[0], &cursor[1]);
@@ -452,7 +460,7 @@ static int node_mouse_select(Main *bmain,
/* 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);
- selected = true;
+ ret_value = OPERATOR_FINISHED;
}
else if (node_find_indicated_socket(snode, &node, &sock, cursor, SOCK_OUT)) {
if (sock->flag & SELECT) {
@@ -460,7 +468,7 @@ static int node_mouse_select(Main *bmain,
node_socket_deselect(node, sock, true);
}
else {
- selected = true;
+ ret_value = OPERATOR_FINISHED;
}
}
else {
@@ -485,7 +493,7 @@ static int node_mouse_select(Main *bmain,
}
}
node_socket_select(node, sock);
- selected = true;
+ ret_value = OPERATOR_FINISHED;
}
}
}
@@ -501,24 +509,43 @@ static int node_mouse_select(Main *bmain,
if (!((node->flag & SELECT) && (node->flag & NODE_ACTIVE) == 0)) {
node_toggle(node);
}
- selected = true;
+ ret_value = OPERATOR_FINISHED;
}
}
else {
if (node != NULL || deselect_all) {
- for (tnode = snode->edittree->nodes.first; tnode; tnode = tnode->next) {
- nodeSetSelected(tnode, false);
+ /* We want to deselect all other nodes in three cases:
+ * - If 'deselect on nothing' is enabled, and we clicked on nothing.
+ * - If we are in single-step selection, or in second step of two-steps selection.
+ * - If we are in first step of two-steps selection, and node on which we clicked
+ * was not selected (that way, click-drag on non-selected node will only move that one,
+ * not also all other previously selected nodes).
+ */
+ if ((deselect_all && node == NULL) || (!use_two_steps || do_second_step) ||
+ (do_first_step && node != NULL && (node->flag & SELECT) == 0)) {
+ for (tnode = snode->edittree->nodes.first; tnode; tnode = tnode->next) {
+ if (tnode == node) {
+ continue;
+ }
+ nodeSetSelected(tnode, false);
+ }
+ if (node != NULL) {
+ nodeSetSelected(node, true);
+ }
+ /* In case we also do de-selection of previously selected nodes, we are done,
+ * no need to keep modal running. */
+ ret_value = OPERATOR_FINISHED;
}
- selected = true;
- if (node != NULL) {
+ else if (node != NULL) {
nodeSetSelected(node, true);
+ ret_value = do_first_step ? OPERATOR_RUNNING_MODAL : OPERATOR_FINISHED;
}
}
}
}
/* update node order */
- if (selected) {
+ if (ret_value != OPERATOR_CANCELLED) {
if (node != NULL) {
ED_node_set_active(bmain, snode->edittree, node);
}
@@ -526,7 +553,7 @@ static int node_mouse_select(Main *bmain,
ED_node_sort(snode->edittree);
}
- return selected;
+ return ret_value;
}
static int node_select_exec(bContext *C, wmOperator *op)
@@ -546,17 +573,90 @@ static int node_select_exec(bContext *C, wmOperator *op)
const bool deselect_all = RNA_boolean_get(op->ptr, "deselect_all");
/* perform the select */
- if (node_mouse_select(bmain, snode, ar, mval, extend, socket_select, deselect_all)) {
+ const int ret_value = node_mouse_select(
+ bmain, snode, ar, mval, extend, socket_select, deselect_all, false, false);
+ if (ret_value & OPERATOR_FINISHED) {
/* send notifiers */
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, NULL);
+ }
+ /* allow tweak event to work too */
+ return ret_value | OPERATOR_PASS_THROUGH;
+}
- /* allow tweak event to work too */
- return OPERATOR_FINISHED | OPERATOR_PASS_THROUGH;
+static int node_select_modal(bContext *C, wmOperator *op, const wmEvent *event)
+{
+ Main *bmain = CTX_data_main(C);
+ SpaceNode *snode = CTX_wm_space_node(C);
+ ARegion *ar = CTX_wm_region(C);
+
+ const short init_event_type = (short)POINTER_AS_INT(op->customdata);
+
+ /* get settings from RNA properties for operator */
+ int mval[2];
+ mval[0] = RNA_int_get(op->ptr, "mouse_x");
+ mval[1] = RNA_int_get(op->ptr, "mouse_y");
+
+ 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) {
+ return node_select_exec(C, op);
}
- else {
- /* allow tweak event to work too */
- return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
+
+ if (init_event_type == 0) {
+ if (event->val == KM_PRESS) {
+ const int ret_value = node_mouse_select(
+ bmain, snode, ar, mval, extend, socket_select, deselect_all, true, false);
+
+ op->customdata = POINTER_FROM_INT((int)event->type);
+ if (ret_value & OPERATOR_RUNNING_MODAL) {
+ WM_event_add_modal_handler(C, op);
+ }
+ if (ret_value & (OPERATOR_RUNNING_MODAL | OPERATOR_FINISHED)) {
+ /* send notifiers */
+ WM_event_add_notifier(C, NC_NODE | NA_SELECTED, NULL);
+ }
+ return ret_value | OPERATOR_PASS_THROUGH;
+ }
+ else {
+ /* If we are in init phase, and cannot validate init of modal operations,
+ * just fall back to basic exec.
+ */
+ return node_select_exec(C, op);
+ }
+ }
+ else if (event->type == init_event_type && event->val == KM_RELEASE) {
+ const int ret_value = node_mouse_select(
+ bmain, snode, ar, mval, extend, socket_select, deselect_all, false, true);
+
+ if (ret_value & (OPERATOR_RUNNING_MODAL | OPERATOR_FINISHED)) {
+ /* send notifiers */
+ WM_event_add_notifier(C, NC_NODE | NA_SELECTED, NULL);
+ }
+
+ return ret_value | OPERATOR_PASS_THROUGH;
}
+ else if (ELEM(event->type, MOUSEMOVE, INBETWEEN_MOUSEMOVE)) {
+ const int dx = mval[0] - event->mval[0];
+ const int dy = mval[1] - event->mval[1];
+ const float tweak_threshold = U.tweak_threshold * U.dpi_fac;
+ /* If user moves mouse more than defined threshold, we consider select operator as
+ * finished. Otherwise, it is still running until we get an 'release' event. In any
+ * case, we pass through event, but select op is not finished yet. */
+ if (abs(dx) + abs(dy) > tweak_threshold) {
+ return OPERATOR_FINISHED | OPERATOR_PASS_THROUGH;
+ }
+ else {
+ /* Important not to return anything other than PASS_THROUGH here,
+ * otherwise it prevents underlying tweak detection code to work properly. */
+ return OPERATOR_PASS_THROUGH;
+ }
+ }
+
+ return OPERATOR_FINISHED | OPERATOR_PASS_THROUGH;
}
static int node_select_invoke(bContext *C, wmOperator *op, const wmEvent *event)
@@ -564,7 +664,9 @@ static int node_select_invoke(bContext *C, wmOperator *op, const wmEvent *event)
RNA_int_set(op->ptr, "mouse_x", event->mval[0]);
RNA_int_set(op->ptr, "mouse_y", event->mval[1]);
- return node_select_exec(C, op);
+ op->customdata = POINTER_FROM_INT(0);
+
+ return node_select_modal(C, op, event);
}
void NODE_OT_select(wmOperatorType *ot)
@@ -577,6 +679,7 @@ void NODE_OT_select(wmOperatorType *ot)
/* api callbacks */
ot->invoke = node_select_invoke;
ot->exec = node_select_exec;
+ ot->modal = node_select_modal;
ot->poll = ED_operator_node_active;
/* flags */

Event Timeline

Bastien Montagne (mont29) created this paste.May 7 2019, 4:00 PM
Bastien Montagne (mont29) edited the content of this paste. (Show Details)May 7 2019, 5:43 PM
Bastien Montagne (mont29) edited the content of this paste. (Show Details)May 7 2019, 5:51 PM
Bastien Montagne (mont29) mentioned this in T63994: Node Editor: Move All Selected Nodes when dragging.May 7 2019, 5:55 PM
William Reynish (billreynish) awarded a token.May 7 2019, 6:09 PM
William Reynish (billreynish) added a subscriber: William Reynish (billreynish).May 7 2019, 6:14 PM

This seems to work well.

Question: How generic is it? Could we re-use same logic for sequencer strips or NLA clips etc?

Bastien Montagne (mont29) added a comment.May 7 2019, 7:47 PM

Copy-pasting similar logic should work, yes, at least for similar selection (like in most 2D editors), would expect some complications if we tried to do so with e.g. 3DView selection (also we might have to do similar things there, cf. can of worms II, T63996). This is not a small change though, as pretty much all select ops do things a bit differently. Kind of like what I did for the 'deselect on nothing' change, but several times bigger every time…

My concern with that change is that it adds a whole other level of complexity over selection logic, which already has to deal with extend, select_all, etc. It also makes behavior in different kind of scenarii more complicated to predict (like, what if user adds a keymap using a totally different shortcut to select nodes? What about macros [e.g. just realized that current patch breaks the 'link to viewer node' macro, the one using sockets selection]? etc. etc.). That code can probably be refactored further to make it more easy to follow, but still… am not so happy with it. This adds technical debt for a bad reason imho. :|

Bastien Montagne (mont29) edited the content of this paste. (Show Details)May 10 2019, 6:26 PM
Bastien Montagne (mont29) edited the content of this paste. (Show Details)May 11 2019, 12:03 PM