Cleanup: Various cleanups to the tree-view API

* Correct URL for documentation (was changed recently).
* Add comments.
* Reevaluate and update which functions are public, protected or
  private.
* Reorder functions and classes to be more logical and readable.
* Add helper class for the public item API so individual functions it
  uses can be made protected/private (the helper class is a friend).
  Also allows splitting API implementation from the C-API.
* Move internal layout builder helper class to the source file, out of
  the header.
* More consistent naming.
* Add alias for item-container, so it's more clear how it can be used.
* Use const.
* Remove unnecessary forward declaration.

1 files changed, 144 insertions, 75 deletions

diff --git a/source/blender/editors/interface/tree_view.cc b/source/blender/editors/interface/tree_view.cc
index 65cc699ee17..3010aaba5a3 100644
--- a/source/blender/editors/interface/tree_view.cc
+++ b/source/blender/editors/interface/tree_view.cc
@@ -87,19 +87,6 @@ bool AbstractTreeView::is_renaming() const
   return rename_buffer_ != nullptr;
 }
 
-void AbstractTreeView::build_layout_from_tree(const TreeViewLayoutBuilder &builder)
-{
-  uiLayout *prev_layout = builder.current_layout();
-
-  uiLayout *box = uiLayoutBox(prev_layout);
-  uiLayoutColumn(box, false);
-
-  foreach_item([&builder](AbstractTreeViewItem &item) { builder.build_row(item); },
-               IterOptions::SkipCollapsed);
-
-  UI_block_layout_set_current(&builder.block(), prev_layout);
-}
-
 void AbstractTreeView::update_from_old(uiBlock &new_block)
 {
   uiBlock *old_block = new_block.oldblock;
@@ -130,8 +117,8 @@ void AbstractTreeView::update_from_old(uiBlock &new_block)
   is_reconstructed_ = true;
 }
 
-void AbstractTreeView::update_children_from_old_recursive(const TreeViewItemContainer &new_items,
-                                                          const TreeViewItemContainer &old_items)
+void AbstractTreeView::update_children_from_old_recursive(const TreeViewOrItem &new_items,
+                                                          const TreeViewOrItem &old_items)
 {
   for (const auto &new_item : new_items.children_) {
     AbstractTreeViewItem *matching_old_item = find_matching_child(*new_item, old_items);
@@ -147,7 +134,7 @@ void AbstractTreeView::update_children_from_old_recursive(const TreeViewItemCont
 }
 
 AbstractTreeViewItem *AbstractTreeView::find_matching_child(
-    const AbstractTreeViewItem &lookup_item, const TreeViewItemContainer &items)
+    const AbstractTreeViewItem &lookup_item, const TreeViewOrItem &items)
 {
   for (const auto &iter_item : items.children_) {
     if (lookup_item.matches(*iter_item)) {
@@ -380,7 +367,7 @@ std::unique_ptr<AbstractTreeViewItemDropController> AbstractTreeViewItem::create
   return nullptr;
 }
 
-bool AbstractTreeViewItem::can_rename() const
+bool AbstractTreeViewItem::supports_renaming() const
 {
   /* No renaming by default. */
   return false;
@@ -414,7 +401,7 @@ bool AbstractTreeViewItem::matches(const AbstractTreeViewItem &other) const
 void AbstractTreeViewItem::begin_renaming()
 {
   AbstractTreeView &tree_view = get_tree_view();
-  if (tree_view.is_renaming() || !can_rename()) {
+  if (tree_view.is_renaming() || !supports_renaming()) {
     return;
   }
 
@@ -444,7 +431,7 @@ AbstractTreeView &AbstractTreeViewItem::get_tree_view() const
 int AbstractTreeViewItem::count_parents() const
 {
   int i = 0;
-  for (TreeViewItemContainer *parent = parent_; parent; parent = parent->parent_) {
+  for (AbstractTreeViewItem *parent = parent_; parent; parent = parent->parent_) {
     i++;
   }
   return i;
@@ -587,22 +574,40 @@ AbstractTreeViewItemDropController::AbstractTreeViewItemDropController(AbstractT
 
 /* ---------------------------------------------------------------------- */
 
-TreeViewBuilder::TreeViewBuilder(uiBlock &block) : block_(block)
+class TreeViewLayoutBuilder {
+  uiBlock &block_;
+
+  friend TreeViewBuilder;
+
+ public:
+  void build_from_tree(const AbstractTreeView &tree_view);
+  void build_row(AbstractTreeViewItem &item) const;
+
+  uiBlock &block() const;
+  uiLayout *current_layout() const;
+
+ private:
+  /* Created through #TreeViewBuilder. */
+  TreeViewLayoutBuilder(uiBlock &block);
+
+  static void polish_layout(const uiBlock &block);
+};
+
+TreeViewLayoutBuilder::TreeViewLayoutBuilder(uiBlock &block) : block_(block)
 {
 }
 
-void TreeViewBuilder::build_tree_view(AbstractTreeView &tree_view)
+void TreeViewLayoutBuilder::build_from_tree(const AbstractTreeView &tree_view)
 {
-  tree_view.build_tree();
-  tree_view.update_from_old(block_);
-  tree_view.change_state_delayed();
-  tree_view.build_layout_from_tree(TreeViewLayoutBuilder(block_));
-}
+  uiLayout *prev_layout = current_layout();
 
-/* ---------------------------------------------------------------------- */
+  uiLayout *box = uiLayoutBox(prev_layout);
+  uiLayoutColumn(box, false);
 
-TreeViewLayoutBuilder::TreeViewLayoutBuilder(uiBlock &block) : block_(block)
-{
+  tree_view.foreach_item([this](AbstractTreeViewItem &item) { build_row(item); },
+                         AbstractTreeView::IterOptions::SkipCollapsed);
+
+  UI_block_layout_set_current(&block(), prev_layout);
 }
 
 void TreeViewLayoutBuilder::polish_layout(const uiBlock &block)
@@ -664,6 +669,22 @@ uiLayout *TreeViewLayoutBuilder::current_layout() const
 
 /* ---------------------------------------------------------------------- */
 
+TreeViewBuilder::TreeViewBuilder(uiBlock &block) : block_(block)
+{
+}
+
+void TreeViewBuilder::build_tree_view(AbstractTreeView &tree_view)
+{
+  tree_view.build_tree();
+  tree_view.update_from_old(block_);
+  tree_view.change_state_delayed();
+
+  TreeViewLayoutBuilder builder(block_);
+  builder.build_from_tree(tree_view);
+}
+
+/* ---------------------------------------------------------------------- */
+
 BasicTreeViewItem::BasicTreeViewItem(StringRef label, BIFIconID icon_) : icon(icon_)
 {
   label_ = label;
@@ -710,8 +731,92 @@ std::optional<bool> BasicTreeViewItem::should_be_active() const
   return std::nullopt;
 }
 
+/* ---------------------------------------------------------------------- */
+
+/**
+ * Helper for a public (C-)API, presenting higher level functionality. Has access to internal
+ * data/functionality (friend of #AbstractTreeViewItem), which is sometimes needed when
+ * functionality of the API needs to be constructed from multiple internal conditions and/or
+ * functions that on their own shouldn't be part of the API.
+ */
+class TreeViewItemAPIWrapper {
+ public:
+  static bool matches(const AbstractTreeViewItem &a, const AbstractTreeViewItem &b)
+  {
+    /* TODO should match the tree-view as well. */
+    return a.matches_including_parents(b);
+  }
+
+  static bool drag_start(bContext &C, const AbstractTreeViewItem &item)
+  {
+    const std::unique_ptr<AbstractTreeViewItemDragController> drag_controller =
+        item.create_drag_controller();
+    if (!drag_controller) {
+      return false;
+    }
+
+    WM_event_start_drag(&C,
+                        ICON_NONE,
+                        drag_controller->get_drag_type(),
+                        drag_controller->create_drag_data(),
+                        0,
+                        WM_DRAG_FREE_DATA);
+    drag_controller->on_drag_start();
+
+    return true;
+  }
+
+  static bool can_drop(const AbstractTreeViewItem &item,
+                       const wmDrag &drag,
+                       const char **r_disabled_hint)
+  {
+    const std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
+        item.create_drop_controller();
+    if (!drop_controller) {
+      return false;
+    }
+
+    return drop_controller->can_drop(drag, r_disabled_hint);
+  }
+
+  static std::string drop_tooltip(const AbstractTreeViewItem &item, const wmDrag &drag)
+  {
+    const std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
+        item.create_drop_controller();
+    if (!drop_controller) {
+      return {};
+    }
+
+    return drop_controller->drop_tooltip(drag);
+  }
+
+  static bool drop_handle(bContext &C, const AbstractTreeViewItem &item, const ListBase &drags)
+  {
+    std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
+        item.create_drop_controller();
+
+    const char *disabled_hint_dummy = nullptr;
+    LISTBASE_FOREACH (const wmDrag *, drag, &drags) {
+      if (drop_controller->can_drop(*drag, &disabled_hint_dummy)) {
+        return drop_controller->on_drop(&C, *drag);
+      }
+    }
+
+    return false;
+  }
+
+  static bool can_rename(const AbstractTreeViewItem &item)
+  {
+    const AbstractTreeView &tree_view = item.get_tree_view();
+    return !tree_view.is_renaming() && item.supports_renaming();
+  }
+};
+
 }  // namespace blender::ui
 
+/* ---------------------------------------------------------------------- */
+/* C-API */
+
 using namespace blender::ui;
 
 bool UI_tree_view_item_is_active(const uiTreeViewItemHandle *item_handle)
@@ -725,28 +830,13 @@ bool UI_tree_view_item_matches(const uiTreeViewItemHandle *a_handle,
 {
   const AbstractTreeViewItem &a = reinterpret_cast<const AbstractTreeViewItem &>(*a_handle);
   const AbstractTreeViewItem &b = reinterpret_cast<const AbstractTreeViewItem &>(*b_handle);
-  /* TODO should match the tree-view as well. */
-  return a.matches_including_parents(b);
+  return TreeViewItemAPIWrapper::matches(a, b);
 }
 
 bool UI_tree_view_item_drag_start(bContext *C, uiTreeViewItemHandle *item_)
 {
   const AbstractTreeViewItem &item = reinterpret_cast<const AbstractTreeViewItem &>(*item_);
-  const std::unique_ptr<AbstractTreeViewItemDragController> drag_controller =
-      item.create_drag_controller();
-  if (!drag_controller) {
-    return false;
-  }
-
-  WM_event_start_drag(C,
-                      ICON_NONE,
-                      drag_controller->get_drag_type(),
-                      drag_controller->create_drag_data(),
-                      0,
-                      WM_DRAG_FREE_DATA);
-  drag_controller->on_drag_start();
-
-  return true;
+  return TreeViewItemAPIWrapper::drag_start(*C, item);
 }
 
 bool UI_tree_view_item_can_drop(const uiTreeViewItemHandle *item_,
@@ -754,50 +844,29 @@ bool UI_tree_view_item_can_drop(const uiTreeViewItemHandle *item_,
                                 const char **r_disabled_hint)
 {
   const AbstractTreeViewItem &item = reinterpret_cast<const AbstractTreeViewItem &>(*item_);
-  const std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
-      item.create_drop_controller();
-  if (!drop_controller) {
-    return false;
-  }
-
-  return drop_controller->can_drop(*drag, r_disabled_hint);
+  return TreeViewItemAPIWrapper::can_drop(item, *drag, r_disabled_hint);
 }
 
 char *UI_tree_view_item_drop_tooltip(const uiTreeViewItemHandle *item_, const wmDrag *drag)
 {
   const AbstractTreeViewItem &item = reinterpret_cast<const AbstractTreeViewItem &>(*item_);
-  const std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
-      item.create_drop_controller();
-  if (!drop_controller) {
-    return nullptr;
-  }
 
-  return BLI_strdup(drop_controller->drop_tooltip(*drag).c_str());
+  const std::string tooltip = TreeViewItemAPIWrapper::drop_tooltip(item, *drag);
+  return tooltip.empty() ? nullptr : BLI_strdup(tooltip.c_str());
 }
 
-bool UI_tree_view_item_drop_handle(struct bContext *C,
-                                   uiTreeViewItemHandle *item_,
+bool UI_tree_view_item_drop_handle(bContext *C,
+                                   const uiTreeViewItemHandle *item_,
                                    const ListBase *drags)
 {
-  AbstractTreeViewItem &item = reinterpret_cast<AbstractTreeViewItem &>(*item_);
-  std::unique_ptr<AbstractTreeViewItemDropController> drop_controller =
-      item.create_drop_controller();
-
-  const char *disabled_hint_dummy = nullptr;
-  LISTBASE_FOREACH (const wmDrag *, drag, drags) {
-    if (drop_controller->can_drop(*drag, &disabled_hint_dummy)) {
-      return drop_controller->on_drop(C, *drag);
-    }
-  }
-
-  return false;
+  const AbstractTreeViewItem &item = reinterpret_cast<const AbstractTreeViewItem &>(*item_);
+  return TreeViewItemAPIWrapper::drop_handle(*C, item, *drags);
 }
 
 bool UI_tree_view_item_can_rename(const uiTreeViewItemHandle *item_handle)
 {
   const AbstractTreeViewItem &item = reinterpret_cast<const AbstractTreeViewItem &>(*item_handle);
-  const AbstractTreeView &tree_view = item.get_tree_view();
-  return !tree_view.is_renaming() && item.can_rename();
+  return TreeViewItemAPIWrapper::can_rename(item);
 }
 
 void UI_tree_view_item_begin_rename(uiTreeViewItemHandle *item_handle)