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).

source/blender/editors/space_outliner/outliner_tree.c

@@ -2321,7 +2321,8 @@ void outliner_build_tree(Main *mainvar,
   outliner_tree_view_destroy(&space_outliner->runtime->tree_view);
 
   TreeSourceData source_data = {.bmain = mainvar, .scene = scene, .view_layer = view_layer};
-  space_outliner->runtime->tree_view = outliner_tree_view_create(space_outliner->outlinevis);
+  space_outliner->runtime->tree_view = outliner_tree_view_create(space_outliner->outlinevis,
+                                                                 space_outliner);
   space_outliner->tree = outliner_tree_view_build_tree(
       space_outliner->runtime->tree_view, &source_data, space_outliner);
 

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;
   }
 

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,

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,32 +38,231 @@
 namespace blender {
 namespace outliner {
 
+/* Convenience/readability. */
+template<typename T> using List = ListBaseWrapper<T>;
+
+class ObjectsChildrenBuilder {
+ public:
+  ObjectsChildrenBuilder(SpaceOutliner &outliner);
+  ~ObjectsChildrenBuilder();
+
+  void operator()(TreeElement &collection_tree_elem);
+
+ private:
+  SpaceOutliner &_outliner;
+  GHash &_object_tree_elements_hash;
+
+  void object_tree_elements_lookup_create_recursive(TreeElement *te_parent);
+  void make_object_parent_hierarchy_collections();
+};
+
+/* -------------------------------------------------------------------- */
+/** \name Tree View for a View Layer.
+ *
+ * \{ */
+
+TreeViewViewLayer::TreeViewViewLayer(SpaceOutliner &space_outliner)
+    : AbstractTreeView(space_outliner)
+{
+}
+
+Tree TreeViewViewLayer::buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner)
+{
+  Tree tree = {nullptr};
+
+  _view_layer = source_data.view_layer;
+  _show_objects = !(space_outliner.filter & SO_FILTER_NO_OBJECT);
+
+  const bool show_children = (space_outliner.filter & SO_FILTER_NO_CHILDREN) == 0;
+
+  if (space_outliner.filter & SO_FILTER_NO_COLLECTION) {
+    /* Show objects in the view layer. */
+    for (Base *base : List<Base>(_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;
+
+    add_view_layer(ten.subtree, ten);
+    if (show_children) {
+      add_layer_collection_objects_children(ten);
+    }
+  }
+
+  return tree;
+}
+
+void TreeViewViewLayer::add_view_layer(ListBase &tree, TreeElement &parent)
+{
+  /* First layer collection is for master collection, don't show it. */
+  LayerCollection *lc = static_cast<LayerCollection *>(_view_layer->layer_collections.first);
+  if (lc == nullptr) {
+    return;
+  }
+
+  add_layer_collections_recursive(tree, lc->layer_collections, parent);
+  if (_show_objects) {
+    add_layer_collection_objects(tree, *lc, parent);
+  }
+}
+
+void TreeViewViewLayer::add_layer_collections_recursive(ListBase &tree,
+                                                        ListBase &layer_collections,
+                                                        TreeElement &parent_ten)
+{
+  for (LayerCollection *lc : List<LayerCollection>(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;
+    }
+    else {
+      ID *id = &lc->collection->id;
+      ten = outliner_add_element(
+          &_space_outliner, &tree, id, &parent_ten, TSE_LAYER_COLLECTION, 0);
+
+      ten->name = id->name + 2;
+      ten->directdata = lc;
+
+      /* Open by default, except linked collections, which may contain many elements. */
+      TreeStoreElem *tselem = TREESTORE(ten);
+      if (!(tselem->used || ID_IS_LINKED(id) || ID_IS_OVERRIDE_LIBRARY(id))) {
+        tselem->flag &= ~TSE_CLOSED;
+      }
+
+      if (exclude || (lc->runtime_flag & LAYER_COLLECTION_VISIBLE_VIEW_LAYER) == 0) {
+        ten->flag |= TE_DISABLED;
+      }
+    }
+
+    add_layer_collections_recursive(ten->subtree, lc->layer_collections, *ten);
+    if (!exclude && _show_objects) {
+      add_layer_collection_objects(ten->subtree, *lc, *ten);
+    }
+  }
+}
+
+void TreeViewViewLayer::add_layer_collection_objects(ListBase &tree,
+                                                     LayerCollection &lc,
+                                                     TreeElement &ten)
+{
+  for (CollectionObject *cob : List<CollectionObject>(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;
+    }
+  }
+}
+
+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);
+}
+
+void ObjectsChildrenBuilder::operator()(TreeElement &collection_tree_elem)
+{
+  object_tree_elements_lookup_create_recursive(&collection_tree_elem);
+  make_object_parent_hierarchy_collections();
+}
+
+/**
+ * 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<TreeElement>(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<ListBase *>(
+          BLI_ghash_lookup(&_object_tree_elements_hash, ob));
+
+      if (tree_elements == nullptr) {
+        tree_elements = static_cast<ListBase *>(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);
+    }
+  }
+}
+
 /**
  * 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)
+void ObjectsChildrenBuilder::make_object_parent_hierarchy_collections()
 {
   GHashIterator gh_iter;
-  GHASH_ITER (gh_iter, object_tree_elements_hash) {
+  GHASH_ITER (gh_iter, &_object_tree_elements_hash) {
     Object *child = static_cast<Object *>(BLI_ghashIterator_getKey(&gh_iter));
 
-    if (child->parent == NULL) {
+    if (child->parent == nullptr) {
       continue;
     }
 
     ListBase *child_ob_tree_elements = static_cast<ListBase *>(
         BLI_ghashIterator_getValue(&gh_iter));
     ListBase *parent_ob_tree_elements = static_cast<ListBase *>(
-        BLI_ghash_lookup(object_tree_elements_hash, child->parent));
-    if (parent_ob_tree_elements == NULL) {
+        BLI_ghash_lookup(&_object_tree_elements_hash, child->parent));
+    if (parent_ob_tree_elements == nullptr) {
       continue;
     }
 
-    LISTBASE_FOREACH (LinkData *, link, parent_ob_tree_elements) {
+    for (LinkData *link : List<LinkData>(parent_ob_tree_elements)) {
       TreeElement *parent_ob_tree_element = static_cast<TreeElement *>(link->data);
-      TreeElement *parent_ob_collection_tree_element = NULL;
+      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.
@@ -74,7 +274,7 @@ static void outliner_make_object_parent_hierarchy_collections(SpaceOutliner *spa
         parent_ob_collection_tree_element = parent_ob_collection_tree_element->parent;
       }
 
-      LISTBASE_FOREACH (LinkData *, link_iter, child_ob_tree_elements) {
+      for (LinkData *link_iter : List<LinkData>(child_ob_tree_elements)) {
         TreeElement *child_ob_tree_element = static_cast<TreeElement *>(link_iter->data);
 
         if (child_ob_tree_element->parent == parent_ob_collection_tree_element) {
@@ -91,7 +291,7 @@ static void outliner_make_object_parent_hierarchy_collections(SpaceOutliner *spa
         /* 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, &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));
@@ -100,159 +300,7 @@ static void outliner_make_object_parent_hierarchy_collections(SpaceOutliner *spa
   }
 }
 
-/**
- * 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)
-{
-  LISTBASE_FOREACH (TreeElement *, te, &te_parent->subtree) {
-    TreeStoreElem *tselem = TREESTORE(te);
-
-    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<ListBase *>(
-          BLI_ghash_lookup(object_tree_elements_hash, ob));
-
-      if (tree_elements == NULL) {
-        tree_elements = static_cast<ListBase *>(MEM_callocN(sizeof(ListBase), __func__));
-        BLI_ghash_insert(object_tree_elements_hash, ob, tree_elements);
-      }
-
-      BLI_addtail(tree_elements, BLI_genericNodeN(te));
-      outliner_object_tree_elements_lookup_create_recursive(object_tree_elements_hash, te);
-    }
-  }
-}
-
-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);
-  }
-  GHASH_FOREACH_END();
-}
-
-static void outliner_add_layer_collection_objects(SpaceOutliner *space_outliner,
-                                                  ListBase *tree,
-                                                  ViewLayer *layer,
-                                                  LayerCollection *lc,
-                                                  TreeElement *ten)
-{
-  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;
-
-    if (!(base->flag & BASE_VISIBLE_VIEWLAYER)) {
-      te_object->flag |= TE_DISABLED;
-    }
-  }
-}
-
-static void outliner_add_layer_collections_recursive(SpaceOutliner *space_outliner,
-                                                     ListBase *tree,
-                                                     ViewLayer *layer,
-                                                     ListBase *layer_collections,
-                                                     TreeElement *parent_ten,
-                                                     const bool show_objects)
-{
-  LISTBASE_FOREACH (LayerCollection *, lc, 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;
-    }
-    else {
-      ID *id = &lc->collection->id;
-      ten = outliner_add_element(space_outliner, tree, id, parent_ten, TSE_LAYER_COLLECTION, 0);
-
-      ten->name = id->name + 2;
-      ten->directdata = lc;
-
-      /* Open by default, except linked collections, which may contain many elements. */
-      TreeStoreElem *tselem = TREESTORE(ten);
-      if (!(tselem->used || ID_IS_LINKED(id) || ID_IS_OVERRIDE_LIBRARY(id))) {
-        tselem->flag &= ~TSE_CLOSED;
-      }
-
-      if (exclude || (lc->runtime_flag & LAYER_COLLECTION_VISIBLE_VIEW_LAYER) == 0) {
-        ten->flag |= TE_DISABLED;
-      }
-    }
-
-    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);
-    }
-  }
-}
-
-static void outliner_add_view_layer(SpaceOutliner *space_outliner,
-                                    ListBase *tree,
-                                    TreeElement *parent,
-                                    ViewLayer *layer,
-                                    const bool show_objects)
-{
-  /* First layer collection is for master collection, don't show it. */
-  LayerCollection *lc = static_cast<LayerCollection *>(layer->layer_collections.first);
-  if (lc == NULL) {
-    return;
-  }
-
-  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);
-  }
-}
-
-Tree TreeViewViewLayer::buildTree(const TreeSourceData &source_data, SpaceOutliner &space_outliner)
-{
-  Tree tree = {nullptr};
-
-  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;
-    }
-
-    if ((space_outliner.filter & SO_FILTER_NO_CHILDREN) == 0) {
-      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;
-
-    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);
-    }
-  }
-
-  return tree;
-}
+/** \} */
 
 }  // namespace outliner
 }  // namespace blender