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:
committed by
Julian Eisel
parent
607ad28604
commit
99fe6785d0
@@ -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();
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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; });
|
||||
|
||||
|
||||
Reference in New Issue
Block a user