Fix issues with asset shelf context menu operators

Shortcut and quick favorite operators wouldn't show up in the regular asset
shelf. In the popup the shortcut operators would but they crashed with use
after frees.

Basic issue is that activating view-items by applying the button didn't work
well. That messes with button storage, but also the operator data (like
`uiBut.optype`) is unset after applying, in this case before the context menu
was built.

Instead, activate the view-item directly, and call the activate operator in its
`on_activate()` callback. The button shouldn't call this operator again, so
added a way to attach operators to buttons without calling them.

Seems generally useful to be able to attach an operator to buttons for
tooltips, "Assign Shortcut" and the like, without it being called on button
apply.

Pull Request: https://projects.blender.org/blender/blender/pulls/124466
This commit is contained in:
Julian Eisel
2024-07-12 10:02:10 +02:00
committed by Julian Eisel
parent 607ad28604
commit 99fe6785d0
7 changed files with 80 additions and 33 deletions

View File

@@ -69,6 +69,7 @@ class AssetViewItem : public ui::PreviewGridItem {
void build_grid_tile(uiLayout &layout) const override;
void build_context_menu(bContext &C, uiLayout &column) const override;
std::optional<bool> should_be_active() const override;
void on_activate(bContext &C) override;
bool should_be_filtered_visible(StringRefNull filter_string) const override;
std::unique_ptr<ui::AbstractViewItemDragController> create_drag_controller() const override;
@@ -195,6 +196,27 @@ void AssetViewItem::disable_asset_drag()
allow_asset_drag_ = false;
}
/**
* Needs freeing with #WM_operator_properties_free() (will be done by button if passed to that) and
* #MEM_freeN().
*/
std::optional<wmOperatorCallParams> create_activate_operator_params(
const StringRefNull op_name, const asset_system::AssetRepresentation &asset)
{
if (op_name.is_empty()) {
return {};
}
wmOperatorType *ot = WM_operatortype_find(op_name.c_str(), true);
if (!ot) {
return {};
}
PointerRNA *op_props = MEM_cnew<PointerRNA>(__func__);
WM_operator_properties_create_ptr(op_props, ot);
asset::operator_asset_reference_props_set(asset, *op_props);
return wmOperatorCallParams{ot, op_props, WM_OP_INVOKE_REGION_WIN};
}
void AssetViewItem::build_grid_tile(uiLayout &layout) const
{
const AssetView &asset_view = reinterpret_cast<const AssetView &>(this->get_view());
@@ -210,14 +232,19 @@ void AssetViewItem::build_grid_tile(uiLayout &layout) const
"active_file",
&file_ptr);
wmOperatorType *ot = WM_operatortype_find(shelf_type.activate_operator.c_str(), true);
PointerRNA op_props = PointerRNA_NULL;
if (ot) {
WM_operator_properties_create_ptr(&op_props, ot);
asset::operator_asset_reference_props_set(*handle_get_representation(&asset_), op_props);
if (std::optional<wmOperatorCallParams> activate_op = create_activate_operator_params(
shelf_type.activate_operator, *handle_get_representation(&asset_)))
{
uiBut *item_but = reinterpret_cast<uiBut *>(this->view_item_button());
/* Attach the operator, but don't call it through the button. We call it using
* #on_activate(). */
UI_but_operator_set(item_but, activate_op->optype, activate_op->opcontext, activate_op->opptr);
UI_but_operator_set_never_call(item_but);
MEM_freeN(activate_op->opptr);
}
ui::PreviewGridItem::build_grid_tile_button(layout, ot, &op_props);
ui::PreviewGridItem::build_grid_tile_button(layout);
}
void AssetViewItem::build_context_menu(bContext &C, uiLayout &column) const
@@ -247,6 +274,21 @@ std::optional<bool> AssetViewItem::should_be_active() const
return matches;
}
void AssetViewItem::on_activate(bContext &C)
{
const AssetView &asset_view = dynamic_cast<const AssetView &>(this->get_view());
const AssetShelfType &shelf_type = *asset_view.shelf_.type;
if (std::optional<wmOperatorCallParams> activate_op = create_activate_operator_params(
shelf_type.activate_operator, *handle_get_representation(&asset_)))
{
WM_operator_name_call_ptr(
&C, activate_op->optype, activate_op->opcontext, activate_op->opptr, nullptr);
WM_operator_properties_free(activate_op->opptr);
MEM_freeN(activate_op->opptr);
}
}
bool AssetViewItem::should_be_filtered_visible(const StringRefNull filter_string) const
{
const StringRefNull asset_name = handle_get_representation(&asset_)->get_name();

View File

@@ -210,12 +210,7 @@ class PreviewGridItem : public AbstractGridViewItem {
void build_grid_tile(uiLayout &layout) const override;
/**
* \note Takes ownership of the operator properties defined in \a op_props.
*/
void build_grid_tile_button(uiLayout &layout,
const wmOperatorType *ot = nullptr,
const PointerRNA *op_props = nullptr) const;
void build_grid_tile_button(uiLayout &layout) const;
/**
* Set a custom callback to execute when activating this view item. This way users don't have to

View File

@@ -1419,6 +1419,12 @@ void UI_but_operator_set(uiBut *but,
wmOperatorType *optype,
wmOperatorCallContext opcontext,
const PointerRNA *opptr = nullptr);
/**
* Disable calling operators from \a but in button handling. Useful to attach an operator to a
* button for tooltips, "Assign Shortcut", etc. without actually making the button execute the
* operator.
*/
void UI_but_operator_set_never_call(uiBut *but);
/** For passing inputs to ButO buttons. */
PointerRNA *UI_but_operator_ptr_ensure(uiBut *but);

View File

@@ -5647,6 +5647,11 @@ void UI_but_operator_set(uiBut *but,
}
}
void UI_but_operator_set_never_call(uiBut *but)
{
but->operator_never_call = true;
}
/* END Button containing both string label and icon */
/* cruft to make uiBlock and uiBut private */

View File

@@ -54,6 +54,7 @@
#include "ED_screen.hh"
#include "ED_undo.hh"
#include "UI_abstract_view.hh"
#include "UI_interface.hh"
#include "UI_interface_c.hh"
#include "UI_string_search.hh"
@@ -877,9 +878,15 @@ static void ui_apply_but_func(bContext *C, uiBut *but)
after->popup_op = block->handle->popup_op;
}
after->optype = but->optype;
after->opcontext = but->opcontext;
after->opptr = but->opptr;
if (!but->operator_never_call) {
after->optype = but->optype;
after->opcontext = but->opcontext;
after->opptr = but->opptr;
but->optype = nullptr;
but->opcontext = wmOperatorCallContext(0);
but->opptr = nullptr;
}
after->rnapoin = but->rnapoin;
after->rnaprop = but->rnaprop;
@@ -918,10 +925,6 @@ static void ui_apply_but_func(bContext *C, uiBut *but)
}
after->drawstr = ui_but_drawstr_without_sep_char(but);
but->optype = nullptr;
but->opcontext = wmOperatorCallContext(0);
but->opptr = nullptr;
}
/* typically call ui_apply_but_undo(), ui_apply_but_autokey() */
@@ -8132,12 +8135,11 @@ static int ui_do_button(bContext *C, uiBlock *block, uiBut *but, const wmEvent *
* right-clicking to spawn the context menu should also activate the item. This makes it
* clear which item will be operated on. Apply the button immediately, so context menu
* polls get the right active item. */
uiBut *clicked_view_item_but = but->type == UI_BTYPE_VIEW_ITEM ?
but :
ui_view_item_find_mouse_over(data->region, event->xy);
uiButViewItem *clicked_view_item_but = static_cast<uiButViewItem *>(
but->type == UI_BTYPE_VIEW_ITEM ? but :
ui_view_item_find_mouse_over(data->region, event->xy));
if (clicked_view_item_but) {
UI_but_execute(C, data->region, clicked_view_item_but);
ui_apply_but_funcs_after(C);
clicked_view_item_but->view_item->activate(*C);
}
/* RMB has two options now */

View File

@@ -270,6 +270,11 @@ struct uiBut {
wmOperatorType *optype = nullptr;
PointerRNA *opptr = nullptr;
wmOperatorCallContext opcontext = WM_OP_INVOKE_DEFAULT;
/**
* Keep an operator attached but never actually call it through the button. See
* #UI_but_operator_set_never_call().
*/
bool operator_never_call = false;
/** When non-zero, this is the key used to activate a menu items (`a-z` always lower case). */
uchar menu_key = 0;

View File

@@ -471,19 +471,11 @@ PreviewGridItem::PreviewGridItem(StringRef identifier, StringRef label, int prev
{
}
void PreviewGridItem::build_grid_tile_button(uiLayout &layout,
const wmOperatorType *ot,
const PointerRNA *op_props) const
void PreviewGridItem::build_grid_tile_button(uiLayout &layout) const
{
const GridViewStyle &style = this->get_view().get_style();
uiBlock *block = uiLayoutGetBlock(&layout);
if (ot) {
UI_but_operator_set(this->view_item_button(),
const_cast<wmOperatorType *>(ot),
WM_OP_INVOKE_REGION_WIN,
op_props);
}
UI_but_func_tooltip_label_set(this->view_item_button(),
[this](const uiBut * /*but*/) { return label; });