Cleanup: General cleanup of Outliner Blender File display mode building

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).
* 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).
* Reduce indentation levels, use `continue` in loops to ensure preconditions
  are met.
* Add asserts for sanity checks.

2 files changed, 131 insertions, 114 deletions

diff --git a/source/blender/editors/space_outliner/tree/tree_view.hh b/source/blender/editors/space_outliner/tree/tree_view.hh
index cb3bb4681c8..a9aed29abc1 100644
--- a/source/blender/editors/space_outliner/tree/tree_view.hh
+++ b/source/blender/editors/space_outliner/tree/tree_view.hh
@@ -26,6 +26,7 @@
 #include "DNA_space_types.h"
 
 struct ListBase;
+struct Main;
 struct SpaceOutliner;
 struct TreeElement;
 struct TreeSourceData;
@@ -100,6 +101,9 @@ class TreeViewLibraries final : public AbstractTreeView {
   ListBase buildTree(const TreeSourceData &source_data) override;
 
  private:
+  TreeElement *add_library_contents(Main &, ListBase &, Library *) const;
+  bool library_id_filter_poll(Library *lib, ID *id) const;
+  short id_filter_get() const;
 };
 
 }  // namespace outliner
diff --git a/source/blender/editors/space_outliner/tree/tree_view_libraries.cc b/source/blender/editors/space_outliner/tree/tree_view_libraries.cc
index 83f4b93df22..77fa1fd3990 100644
--- a/source/blender/editors/space_outliner/tree/tree_view_libraries.cc
+++ b/source/blender/editors/space_outliner/tree/tree_view_libraries.cc
@@ -41,95 +41,134 @@ TreeViewLibraries::TreeViewLibraries(SpaceOutliner &space_outliner)
 {
 }
 
-static bool outliner_library_id_show(Library *lib, ID *id, short filter_id_type)
+ListBase TreeViewLibraries::buildTree(const TreeSourceData &source_data)
 {
-  if (id->lib != lib) {
-    return false;
-  }
+  ListBase tree = {nullptr};
 
-  if (filter_id_type == ID_GR) {
-    /* Don't show child collections of non-scene master collection,
-     * they are already shown as children. */
-    Collection *collection = (Collection *)id;
-    bool has_non_scene_parent = false;
+  {
+    /* current file first - mainvar provides tselem with unique pointer - not used */
+    TreeElement *ten = add_library_contents(*source_data.bmain, tree, nullptr);
+    TreeStoreElem *tselem;
 
-    for (CollectionParent *cparent : List<CollectionParent>(collection->parents)) {
-      if (!(cparent->collection->flag & COLLECTION_IS_MASTER)) {
-        has_non_scene_parent = true;
+    if (ten) {
+      tselem = TREESTORE(ten);
+      if (!tselem->used) {
+        tselem->flag &= ~TSE_CLOSED;
       }
     }
+  }
 
-    if (has_non_scene_parent) {
-      return false;
+  for (ID *id : List<ID>(source_data.bmain->libraries)) {
+    Library *lib = reinterpret_cast<Library *>(id);
+    TreeElement *ten = add_library_contents(*source_data.bmain, tree, lib);
+    /* NULL-check matters, due to filtering there may not be a new element. */
+    if (ten) {
+      lib->id.newid = (ID *)ten;
     }
   }
 
-  return true;
+  /* make hierarchy */
+  for (TreeElement *ten : List<TreeElement>(tree)) {
+    if (ten == tree.first) {
+      /* First item is main, skip. */
+      continue;
+    }
+
+    TreeStoreElem *tselem = TREESTORE(ten);
+    Library *lib = (Library *)tselem->id;
+    BLI_assert(!lib || (GS(lib->id.name) == ID_LI));
+    if (!lib || !lib->parent) {
+      continue;
+    }
+
+    TreeElement *parent = (TreeElement *)lib->parent->id.newid;
+
+    if (tselem->id->tag & LIB_TAG_INDIRECT) {
+      /* Only remove from 'first level' if lib is not also directly used. */
+      BLI_remlink(&tree, ten);
+      BLI_addtail(&parent->subtree, ten);
+      ten->parent = parent;
+    }
+    else {
+      /* Else, make a new copy of the libtree for our parent. */
+      TreeElement *dupten = add_library_contents(*source_data.bmain, parent->subtree, lib);
+      if (dupten) {
+        dupten->parent = parent;
+      }
+    }
+  }
+  /* restore newid pointers */
+  for (ID *library_id : List<ID>(source_data.bmain->libraries)) {
+    library_id->newid = nullptr;
+  }
+
+  return tree;
 }
 
-static TreeElement *outliner_add_library_contents(Main *mainvar,
-                                                  SpaceOutliner *space_outliner,
-                                                  ListBase *lb,
-                                                  Library *lib)
+TreeElement *TreeViewLibraries::add_library_contents(Main &mainvar,
+                                                     ListBase &lb,
+                                                     Library *lib) const
 {
-  TreeElement *ten, *tenlib = nullptr;
-  ListBase *lbarray[MAX_LIBARRAY];
-  int a, tot;
-  short filter_id_type = (space_outliner->filter & SO_FILTER_ID_TYPE) ?
-                             space_outliner->filter_id_type :
-                             0;
+  const short filter_id_type = id_filter_get();
 
+  ListBase *lbarray[MAX_LIBARRAY];
+  int tot;
   if (filter_id_type) {
-    lbarray[0] = which_libbase(mainvar, space_outliner->filter_id_type);
+    lbarray[0] = which_libbase(&mainvar, _space_outliner.filter_id_type);
     tot = 1;
   }
   else {
-    tot = set_listbasepointers(mainvar, lbarray);
+    tot = set_listbasepointers(&mainvar, lbarray);
   }
 
-  for (a = 0; a < tot; a++) {
-    if (lbarray[a] && lbarray[a]->first) {
-      ID *id = static_cast<ID *>(lbarray[a]->first);
-      const bool is_library = (GS(id->name) == ID_LI) && (lib != nullptr);
+  TreeElement *tenlib = nullptr;
+  for (int a = 0; a < tot; a++) {
+    if (!lbarray[a] || !lbarray[a]->first) {
+      continue;
+    }
+
+    ID *id = static_cast<ID *>(lbarray[a]->first);
+    const bool is_library = (GS(id->name) == ID_LI) && (lib != nullptr);
 
-      /* check if there's data in current lib */
-      for (ID *id_iter : List<ID>(lbarray[a])) {
-        if (id_iter->lib == lib) {
-          id = id_iter;
-          break;
-        }
+    /* check if there's data in current lib */
+    for (ID *id_iter : List<ID>(lbarray[a])) {
+      if (id_iter->lib == lib) {
+        id = id_iter;
+        break;
       }
+    }
 
-      /* We always want to create an entry for libraries, even if/when we have no more IDs from
-       * them. This invalid state is important to show to user as well.*/
-      if (id != nullptr || is_library) {
-        if (!tenlib) {
-          /* Create library tree element on demand, depending if there are any data-blocks. */
-          if (lib) {
-            tenlib = outliner_add_element(space_outliner, lb, lib, nullptr, 0, 0);
-          }
-          else {
-            tenlib = outliner_add_element(space_outliner, lb, mainvar, nullptr, TSE_ID_BASE, 0);
-            tenlib->name = IFACE_("Current File");
-          }
+    /* We always want to create an entry for libraries, even if/when we have no more IDs from
+     * them. This invalid state is important to show to user as well.*/
+    if (id != nullptr || is_library) {
+      if (!tenlib) {
+        /* Create library tree element on demand, depending if there are any data-blocks. */
+        if (lib) {
+          tenlib = outliner_add_element(&_space_outliner, &lb, lib, nullptr, 0, 0);
+        }
+        else {
+          tenlib = outliner_add_element(&_space_outliner, &lb, &mainvar, nullptr, TSE_ID_BASE, 0);
+          tenlib->name = IFACE_("Current File");
         }
+      }
 
-        /* Create data-block list parent element on demand. */
-        if (id != nullptr) {
-          if (filter_id_type) {
-            ten = tenlib;
-          }
-          else {
-            ten = outliner_add_element(
-                space_outliner, &tenlib->subtree, lbarray[a], nullptr, TSE_ID_BASE, 0);
-            ten->directdata = lbarray[a];
-            ten->name = outliner_idcode_to_plural(GS(id->name));
-          }
+      /* Create data-block list parent element on demand. */
+      if (id != nullptr) {
+        TreeElement *ten;
+
+        if (filter_id_type) {
+          ten = tenlib;
+        }
+        else {
+          ten = outliner_add_element(
+              &_space_outliner, &tenlib->subtree, lbarray[a], nullptr, TSE_ID_BASE, 0);
+          ten->directdata = lbarray[a];
+          ten->name = outliner_idcode_to_plural(GS(id->name));
+        }
 
-          for (ID *id : List<ID>(lbarray[a])) {
-            if (outliner_library_id_show(lib, id, filter_id_type)) {
-              outliner_add_element(space_outliner, &ten->subtree, id, ten, 0, 0);
-            }
+        for (ID *id : List<ID>(lbarray[a])) {
+          if (library_id_filter_poll(lib, id)) {
+            outliner_add_element(&_space_outliner, &ten->subtree, id, ten, 0, 0);
           }
         }
       }
@@ -139,64 +178,38 @@ static TreeElement *outliner_add_library_contents(Main *mainvar,
   return tenlib;
 }
 
-ListBase TreeViewLibraries::buildTree(const TreeSourceData &source_data)
+short TreeViewLibraries::id_filter_get() const
 {
-  ListBase tree = {nullptr};
-
-  /* current file first - mainvar provides tselem with unique pointer - not used */
-  TreeElement *ten = outliner_add_library_contents(
-      source_data.bmain, &_space_outliner, &tree, nullptr);
-  TreeStoreElem *tselem;
-
-  if (ten) {
-    tselem = TREESTORE(ten);
-    if (!tselem->used) {
-      tselem->flag &= ~TSE_CLOSED;
-    }
+  if (_space_outliner.filter & SO_FILTER_ID_TYPE) {
+    return _space_outliner.filter_id_type;
   }
+  return 0;
+}
 
-  for (ID *id : List<ID>(source_data.bmain->libraries)) {
-    Library *lib = reinterpret_cast<Library *>(id);
-    ten = outliner_add_library_contents(source_data.bmain, &_space_outliner, &tree, lib);
-    /* NULL-check matters, due to filtering there may not be a new element. */
-    if (ten) {
-      lib->id.newid = (ID *)ten;
-    }
+bool TreeViewLibraries::library_id_filter_poll(Library *lib, ID *id) const
+{
+  if (id->lib != lib) {
+    return false;
   }
-  /* make hierarchy */
-  ten = static_cast<TreeElement *>(tree.first);
-  if (ten != nullptr) {
-    ten = ten->next; /* first one is main */
-    while (ten) {
-      TreeElement *nten = ten->next, *par;
-      tselem = TREESTORE(ten);
-      Library *lib = (Library *)tselem->id;
-      if (lib && lib->parent) {
-        par = (TreeElement *)lib->parent->id.newid;
-        if (tselem->id->tag & LIB_TAG_INDIRECT) {
-          /* Only remove from 'first level' if lib is not also directly used. */
-          BLI_remlink(&tree, ten);
-          BLI_addtail(&par->subtree, ten);
-          ten->parent = par;
-        }
-        else {
-          /* Else, make a new copy of the libtree for our parent. */
-          TreeElement *dupten = outliner_add_library_contents(
-              source_data.bmain, &_space_outliner, &par->subtree, lib);
-          if (dupten) {
-            dupten->parent = par;
-          }
-        }
+
+  if (id_filter_get() == ID_GR) {
+    /* Don't show child collections of non-scene master collection,
+     * they are already shown as children. */
+    Collection *collection = (Collection *)id;
+    bool has_non_scene_parent = false;
+
+    for (CollectionParent *cparent : List<CollectionParent>(collection->parents)) {
+      if (!(cparent->collection->flag & COLLECTION_IS_MASTER)) {
+        has_non_scene_parent = true;
       }
-      ten = nten;
     }
-  }
-  /* restore newid pointers */
-  for (ID *library_id : List<ID>(source_data.bmain->libraries)) {
-    library_id->newid = nullptr;
+
+    if (has_non_scene_parent) {
+      return false;
+    }
   }
 
-  return tree;
+  return true;
 }
 
 }  // namespace outliner