bartvdbraak/blender
1
0
Fork 4
Code Issues 4 Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

Fix #121686: Regression: Context Menu has many operations disabled

Browse Source 
Resolve own regression in [0] which caused operators poll functions
in context menus to fail because the menus own region was set
causing the button clicked on not to be detected as the active button.

Resolve by passing "can_refresh" as an argument to ui_popup_block_create
which only sets the "region_popup" context item for popups that can
refresh. This is done because previously "region_popup" was only ever
set for temporary regions that refreshed (details in code-comments).

[0]: 38d11482f5e961e4d05930550452436da446dd4c
This commit is contained in:
Campbell Barton 2024-05-14 14:08:28 +10:00
parent 01a78bf3e6
commit 9ed5ed1a2b
7 changed files with 65 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
source/blender/editors/include/UI_interface_c.hh
View File

@ -786,6 +786,11 @@ using uiBlockCreateFunc = uiBlock *(*)(bContext *C, ARegion *region, void *arg1)
using uiBlockCancelFunc = void (*)(bContext *C, void *arg1);
void UI_popup_block_invoke(bContext *C, uiBlockCreateFunc func, void *arg, uiFreeArgFunc arg_free);
/**
* \param can_refresh: When true, the popup may be refreshed (updated after creation).
* \note It can be useful to disable refresh (even though it will work)
* as this exits text fields which can be disruptive if refresh isn't needed.
*/
void UI_popup_block_invoke_ex(
bContext *C, uiBlockCreateFunc func, void *arg, uiFreeArgFunc arg_free, bool can_refresh);
void UI_popup_block_ex(bContext *C,

3
source/blender/editors/interface/interface_handlers.cc
View File

@ -4336,7 +4336,8 @@ static void ui_block_open_begin(bContext *C, uiBut *but, uiHandleButtonData *dat
}
if (func || handlefunc) {
data->menu = ui_popup_block_create(C, data->region, but, func, handlefunc, arg, nullptr);
data->menu = ui_popup_block_create(
C, data->region, but, func, handlefunc, arg, nullptr, false);
if (but->block->handle) {
data->menu->popup = but->block->handle->popup;
}

10
source/blender/editors/interface/interface_intern.hh
View File

@ -868,7 +868,12 @@ struct uiPopupBlockHandle {
/** Store data for refreshing popups. */
uiPopupBlockCreate popup_create_vars;
/** True if we can re-create the popup using #uiPopupBlockHandle.popup_create_vars. */
/**
* True if we can re-create the popup using #uiPopupBlockHandle.popup_create_vars.
*
* \note Popups that can refresh are called with #bContext::wm::region_popup set
* to the #uiPopupBlockHandle::region both on initial creation and when refreshing.
*/
bool can_refresh;
bool refresh;
@ -985,7 +990,8 @@ uiPopupBlockHandle *ui_popup_block_create(bContext *C,
uiBlockCreateFunc create_func,
uiBlockHandleCreateFunc handle_create_func,
void *arg,
uiFreeArgFunc arg_free);
uiFreeArgFunc arg_free,
bool can_refresh);
uiPopupBlockHandle *ui_popup_menu_create(
bContext *C, ARegion *butregion, uiBut *but, uiMenuCreateFunc menu_func, void *arg);

3
source/blender/editors/interface/regions/interface_region_menu_pie.cc
View File

@ -175,7 +175,8 @@ void UI_pie_menu_end(bContext *C, uiPieMenu *pie)
wmWindow *window = CTX_wm_window(C);
uiPopupBlockHandle *menu;
menu = ui_popup_block_create(C, nullptr, nullptr, nullptr, ui_block_func_PIE, pie, nullptr);
menu = ui_popup_block_create(
C, nullptr, nullptr, nullptr, ui_block_func_PIE, pie, nullptr, false);
menu->popup = true;
menu->towardstime = BLI_time_now_seconds();

40
source/blender/editors/interface/regions/interface_region_menu_popup.cc
View File

@ -390,7 +390,8 @@ static uiPopupBlockHandle *ui_popup_menu_create_impl(
ARegion *butregion,
uiBut *but,
const char *title,
std::function<void(bContext *, uiLayout *)> menu_func)
std::function<void(bContext *, uiLayout *)> menu_func,
const bool can_refresh)
{
wmWindow *window = CTX_wm_window(C);
@ -415,7 +416,7 @@ static uiPopupBlockHandle *ui_popup_menu_create_impl(
pup->popup = true;
}
uiPopupBlockHandle *handle = ui_popup_block_create(
C, butregion, but, nullptr, ui_block_func_POPUP, pup, ui_block_free_func_POPUP);
C, butregion, but, nullptr, ui_block_func_POPUP, pup, ui_block_free_func_POPUP, can_refresh);
if (!but) {
handle->popup = true;
@ -431,9 +432,12 @@ uiPopupBlockHandle *ui_popup_menu_create(
bContext *C, ARegion *butregion, uiBut *but, uiMenuCreateFunc menu_func, void *arg)
{
return ui_popup_menu_create_impl(
C, butregion, but, nullptr, [menu_func, arg](bContext *C, uiLayout *layout) {
menu_func(C, layout, arg);
});
C,
butregion,
but,
nullptr,
[menu_func, arg](bContext *C, uiLayout *layout) { menu_func(C, layout, arg); },
false);
}
/** \} */
@ -509,7 +513,7 @@ void UI_popup_menu_end(bContext *C, uiPopupMenu *pup)
}
uiPopupBlockHandle *menu = ui_popup_block_create(
C, butregion, but, nullptr, ui_block_func_POPUP, pup, nullptr);
C, butregion, but, nullptr, ui_block_func_POPUP, pup, nullptr, false);
menu->popup = true;
UI_popup_handlers_add(C, &window->modalhandlers, menu, 0);
@ -601,15 +605,19 @@ static void ui_popup_menu_create_from_menutype(bContext *C,
const int icon)
{
uiPopupBlockHandle *handle = ui_popup_menu_create_impl(
C, nullptr, nullptr, title, [mt, title, icon](bContext *C, uiLayout *layout) -> void {
C,
nullptr,
nullptr,
title,
[mt, title, icon](bContext *C, uiLayout *layout) -> void {
if (title && title[0]) {
create_title_button(layout, title, icon);
}
ui_item_menutype_func(C, layout, mt);
});
},
true);
STRNCPY(handle->menu_idname, mt->idname);
handle->can_refresh = true;
WorkspaceStatus status(C);
if (bool(mt->flag & MenuTypeFlag::SearchOnKeyPress)) {
@ -659,18 +667,14 @@ int UI_popup_menu_invoke(bContext *C, const char *idname, ReportList *reports)
* \{ */
void UI_popup_block_invoke_ex(
bContext *C, uiBlockCreateFunc func, void *arg, uiFreeArgFunc arg_free, bool can_refresh)
bContext *C, uiBlockCreateFunc func, void *arg, uiFreeArgFunc arg_free, const bool can_refresh)
{
wmWindow *window = CTX_wm_window(C);
uiPopupBlockHandle *handle = ui_popup_block_create(
C, nullptr, nullptr, func, nullptr, arg, arg_free);
C, nullptr, nullptr, func, nullptr, arg, arg_free, can_refresh);
handle->popup = true;
/* It can be useful to disable refresh (even though it will work)
* as this exists text fields which can be disruptive if refresh isn't needed. */
handle->can_refresh = can_refresh;
UI_popup_handlers_add(C, &window->modalhandlers, handle, 0);
UI_block_active_only_flagged_buttons(
C, handle->region, static_cast<uiBlock *>(handle->region->uiblocks.first));
@ -692,10 +696,9 @@ void UI_popup_block_ex(bContext *C,
wmWindow *window = CTX_wm_window(C);
uiPopupBlockHandle *handle = ui_popup_block_create(
C, nullptr, nullptr, func, nullptr, arg, nullptr);
C, nullptr, nullptr, func, nullptr, arg, nullptr, true);
handle->popup = true;
handle->retvalue = 1;
handle->can_refresh = true;
handle->popup_op = op;
handle->popup_arg = arg;
@ -718,10 +721,9 @@ void uiPupBlockOperator(bContext *C,
wmWindow *window = CTX_wm_window(C);
uiPopupBlockHandle *handle;
handle = ui_popup_block_create(C, nullptr, nullptr, func, nullptr, op, nullptr);
handle = ui_popup_block_create(C, nullptr, nullptr, func, nullptr, op, nullptr, true);
handle->popup = 1;
handle->retvalue = 1;
handle->can_refresh = true;
handle->popup_arg = op;
handle->popup_func = operator_cb;

6
source/blender/editors/interface/regions/interface_region_popover.cc
View File

@ -273,8 +273,7 @@ uiPopupBlockHandle *ui_popover_panel_create(bContext *C,
/* Create popup block. */
uiPopupBlockHandle *handle;
handle = ui_popup_block_create(
C, butregion, but, nullptr, ui_block_func_POPOVER, pup, ui_block_free_func_POPOVER);
handle->can_refresh = true;
C, butregion, but, nullptr, ui_block_func_POPOVER, pup, ui_block_free_func_POPOVER, true);
/* Add handlers. If attached to a button, the button will already
* add a modal handler and pass on events. */
@ -393,7 +392,8 @@ void UI_popover_end(bContext *C, uiPopover *pup, wmKeyMap *keymap)
nullptr,
ui_block_func_POPOVER,
pup,
ui_block_free_func_POPOVER);
ui_block_free_func_POPOVER,
false);
/* Add handlers. */
UI_popup_handlers_add(C, &window->modalhandlers, handle, 0);

32
source/blender/editors/interface/regions/interface_region_popup.cc
View File

@ -852,7 +852,8 @@ uiPopupBlockHandle *ui_popup_block_create(bContext *C,
uiBlockCreateFunc create_func,
uiBlockHandleCreateFunc handle_create_func,
void *arg,
uiFreeArgFunc arg_free)
uiFreeArgFunc arg_free,
const bool can_refresh)
{
wmWindow *window = CTX_wm_window(C);
uiBut *activebut = UI_context_active_but_get(C);
@ -870,6 +871,7 @@ uiPopupBlockHandle *ui_popup_block_create(bContext *C,
/* store context for operator */
handle->ctx_area = CTX_wm_area(C);
handle->ctx_region = CTX_wm_region(C);
handle->can_refresh = can_refresh;
/* store vars to refresh popup (RGN_REFRESH_UI) */
handle->popup_create_vars.create_func = create_func;
@ -880,9 +882,6 @@ uiPopupBlockHandle *ui_popup_block_create(bContext *C,
handle->popup_create_vars.butregion = but ? butregion : nullptr;
copy_v2_v2_int(handle->popup_create_vars.event_xy, window->eventstate->xy);
/* don't allow by default, only if popup type explicitly supports it */
handle->can_refresh = false;
/* create area region */
ARegion *region = ui_region_temp_add(CTX_wm_screen(C));
handle->region = region;
@ -901,14 +900,31 @@ uiPopupBlockHandle *ui_popup_block_create(bContext *C,
* Whereas this only runs on initial creation.
* Set the region here so drawing logic can rely on it being set.
* Note that restoring the previous value may not be needed, it just avoids potential
* problems caused by popups manipulating the context which created them. */
ARegion *region_popup_prev = CTX_wm_region_popup(C);
CTX_wm_region_popup_set(C, region);
* problems caused by popups manipulating the context which created them.
*
* The check for `can_refresh` exists because the context when refreshing sets the "region_popup"
* so failing to do so here would cause callbacks draw function to have a different context
* the first time it's called. Setting this in every context causes button context menus to
* fail because setting the "region_popup" causes poll functions to reference the popup region
* instead of the region where the button was created, see #121728.
*
* NOTE(@ideasman42): the logic for which popups run with their region set to
* #bContext::wm::region_popup could be adjusted, making this context member depend on
* the ability to refresh seems somewhat arbitrary although it does make *some* sense
* because accessing the region later (to tag for refreshing for example)
* only makes sense if that region supports refreshing. */
ARegion *region_popup_prev = nullptr;
if (can_refresh) {
region_popup_prev = CTX_wm_region_popup(C);
CTX_wm_region_popup_set(C, region);
}
uiBlock *block = ui_popup_block_refresh(C, handle, butregion, but);
handle = block->handle;
CTX_wm_region_popup_set(C, region_popup_prev);
if (can_refresh) {
CTX_wm_region_popup_set(C, region_popup_prev);
}
/* keep centered on window resizing */
if (block->bounds_type == UI_BLOCK_BOUNDS_POPUP_CENTER) {