[Bf-blender-cvs] [2e221de4cee] master: UI Code Quality: Start refactoring Outliner tree-element building (using C++)

Julian Eisel noreply at git.blender.org
Mon Dec 7 14:53:15 CET 2020


Commit: 2e221de4ceeedcaf6e322d9cd72b148cb04cdac4
Author: Julian Eisel
Date:   Mon Dec 7 14:50:08 2020 +0100
Branches: master
https://developer.blender.org/rB2e221de4ceeedcaf6e322d9cd72b148cb04cdac4

UI Code Quality: Start refactoring Outliner tree-element building (using C++)

Continuation of the work started with 249e4df110e0. After all display modes
were ported to this new design, this commit starts the (more complex) work on
the individual tree-element types. More concretely it ports animation
tree-elements (action data-blocks, drivers and NLA data).

The commit above explains motivations. In short, we need a better design that's
easier to reason about and better testable.

Changes done here are pretty straight forward and introduce similar class
hierarchy and building patterns as introduced for the display modes already.
I.e. an abstract base class, `AbstractTreeElement` with derived classes for the
concrete types, and a C-API with a switch to create the needed objects from a
type enum. The latter should be replacable with something nicer later on (RAII
based, and type-safer through meta-programming).
Each tree-element type has its own class, with an own header and source file
(okay some closely related types can share a header and source file, like the
NLA ones).

I added some further temporary bits for the transition to the new design, such
as the `TreeElement.type`. It should entirely replace `TreeElement` eventually,
just as `outliner_add_element()` should be quite small by then and easily
replacable by a `TreeBuilder` helper.

===================================================================

M	source/blender/editors/space_outliner/CMakeLists.txt
M	source/blender/editors/space_outliner/outliner_intern.h
M	source/blender/editors/space_outliner/outliner_tree.c
A	source/blender/editors/space_outliner/tree/tree_element.cc
A	source/blender/editors/space_outliner/tree/tree_element.h
A	source/blender/editors/space_outliner/tree/tree_element.hh
A	source/blender/editors/space_outliner/tree/tree_element_anim_data.cc
A	source/blender/editors/space_outliner/tree/tree_element_anim_data.hh
A	source/blender/editors/space_outliner/tree/tree_element_driver_base.cc
A	source/blender/editors/space_outliner/tree/tree_element_driver_base.hh
A	source/blender/editors/space_outliner/tree/tree_element_nla.cc
A	source/blender/editors/space_outliner/tree/tree_element_nla.hh

===================================================================

diff --git a/source/blender/editors/space_outliner/CMakeLists.txt b/source/blender/editors/space_outliner/CMakeLists.txt
index b21b969493a..74f99540bee 100644
--- a/source/blender/editors/space_outliner/CMakeLists.txt
+++ b/source/blender/editors/space_outliner/CMakeLists.txt
@@ -52,10 +52,19 @@ set(SRC
   tree/tree_display_orphaned.cc
   tree/tree_display_scenes.cc
   tree/tree_display_data.cc
+  tree/tree_element.cc
+  tree/tree_element_anim_data.cc
+  tree/tree_element_driver_base.cc
+  tree/tree_element_nla.cc
 
   outliner_intern.h
   tree/tree_display.h
   tree/tree_display.hh
+  tree/tree_element.h
+  tree/tree_element.hh
+  tree/tree_element_anim_data.hh
+  tree/tree_element_driver_base.hh
+  tree/tree_element_nla.hh
 )
 
 set(LIB
diff --git a/source/blender/editors/space_outliner/outliner_intern.h b/source/blender/editors/space_outliner/outliner_intern.h
index ecf8cc0f800..0294b4836c8 100644
--- a/source/blender/editors/space_outliner/outliner_intern.h
+++ b/source/blender/editors/space_outliner/outliner_intern.h
@@ -75,6 +75,14 @@ typedef TreeTraversalAction (*TreeTraversalFunc)(struct TreeElement *te, void *c
 
 typedef struct TreeElement {
   struct TreeElement *next, *prev, *parent;
+
+  /**
+   * Handle to the new C++ object (a derived type of base #AbstractTreeElement) that should replace
+   * #TreeElement. Step by step, data should be moved to it and operations based on the type should
+   * become virtual methods of the class hierarchy.
+   */
+  struct TreeElementType *type;
+
   ListBase subtree;
   int xs, ys;                /* Do selection. */
   TreeStoreElem *store_elem; /* Element in tree store. */
diff --git a/source/blender/editors/space_outliner/outliner_tree.c b/source/blender/editors/space_outliner/outliner_tree.c
index a02a8441620..7308b161d18 100644
--- a/source/blender/editors/space_outliner/outliner_tree.c
+++ b/source/blender/editors/space_outliner/outliner_tree.c
@@ -84,6 +84,7 @@
 
 #include "outliner_intern.h"
 #include "tree/tree_display.h"
+#include "tree/tree_element.h"
 
 #ifdef WIN32
 #  include "BLI_math_base.h" /* M_PI */
@@ -230,6 +231,7 @@ void outliner_free_tree_element(TreeElement *element, ListBase *parent_subtree)
   if (element->flag & TE_FREE_NAME) {
     MEM_freeN((void *)element->name);
   }
+  outliner_tree_element_type_free(&element->type);
   MEM_freeN(element);
 }
 
@@ -959,6 +961,11 @@ TreeElement *outliner_add_element(SpaceOutliner *space_outliner,
 
   te->parent = parent;
   te->index = index; /* For data arrays. */
+
+  /* New C++ based type handle (`TreeElementType` in C, `AbstractTreeElement` in C++). Only some
+   * support this, eventually this should replace `TreeElement` entirely. */
+  te->type = outliner_tree_element_type_create(type, te, idv);
+
   if (ELEM(type, TSE_SEQUENCE, TSE_SEQ_STRIP, TSE_SEQUENCE_DUP)) {
     /* pass */
   }
@@ -994,7 +1001,10 @@ TreeElement *outliner_add_element(SpaceOutliner *space_outliner,
     te->idcode = GS(id->name);
   }
 
-  if (type == 0) {
+  if (te->type) {
+    outliner_tree_element_type_expand(te->type, space_outliner);
+  }
+  else if (type == 0) {
     TreeStoreElem *tsepar = parent ? TREESTORE(parent) : NULL;
 
     /* ID data-block. */
@@ -1002,68 +1012,9 @@ TreeElement *outliner_add_element(SpaceOutliner *space_outliner,
       outliner_add_id_contents(space_outliner, te, tselem, id);
     }
   }
-  else if (type == TSE_ANIM_DATA) {
-    IdAdtTemplate *iat = (IdAdtTemplate *)idv;
-    AnimData *adt = (AnimData *)iat->adt;
-
-    /* this element's info */
-    te->name = IFACE_("Animation");
-
-    /* Action */
-    outliner_add_element(space_outliner, &te->subtree, adt->action, te, 0, 0);
-
-    /* Drivers */
-    if (adt->drivers.first) {
-      TreeElement *ted = outliner_add_element(
-          space_outliner, &te->subtree, adt, te, TSE_DRIVER_BASE, 0);
-      ID *lastadded = NULL;
-      FCurve *fcu;
-
-      ted->name = IFACE_("Drivers");
-
-      for (fcu = adt->drivers.first; fcu; fcu = fcu->next) {
-        if (fcu->driver && fcu->driver->variables.first) {
-          ChannelDriver *driver = fcu->driver;
-          DriverVar *dvar;
-
-          for (dvar = driver->variables.first; dvar; dvar = dvar->next) {
-            /* loop over all targets used here */
-            DRIVER_TARGETS_USED_LOOPER_BEGIN (dvar) {
-              if (lastadded != dtar->id) {
-                /* XXX this lastadded check is rather lame, and also fails quite badly... */
-                outliner_add_element(
-                    space_outliner, &ted->subtree, dtar->id, ted, TSE_LINKED_OB, 0);
-                lastadded = dtar->id;
-              }
-            }
-            DRIVER_TARGETS_LOOPER_END;
-          }
-        }
-      }
-    }
-
-    /* NLA Data */
-    if (adt->nla_tracks.first) {
-      TreeElement *tenla = outliner_add_element(space_outliner, &te->subtree, adt, te, TSE_NLA, 0);
-      NlaTrack *nlt;
-      int a = 0;
-
-      tenla->name = IFACE_("NLA Tracks");
-
-      for (nlt = adt->nla_tracks.first; nlt; nlt = nlt->next) {
-        TreeElement *tenlt = outliner_add_element(
-            space_outliner, &tenla->subtree, nlt, tenla, TSE_NLA_TRACK, a);
-        NlaStrip *strip;
-        int b = 0;
-
-        tenlt->name = nlt->name;
-
-        for (strip = nlt->strips.first; strip; strip = strip->next, b++) {
-          outliner_add_element(
-              space_outliner, &tenlt->subtree, strip->act, tenlt, TSE_NLA_ACTION, b);
-        }
-      }
-    }
+  else if (ELEM(type, TSE_ANIM_DATA, TSE_DRIVER_BASE, TSE_NLA, TSE_NLA_ACTION, TSE_NLA_TRACK)) {
+    /* Should already use new AbstractTreeElement design. */
+    BLI_assert(0);
   }
   else if (type == TSE_GP_LAYER) {
     bGPDlayer *gpl = (bGPDlayer *)idv;
diff --git a/source/blender/editors/space_outliner/tree/tree_element.cc b/source/blender/editors/space_outliner/tree/tree_element.cc
new file mode 100644
index 00000000000..ce2a8fa634d
--- /dev/null
+++ b/source/blender/editors/space_outliner/tree/tree_element.cc
@@ -0,0 +1,88 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+/** \file
+ * \ingroup spoutliner
+ */
+
+#include "DNA_listBase.h"
+
+#include "../outliner_intern.h"
+
+#include "tree_element_anim_data.hh"
+#include "tree_element_driver_base.hh"
+#include "tree_element_nla.hh"
+
+#include "tree_element.h"
+#include "tree_element.hh"
+
+namespace blender::ed::outliner {
+
+static AbstractTreeElement *tree_element_create(int type, TreeElement &legacy_te, void *idv)
+{
+  /* Would be nice to get rid of void * here, can we somehow expect the right type right away?
+   * Perfect forwarding maybe, once the API is C++ only? */
+  ID &id = *static_cast<ID *>(idv);
+
+  switch (type) {
+    case TSE_ANIM_DATA:
+      return new TreeElementAnimData(legacy_te, id);
+    case TSE_DRIVER_BASE:
+      return new TreeElementDriverBase(legacy_te, *static_cast<AnimData *>(idv));
+    case TSE_NLA:
+      return new TreeElementNLA(legacy_te, *static_cast<AnimData *>(idv));
+    case TSE_NLA_TRACK:
+      return new TreeElementNLATrack(legacy_te, *static_cast<NlaTrack *>(idv));
+    case TSE_NLA_ACTION:
+      return new TreeElementNLAAction(legacy_te);
+    default:
+      break;
+  }
+
+  return nullptr;
+}
+
+static void tree_element_free(AbstractTreeElement **tree_element)
+{
+  delete *tree_element;
+  *tree_element = nullptr;
+}
+
+static void tree_element_expand(AbstractTreeElement &tree_element, SpaceOutliner &space_outliner)
+{
+  tree_element.expand(space_outliner);
+}
+
+}  // namespace blender::ed::outliner
+
+namespace outliner = blender::ed::outliner;
+
+TreeElementType *outliner_tree_element_type_create(int type, TreeElement *legacy_te, void *idv)
+{
+  outliner::AbstractTreeElement *element = outliner::tree_element_create(type, *legacy_te, idv);
+  return reinterpret_cast<TreeElementType *>(element);
+}
+
+void outliner_tree_element_type_expand(TreeElementType *type, SpaceOutliner *space_outliner)
+{
+  outliner::tree_element_expand(reinterpret_cast<outliner::AbstractTreeElement &>(*type),
+                                *space_outliner);
+}
+
+void outliner_tree_element_type_free(TreeElementType **type)
+{
+  outliner::tree_element_free(reinterpret_cast<outliner::AbstractTreeElement **>(type));
+}
diff --git a/source/blender/editors/space_outliner/tree/tree_element.h b/source/blender/editors/space_outliner/tree/tree_element.h
new file mode 100644
index 00000000000..9012321a323
--- /dev/null
+++ b/source/blender/editors/space_outliner/tree/tree_element.h
@@ -0,0 +1,45 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+/** \file
+ * \ingroup spoutliner
+ *
+ * C-API for the Tree-Element types.
+ * This API shouldn't stay for long. All tree building should eventually be done through C++ types,
+ * with more type safety and an easier to reason about design.
+ */
+
+#pragma once
+
+#include "DNA_space_ty

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list