diff --git a/source/blender/editors/include/UI_grid_view.hh b/source/blender/editors/include/UI_grid_view.hh index 21493155bda..3562b5dcc61 100644 --- a/source/blender/editors/include/UI_grid_view.hh +++ b/source/blender/editors/include/UI_grid_view.hh @@ -56,11 +56,17 @@ class AbstractGridViewItem : public AbstractViewItem { /** See AbstractViewItem::matches(). */ /* virtual */ bool matches(const AbstractViewItem &other) const override; - /** Called when the item's state changes from inactive to active. */ + /** + * 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. + * 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; @@ -69,7 +75,10 @@ class AbstractGridViewItem : public AbstractViewItem { /** * Activates this item, deactivates other items, and calls the - * #AbstractGridViewItem::on_activate() function. + * #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 tree to have completed reconstruction, see #is_reconstructed(). Otherwise the * actual item state is unknown, possibly calling state-change update functions incorrectly. */ @@ -79,6 +88,12 @@ class AbstractGridViewItem : public AbstractViewItem { private: /** 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. + */ + bool set_state_active(); static void grid_tile_click_fn(bContext *, void *but_arg1, void *); void add_grid_tile_button(uiBlock &block); }; diff --git a/source/blender/editors/include/UI_tree_view.hh b/source/blender/editors/include/UI_tree_view.hh index be074d816da..77d237649d5 100644 --- a/source/blender/editors/include/UI_tree_view.hh +++ b/source/blender/editors/include/UI_tree_view.hh @@ -210,12 +210,16 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain protected: /** - * Called when the items state changes from inactive to active. + * 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. + * 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; @@ -250,7 +254,10 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain /** * 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). + * 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. */ @@ -273,6 +280,12 @@ class AbstractTreeViewItem : public AbstractViewItem, public TreeViewItemContain /** 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. + */ + bool set_state_active(); void add_treerow_button(uiBlock &block); int indent_width() const; diff --git a/source/blender/editors/interface/views/grid_view.cc b/source/blender/editors/interface/views/grid_view.cc index ccb3bd3ba07..d32c0b763cd 100644 --- a/source/blender/editors/interface/views/grid_view.cc +++ b/source/blender/editors/interface/views/grid_view.cc @@ -186,28 +186,36 @@ void AbstractGridViewItem::change_state_delayed() { const std::optional should_be_active = this->should_be_active(); if (should_be_active.has_value() && *should_be_active) { - activate(); + /* Don't call #activate() here, since this reflects an external state change and therefore + * shouldn't call #on_activate(). */ + set_state_active(); } } -void AbstractGridViewItem::activate() +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; + return false; } if (is_active()) { - return; + return false; } - /* Deactivate other items in the tree. */ + /* Deactivate other items in the view. */ get_view().foreach_item([](auto &item) { item.deactivate(); }); - on_activate(); - is_active_ = true; + return true; +} + +void AbstractGridViewItem::activate() +{ + if (set_state_active()) { + on_activate(); + } } void AbstractGridViewItem::deactivate() diff --git a/source/blender/editors/interface/views/tree_view.cc b/source/blender/editors/interface/views/tree_view.cc index 1d25ad74254..f4bf91be8a3 100644 --- a/source/blender/editors/interface/views/tree_view.cc +++ b/source/blender/editors/interface/views/tree_view.cc @@ -494,26 +494,32 @@ int AbstractTreeViewItem::count_parents() const return i; } -void AbstractTreeViewItem::activate() +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; + return false; } if (is_active()) { - return; + return false; } /* Deactivate other items in the tree. */ get_tree_view().foreach_item([](auto &item) { item.deactivate(); }); - - on_activate(); /* 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() @@ -595,7 +601,9 @@ void AbstractTreeViewItem::change_state_delayed() { const std::optional should_be_active = this->should_be_active(); if (should_be_active.has_value() && *should_be_active) { - activate(); + /* Don't call #activate() here, since this reflects an external state change and therefore + * shouldn't call #on_activate(). */ + set_state_active(); } }