diff --git a/source/blender/editors/include/UI_tree_view.hh b/source/blender/editors/include/UI_tree_view.hh index 4028bee9e15..46eaf56a3c0 100644 --- a/source/blender/editors/include/UI_tree_view.hh +++ b/source/blender/editors/include/UI_tree_view.hh @@ -139,11 +139,17 @@ class AbstractTreeView : public TreeViewItemContainer { friend TreeViewBuilder; friend TreeViewLayoutBuilder; + bool is_reconstructed_ = false; + public: virtual ~AbstractTreeView() = default; void foreach_item(ItemIterFn iter_fn, IterOptions options = IterOptions::None) const; + /** Check if the tree is fully (re-)constructed. That means, both #build_tree() and + * #update_from_old() have finished. */ + bool is_reconstructed() const; + protected: virtual void build_tree() = 0; @@ -156,6 +162,10 @@ class AbstractTreeView : public TreeViewItemContainer { const TreeViewItemContainer &old_items); static AbstractTreeViewItem *find_matching_child(const AbstractTreeViewItem &lookup_item, const TreeViewItemContainer &items); + /** Items may want to do additional work when state changes. But these state changes can only be + * reliably detected after the tree was reconstructed (see #is_reconstructed()). So this is done + * delayed. */ + void change_state_delayed(); void build_layout_from_tree(const TreeViewLayoutBuilder &builder); }; @@ -175,9 +185,15 @@ class AbstractTreeView : public TreeViewItemContainer { class AbstractTreeViewItem : public TreeViewItemContainer { friend class AbstractTreeView; + public: + using IsActiveFn = std::function; + + private: bool is_open_ = false; bool is_active_ = false; + IsActiveFn is_active_fn_; + protected: /** This label is used for identifying an item (together with its parent's labels). */ std::string label_{}; @@ -188,6 +204,7 @@ class AbstractTreeViewItem : public TreeViewItemContainer { virtual void build_row(uiLayout &row) = 0; virtual void on_activate(); + virtual void is_active(IsActiveFn is_active_fn); virtual bool on_drop(const wmDrag &drag); virtual bool can_drop(const wmDrag &drag) const; /** Custom text to display when dragging over a tree item. Should explain what happens when @@ -211,16 +228,29 @@ class AbstractTreeViewItem : public TreeViewItemContainer { const AbstractTreeView &get_tree_view() const; int count_parents() const; - /** Activates this item, deactivates other items, calls the #AbstractTreeViewItem::on_activate() - * function and ensures this item's parents are not collapsed (so the item is visible). */ - void activate(); void deactivate(); + /** Must not be called before the tree was reconstructed (see #is_reconstructed()). Otherwise we + * can't be sure about the item state. */ bool is_active() const; void toggle_collapsed(); + /** Must not be called before the tree was reconstructed (see #is_reconstructed()). Otherwise we + * can't be sure about the item state. */ bool is_collapsed() const; void set_collapsed(bool collapsed); bool is_collapsible() const; void ensure_parents_uncollapsed(); + + protected: + /** Activates this item, deactivates other items, calls the #AbstractTreeViewItem::on_activate() + * function and ensures this item's parents are not collapsed (so the item is visible). + * Must not be called before the tree was reconstructed (see #is_reconstructed()). Otherwise we + * can't be sure about the current item state and may call state-change update functions + * incorrectly. */ + void activate(); + + private: + /** See #AbstractTreeView::change_state_delayed() */ + void change_state_delayed(); }; /** \} */ @@ -242,7 +272,6 @@ class BasicTreeViewItem : public AbstractTreeViewItem { BasicTreeViewItem(StringRef label, BIFIconID icon = ICON_NONE); void build_row(uiLayout &row) override; - void on_activate() override; void on_activate(ActivateFn fn); protected: @@ -255,6 +284,11 @@ class BasicTreeViewItem : public AbstractTreeViewItem { uiBut *button(); BIFIconID get_draw_icon() const; + + private: + static void tree_row_click_fn(struct bContext *C, void *arg1, void *arg2); + + void on_activate() override; }; /** \} */ diff --git a/source/blender/editors/interface/tree_view.cc b/source/blender/editors/interface/tree_view.cc index 63b12d4fc89..28c757ddc79 100644 --- a/source/blender/editors/interface/tree_view.cc +++ b/source/blender/editors/interface/tree_view.cc @@ -92,17 +92,20 @@ void AbstractTreeView::update_from_old(uiBlock &new_block) { uiBlock *old_block = new_block.oldblock; if (!old_block) { + /* Initial construction, nothing to update. */ + is_reconstructed_ = true; return; } uiTreeViewHandle *old_view_handle = ui_block_view_find_matching_in_old_block( &new_block, reinterpret_cast(this)); - if (!old_view_handle) { - return; - } + BLI_assert(old_view_handle); AbstractTreeView &old_view = reinterpret_cast(*old_view_handle); update_children_from_old_recursive(*this, old_view); + + /* Finished (re-)constructing the tree. */ + is_reconstructed_ = true; } void AbstractTreeView::update_children_from_old_recursive(const TreeViewItemContainer &new_items, @@ -134,6 +137,19 @@ AbstractTreeViewItem *AbstractTreeView::find_matching_child( return nullptr; } +bool AbstractTreeView::is_reconstructed() const +{ + return is_reconstructed_; +} + +void AbstractTreeView::change_state_delayed() +{ + BLI_assert_msg( + is_reconstructed(), + "These state changes are supposed to be delayed until reconstruction is completed"); + foreach_item([](AbstractTreeViewItem &item) { item.change_state_delayed(); }); +} + /* ---------------------------------------------------------------------- */ void AbstractTreeViewItem::on_activate() @@ -141,6 +157,11 @@ void AbstractTreeViewItem::on_activate() /* Do nothing by default. */ } +void AbstractTreeViewItem::is_active(IsActiveFn is_active_fn) +{ + is_active_fn_ = is_active_fn; +} + bool AbstractTreeViewItem::on_drop(const wmDrag & /*drag*/) { /* Do nothing by default. */ @@ -186,6 +207,9 @@ int AbstractTreeViewItem::count_parents() const void AbstractTreeViewItem::activate() { + BLI_assert_msg(get_tree_view().is_reconstructed(), + "Item activation can't be done until reconstruction is completed"); + if (is_active()) { return; } @@ -207,11 +231,15 @@ void AbstractTreeViewItem::deactivate() bool AbstractTreeViewItem::is_active() const { + BLI_assert_msg(get_tree_view().is_reconstructed(), + "State can't be queried until reconstruction is completed"); return is_active_; } bool AbstractTreeViewItem::is_collapsed() const { + BLI_assert_msg(get_tree_view().is_reconstructed(), + "State can't be queried until reconstruction is completed"); return is_collapsible() && !is_open_; } @@ -237,6 +265,13 @@ void AbstractTreeViewItem::ensure_parents_uncollapsed() } } +void AbstractTreeViewItem::change_state_delayed() +{ + if (is_active_fn_()) { + activate(); + } +} + /* ---------------------------------------------------------------------- */ TreeViewBuilder::TreeViewBuilder(uiBlock &block) : block_(block) @@ -247,6 +282,7 @@ void TreeViewBuilder::build_tree_view(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_)); } @@ -283,11 +319,10 @@ BasicTreeViewItem::BasicTreeViewItem(StringRef label, BIFIconID icon_) : icon(ic label_ = label; } -static void tree_row_click_fn(struct bContext *UNUSED(C), void *but_arg1, void *UNUSED(arg2)) +void BasicTreeViewItem::tree_row_click_fn(struct bContext * /*C*/, void *but_arg1, void * /*arg2*/) { uiButTreeRow *tree_row_but = (uiButTreeRow *)but_arg1; - AbstractTreeViewItem &tree_item = reinterpret_cast( - *tree_row_but->tree_item); + BasicTreeViewItem &tree_item = reinterpret_cast(*tree_row_but->tree_item); /* Let a click on an opened item activate it, a second click will close it then. * TODO Should this be for asset catalogs only? */ diff --git a/source/blender/editors/space_file/asset_catalog_tree_view.cc b/source/blender/editors/space_file/asset_catalog_tree_view.cc index ff8775155c2..291582dac08 100644 --- a/source/blender/editors/space_file/asset_catalog_tree_view.cc +++ b/source/blender/editors/space_file/asset_catalog_tree_view.cc @@ -151,9 +151,7 @@ ui::BasicTreeViewItem &AssetCatalogTreeView::build_catalog_items_recursive( { ui::BasicTreeViewItem &view_item = view_parent_item.add_tree_item( &catalog); - if (is_active_catalog(catalog.get_catalog_id())) { - view_item.activate(); - } + view_item.is_active([this, &catalog]() { return is_active_catalog(catalog.get_catalog_id()); }); catalog.foreach_child([&view_item, this](AssetCatalogTreeItem &child) { build_catalog_items_recursive(view_item, child); @@ -171,9 +169,8 @@ void AssetCatalogTreeView::add_all_item() params->asset_catalog_visibility = FILE_SHOW_ASSETS_ALL_CATALOGS; WM_main_add_notifier(NC_SPACE | ND_SPACE_ASSET_PARAMS, nullptr); }); - if (params->asset_catalog_visibility == FILE_SHOW_ASSETS_ALL_CATALOGS) { - item.activate(); - } + item.is_active( + [params]() { return params->asset_catalog_visibility == FILE_SHOW_ASSETS_ALL_CATALOGS; }); } void AssetCatalogTreeView::add_unassigned_item() @@ -187,10 +184,8 @@ void AssetCatalogTreeView::add_unassigned_item() params->asset_catalog_visibility = FILE_SHOW_ASSETS_WITHOUT_CATALOG; WM_main_add_notifier(NC_SPACE | ND_SPACE_ASSET_PARAMS, nullptr); }); - - if (params->asset_catalog_visibility == FILE_SHOW_ASSETS_WITHOUT_CATALOG) { - item.activate(); - } + item.is_active( + [params]() { return params->asset_catalog_visibility == FILE_SHOW_ASSETS_WITHOUT_CATALOG; }); } bool AssetCatalogTreeView::is_active_catalog(CatalogID catalog_id) const