diff --git a/source/blender/editors/include/UI_abstract_view.hh b/source/blender/editors/include/UI_abstract_view.hh index 163ea81943f..b6468d0d26b 100644 --- a/source/blender/editors/include/UI_abstract_view.hh +++ b/source/blender/editors/include/UI_abstract_view.hh @@ -88,6 +88,8 @@ class AbstractView { virtual void draw_overlays(const ARegion ®ion) const; + virtual void foreach_view_item(FunctionRef iter_fn) const = 0; + /** * Makes \a item valid for display in this view. Behavior is undefined for items not registered * with this. @@ -110,6 +112,15 @@ class AbstractView { protected: AbstractView() = default; + /** + * Items may want to do additional work when state changes. But these state changes can only be + * reliably detected after the view has completed reconstruction (see #is_reconstructed()). So + * the actual state changes are done in a delayed manner through this function. + * + * Overrides should call the base class implementation. + */ + virtual void change_state_delayed(); + virtual void update_children_from_old(const AbstractView &old_view) = 0; /** @@ -151,6 +162,20 @@ class AbstractViewItem { virtual void build_context_menu(bContext &C, uiLayout &column) const; + /** + * Called when the view changes an item's state from inactive to active. Will only be called if + * the state change is triggered through the view, not through external changes. E.g. a click on + * an item calls it, a change in the value returned by #should_be_active() to reflect an external + * state change does not. + */ + virtual void on_activate(); + /** + * If the result is not empty, it controls whether the item should be active or not, usually + * depending on the data that the view represents. Note that since this is meant to reflect + * externally managed state changes, #on_activate() will never be called if this returns true. + */ + virtual std::optional should_be_active() const; + /** * Queries if the view item supports renaming in principle. Renaming may still fail, e.g. if * another item is already being renamed. @@ -204,7 +229,17 @@ class AbstractViewItem { bool is_interactive() const; void disable_activatable(); - + /** + * Activates this item, deactivates other items, and calls the #AbstractViewItem::on_activate() + * function. Should only be called when the item was activated through the view (e.g. through a + * click), not if the view reflects an external change (e.g. + * #AbstractViewItem::should_be_active() changes from returning false to returning true). + * + * Requires the view to have completed reconstruction, see #is_reconstructed(). Otherwise the + * actual item state is unknown, possibly calling state-change update functions incorrectly. + */ + void activate(); + void deactivate(); /** * Requires the view to have completed reconstruction, see #is_reconstructed(). Otherwise we * can't be sure about the item state. @@ -240,6 +275,20 @@ class AbstractViewItem { */ virtual void update_from_old(const AbstractViewItem &old); + /** + * Like #activate() but does not call #on_activate(). Use it to reflect changes in the active + * state that happened externally. + * Can be overridden to customize behavior but should always call the base class implementation. + * \return true of the item was activated. + */ + virtual bool set_state_active(); + + /** + * See #AbstractView::change_state_delayed(). Overrides should call the base class + * implementation. + */ + virtual void change_state_delayed(); + /** * \note Do not call this directly to avoid constantly rechecking the filter state. Instead use * #is_filtered_visible_cached() for querying. diff --git a/source/blender/editors/include/UI_grid_view.hh b/source/blender/editors/include/UI_grid_view.hh index a81539979da..77e28cdb941 100644 --- a/source/blender/editors/include/UI_grid_view.hh +++ b/source/blender/editors/include/UI_grid_view.hh @@ -56,44 +56,10 @@ class AbstractGridViewItem : public AbstractViewItem { /** See AbstractViewItem::matches(). */ /* virtual */ bool matches(const AbstractViewItem &other) const override; - /** - * Called when the grid view changes an item's state from inactive to active. Will only be called - * if the state change is triggered through the grid view, not through external changes. E.g. a - * click on an item calls it, a change in the value returned by #should_be_active() to reflect an - * external state change does not. - */ - virtual void on_activate(); - /** - * If the result is not empty, it controls whether the item should be active or not, usually - * depending on the data that the view represents. Note that since this is meant to reflect - * externally managed state changes, #on_activate() will never be called if this returns true. - */ - virtual std::optional should_be_active() const; - /* virtual */ std::unique_ptr create_item_drop_target() final; virtual std::unique_ptr create_drop_target(); - /** - * Activates this item, deactivates other items, and calls the - * #AbstractGridViewItem::on_activate() function. Should only be called when the item was - * activated through the grid view (e.g. through a click), not if the grid view reflects an - * external change (e.g. #AbstractGridViewItem::should_be_active() changes from returning false - * to returning true). - * Requires the view to have completed reconstruction, see #is_reconstructed(). Otherwise the - * actual item state is unknown, possibly calling state-change update functions incorrectly. - */ - void activate(); - void deactivate(); - private: - /** See #AbstractGridView::change_state_delayed() */ - void change_state_delayed(); - /** - * Like #activate() but does not call #on_activate(). Use it to reflect changes in the active - * state that happened externally. - * \return true of the item was activated. - */ - bool set_state_active(); static void grid_tile_click_fn(bContext *, void *but_arg1, void *); void add_grid_tile_button(uiBlock &block); }; @@ -156,15 +122,10 @@ class AbstractGridView : public AbstractView { virtual void build_items() = 0; private: + void foreach_view_item(FunctionRef iter_fn) const final; void update_children_from_old(const AbstractView &old_view) override; AbstractGridViewItem *find_matching_item(const AbstractGridViewItem &item_to_match, const AbstractGridView &view_to_search_in) const; - /** - * Items may want to do additional work when state changes. But these state changes can only be - * reliably detected after the view has completed reconstruction (see #is_reconstructed()). So - * the actual state changes are done in a delayed manner through this function. - */ - void change_state_delayed(); /** * Add an already constructed item, moving ownership to the grid-view. diff --git a/source/blender/editors/include/UI_tree_view.hh b/source/blender/editors/include/UI_tree_view.hh index 77d237649d5..f3233f421b1 100644 --- a/source/blender/editors/include/UI_tree_view.hh +++ b/source/blender/editors/include/UI_tree_view.hh @@ -138,18 +138,13 @@ class AbstractTreeView : public AbstractView, public TreeViewItemContainer { virtual void build_tree() = 0; private: + void foreach_view_item(FunctionRef iter_fn) const final; void update_children_from_old(const AbstractView &old_view) override; static void update_children_from_old_recursive(const TreeViewOrItem &new_items, const TreeViewOrItem &old_items); static AbstractTreeViewItem *find_matching_child(const AbstractTreeViewItem &lookup_item, const TreeViewOrItem &items); - /** - * Items may want to do additional work when state changes. But these state changes can only be - * reliably detected after the tree has completed reconstruction (see #is_reconstructed()). So - * the actual state changes are done in a delayed manner through this function. - */ - void change_state_delayed(); void draw_hierarchy_lines(const ARegion ®ion) const; void draw_hierarchy_lines_recursive(const ARegion ®ion, const TreeViewOrItem &parent, @@ -209,20 +204,6 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain bool is_collapsible() const; protected: - /** - * Called when the tree view changes an item's state from inactive to active. Will only be called - * if the state change is triggered through the tree view, not through external changes. E.g. a - * click on an item calls it, a change in the value returned by #should_be_active() to reflect an - * external state change does not. - */ - virtual void on_activate(); - /** - * If the result is not empty, it controls whether the item should be active or not, usually - * depending on the data that the view represents. Note that since this is meant to reflect - * externally managed state changes, #on_activate() will never be called if this returns true. - */ - virtual std::optional should_be_active() const; - /** See AbstractViewItem::get_rename_string(). */ /* virtual */ StringRef get_rename_string() const override; /** See AbstractViewItem::rename(). */ @@ -252,18 +233,6 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain */ virtual bool matches_single(const AbstractTreeViewItem &other) 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). Should - * only be called when the item was activated through the tree view (e.g. through a click), not - * if the tree view reflects an external change (e.g. #AbstractTreeViewItem::should_be_active() - * changes from returning false to returning true). - * Requires the tree to have completed reconstruction, see #is_reconstructed(). Otherwise the - * actual item state is unknown, possibly calling state-change update functions incorrectly. - */ - void activate(); - void deactivate(); - /** * Can be called from the #AbstractTreeViewItem::build_row() implementation, but not earlier. The * hovered state can't be queried reliably otherwise. @@ -278,14 +247,11 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain static void collapse_chevron_click_fn(bContext *, void *but_arg1, void *); static bool is_collapse_chevron_but(const uiBut *but); - /** See #AbstractTreeView::change_state_delayed() */ - void change_state_delayed(); /** - * Like #activate() but does not call #on_activate(). Use it to reflect changes in the active - * state that happened externally. - * \return true of the item was activated. + * Override of #AbstractViewItem::set_state_active() that also ensures the parents of this + * element are uncollapsed so that the item is visible. */ - bool set_state_active(); + bool set_state_active() final; void add_treerow_button(uiBlock &block); int indent_width() const; diff --git a/source/blender/editors/interface/views/abstract_view.cc b/source/blender/editors/interface/views/abstract_view.cc index 8d9c347f051..8319a520f1f 100644 --- a/source/blender/editors/interface/views/abstract_view.cc +++ b/source/blender/editors/interface/views/abstract_view.cc @@ -62,6 +62,34 @@ void AbstractView::update_from_old(uiBlock &new_block) /** \} */ +/* ---------------------------------------------------------------------- */ +/** \name State Managment + * \{ */ + +void AbstractView::change_state_delayed() +{ + BLI_assert_msg( + is_reconstructed(), + "These state changes are supposed to be delayed until reconstruction is completed"); + +/* Debug-only sanity check: Ensure only one item requests to be active. */ +#ifndef NDEBUG + bool has_active = false; + foreach_view_item([&has_active](AbstractViewItem &item) { + if (item.should_be_active().value_or(false)) { + BLI_assert_msg( + !has_active, + "Only one view item should ever return true for its `should_be_active()` method"); + has_active = true; + } + }); +#endif + + foreach_view_item([](AbstractViewItem &item) { item.change_state_delayed(); }); +} + +/** \} */ + /* ---------------------------------------------------------------------- */ /** \name Default implementations of virtual functions * \{ */ diff --git a/source/blender/editors/interface/views/abstract_view_item.cc b/source/blender/editors/interface/views/abstract_view_item.cc index 8df472ded8f..0c6df3cda12 100644 --- a/source/blender/editors/interface/views/abstract_view_item.cc +++ b/source/blender/editors/interface/views/abstract_view_item.cc @@ -32,6 +32,69 @@ void AbstractViewItem::update_from_old(const AbstractViewItem &old) /** \} */ +/* ---------------------------------------------------------------------- */ +/** \name Active Item State + * \{ */ + +void AbstractViewItem::on_activate() +{ + /* Do nothing by default. */ +} + +std::optional AbstractViewItem::should_be_active() const +{ + return std::nullopt; +} + +bool AbstractViewItem::set_state_active() +{ + BLI_assert_msg(get_view().is_reconstructed(), + "Item activation can't be done until reconstruction is completed"); + + if (!is_activatable_) { + return false; + } + if (is_active()) { + return false; + } + + /* Deactivate other items in the view. */ + get_view().foreach_view_item([](auto &item) { item.deactivate(); }); + + is_active_ = true; + return true; +} + +void AbstractViewItem::activate() +{ + if (set_state_active()) { + on_activate(); + } +} + +void AbstractViewItem::deactivate() +{ + is_active_ = false; +} + +/** \} */ + +/* ---------------------------------------------------------------------- */ +/** \name General State Management + * \{ */ + +void AbstractViewItem::change_state_delayed() +{ + const std::optional should_be_active = this->should_be_active(); + if (should_be_active.has_value() && *should_be_active) { + /* Don't call #activate() here, since this reflects an external state change and therefore + * shouldn't call #on_activate(). */ + set_state_active(); + } +} + +/** \} */ + /* ---------------------------------------------------------------------- */ /** \name Renaming * \{ */ diff --git a/source/blender/editors/interface/views/grid_view.cc b/source/blender/editors/interface/views/grid_view.cc index d32c0b763cd..dd3ad340555 100644 --- a/source/blender/editors/interface/views/grid_view.cc +++ b/source/blender/editors/interface/views/grid_view.cc @@ -37,6 +37,14 @@ AbstractGridViewItem &AbstractGridView::add_item(std::unique_ptr iter_fn) const +{ + for (const auto &item_ptr : items_) { + iter_fn(*item_ptr); + } +} + void AbstractGridView::foreach_item(ItemIterFn iter_fn) const { for (const auto &item_ptr : items_) { @@ -63,28 +71,6 @@ AbstractGridViewItem *AbstractGridView::find_matching_item( return match ? *match : nullptr; } -void AbstractGridView::change_state_delayed() -{ - BLI_assert_msg( - is_reconstructed(), - "These state changes are supposed to be delayed until reconstruction is completed"); - -/* Debug-only sanity check: Ensure only one item requests to be active. */ -#ifndef NDEBUG - bool has_active = false; - foreach_item([&has_active](AbstractGridViewItem &item) { - if (item.should_be_active().value_or(false)) { - BLI_assert_msg( - !has_active, - "Only one view item should ever return true for its `should_be_active()` method"); - has_active = true; - } - }); -#endif - - foreach_item([](AbstractGridViewItem &item) { item.change_state_delayed(); }); -} - void AbstractGridView::update_children_from_old(const AbstractView &old_view) { const AbstractGridView &old_grid_view = dynamic_cast(old_view); @@ -172,57 +158,6 @@ void AbstractGridViewItem::add_grid_tile_button(uiBlock &block) UI_but_func_set(view_item_but_, grid_tile_click_fn, view_item_but_, nullptr); } -void AbstractGridViewItem::on_activate() -{ - /* Do nothing by default. */ -} - -std::optional AbstractGridViewItem::should_be_active() const -{ - return std::nullopt; -} - -void AbstractGridViewItem::change_state_delayed() -{ - const std::optional should_be_active = this->should_be_active(); - if (should_be_active.has_value() && *should_be_active) { - /* Don't call #activate() here, since this reflects an external state change and therefore - * shouldn't call #on_activate(). */ - set_state_active(); - } -} - -bool AbstractGridViewItem::set_state_active() -{ - BLI_assert_msg(get_view().is_reconstructed(), - "Item activation can't be done until reconstruction is completed"); - - if (!is_activatable_) { - return false; - } - if (is_active()) { - return false; - } - - /* Deactivate other items in the view. */ - get_view().foreach_item([](auto &item) { item.deactivate(); }); - - is_active_ = true; - return true; -} - -void AbstractGridViewItem::activate() -{ - if (set_state_active()) { - on_activate(); - } -} - -void AbstractGridViewItem::deactivate() -{ - is_active_ = false; -} - AbstractGridView &AbstractGridViewItem::get_view() const { if (UNLIKELY(!view_)) { diff --git a/source/blender/editors/interface/views/tree_view.cc b/source/blender/editors/interface/views/tree_view.cc index f4bf91be8a3..7b3220f89e6 100644 --- a/source/blender/editors/interface/views/tree_view.cc +++ b/source/blender/editors/interface/views/tree_view.cc @@ -73,6 +73,12 @@ void TreeViewItemContainer::foreach_item_recursive(ItemIterFn iter_fn, IterOptio /* ---------------------------------------------------------------------- */ +/* Implementation for the base class virtual function. More specialized iterators below. */ +void AbstractTreeView::foreach_view_item(FunctionRef iter_fn) const +{ + foreach_item_recursive(iter_fn); +} + void AbstractTreeView::foreach_item(ItemIterFn iter_fn, IterOptions options) const { foreach_item_recursive(iter_fn, options); @@ -224,28 +230,6 @@ AbstractTreeViewItem *AbstractTreeView::find_matching_child( return nullptr; } -void AbstractTreeView::change_state_delayed() -{ - BLI_assert_msg( - is_reconstructed(), - "These state changes are supposed to be delayed until reconstruction is completed"); - -/* Debug-only sanity check: Ensure only one item requests to be active. */ -#ifndef NDEBUG - bool has_active = false; - foreach_item([&has_active](AbstractTreeViewItem &item) { - if (item.should_be_active().value_or(false)) { - BLI_assert_msg( - !has_active, - "Only one view item should ever return true for its `should_be_active()` method"); - has_active = true; - } - }); -#endif - - foreach_item([](AbstractTreeViewItem &item) { item.change_state_delayed(); }); -} - /* ---------------------------------------------------------------------- */ TreeViewItemDropTarget::TreeViewItemDropTarget(AbstractTreeView &view, DropBehavior behavior) @@ -416,16 +400,6 @@ bool AbstractTreeViewItem::has_active_child() const return found; } -void AbstractTreeViewItem::on_activate() -{ - /* Do nothing by default. */ -} - -std::optional AbstractTreeViewItem::should_be_active() const -{ - return std::nullopt; -} - bool AbstractTreeViewItem::supports_collapsing() const { return true; @@ -496,35 +470,13 @@ int AbstractTreeViewItem::count_parents() const bool AbstractTreeViewItem::set_state_active() { - BLI_assert_msg(get_tree_view().is_reconstructed(), - "Item activation can't be done until reconstruction is completed"); - - if (!is_activatable_) { - return false; - } - if (is_active()) { - return false; + if (AbstractViewItem::set_state_active()) { + /* Make sure the active item is always visible. */ + ensure_parents_uncollapsed(); + return true; } - /* Deactivate other items in the tree. */ - get_tree_view().foreach_item([](auto &item) { item.deactivate(); }); - /* Make sure the active item is always visible. */ - ensure_parents_uncollapsed(); - - is_active_ = true; - return true; -} - -void AbstractTreeViewItem::activate() -{ - if (set_state_active()) { - on_activate(); - } -} - -void AbstractTreeViewItem::deactivate() -{ - is_active_ = false; + return false; } bool AbstractTreeViewItem::is_hovered() const @@ -597,16 +549,6 @@ bool AbstractTreeViewItem::matches(const AbstractViewItem &other) const return true; } -void AbstractTreeViewItem::change_state_delayed() -{ - const std::optional should_be_active = this->should_be_active(); - if (should_be_active.has_value() && *should_be_active) { - /* Don't call #activate() here, since this reflects an external state change and therefore - * shouldn't call #on_activate(). */ - set_state_active(); - } -} - /* ---------------------------------------------------------------------- */ class TreeViewLayoutBuilder {