From c9cc03b688230d0b898467135e75e2cbac8a3226 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Fri, 6 Nov 2020 22:28:15 +0100 Subject: UI Code Quality: General refactor of Outliner View Layer display mode creation See https://developer.blender.org/D9499. * Turn functions into member functions (makes API for a type more obvious & local, allows implicitly sharing data through member variables, enables order independend definition of functions, allows more natural language for function names because of the obvious context). * Move important variables to classes rather than passing around all the time (shorter, more task-focused code, localizes important data names). * Add helper class for adding object children sub-trees (smaller, more focused units are easier to reason about, have higher coherence, better testability, can manage own resources easily with RAII). * Use C++ iterators over C-macros (arguably more readable, less macros is generally preferred) * Add doxygen groups (visually emphasizes the coherence of code sections, provide place for higher level comments on sections). * Prefer references over pointers for passing by reference (makes clear that NULL is not a valid value and that the current scope is not the owner). --- .../editors/space_outliner/tree/tree_view.cc | 4 +- .../editors/space_outliner/tree/tree_view.hh | 30 +- .../space_outliner/tree/tree_view_view_layer.cc | 352 ++++++++++++--------- 3 files changed, 229 insertions(+), 157 deletions(-) (limited to 'source/blender/editors/space_outliner/tree') diff --git a/source/blender/editors/space_outliner/tree/tree_view.cc b/source/blender/editors/space_outliner/tree/tree_view.cc index de9bc1237b5..8352a23080d 100644 --- a/source/blender/editors/space_outliner/tree/tree_view.cc +++ b/source/blender/editors/space_outliner/tree/tree_view.cc @@ -28,7 +28,7 @@ namespace outliner = blender::outliner; /* Convenience. */ using blender::outliner::AbstractTreeView; -TreeView *outliner_tree_view_create(eSpaceOutliner_Mode mode) +TreeView *outliner_tree_view_create(eSpaceOutliner_Mode mode, SpaceOutliner *space_outliner) { AbstractTreeView *tree_view = nullptr; @@ -40,7 +40,7 @@ TreeView *outliner_tree_view_create(eSpaceOutliner_Mode mode) case SO_ID_ORPHANS: break; case SO_VIEW_LAYER: - tree_view = new outliner::TreeViewViewLayer(); + tree_view = new outliner::TreeViewViewLayer(*space_outliner); break; } diff --git a/source/blender/editors/space_outliner/tree/tree_view.hh b/source/blender/editors/space_outliner/tree/tree_view.hh index b7d71c0b608..57ee7c58c29 100644 --- a/source/blender/editors/space_outliner/tree/tree_view.hh +++ b/source/blender/editors/space_outliner/tree/tree_view.hh @@ -25,6 +25,7 @@ struct bContext; struct ListBase; struct SpaceOutliner; +struct TreeElement; struct TreeSourceData; #ifdef __cplusplus @@ -34,17 +35,40 @@ namespace outliner { using Tree = ListBase; +/* -------------------------------------------------------------------- */ +/* Tree-View Interface */ + class AbstractTreeView { public: + AbstractTreeView(SpaceOutliner &space_outliner) : _space_outliner(space_outliner) + { + } virtual ~AbstractTreeView() = default; /** Build a tree for this view and the current context. */ virtual Tree buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner) = 0; + + protected: + SpaceOutliner &_space_outliner; }; -class TreeViewViewLayer : public AbstractTreeView { +/* -------------------------------------------------------------------- */ +/* View Layer Tree-View */ + +class TreeViewViewLayer final : public AbstractTreeView { public: - Tree buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner) override final; + TreeViewViewLayer(SpaceOutliner &space_outliner); + + Tree buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner) override; + + private: + ViewLayer *_view_layer; + bool _show_objects = true; + + void add_view_layer(ListBase &, TreeElement &); + void add_layer_collections_recursive(ListBase &, ListBase &, TreeElement &); + void add_layer_collection_objects(ListBase &, LayerCollection &, TreeElement &); + void add_layer_collection_objects_children(TreeElement &); }; } // namespace outliner @@ -67,7 +91,7 @@ typedef struct TreeSourceData { struct ViewLayer *view_layer; } TreeSourceData; -TreeView *outliner_tree_view_create(eSpaceOutliner_Mode mode); +TreeView *outliner_tree_view_create(eSpaceOutliner_Mode mode, SpaceOutliner *space_outliner); void outliner_tree_view_destroy(TreeView **tree_view); ListBase outliner_tree_view_build_tree(TreeView *tree_view, diff --git a/source/blender/editors/space_outliner/tree/tree_view_view_layer.cc b/source/blender/editors/space_outliner/tree/tree_view_view_layer.cc index 665e8d1954f..0d35db36e5b 100644 --- a/source/blender/editors/space_outliner/tree/tree_view_view_layer.cc +++ b/source/blender/editors/space_outliner/tree/tree_view_view_layer.cc @@ -26,6 +26,7 @@ #include "BLI_ghash.h" #include "BLI_listbase.h" +#include "BLI_listbase_wrapper.hh" #include "BLT_translation.h" @@ -37,140 +38,100 @@ namespace blender { namespace outliner { -/** - * For all objects in the tree, lookup the parent in this map, - * and move or add tree elements as needed. - */ -static void outliner_make_object_parent_hierarchy_collections(SpaceOutliner *space_outliner, - GHash *object_tree_elements_hash) -{ - GHashIterator gh_iter; - GHASH_ITER (gh_iter, object_tree_elements_hash) { - Object *child = static_cast(BLI_ghashIterator_getKey(&gh_iter)); - - if (child->parent == NULL) { - continue; - } +/* Convenience/readability. */ +template using List = ListBaseWrapper; - ListBase *child_ob_tree_elements = static_cast( - BLI_ghashIterator_getValue(&gh_iter)); - ListBase *parent_ob_tree_elements = static_cast( - BLI_ghash_lookup(object_tree_elements_hash, child->parent)); - if (parent_ob_tree_elements == NULL) { - continue; - } +class ObjectsChildrenBuilder { + public: + ObjectsChildrenBuilder(SpaceOutliner &outliner); + ~ObjectsChildrenBuilder(); - LISTBASE_FOREACH (LinkData *, link, parent_ob_tree_elements) { - TreeElement *parent_ob_tree_element = static_cast(link->data); - TreeElement *parent_ob_collection_tree_element = NULL; - bool found = false; + void operator()(TreeElement &collection_tree_elem); - /* We always want to remove the child from the direct collection its parent is nested under. - * This is particularly important when dealing with multi-level nesting (grandchildren). */ - parent_ob_collection_tree_element = parent_ob_tree_element->parent; - while (!ELEM(TREESTORE(parent_ob_collection_tree_element)->type, - TSE_VIEW_COLLECTION_BASE, - TSE_LAYER_COLLECTION)) { - parent_ob_collection_tree_element = parent_ob_collection_tree_element->parent; - } + private: + SpaceOutliner &_outliner; + GHash &_object_tree_elements_hash; - LISTBASE_FOREACH (LinkData *, link_iter, child_ob_tree_elements) { - TreeElement *child_ob_tree_element = static_cast(link_iter->data); + void object_tree_elements_lookup_create_recursive(TreeElement *te_parent); + void make_object_parent_hierarchy_collections(); +}; - if (child_ob_tree_element->parent == parent_ob_collection_tree_element) { - /* Move from the collection subtree into the parent object subtree. */ - BLI_remlink(&parent_ob_collection_tree_element->subtree, child_ob_tree_element); - BLI_addtail(&parent_ob_tree_element->subtree, child_ob_tree_element); - child_ob_tree_element->parent = parent_ob_tree_element; - found = true; - break; - } - } +/* -------------------------------------------------------------------- */ +/** \name Tree View for a View Layer. + * + * \{ */ - if (!found) { - /* We add the child in the tree even if it is not in the collection. - * We deliberately clear its sub-tree though, to make it less prominent. */ - TreeElement *child_ob_tree_element = outliner_add_element( - space_outliner, &parent_ob_tree_element->subtree, child, parent_ob_tree_element, 0, 0); - outliner_free_tree(&child_ob_tree_element->subtree); - child_ob_tree_element->flag |= TE_CHILD_NOT_IN_COLLECTION; - BLI_addtail(child_ob_tree_elements, BLI_genericNodeN(child_ob_tree_element)); - } - } - } +TreeViewViewLayer::TreeViewViewLayer(SpaceOutliner &space_outliner) + : AbstractTreeView(space_outliner) +{ } -/** - * Build a map from Object* to a list of TreeElement* matching the object. - */ -static void outliner_object_tree_elements_lookup_create_recursive(GHash *object_tree_elements_hash, - TreeElement *te_parent) +Tree TreeViewViewLayer::buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner) { - LISTBASE_FOREACH (TreeElement *, te, &te_parent->subtree) { - TreeStoreElem *tselem = TREESTORE(te); + Tree tree = {nullptr}; - if (tselem->type == TSE_LAYER_COLLECTION) { - outliner_object_tree_elements_lookup_create_recursive(object_tree_elements_hash, te); - } - else if (tselem->type == 0 && te->idcode == ID_OB) { - Object *ob = (Object *)tselem->id; - ListBase *tree_elements = static_cast( - BLI_ghash_lookup(object_tree_elements_hash, ob)); + _view_layer = source_data.view_layer; + _show_objects = !(space_outliner.filter & SO_FILTER_NO_OBJECT); - if (tree_elements == NULL) { - tree_elements = static_cast(MEM_callocN(sizeof(ListBase), __func__)); - BLI_ghash_insert(object_tree_elements_hash, ob, tree_elements); - } + const bool show_children = (space_outliner.filter & SO_FILTER_NO_CHILDREN) == 0; - BLI_addtail(tree_elements, BLI_genericNodeN(te)); - outliner_object_tree_elements_lookup_create_recursive(object_tree_elements_hash, te); + if (space_outliner.filter & SO_FILTER_NO_COLLECTION) { + /* Show objects in the view layer. */ + for (Base *base : List(_view_layer->object_bases)) { + TreeElement *te_object = outliner_add_element( + &space_outliner, &tree, base->object, nullptr, 0, 0); + te_object->directdata = base; + } + + if (show_children) { + outliner_make_object_parent_hierarchy(&tree); } } -} + else { + /* Show collections in the view layer. */ + TreeElement &ten = *outliner_add_element( + &space_outliner, &tree, source_data.scene, nullptr, TSE_VIEW_COLLECTION_BASE, 0); + ten.name = IFACE_("Scene Collection"); + TREESTORE(&ten)->flag &= ~TSE_CLOSED; -static void outliner_object_tree_elements_lookup_free(GHash *object_tree_elements_hash) -{ - GHASH_FOREACH_BEGIN (ListBase *, tree_elements, object_tree_elements_hash) { - BLI_freelistN(tree_elements); - MEM_freeN(tree_elements); + add_view_layer(ten.subtree, ten); + if (show_children) { + add_layer_collection_objects_children(ten); + } } - GHASH_FOREACH_END(); + + return tree; } -static void outliner_add_layer_collection_objects(SpaceOutliner *space_outliner, - ListBase *tree, - ViewLayer *layer, - LayerCollection *lc, - TreeElement *ten) +void TreeViewViewLayer::add_view_layer(ListBase &tree, TreeElement &parent) { - LISTBASE_FOREACH (CollectionObject *, cob, &lc->collection->gobject) { - Base *base = BKE_view_layer_base_find(layer, cob->ob); - TreeElement *te_object = outliner_add_element(space_outliner, tree, base->object, ten, 0, 0); - te_object->directdata = base; + /* First layer collection is for master collection, don't show it. */ + LayerCollection *lc = static_cast(_view_layer->layer_collections.first); + if (lc == nullptr) { + return; + } - if (!(base->flag & BASE_VISIBLE_VIEWLAYER)) { - te_object->flag |= TE_DISABLED; - } + add_layer_collections_recursive(tree, lc->layer_collections, parent); + if (_show_objects) { + add_layer_collection_objects(tree, *lc, parent); } } -static void outliner_add_layer_collections_recursive(SpaceOutliner *space_outliner, - ListBase *tree, - ViewLayer *layer, - ListBase *layer_collections, - TreeElement *parent_ten, - const bool show_objects) +void TreeViewViewLayer::add_layer_collections_recursive(ListBase &tree, + ListBase &layer_collections, + TreeElement &parent_ten) { - LISTBASE_FOREACH (LayerCollection *, lc, layer_collections) { + for (LayerCollection *lc : List(layer_collections)) { const bool exclude = (lc->flag & LAYER_COLLECTION_EXCLUDE) != 0; TreeElement *ten; - if (exclude && ((space_outliner->show_restrict_flags & SO_RESTRICT_ENABLE) == 0)) { - ten = parent_ten; + if (exclude && ((_space_outliner.show_restrict_flags & SO_RESTRICT_ENABLE) == 0)) { + ten = &parent_ten; } else { ID *id = &lc->collection->id; - ten = outliner_add_element(space_outliner, tree, id, parent_ten, TSE_LAYER_COLLECTION, 0); + ten = outliner_add_element( + &_space_outliner, &tree, id, &parent_ten, TSE_LAYER_COLLECTION, 0); ten->name = id->name + 2; ten->directdata = lc; @@ -186,73 +147,160 @@ static void outliner_add_layer_collections_recursive(SpaceOutliner *space_outlin } } - outliner_add_layer_collections_recursive( - space_outliner, &ten->subtree, layer, &lc->layer_collections, ten, show_objects); - if (!exclude && show_objects) { - outliner_add_layer_collection_objects(space_outliner, &ten->subtree, layer, lc, ten); + add_layer_collections_recursive(ten->subtree, lc->layer_collections, *ten); + if (!exclude && _show_objects) { + add_layer_collection_objects(ten->subtree, *lc, *ten); } } } -static void outliner_add_view_layer(SpaceOutliner *space_outliner, - ListBase *tree, - TreeElement *parent, - ViewLayer *layer, - const bool show_objects) +void TreeViewViewLayer::add_layer_collection_objects(ListBase &tree, + LayerCollection &lc, + TreeElement &ten) { - /* First layer collection is for master collection, don't show it. */ - LayerCollection *lc = static_cast(layer->layer_collections.first); - if (lc == NULL) { - return; + for (CollectionObject *cob : List(lc.collection->gobject)) { + Base *base = BKE_view_layer_base_find(_view_layer, cob->ob); + TreeElement *te_object = outliner_add_element( + &_space_outliner, &tree, base->object, &ten, 0, 0); + te_object->directdata = base; + + if (!(base->flag & BASE_VISIBLE_VIEWLAYER)) { + te_object->flag |= TE_DISABLED; + } } +} - outliner_add_layer_collections_recursive( - space_outliner, tree, layer, &lc->layer_collections, parent, show_objects); - if (show_objects) { - outliner_add_layer_collection_objects(space_outliner, tree, layer, lc, parent); +void TreeViewViewLayer::add_layer_collection_objects_children(TreeElement &collection_tree_elem) +{ + /* Call helper to add children. */ + ObjectsChildrenBuilder child_builder{_space_outliner}; + child_builder(collection_tree_elem); +} + +/** \} */ + +/* -------------------------------------------------------------------- */ +/** \name Object Chilren helper. + * + * Helper to add child objects to the sub-tree of their parent, recursively covering all nested + * collections. + * + * \{ */ + +ObjectsChildrenBuilder::ObjectsChildrenBuilder(SpaceOutliner &outliner) + : _outliner(outliner), + _object_tree_elements_hash( + *BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__)) +{ +} + +ObjectsChildrenBuilder::~ObjectsChildrenBuilder() +{ + GHASH_FOREACH_BEGIN (ListBase *, tree_elements, &_object_tree_elements_hash) { + BLI_freelistN(tree_elements); + MEM_freeN(tree_elements); } + GHASH_FOREACH_END(); + + BLI_ghash_free(&_object_tree_elements_hash, nullptr, nullptr); } -Tree TreeViewViewLayer::buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner) +void ObjectsChildrenBuilder::operator()(TreeElement &collection_tree_elem) { - Tree tree = {nullptr}; + object_tree_elements_lookup_create_recursive(&collection_tree_elem); + make_object_parent_hierarchy_collections(); +} - if (space_outliner.filter & SO_FILTER_NO_COLLECTION) { - /* Show objects in the view layer. */ - LISTBASE_FOREACH (Base *, base, &source_data.view_layer->object_bases) { - TreeElement *te_object = outliner_add_element( - &space_outliner, &tree, base->object, nullptr, 0, 0); - te_object->directdata = base; +/** + * Build a map from Object* to a list of TreeElement* matching the object. + */ +void ObjectsChildrenBuilder::object_tree_elements_lookup_create_recursive(TreeElement *te_parent) +{ + for (TreeElement *te : List(te_parent->subtree)) { + TreeStoreElem *tselem = TREESTORE(te); + + if (tselem->type == TSE_LAYER_COLLECTION) { + object_tree_elements_lookup_create_recursive(te); } + else if (tselem->type == 0 && te->idcode == ID_OB) { + Object *ob = (Object *)tselem->id; + ListBase *tree_elements = static_cast( + BLI_ghash_lookup(&_object_tree_elements_hash, ob)); - if ((space_outliner.filter & SO_FILTER_NO_CHILDREN) == 0) { - outliner_make_object_parent_hierarchy(&tree); + if (tree_elements == nullptr) { + tree_elements = static_cast(MEM_callocN(sizeof(ListBase), __func__)); + BLI_ghash_insert(&_object_tree_elements_hash, ob, tree_elements); + } + + BLI_addtail(tree_elements, BLI_genericNodeN(te)); + object_tree_elements_lookup_create_recursive(te); } } - else { - /* Show collections in the view layer. */ - TreeElement *ten = outliner_add_element( - &space_outliner, &tree, source_data.scene, nullptr, TSE_VIEW_COLLECTION_BASE, 0); - ten->name = IFACE_("Scene Collection"); - TREESTORE(ten)->flag &= ~TSE_CLOSED; - - bool show_objects = !(space_outliner.filter & SO_FILTER_NO_OBJECT); - outliner_add_view_layer( - &space_outliner, &ten->subtree, ten, source_data.view_layer, show_objects); - - if ((space_outliner.filter & SO_FILTER_NO_CHILDREN) == 0) { - GHash *object_tree_elements_hash = BLI_ghash_new( - BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__); - outliner_object_tree_elements_lookup_create_recursive(object_tree_elements_hash, ten); - outliner_make_object_parent_hierarchy_collections(&space_outliner, - object_tree_elements_hash); - outliner_object_tree_elements_lookup_free(object_tree_elements_hash); - BLI_ghash_free(object_tree_elements_hash, nullptr, nullptr); +} + +/** + * For all objects in the tree, lookup the parent in this map, + * and move or add tree elements as needed. + */ +void ObjectsChildrenBuilder::make_object_parent_hierarchy_collections() +{ + GHashIterator gh_iter; + GHASH_ITER (gh_iter, &_object_tree_elements_hash) { + Object *child = static_cast(BLI_ghashIterator_getKey(&gh_iter)); + + if (child->parent == nullptr) { + continue; } - } - return tree; + ListBase *child_ob_tree_elements = static_cast( + BLI_ghashIterator_getValue(&gh_iter)); + ListBase *parent_ob_tree_elements = static_cast( + BLI_ghash_lookup(&_object_tree_elements_hash, child->parent)); + if (parent_ob_tree_elements == nullptr) { + continue; + } + + for (LinkData *link : List(parent_ob_tree_elements)) { + TreeElement *parent_ob_tree_element = static_cast(link->data); + TreeElement *parent_ob_collection_tree_element = nullptr; + bool found = false; + + /* We always want to remove the child from the direct collection its parent is nested under. + * This is particularly important when dealing with multi-level nesting (grandchildren). */ + parent_ob_collection_tree_element = parent_ob_tree_element->parent; + while (!ELEM(TREESTORE(parent_ob_collection_tree_element)->type, + TSE_VIEW_COLLECTION_BASE, + TSE_LAYER_COLLECTION)) { + parent_ob_collection_tree_element = parent_ob_collection_tree_element->parent; + } + + for (LinkData *link_iter : List(child_ob_tree_elements)) { + TreeElement *child_ob_tree_element = static_cast(link_iter->data); + + if (child_ob_tree_element->parent == parent_ob_collection_tree_element) { + /* Move from the collection subtree into the parent object subtree. */ + BLI_remlink(&parent_ob_collection_tree_element->subtree, child_ob_tree_element); + BLI_addtail(&parent_ob_tree_element->subtree, child_ob_tree_element); + child_ob_tree_element->parent = parent_ob_tree_element; + found = true; + break; + } + } + + if (!found) { + /* We add the child in the tree even if it is not in the collection. + * We deliberately clear its sub-tree though, to make it less prominent. */ + TreeElement *child_ob_tree_element = outliner_add_element( + &_outliner, &parent_ob_tree_element->subtree, child, parent_ob_tree_element, 0, 0); + outliner_free_tree(&child_ob_tree_element->subtree); + child_ob_tree_element->flag |= TE_CHILD_NOT_IN_COLLECTION; + BLI_addtail(child_ob_tree_elements, BLI_genericNodeN(child_ob_tree_element)); + } + } + } } +/** \} */ + } // namespace outliner } // namespace blender -- cgit v1.2.3