blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 777 Projects 19 Wiki Activity

Outliner: Double-click on item icon to select contents/hierarchy #110151

Merged
Harley Acheson merged 12 commits from mgradysaunders/blender:temp-outliner-selection into main 2024-01-17 00:45:52 +01:00
Conversation 18 Commits 12 Files Changed 4 +12 -13
1 changed files with 12 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 9cfa701a8a - Show all commits

25
source/blender/editors/space_outliner/outliner_select.cc
View File

@ -1569,14 +1569,13 @@ void outliner_item_select(bContext *C,
}
}
/* A simple but apparently effective solution for recursive selections. */
static void do_outliner_select_recursive(ListBase *lb, bool selecting, Collection *in_collection)
{
LISTBASE_FOREACH (TreeElement *, te, lb) {
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-09-22 18:18:30 +02:00
Outdated
Review
Copy Link

This comment doesn't really add much, would remove it.

This comment doesn't really add much, would remove it.
TreeStoreElem *tselem = TREESTORE(te);
/* The desired behavior is only to select collections and object hierarchies
recursively. So if this isn't a collection or an object, skip it. */
* recursively. So if this isn't a collection or an object, skip it. */
if (tselem->type == TSE_LAYER_COLLECTION) {
tselem->flag = selecting ? (tselem->flag | TSE_SELECTED) : (tselem->flag & ~TSE_SELECTED);
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-09-22 18:41:26 +02:00
Outdated
Review
Copy Link

Code-style: Every multi-line comment should have a * in new lines, see https://wiki.blender.org/wiki/Style_Guide/C_Cpp#C.2FC.2B.2B_Comments

Code-style: Every multi-line comment should have a `*` in new lines, see https://wiki.blender.org/wiki/Style_Guide/C_Cpp#C.2FC.2B.2B_Comments
/* Restrict sub-tree selections to this collection. This prevents undesirable behavior in
@ -1640,12 +1639,12 @@ static bool do_outliner_range_select_recursive(ListBase *lb,
if (!(tselem->flag & TSE_CLOSED) || recurse) {
/* If this tree element is a collection, then it sets the precedent for inclusion
of its subobjects. */
Collection *in_collection2 = in_collection;
Collection *child_collection = in_collection;
Harley marked this conversation as resolved Outdated
Pratik Borhade commented 2023-08-20 08:56:04 +02:00
Outdated
Review
Copy Link

There is code duplication.
in_collection is already found for active tree element in outliner_item_do_activate_from_cursor. So just pass it as argument to this function?

There is code duplication. `in_collection` is already found for active tree element in `outliner_item_do_activate_from_cursor`. So just pass it as argument to this function?
if (tselem->type == TSE_LAYER_COLLECTION) {
in_collection2 = static_cast<LayerCollection *>(te->directdata)->collection;
child_collection = static_cast<LayerCollection *>(te->directdata)->collection;
}
Harley marked this conversation as resolved
Julian Eisel commented 2023-09-22 18:45:20 +02:00
Review
Copy Link

Could use a better name, like nested_collection or child_collection.

Could use a better name, like `nested_collection` or `child_collection`.
selecting = do_outliner_range_select_recursive(
Harley marked this conversation as resolved Outdated
Pratik Borhade commented 2024-01-12 13:18:05 +01:00
Outdated
Review
Copy Link

Maybe we can wrap this inside if(can_select). Otherwise unnecessarily we will iterate the children of object that are not present in current collection

Maybe we can wrap this inside `if(can_select)`. Otherwise unnecessarily we will iterate the children of object that are not present in current collection
&te->subtree, active, cursor, selecting, recurse, in_collection2);
&te->subtree, active, cursor, selecting, recurse, child_collection);
}
}
@ -1777,13 +1776,13 @@ static int outliner_item_do_activate_from_cursor(bContext *C,
to prevent selecting across collection boundaries. (Object hierarchies might cross
collection boundaries, i.e., children may be in different collections from their
parents.) */
Collection *in_collection = nullptr;
Collection *parent_collection = nullptr;
if (recurse) {
if (activate_tselem->type == TSE_LAYER_COLLECTION) {
in_collection = static_cast<LayerCollection *>(activate_te->directdata)->collection;
parent_collection = static_cast<LayerCollection *>(activate_te->directdata)->collection;
Harley marked this conversation as resolved Outdated
Julian Eisel commented 2023-09-22 18:50:43 +02:00
Outdated
Review
Copy Link

in_collection sounds like a boolean variable. Maybe parent_collection?

`in_collection` sounds like a boolean variable. Maybe `parent_collection`?
}
else if (activate_tselem->type == TSE_SOME_ID && activate_te->idcode == ID_OB) {
in_collection = BKE_collection_object_find(
parent_collection = BKE_collection_object_find(
CTX_data_main(C),
CTX_data_scene(C),
nullptr,
@ -1802,9 +1801,9 @@ static int outliner_item_do_activate_from_cursor(bContext *C,
}
if (use_range) {
do_outliner_range_select(C, space_outliner, activate_te, extend, recurse, in_collection);
do_outliner_range_select(C, space_outliner, activate_te, extend, recurse, parent_collection);
if (recurse) {
do_outliner_select_recursive(&activate_te->subtree, /*selecting=*/true, in_collection);
do_outliner_select_recursive(&activate_te->subtree, /*selecting=*/true, parent_collection);
}
}
else {
@ -1812,8 +1811,8 @@ static int outliner_item_do_activate_from_cursor(bContext *C,
view_mval[0]);
/* Always select unless already active and selected */
Harley marked this conversation as resolved Outdated
Pratik Borhade commented 2023-08-21 14:40:40 +02:00
Outdated
Review
Copy Link

use brackets {}

use brackets {}
bool select = !extend ||
!(activate_tselem->flag & TSE_ACTIVE && activate_tselem->flag & TSE_SELECTED);
bool select = !extend || !(activate_tselem->flag & TSE_ACTIVE) ||
!(activate_tselem->flag & TSE_SELECTED);
/* If we're CTRL+double-clicking and the element is aleady selected, skip the activation
straight to deselection */
Harley marked this conversation as resolved Outdated
Pratik Borhade commented 2023-08-21 14:27:27 +02:00
Outdated
Review
Copy Link

use brackets {}

use brackets {}
Julian Eisel commented 2023-09-22 18:48:28 +02:00
Outdated
Review
Copy Link

I think this reads easier:

bool select = !extend || !(activate_tselem->flag & TSE_ACTIVE) || ! (activate_tselem->flag & TSE_SELECTED);
I think this reads easier: ``` bool select = !extend || !(activate_tselem->flag & TSE_ACTIVE) || ! (activate_tselem->flag & TSE_SELECTED); ```
@ -1832,7 +1831,7 @@ static int outliner_item_do_activate_from_cursor(bContext *C,
/* Select or deselect object hierarchy recursively. */
outliner_item_select(C, space_outliner, activate_te, select_flag);
do_outliner_select_recursive(&activate_te->subtree, select, in_collection);
do_outliner_select_recursive(&activate_te->subtree, select, parent_collection);
}
else {
/* Double-clicked, but it wasn't on the icon. */