Node Editor: Various usability and code tidyups

* Fixed a nasty bug which meant that it was impossible to set an active node. Was caused by ntreeCopyTree() getting called when compo updates were done and clearing the active flags. The active flag clearing is only really needed for the "internal_select" case which is only used for duplicating selected nodes (from Shift-D duplicate).

* Recoded click-selection code. Was a mess of old code, bad exceptions from the old code half ported, duplicate operators, unnecessary flags/modes.

* Fixed bug #19927: compositing node groups can't be access via "tab" or ungrouped via "alt+g". Was probably related to the active group not being able to be set.

* Made resizing nodes work again. Again, this was due to the active node bug.

* Made adding a new group with Ctrl-G correctly update the views

8 files changed, 68 insertions, 290 deletions

diff --git a/source/blender/blenkernel/intern/node.c b/source/blender/blenkernel/intern/node.c
index 1b0f1f28d2c..4c378d25a8c 100644
--- a/source/blender/blenkernel/intern/node.c
+++ b/source/blender/blenkernel/intern/node.c
@@ -1072,6 +1072,11 @@ bNodeTree *ntreeAddTree(int type)
 	return ntree;
 }
 
+/* Warning: this function gets called during some rather unexpected times
+ *	- internal_select is only 1 when used for duplicating selected nodes (i.e. Shift-D duplicate operator)
+ *	- this gets called when executing compositing updates (for threaded previews)
+ *	- when the nodetree datablock needs to be copied (i.e. when users get copied)
+ */
 bNodeTree *ntreeCopyTree(bNodeTree *ntree, int internal_select)
 {
 	bNodeTree *newtree;
@@ -1103,10 +1108,9 @@ bNodeTree *ntreeCopyTree(bNodeTree *ntree, int internal_select)
 		if(internal_select==0 || (node->flag & NODE_SELECT)) {
 			nnode= nodeCopyNode(newtree, node, internal_select);	/* sets node->new */
 			if(internal_select) {
-				node->flag &= ~NODE_SELECT;
+				node->flag &= ~(NODE_SELECT|NODE_ACTIVE);
 				nnode->flag |= NODE_SELECT;
 			}
-			node->flag &= ~NODE_ACTIVE;
 
 			/* deselect original sockets */
 			for(sock= node->inputs.first; sock; sock= sock->next) {
diff --git a/source/blender/editors/space_node/node_buttons.c b/source/blender/editors/space_node/node_buttons.c
index 20650812d52..52d8fc48a1a 100644
--- a/source/blender/editors/space_node/node_buttons.c
+++ b/source/blender/editors/space_node/node_buttons.c
@@ -103,8 +103,8 @@ static int active_node_poll(const bContext *C, PanelType *pt)
 static void active_node_panel(const bContext *C, Panel *pa)
 {
 	SpaceNode *snode= CTX_wm_space_node(C);
-	bNodeTree *ntree= (snode) ? snode->nodetree : NULL; // XXX what's up with edittree then?
-	bNode *node = (ntree) ? nodeGetActive(ntree) : NULL;
+	bNodeTree *ntree= (snode) ? snode->edittree : NULL;
+	bNode *node = (ntree) ? nodeGetActive(ntree) : NULL; // xxx... for editing group nodes
 	uiLayout *layout= pa->layout;
 	uiBlock *block;
 	PointerRNA ptr;
@@ -112,7 +112,10 @@ static void active_node_panel(const bContext *C, Panel *pa)
 	/* verify pointers, and create RNA pointer for the node */
 	if ELEM(NULL, ntree, node)
 		return;
-	RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr);
+	//if (node->id) /* for group nodes */
+	//	RNA_pointer_create(node->id, &RNA_Node, node, &ptr);
+	//else
+		RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr); 
 	
 	/* set update callback */
 	// xxx is this really needed
diff --git a/source/blender/editors/space_node/node_edit.c b/source/blender/editors/space_node/node_edit.c
index b76f5d7ee22..d0692f1511f 100644
--- a/source/blender/editors/space_node/node_edit.c
+++ b/source/blender/editors/space_node/node_edit.c
@@ -712,7 +712,7 @@ void node_set_active(SpaceNode *snode, bNode *node)
 			/* make active viewer, currently only 1 supported... */
 			if( ELEM(node->type, CMP_NODE_VIEWER, CMP_NODE_SPLITVIEWER)) {
 				bNode *tnode;
-				int was_output= node->flag & NODE_DO_OUTPUT;
+				int was_output= (node->flag & NODE_DO_OUTPUT);
 
 				for(tnode= snode->edittree->nodes.first; tnode; tnode= tnode->next)
 					if( ELEM(tnode->type, CMP_NODE_VIEWER, CMP_NODE_SPLITVIEWER))
@@ -1206,8 +1206,11 @@ static int node_resize_invoke(bContext *C, wmOperator *op, wmEvent *event)
 	if(node) {
 		rctf totr;
 		
+		/* convert mouse coordinates to v2d space */
 		UI_view2d_region_to_view(&ar->v2d, event->x - ar->winrct.xmin, event->y - ar->winrct.ymin, 
 								 &snode->mx, &snode->my);
+		
+		/* rect we're interested in is just the bottom right corner */
 		totr= node->totr;
 		totr.xmin= totr.xmax-10.0f;
 		totr.ymax= totr.ymin+10.0f;
@@ -1230,7 +1233,7 @@ static int node_resize_invoke(bContext *C, wmOperator *op, wmEvent *event)
 			return OPERATOR_RUNNING_MODAL;
 		}
 	}
-	return OPERATOR_PASS_THROUGH;
+	return OPERATOR_CANCELLED|OPERATOR_PASS_THROUGH;
 }
 
 void NODE_OT_resize(wmOperatorType *ot)
@@ -2096,30 +2099,6 @@ void node_hide(SpaceNode *snode)
 
 #if 0
 
-void node_insert_key(SpaceNode *snode)
-{
-	bNode *node= editnode_get_active(snode->edittree);
-	
-	if(node == NULL)
-		return;
-	
-	if(node->type==CMP_NODE_TIME) {
-		if(node->custom1<node->custom2) {
-
-			CurveMapping *cumap= node->storage;
-			float fval, curval;
-		
-			curval= (float)(CFRA - node->custom1)/(float)(node->custom2-node->custom1);
-			fval= curvemapping_evaluateF(cumap, 0, curval);
-			
-			if(fbutton(&fval, 0.0f, 1.0f, 10, 10, "Insert Value")) {
-				curvemap_insert(cumap->cm, curval, fval);
-
-			}
-		}
-	}
-}
-
 /* makes a link between selected output and input sockets */
 void node_make_link(SpaceNode *snode)
 {
@@ -2201,7 +2180,7 @@ static int cut_links_exec(bContext *C, wmOperator *op)
 		return OPERATOR_FINISHED;
 	}
 	
-	return OPERATOR_PASS_THROUGH;;
+	return OPERATOR_CANCELLED|OPERATOR_PASS_THROUGH;
 }
 
 void NODE_OT_links_cut(wmOperatorType *ot)
@@ -2330,6 +2309,8 @@ static int node_group_make_exec(bContext *C, wmOperator *op)
 		nodeSetActive(snode->nodetree, gnode);
 		ntreeSolveOrder(snode->nodetree);
 	}
+	
+	snode_handle_recalc(C, snode);
 
 	return OPERATOR_FINISHED;
 }
diff --git a/source/blender/editors/space_node/node_header.c b/source/blender/editors/space_node/node_header.c
index a0eae40f579..4cedbe3f666 100644
--- a/source/blender/editors/space_node/node_header.c
+++ b/source/blender/editors/space_node/node_header.c
@@ -202,131 +202,3 @@ void node_menus_register(ARegionType *art)
 	WM_menutype_add(mt);
 }
 
-#if 0
-static void do_node_nodemenu(bContext *C, void *arg, int event)
-{
-	ScrArea *curarea= CTX_wm_area(C);
-	SpaceNode *snode= CTX_wm_space_node(C); 
-	int fromlib=0;
-	
-	/* functions in editnode.c assume there's a tree */
-	if(snode->nodetree==NULL)
-		return;
-	fromlib= (snode->id && snode->id->lib);
-	
-	switch(event) {
-		case 1: /* grab/move */
-			// XXX node_transform_ext(0,0);
-			break;
-		case 2: /* duplicate */
-			if(fromlib) fromlib= -1;
-			else ; // XXX node_adduplicate(snode);
-			break;
-		case 3: /* delete */
-			if(fromlib) fromlib= -1;
-			else ; // XXX node_delete(snode);
-			break;
-		case 4: /* make group */
-			// XXX node_make_group(snode);
-			break;
-		case 5: /* ungroup */
-			// XXX node_ungroup(snode);
-			break;
-		case 6: /* edit group */
-			if(fromlib) fromlib= -1;
-			else ; // XXX snode_make_group_editable(snode, NULL);
-			break;
-		case 7: /* hide/unhide */
-			// XXX node_hide(snode);
-			break;
-		case 8: /* read saved render layers */
-			// XXX node_read_renderlayers(snode);
-			break;
-		case 9: /* show cyclic */
-			// XXX ntreeSolveOrder(snode->edittree);
-			break;
-		case 10: /* execute */
-			// XXXX addqueue(curarea->win, UI_BUT_EVENT, B_NODE_TREE_EXEC);
-			break;
-		case 11: /* make link */
-			// XXX node_make_link(snode);
-			break;
-		case 12: /* rename */
-			// XXX node_rename(snode);
-			break;
-		case 13: /* read saved full sample layers */
-			// XXX node_read_fullsamplelayers(snode);
-			break;
-		case 14: /* connect viewer */
-			// XXX node_active_link_viewer(snode);
-			break;
-			
-	}
-	
-	// XXX if(fromlib==-1) error_libdata();
-	
-	ED_area_tag_redraw(curarea);
-}
-
-static uiBlock *node_nodemenu(bContext *C, ARegion *ar, void *arg_unused)
-{
-	ScrArea *curarea= CTX_wm_area(C);
-	SpaceNode *snode= CTX_wm_space_node(C);
-	uiBlock *block;
-	short yco= 0, menuwidth=120;
-	
-	block= uiBeginBlock(C, ar, "node_nodemenu", UI_EMBOSSP);
-	uiBlockSetButmFunc(block, do_node_nodemenu, NULL);
-	
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Grab/Move|G", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 1, "");
-	
-	uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-	
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Duplicate|Shift D", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 2, "");
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Delete|X", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 3, "");
-	
-	uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Make Link|F", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 11, "");
-
-	uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-	
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Make Group|Ctrl G", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 4, "");
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Ungroup|Alt G", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 5, "");
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Edit Group|Tab", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 6, "");
-	
-	uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-	
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Hide/Unhide|H", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 7, "");
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Rename|Ctrl R", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 12, "");
-	
-	uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-	
-	if(snode->treetype==NTREE_COMPOSIT) {
-		uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Execute Composite|E", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 10, "");
-		uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Read Saved Render Results|R", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 8, "");
-		uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Read Saved Full Sample Results|Shift R", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 13, "");
-		
-		uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-		
-		uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Connect Node to Viewer|Ctrl RMB", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 14, "");
-		
-		uiDefBut(block, SEPR, 0, "",        0, yco-=6, menuwidth, 6, NULL, 0.0, 0.0, 0, 0, "");
-	}
-	uiDefIconTextBut(block, BUTM, 1, ICON_BLANK1, "Show Cyclic Dependencies|C", 0, yco-=20, menuwidth, 19, NULL, 0.0, 0.0, 1, 9, "");
-	
-	if(curarea->headertype==HEADERTOP) {
-		uiBlockSetDirection(block, UI_DOWN);
-	}
-	else {
-		uiBlockSetDirection(block, UI_TOP);
-		uiBlockFlipOrder(block);
-	}
-	
-	uiTextBoundsBlock(block, 50);
-	uiEndBlock(C, block);
-	
-	return block;
-}
-#endif
-
diff --git a/source/blender/editors/space_node/node_intern.h b/source/blender/editors/space_node/node_intern.h
index 2d4d7035568..ea33662ecab 100644
--- a/source/blender/editors/space_node/node_intern.h
+++ b/source/blender/editors/space_node/node_intern.h
@@ -36,14 +36,6 @@ struct View2D;
 struct bContext;
 struct wmWindowManager;
 
-/* defines */
-
-#define NODE_SELECT_MOUSE		1
-
-/* border select defines XXX these seem to be hardcode values still in border select invoke, check */
-#define NODE_EXTEND				1
-#define NODE_EXCLUSIVE			3
-
 /* space_node.c */
 ARegion *node_has_buttons_region(ScrArea *sa);
 
@@ -64,7 +56,6 @@ void node_keymap(wmKeyConfig *keyconf);
 
 /* node_select.c */
 void NODE_OT_select(struct wmOperatorType *ot);
-void NODE_OT_select_extend(struct wmOperatorType *ot);
 void NODE_OT_select_all(wmOperatorType *ot);
 void NODE_OT_select_linked_to(wmOperatorType *ot);
 void NODE_OT_select_linked_from(wmOperatorType *ot);
diff --git a/source/blender/editors/space_node/node_ops.c b/source/blender/editors/space_node/node_ops.c
index a77ba68dc24..ed199bd69d0 100644
--- a/source/blender/editors/space_node/node_ops.c
+++ b/source/blender/editors/space_node/node_ops.c
@@ -52,7 +52,6 @@ void node_operatortypes(void)
 	WM_operatortype_append(NODE_OT_properties);
 	
 	WM_operatortype_append(NODE_OT_select);
-	WM_operatortype_append(NODE_OT_select_extend);
 	WM_operatortype_append(NODE_OT_select_all);
 	WM_operatortype_append(NODE_OT_select_linked_to);
 	WM_operatortype_append(NODE_OT_select_linked_from);
@@ -72,7 +71,7 @@ void node_operatortypes(void)
 void node_keymap(struct wmKeyConfig *keyconf)
 {
 	wmKeyMap *keymap;
-//	wmKeyMapItem *kmi;
+	wmKeyMapItem *kmi;
 	
 	/* Entire Editor only ----------------- */
 	keymap= WM_keymap_find(keyconf, "Node Generic", SPACE_NODE, 0);
@@ -82,18 +81,24 @@ void node_keymap(struct wmKeyConfig *keyconf)
 	/* Main Area only ----------------- */
 	keymap= WM_keymap_find(keyconf, "Node", SPACE_NODE, 0);
 	
-	/* mouse select in nodes used to be both keys, it's UI elements... */
-	RNA_enum_set(WM_keymap_add_item(keymap, "NODE_OT_select", ACTIONMOUSE, KM_PRESS, 0, 0)->ptr, "select_type", NODE_SELECT_MOUSE);
-	RNA_enum_set(WM_keymap_add_item(keymap, "NODE_OT_select", SELECTMOUSE, KM_PRESS, 0, 0)->ptr, "select_type", NODE_SELECT_MOUSE);
-	RNA_enum_set(WM_keymap_add_item(keymap, "NODE_OT_select_extend", ACTIONMOUSE, KM_PRESS, KM_SHIFT, 0)->ptr, "select_type", NODE_SELECT_MOUSE);
-	RNA_enum_set(WM_keymap_add_item(keymap, "NODE_OT_select_extend", SELECTMOUSE, KM_PRESS, KM_SHIFT, 0)->ptr, "select_type", NODE_SELECT_MOUSE);
-	
-	WM_keymap_add_item(keymap, "NODE_OT_duplicate", DKEY, KM_PRESS, KM_SHIFT, 0);
+	/* mouse select in nodes used to be both keys, but perhaps this should be reduced? 
+	 * NOTE: mouse-clicks on left-mouse will fall through to allow transform-tweak, but also link/resize
+	 */
+	WM_keymap_add_item(keymap, "NODE_OT_select", ACTIONMOUSE, KM_PRESS, 0, 0);
+	WM_keymap_add_item(keymap, "NODE_OT_select", SELECTMOUSE, KM_PRESS, 0, 0);
+	kmi= WM_keymap_add_item(keymap, "NODE_OT_select", ACTIONMOUSE, KM_PRESS, KM_SHIFT, 0);
+		RNA_boolean_set(kmi->ptr, "extend", 1);
+	kmi= WM_keymap_add_item(keymap, "NODE_OT_select", SELECTMOUSE, KM_PRESS, KM_SHIFT, 0);
+		RNA_boolean_set(kmi->ptr, "extend", 1);
 	
+	/* each of these falls through if not handled... */
 	WM_keymap_add_item(keymap, "NODE_OT_link", LEFTMOUSE, KM_PRESS, 0, 0);
-	WM_keymap_add_item(keymap, "NODE_OT_resize", LEFTMOUSE, KM_PRESS, 0, 0);
+	WM_keymap_add_item(keymap, "NODE_OT_resize", LEFTMOUSE, KM_PRESS, 0, 0);	// XXX not working..
 	WM_keymap_add_item(keymap, "NODE_OT_visibility_toggle", LEFTMOUSE, KM_PRESS, 0, 0);
-	WM_keymap_add_item(keymap, "NODE_OT_links_cut", LEFTMOUSE, KM_PRESS, KM_ALT, 0);
+	
+	WM_keymap_add_item(keymap, "NODE_OT_links_cut", RIGHTMOUSE, KM_PRESS, KM_CTRL|KM_ALT, 0);
+	
+	WM_keymap_add_item(keymap, "NODE_OT_duplicate", DKEY, KM_PRESS, KM_SHIFT, 0);
 	
 	WM_keymap_add_item(keymap, "NODE_OT_view_all", HOMEKEY, KM_PRESS, 0, 0);
 	WM_keymap_add_item(keymap, "NODE_OT_select_border", BKEY, KM_PRESS, 0, 0);
diff --git a/source/blender/editors/space_node/node_select.c b/source/blender/editors/space_node/node_select.c
index 67c7915694d..87bd6075ec5 100644
--- a/source/blender/editors/space_node/node_select.c
+++ b/source/blender/editors/space_node/node_select.c
@@ -61,48 +61,28 @@ static bNode *node_mouse_select(SpaceNode *snode, ARegion *ar, short *mval, shor
 	bNode *node;
 	float mx, my;
 	
+	/* get mouse coordinates in view2d space */
 	mx= (float)mval[0];
 	my= (float)mval[1];
 	
 	UI_view2d_region_to_view(&ar->v2d, mval[0], mval[1], &mx, &my);
 	
+	/* find the closest visible node */
 	for(next_node(snode->edittree); (node=next_node(NULL));) {
-		
-		/* first check for the headers or scaling widget */
-		/* XXX if(node->flag & NODE_HIDDEN) {
-			if(do_header_hidden_node(snode, node, mx, my))
-				return 1;
-		}
-		else {
-			if(do_header_node(snode, node, mx, my))
-				return 1;
-		}*/
-		
-		/* node body */
-		if(BLI_in_rctf(&node->totr, mx, my))
+		/* node body (header and scale are in other operators) */
+		if (BLI_in_rctf(&node->totr, mx, my))
 			break;
 	}
-	if(node) {
-		if((extend & KM_SHIFT)==0)
+	
+	if (node) {
+		if (extend == 0) {
 			node_deselectall(snode);
-		
-		if(extend & KM_SHIFT) {
-			if(node->flag & SELECT)
-				node->flag &= ~SELECT;
-			else
-				node->flag |= SELECT;
+			node->flag |= SELECT;
 		}
 		else
-			node->flag |= SELECT;
-		
+			node->flag ^= SELECT;
+			
 		node_set_active(snode, node);
-		
-		/* viewer linking */
-		if(extend & KM_CTRL)
-			;//	node_link_viewer(snode, node);
-		
-		//std_rmouse_transform(node_transform_ext);	/* does undo push for select */
-
 	}
 
 	return node;
@@ -110,27 +90,23 @@ static bNode *node_mouse_select(SpaceNode *snode, ARegion *ar, short *mval, shor
 
 static int node_select_exec(bContext *C, wmOperator *op)
 {
-	// XXX wmWindow *window=  CTX_wm_window(C);
 	SpaceNode *snode= CTX_wm_space_node(C);
 	ARegion *ar= CTX_wm_region(C);
-	int select_type;
 	short mval[2];
 	short extend;
 	bNode *node= NULL;
-
-	select_type = RNA_enum_get(op->ptr, "select_type");
 	
-	switch (select_type) {
-		case NODE_SELECT_MOUSE:
-			mval[0] = RNA_int_get(op->ptr, "mouse_x");
-			mval[1] = RNA_int_get(op->ptr, "mouse_y");
-			extend = RNA_boolean_get(op->ptr, "extend");
-			node= node_mouse_select(snode, ar, mval, extend);
-			break;
-	}
+	/* get settings from RNA properties for operator */
+	mval[0] = RNA_int_get(op->ptr, "mouse_x");
+	mval[1] = RNA_int_get(op->ptr, "mouse_y");
+	
+	extend = RNA_boolean_get(op->ptr, "extend");
+	
+	/* perform the select */
+	node= node_mouse_select(snode, ar, mval, extend);
 
 	/* need refresh/a notifier vs compo notifier */
-	// XXX WM_event_add_notifier(C, NC_SCENE|ND_NODES, NULL); /* Do we need to pass the scene? */
+	WM_event_add_notifier(C, NC_SCENE|ND_NODES, NULL); /* Do we need to pass the scene? */
 
 	/* WATCH THIS, there are a few other ways to change the active material */
 	if(node) {
@@ -138,9 +114,7 @@ static int node_select_exec(bContext *C, wmOperator *op)
 			WM_event_add_notifier(C, NC_MATERIAL|ND_SHADING_DRAW, node->id);
 		}
 	}
-
-	ED_region_tag_redraw(ar);
-    
+	
     /* send notifiers */
 	WM_event_add_notifier(C, NC_NODE|ND_NODE_SELECT, NULL);
 	
@@ -148,27 +122,6 @@ static int node_select_exec(bContext *C, wmOperator *op)
 	return OPERATOR_FINISHED|OPERATOR_PASS_THROUGH;
 }
 
-static int node_select_modal(bContext *C, wmOperator *op, wmEvent *event)
-{
-	/* execute the events */
-	switch (event->type) {
-		case MOUSEMOVE:
-			printf("%d %d\n", event->x, event->y);
-			break;
-		case SELECTMOUSE:
-			//if (event->val==KM_RELEASE) {
-				/* calculate overall delta mouse-movement for redo */
-				printf("done translating\n");
-				//WM_cursor_restore(CTX_wm_window(C));
-				
-				return OPERATOR_FINISHED;
-			//}
-			break;
-	}
-
-	return OPERATOR_RUNNING_MODAL;
-}
-
 static int node_select_invoke(bContext *C, wmOperator *op, wmEvent *event)
 {
 	ARegion *ar= CTX_wm_region(C);
@@ -183,56 +136,21 @@ static int node_select_invoke(bContext *C, wmOperator *op, wmEvent *event)
 	return node_select_exec(C,op);
 }
 
-static int node_extend_select_invoke(bContext *C, wmOperator *op, wmEvent *event)
-{
-	RNA_boolean_set(op->ptr, "extend", KM_SHIFT);
-
-	return node_select_invoke(C, op, event);
-}
-
-/* operators */
-
-static EnumPropertyItem prop_select_items[] = {
-	{NODE_SELECT_MOUSE, "NORMAL", 0, "Normal Select", "Select using the mouse"},
-	{0, NULL, 0, NULL, NULL}};
-
-void NODE_OT_select_extend(wmOperatorType *ot)
-{
-	// XXX - Todo - This should just be a toggle option for NODE_OT_select not its own op
-	/* identifiers */
-	ot->name= "Activate/Select (Shift)";
-	ot->idname= "NODE_OT_select_extend";
-	
-	/* api callbacks */
-	ot->invoke= node_extend_select_invoke;
-	ot->poll= ED_operator_node_active;
-	
-	/* flags */
-	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
-	
-	RNA_def_enum(ot->srna, "select_type", prop_select_items, 0, "Select Type", "");
-	
-	RNA_def_int(ot->srna, "mouse_x", 0, INT_MIN, INT_MAX, "Mouse X", "", INT_MIN, INT_MAX);
-	RNA_def_int(ot->srna, "mouse_y", 0, INT_MIN, INT_MAX, "Mouse Y", "", INT_MIN, INT_MAX);
-	RNA_def_boolean(ot->srna, "extend", 0, "Extend", "");
-}
 
 void NODE_OT_select(wmOperatorType *ot)
 {
 	/* identifiers */
-	ot->name= "Activate/Select";
+	ot->name= "Select";
 	ot->idname= "NODE_OT_select";
 	
 	/* api callbacks */
 	ot->invoke= node_select_invoke;
 	ot->poll= ED_operator_node_active;
-	ot->modal= node_select_modal;
 	
 	/* flags */
-	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO|OPTYPE_BLOCKING;
-	
-	RNA_def_enum(ot->srna, "select_type", prop_select_items, 0, "Select Type", "");
+	ot->flag= OPTYPE_REGISTER|OPTYPE_UNDO;
 	
+	/* properties */
 	RNA_def_int(ot->srna, "mouse_x", 0, INT_MIN, INT_MAX, "Mouse X", "", INT_MIN, INT_MAX);
 	RNA_def_int(ot->srna, "mouse_y", 0, INT_MIN, INT_MAX, "Mouse Y", "", INT_MIN, INT_MAX);
 	RNA_def_boolean(ot->srna, "extend", 0, "Extend", "");
@@ -346,13 +264,15 @@ static int node_select_linked_to_exec(bContext *C, wmOperator *op)
 	for (node=snode->edittree->nodes.first; node; node=node->next)
 		node->flag &= ~NODE_TEST;
 
-	for (link=snode->edittree->links.first; link; link=link->next)
+	for (link=snode->edittree->links.first; link; link=link->next) {
 		if (link->fromnode->flag & NODE_SELECT)
 			link->tonode->flag |= NODE_TEST;
+	}
 	
-	for (node=snode->edittree->nodes.first; node; node=node->next)
+	for (node=snode->edittree->nodes.first; node; node=node->next) {
 		if (node->flag & NODE_TEST)
 			node->flag |= NODE_SELECT;
+	}
 	
 	WM_event_add_notifier(C, NC_SCENE|ND_NODES, NULL);
 	return OPERATOR_FINISHED;
@@ -384,13 +304,15 @@ static int node_select_linked_from_exec(bContext *C, wmOperator *op)
 	for(node=snode->edittree->nodes.first; node; node=node->next)
 		node->flag &= ~NODE_TEST;
 
-	for(link=snode->edittree->links.first; link; link=link->next)
+	for(link=snode->edittree->links.first; link; link=link->next) {
 		if(link->tonode->flag & NODE_SELECT)
 			link->fromnode->flag |= NODE_TEST;
+	}
 	
-	for(node=snode->edittree->nodes.first; node; node=node->next)
+	for(node=snode->edittree->nodes.first; node; node=node->next) {
 		if(node->flag & NODE_TEST)
 			node->flag |= NODE_SELECT;
+	}
 	
 	WM_event_add_notifier(C, NC_SCENE|ND_NODES, NULL);
 	return OPERATOR_FINISHED;
diff --git a/source/blender/editors/space_node/node_state.c b/source/blender/editors/space_node/node_state.c
index dcab3dc0895..5214d2ef516 100644
--- a/source/blender/editors/space_node/node_state.c
+++ b/source/blender/editors/space_node/node_state.c
@@ -149,7 +149,7 @@ static int node_toggle_visibility_exec(bContext *C, wmOperator *op)
 	if(node_toggle_visibility(snode, ar, mval))
 		return OPERATOR_FINISHED;
 	else
-		return OPERATOR_PASS_THROUGH;
+		return OPERATOR_CANCELLED|OPERATOR_PASS_THROUGH;
 }
 
 static int node_toggle_visibility_invoke(bContext *C, wmOperator *op, wmEvent *event)