UI: restore assert in CTX_wm_region_popup_set

This just hid that freed memory was being passed to
CTX_wm_region_popup_set.

The change from [0] exposed this issue, however the problem has
existed for a long time (likely since the inclusion of popups in the
context) it's just that setting the context member when popups
are first displayed made problems more likely to show up.

[0]: 38d11482f5
This commit is contained in:
Campbell Barton
2024-05-14 19:27:16 +10:00
parent 60c9f4026b
commit 566a77f605
2 changed files with 36 additions and 1 deletions

View File

@@ -997,7 +997,7 @@ void CTX_wm_region_set(bContext *C, ARegion *region)
void CTX_wm_region_popup_set(bContext *C, ARegion *region_popup)
{
/* BLI_assert(region_popup == nullptr || region_popup->regiontype == RGN_TYPE_TEMPORARY); */
BLI_assert(region_popup == nullptr || region_popup->regiontype == RGN_TYPE_TEMPORARY);
C->wm.region_popup = region_popup;
}

View File

@@ -136,6 +136,35 @@ static void wm_event_state_update_and_click_set_ex(wmEvent *event,
const bool is_keyboard,
const bool check_double_click);
/* -------------------------------------------------------------------- */
/** \name Private Utiilities
* \{ */
/**
* Return true if `region` exists in any screen.
* Note that `region` may be freed memory so it's contents should never be read.
*/
static bool screen_temp_region_exists(const ARegion *region)
{
/* TODO(@ideasman42): this function would ideally not be needed.
* It avoids problems restoring the #bContext::wm::region_popup
* when it's not known if the popup was removed, however it would be better to
* resolve this by ensuring the contexts previous state never references stale data.
*
* This could be done using a context "stack" allowing freeing windowing data to clear
* references at all levels in the stack. */
Main *bmain = G_MAIN;
LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) {
if (BLI_findindex(&screen->regionbase, region) != -1) {
return true;
}
}
return false;
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Event Management
* \{ */
@@ -2204,6 +2233,12 @@ void WM_event_remove_handlers(bContext *C, ListBase *handlers)
handler->remove_fn(C, handler->user_data);
/* Currently we don't have a practical way to check if this region
* was a temporary region created by `handler`, so do a full lookup. */
if (region_popup_prev && !screen_temp_region_exists(region_popup_prev)) {
region_popup_prev = nullptr;
}
CTX_wm_area_set(C, area_prev);
CTX_wm_region_set(C, region_prev);
CTX_wm_region_popup_set(C, region_popup_prev);