Outliner: Rework element selection behavior

* LMB now replaces selection instead of adding to it. Shift+LMB adds to selection (or removes if already selected). This is usual selection behavior Blender.
* Outliner selection isn't completely separate from object/sequencer-strip/render-layer/... selection anymore, when selecting an outliner item we now always try to select (and activate) the object it belongs to. Previously you had to click the name or icon of an item to select the object (or whatever) and on empty space within the row to set outliner selection.
* Collapsed items may show click-able icons for their children (nothing new). Clicking on such an icon will also select the hidden item it represents now, you'll notice after opening the parent. This valid from a technical POV, I'm not sure if this is wanted from user POV though. Changing would be easy, feedback welcome!
* Code cleanup.

Part of T37430.

5 files changed, 168 insertions, 185 deletions

diff --git a/source/blender/editors/space_outliner/outliner_draw.c b/source/blender/editors/space_outliner/outliner_draw.c
index c2cd859dcd8..7306afc08ff 100644
--- a/source/blender/editors/space_outliner/outliner_draw.c
+++ b/source/blender/editors/space_outliner/outliner_draw.c
@@ -1411,11 +1411,14 @@ static void outliner_draw_iconrow(bContext *C, uiBlock *block, Scene *scene, Spa
 static void outliner_set_coord_tree_element(SpaceOops *soops, TreeElement *te, int startx, int starty)
 {
 	TreeElement *ten;
-	
-	/* store coord and continue, we need coordinates for elements outside view too */
-	te->xs = startx;
-	te->ys = starty;
-	
+
+	/* closed items may be displayed in row of parent, don't change their coordinate! */
+	if ((te->flag & TE_ICONROW) == 0) {
+		/* store coord and continue, we need coordinates for elements outside view too */
+		te->xs = startx;
+		te->ys = starty;
+	}
+
 	for (ten = te->subtree.first; ten; ten = ten->next) {
 		outliner_set_coord_tree_element(soops, ten, startx + UI_UNIT_X, starty);
 	}
diff --git a/source/blender/editors/space_outliner/outliner_edit.c b/source/blender/editors/space_outliner/outliner_edit.c
index b8883371213..87e6be76de8 100644
--- a/source/blender/editors/space_outliner/outliner_edit.c
+++ b/source/blender/editors/space_outliner/outliner_edit.c
@@ -150,16 +150,11 @@ TreeElement *outliner_dropzone_find(const SpaceOops *soops, const float fmval[2]
 	return NULL;
 }
 
-/* ************************************************************** */
-
-/* Highlight --------------------------------------------------- */
-
 /**
  * Try to find an item under y-coordinate \a view_co_y (view-space).
  * \note Recursive
  */
-static TreeElement *outliner_find_item_at_y(
-        const SpaceOops *soops, const ListBase *tree, float view_co_y)
+TreeElement *outliner_find_item_at_y(const SpaceOops *soops, const ListBase *tree, float view_co_y)
 {
 	for (TreeElement *te_iter = tree->first; te_iter; te_iter = te_iter->next) {
 		if (view_co_y < (te_iter->ys + UI_UNIT_Y)) {
@@ -180,6 +175,35 @@ static TreeElement *outliner_find_item_at_y(
 	return NULL;
 }
 
+/**
+ * Collapsed items can show their children as click-able icons. This function tries to find
+ * such an icon that represents the child item at x-coordinate \a view_co_x (view-space).
+ *
+ * \return a hovered child item or \a parent_te (if no hovered child found).
+ */
+TreeElement *outliner_find_item_at_x_in_row(const SpaceOops *soops, const TreeElement *parent_te, float view_co_x)
+{
+	if (!TSELEM_OPEN(TREESTORE(parent_te), soops)) { /* if parent_te is opened, it doesn't show childs in row */
+		/* no recursion, items can only display their direct children in the row */
+		for (TreeElement *child_te = parent_te->subtree.first;
+		     child_te && view_co_x >= child_te->xs; /* don't look further if co_x is smaller than child position*/
+		     child_te = child_te->next)
+		{
+			if ((child_te->flag & TE_ICONROW) && (view_co_x > child_te->xs) && (view_co_x < child_te->xend)) {
+				return child_te;
+			}
+		}
+	}
+
+	/* return parent if no child is hovered */
+	return (TreeElement *)parent_te;
+}
+
+
+/* ************************************************************** */
+
+/* Highlight --------------------------------------------------- */
+
 static int outliner_highlight_update(bContext *C, wmOperator *UNUSED(op), const wmEvent *event)
 {
 	ARegion *ar = CTX_wm_region(C);
diff --git a/source/blender/editors/space_outliner/outliner_intern.h b/source/blender/editors/space_outliner/outliner_intern.h
index 74c59fb4533..db614631bf7 100644
--- a/source/blender/editors/space_outliner/outliner_intern.h
+++ b/source/blender/editors/space_outliner/outliner_intern.h
@@ -65,10 +65,14 @@ typedef struct TreeElement {
 	 ELEM(GS((_id)->name), ID_SCR, ID_WM, ID_TXT, ID_VF, ID_SO, ID_CF, ID_PAL))  /* Only in 'blendfile' mode ... :/ */
 
 /* TreeElement->flag */
-#define TE_ACTIVE       1
-#define TE_ICONROW      2
-#define TE_LAZY_CLOSED  4
-#define TE_FREE_NAME    8
+enum {
+	TE_ACTIVE      = (1 << 0),
+	/* Closed items display their children as icon within the row. TE_ICONROW is for
+	 * these child-items that are visible but only within the row of the closed parent. */
+	TE_ICONROW     = (1 << 1),
+	TE_LAZY_CLOSED = (1 << 2),
+	TE_FREE_NAME   = (1 << 3),
+};
 
 /* button events */
 #define OL_NAMEBUTTON       1
@@ -150,7 +154,7 @@ eOLDrawState tree_element_type_active(
         TreeElement *te, TreeStoreElem *tselem, const eOLSetState set, bool recursive);
 eOLDrawState tree_element_active(struct bContext *C, struct Scene *scene, SpaceOops *soops,
                                  TreeElement *te, const eOLSetState set, const bool handle_all_types);
-int outliner_item_do_activate(struct bContext *C, int x, int y, bool extend, bool recursive);
+int outliner_item_activate_or_toggle_closed(struct bContext *C, int x, int y, bool extend, bool recursive);
 
 /* outliner_edit.c ---------------------------------------------- */
 typedef void (*outliner_operation_cb)(
@@ -208,6 +212,10 @@ void id_remap_cb(
         struct TreeStoreElem *tsep, struct TreeStoreElem *tselem, void *user_data);
 
 TreeElement *outliner_dropzone_find(const struct SpaceOops *soops, const float fmval[2], const bool children);
+
+TreeElement *outliner_find_item_at_y(const SpaceOops *soops, const ListBase *tree, float view_co_y);
+TreeElement *outliner_find_item_at_x_in_row(const SpaceOops *soops, const TreeElement *parent_te, float view_co_x);
+
 /* ...................................................... */
 
 void OUTLINER_OT_highlight_update(struct wmOperatorType *ot);
diff --git a/source/blender/editors/space_outliner/outliner_select.c b/source/blender/editors/space_outliner/outliner_select.c
index 17b6930e2d9..635c7923236 100644
--- a/source/blender/editors/space_outliner/outliner_select.c
+++ b/source/blender/editors/space_outliner/outliner_select.c
@@ -68,55 +68,6 @@
 
 #include "outliner_intern.h"
 
-/* ****************************************************** */
-/* Outliner Selection (gray-blue highlight for rows) */
-
-static int outliner_select(SpaceOops *soops, ListBase *lb, int *index, short *selecting)
-{
-	TreeElement *te;
-	TreeStoreElem *tselem;
-	bool changed = false;
-	
-	for (te = lb->first; te && *index >= 0; te = te->next, (*index)--) {
-		tselem = TREESTORE(te);
-		
-		/* if we've encountered the right item, set its 'Outliner' selection status */
-		if (*index == 0) {
-			/* this should be the last one, so no need to do anything with index */
-			if ((te->flag & TE_ICONROW) == 0) {
-				/* -1 value means toggle testing for now... */
-				if (*selecting == -1) {
-					if (tselem->flag & TSE_SELECTED) 
-						*selecting = 0;
-					else 
-						*selecting = 1;
-				}
-				
-				/* set selection */
-				if (*selecting) 
-					tselem->flag |= TSE_SELECTED;
-				else 
-					tselem->flag &= ~TSE_SELECTED;
-
-				changed |= true;
-			}
-		}
-		else if (TSELEM_OPEN(tselem, soops)) {
-			/* Only try selecting sub-elements if we haven't hit the right element yet
-			 *
-			 * Hack warning:
-			 *  Index must be reduced before supplying it to the sub-tree to try to do
-			 *  selection, however, we need to increment it again for the next loop to
-			 *  function correctly
-			 */
-			(*index)--;
-			changed |= outliner_select(soops, &te->subtree, index, selecting);
-			(*index)++;
-		}
-	}
-
-	return changed;
-}
 
 /* ****************************************************** */
 /* Outliner Element Selection/Activation on Click */
@@ -870,163 +821,160 @@ eOLDrawState tree_element_type_active(
 
 /* ================================================ */
 
-static bool do_outliner_item_activate(bContext *C, Scene *scene, ARegion *ar, SpaceOops *soops,
-                                      TreeElement *te, bool extend, bool recursive, const float mval[2])
+static void outliner_item_activate(
+        bContext *C, SpaceOops *soops, TreeElement *te,
+        const bool extend, const bool recursive)
 {
-	
-	if (mval[1] > te->ys && mval[1] < te->ys + UI_UNIT_Y) {
-		TreeStoreElem *tselem = TREESTORE(te);
-		bool openclose = false;
-		
-		/* open close icon */
-		if ((te->flag & TE_ICONROW) == 0) {               // hidden icon, no open/close
-			if (mval[0] > te->xs && mval[0] < te->xs + UI_UNIT_X)
-				openclose = true;
-		}
-		
-		if (openclose) {
-			/* all below close/open? */
-			if (extend) {
-				tselem->flag &= ~TSE_CLOSED;
-				outliner_set_flag(soops, &te->subtree, TSE_CLOSED, !outliner_has_one_flag(soops, &te->subtree, TSE_CLOSED, 1));
-			}
-			else {
-				if (tselem->flag & TSE_CLOSED) tselem->flag &= ~TSE_CLOSED;
-				else tselem->flag |= TSE_CLOSED;
-				
+	Scene *scene = CTX_data_scene(C);
+	TreeStoreElem *tselem = TREESTORE(te);
+
+	/* always makes active object, except for some specific types.
+	 * Note about TSE_EBONE: In case of a same ID_AR datablock shared among several objects, we do not want
+	 * to switch out of edit mode (see T48328 for details). */
+	if (!ELEM(tselem->type, TSE_SEQUENCE, TSE_SEQ_STRIP, TSE_SEQUENCE_DUP, TSE_EBONE)) {
+		tree_element_set_active_object(C, scene, soops, te,
+		                               (extend && tselem->type == 0) ? OL_SETSEL_EXTEND : OL_SETSEL_NORMAL,
+		                               recursive && tselem->type == 0);
+	}
+
+	if (tselem->type == 0) { // the lib blocks
+		/* editmode? */
+		if (te->idcode == ID_SCE) {
+			if (scene != (Scene *)tselem->id) {
+				ED_screen_set_scene(C, CTX_wm_screen(C), (Scene *)tselem->id);
 			}
-			
-			return true;
 		}
-		/* name and first icon */
-		else if (mval[0] > te->xs + UI_UNIT_X && mval[0] < te->xend) {
-			
-			/* always makes active object, except for some specific types.
-			 * Note about TSE_EBONE: In case of a same ID_AR datablock shared among several objects, we do not want
-			 * to switch out of edit mode (see T48328 for details). */
-			if (!ELEM(tselem->type, TSE_SEQUENCE, TSE_SEQ_STRIP, TSE_SEQUENCE_DUP, TSE_EBONE)) {
-				tree_element_set_active_object(C, scene, soops, te,
-				                               (extend && tselem->type == 0) ? OL_SETSEL_EXTEND : OL_SETSEL_NORMAL,
-				                               recursive && tselem->type == 0);
-			}
+		else if (te->idcode == ID_GR) {
+			Group *gr = (Group *)tselem->id;
+			GroupObject *gob;
 			
-			if (tselem->type == 0) { // the lib blocks
-				/* editmode? */
-				if (te->idcode == ID_SCE) {
-					if (scene != (Scene *)tselem->id) {
-						ED_screen_set_scene(C, CTX_wm_screen(C), (Scene *)tselem->id);
-					}
-				}
-				else if (te->idcode == ID_GR) {
-					Group *gr = (Group *)tselem->id;
-					GroupObject *gob;
-					
-					if (extend) {
-						int sel = BA_SELECT;
-						for (gob = gr->gobject.first; gob; gob = gob->next) {
-							if (gob->ob->flag & SELECT) {
-								sel = BA_DESELECT;
-								break;
-							}
-						}
-						
-						for (gob = gr->gobject.first; gob; gob = gob->next) {
-							ED_base_object_select(BKE_scene_base_find(scene, gob->ob), sel);
-						}
-					}
-					else {
-						BKE_scene_base_deselect_all(scene);
-						
-						for (gob = gr->gobject.first; gob; gob = gob->next) {
-							if ((gob->ob->flag & SELECT) == 0)
-								ED_base_object_select(BKE_scene_base_find(scene, gob->ob), BA_SELECT);
-						}
+			if (extend) {
+				int sel = BA_SELECT;
+				for (gob = gr->gobject.first; gob; gob = gob->next) {
+					if (gob->ob->flag & SELECT) {
+						sel = BA_DESELECT;
+						break;
 					}
-					
-					WM_event_add_notifier(C, NC_SCENE | ND_OB_SELECT, scene);
-				}
-				else if (ELEM(te->idcode, ID_ME, ID_CU, ID_MB, ID_LT, ID_AR)) {
-					WM_operator_name_call(C, "OBJECT_OT_editmode_toggle", WM_OP_INVOKE_REGION_WIN, NULL);
-				}
-				else {  // rest of types
-					tree_element_active(C, scene, soops, te, OL_SETSEL_NORMAL, false);
 				}
 
+				for (gob = gr->gobject.first; gob; gob = gob->next) {
+					ED_base_object_select(BKE_scene_base_find(scene, gob->ob), sel);
+				}
 			}
 			else {
-				tree_element_type_active(C, scene, soops, te, tselem,
-				                         extend ? OL_SETSEL_EXTEND : OL_SETSEL_NORMAL,
-				                         recursive);
+				BKE_scene_base_deselect_all(scene);
+
+				for (gob = gr->gobject.first; gob; gob = gob->next) {
+					if ((gob->ob->flag & SELECT) == 0)
+						ED_base_object_select(BKE_scene_base_find(scene, gob->ob), BA_SELECT);
+				}
 			}
 			
-			return true;
+			WM_event_add_notifier(C, NC_SCENE | ND_OB_SELECT, scene);
 		}
-	}
-	
-	for (te = te->subtree.first; te; te = te->next) {
-		if (do_outliner_item_activate(C, scene, ar, soops, te, extend, recursive, mval)) {
-			return true;
+		else if (ELEM(te->idcode, ID_ME, ID_CU, ID_MB, ID_LT, ID_AR)) {
+			WM_operator_name_call(C, "OBJECT_OT_editmode_toggle", WM_OP_INVOKE_REGION_WIN, NULL);
 		}
+		else {  // rest of types
+			tree_element_active(C, scene, soops, te, OL_SETSEL_NORMAL, false);
+		}
+
+	}
+	else {
+		tree_element_type_active(C, scene, soops, te, tselem,
+		                         extend ? OL_SETSEL_EXTEND : OL_SETSEL_NORMAL,
+		                         recursive);
 	}
-	return false;
 }
 
-int outliner_item_do_activate(bContext *C, int x, int y, bool extend, bool recursive)
+/**
+ * \param extend: Don't deselect other items, only modify \a te.
+ * \param toggle: Select \a te when not selected, deselect when selected.
+ */
+static void outliner_item_select(SpaceOops *soops, const TreeElement *te, const bool extend, const bool toggle)
+{
+	TreeStoreElem *tselem = TREESTORE(te);
+	const short new_flag = toggle ? (tselem->flag ^ TSE_SELECTED) : (tselem->flag | TSE_SELECTED);
+
+	if (extend == false) {
+		outliner_set_flag(soops, &soops->tree, TSE_SELECTED, false);
+	}
+	tselem->flag = new_flag;
+}
+
+static void outliner_item_toggle_closed(SpaceOops *soops, TreeElement *te, const bool toggle_children)
+{
+	TreeStoreElem *tselem = TREESTORE(te);
+	if (toggle_children) {
+		tselem->flag &= ~TSE_CLOSED;
+
+		const bool all_opened = !outliner_has_one_flag(soops, &te->subtree, TSE_CLOSED, 1);
+		outliner_set_flag(soops, &te->subtree, TSE_CLOSED, all_opened);
+	}
+	else {
+		tselem->flag ^= TSE_CLOSED;
+	}
+}
+
+static bool outliner_item_is_co_within_close_toggle(TreeElement *te, float view_co_x)
+{
+	return ((te->flag & TE_ICONROW) == 0) && (view_co_x > te->xs) && (view_co_x < te->xs + UI_UNIT_X);
+}
+
+static bool outliner_is_co_within_restrict_columns(const SpaceOops *soops, const ARegion *ar, float view_co_x)
+{
+	return (!ELEM(soops->outlinevis, SO_DATABLOCKS, SO_USERDEF) &&
+	        !(soops->flag & SO_HIDE_RESTRICTCOLS) &&
+	        (view_co_x > ar->v2d.cur.xmax - OL_TOG_RESTRICT_VIEWX));
+}
+
+int outliner_item_activate_or_toggle_closed(bContext *C, int x, int y, bool extend, bool recursive)
 {
-	Scene *scene = CTX_data_scene(C);
 	ARegion *ar = CTX_wm_region(C);
 	SpaceOops *soops = CTX_wm_space_outliner(C);
 	TreeElement *te;
-	float fmval[2];
+	float view_mval[2];
+	bool changed = false;
 
-	UI_view2d_region_to_view(&ar->v2d, x, y, &fmval[0], &fmval[1]);
+	UI_view2d_region_to_view(&ar->v2d, x, y, &view_mval[0], &view_mval[1]);
 
-	if (!ELEM(soops->outlinevis, SO_DATABLOCKS, SO_USERDEF) &&
-	    !(soops->flag & SO_HIDE_RESTRICTCOLS) &&
-	    (fmval[0] > ar->v2d.cur.xmax - OL_TOG_RESTRICT_VIEWX))
-	{
+	if (outliner_is_co_within_restrict_columns(soops, ar, view_mval[0])) {
 		return OPERATOR_CANCELLED;
 	}
 
-	for (te = soops->tree.first; te; te = te->next) {
-		if (do_outliner_item_activate(C, scene, ar, soops, te, extend, recursive, fmval)) break;
+	if (!(te = outliner_find_item_at_y(soops, &soops->tree, view_mval[1]))) {
+		/* skip */
 	}
-	
-	if (te) {
-		ED_undo_push(C, "Outliner click event");
+	else if (outliner_item_is_co_within_close_toggle(te, view_mval[0])) {
+		outliner_item_toggle_closed(soops, te, extend);
+		changed = true;
 	}
 	else {
-		short selecting = -1;
-		int row;
-		
-		/* get row number - 100 here is just a dummy value since we don't need the column */
-		UI_view2d_listview_view_to_cell(&ar->v2d, 1000, UI_UNIT_Y, 0.0f, OL_Y_OFFSET, 
-		                                fmval[0], fmval[1], NULL, &row);
-		
-		/* select relevant row */
-		if (outliner_select(soops, &soops->tree, &row, &selecting)) {
-		
-			soops->storeflag |= SO_TREESTORE_REDRAW;
-		
-			/* no need for undo push here, only changing outliner data which is
-			 * scene level - campbell */
-			/* ED_undo_push(C, "Outliner selection event"); */
-		}
+		/* the row may also contain children, if one is hovered we want this instead of current te */
+		TreeElement *activate_te = outliner_find_item_at_x_in_row(soops, te, view_mval[0]);
+
+		outliner_item_select(soops, activate_te, extend, extend);
+		outliner_item_activate(C, soops, activate_te, extend, recursive);
+		changed = true;
+	}
+
+	if (changed) {
+		soops->storeflag |= SO_TREESTORE_REDRAW; /* only needs to redraw, no rebuild */
+		ED_undo_push(C, "Outliner selection change");
+		ED_region_tag_redraw(ar);
 	}
-	
-	ED_region_tag_redraw(ar);
 
 	return OPERATOR_FINISHED;
 }
 
 /* event can enterkey, then it opens/closes */
-static int outliner_item_activate(bContext *C, wmOperator *op, const wmEvent *event)
+static int outliner_item_activate_invoke(bContext *C, wmOperator *op, const wmEvent *event)
 {
 	bool extend    = RNA_boolean_get(op->ptr, "extend");
 	bool recursive = RNA_boolean_get(op->ptr, "recursive");
 	int x = event->mval[0];
 	int y = event->mval[1];
-	return outliner_item_do_activate(C, x, y, extend, recursive);
+	return outliner_item_activate_or_toggle_closed(C, x, y, extend, recursive);
 }
 
 void OUTLINER_OT_item_activate(wmOperatorType *ot)
@@ -1035,7 +983,7 @@ void OUTLINER_OT_item_activate(wmOperatorType *ot)
 	ot->idname = "OUTLINER_OT_item_activate";
 	ot->description = "Handle mouse clicks to activate/select items";
 	
-	ot->invoke = outliner_item_activate;
+	ot->invoke = outliner_item_activate_invoke;
 	
 	ot->poll = ED_operator_outliner_active;
 	
diff --git a/source/blender/editors/space_outliner/outliner_tools.c b/source/blender/editors/space_outliner/outliner_tools.c
index ecb4949350f..70832a541de 100644
--- a/source/blender/editors/space_outliner/outliner_tools.c
+++ b/source/blender/editors/space_outliner/outliner_tools.c
@@ -380,7 +380,7 @@ static void object_select_hierarchy_cb(
 	wmWindow *win = CTX_wm_window(C);
 	int x = win->eventstate->mval[0];
 	int y = win->eventstate->mval[1];
-	outliner_item_do_activate(C, x, y, true, true);
+	outliner_item_activate_or_toggle_closed(C, x, y, true, true);
 }