[Bf-blender-cvs] [bc8e030a840] blender-v3.2-release: Fix T88570: Crash when saving after pressing ctrl+S twice.
Julian Eisel
noreply at git.blender.org
Mon May 23 18:02:10 CEST 2022
Commit: bc8e030a84001cc4b0ed870e607b3f94d32faf82
Author: Julian Eisel
Date: Tue May 10 12:27:04 2022 +0200
Branches: blender-v3.2-release
https://developer.blender.org/rBbc8e030a84001cc4b0ed870e607b3f94d32faf82
Fix T88570: Crash when saving after pressing ctrl+S twice.
Existing code to replace the file operation was failing when done from
the window for the file operation itself.
Basically, this patch does two things:
- Implement a well defined window context to use as the "owner" or
"root" of the File Browser. This will be used for managing the File
Browser and to execute the file operation, even after the File Browser
was closed.
- Ensure the context is valid when dealing with file File Browser event
handlers.
Previously the window context just wasn't well defined and just happened
to work well enough in most cases. Addressing this may unveil further
issues, see T88570#1355740.
Differential Revision: https://developer.blender.org/D13441
Reviewed by: Campbell Barton
===================================================================
M source/blender/editors/include/ED_fileselect.h
M source/blender/editors/space_file/filesel.c
M source/blender/windowmanager/intern/wm_event_system.c
===================================================================
diff --git a/source/blender/editors/include/ED_fileselect.h b/source/blender/editors/include/ED_fileselect.h
index c367072e6e7..e9fcd2bd5fe 100644
--- a/source/blender/editors/include/ED_fileselect.h
+++ b/source/blender/editors/include/ED_fileselect.h
@@ -164,8 +164,16 @@ void ED_fileselect_window_params_get(const struct wmWindow *win,
int win_size[2],
bool *is_maximized);
+/**
+ * Return the File Browser area in which \a file_operator is active.
+ */
struct ScrArea *ED_fileselect_handler_area_find(const struct wmWindow *win,
const struct wmOperator *file_operator);
+/**
+ * Check if there is any area in \a win that acts as a modal File Browser (#SpaceFile.op is set)
+ * and return it.
+ */
+struct ScrArea *ED_fileselect_handler_area_find_any_with_op(const struct wmWindow *win);
/* TODO: Maybe we should move this to BLI?
* On the other hand, it's using defines from space-file area, so not sure... */
diff --git a/source/blender/editors/space_file/filesel.c b/source/blender/editors/space_file/filesel.c
index 011506368ee..ce36e3e4e4f 100644
--- a/source/blender/editors/space_file/filesel.c
+++ b/source/blender/editors/space_file/filesel.c
@@ -1364,3 +1364,21 @@ ScrArea *ED_fileselect_handler_area_find(const wmWindow *win, const wmOperator *
return NULL;
}
+
+ScrArea *ED_fileselect_handler_area_find_any_with_op(const wmWindow *win)
+{
+ const bScreen *screen = WM_window_get_active_screen(win);
+
+ ED_screen_areas_iter (win, screen, area) {
+ if (area->spacetype != SPACE_FILE) {
+ continue;
+ }
+
+ const SpaceFile *sfile = area->spacedata.first;
+ if (sfile->op) {
+ return area;
+ }
+ }
+
+ return NULL;
+}
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 4b8552a4ec3..47b63869094 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -1899,8 +1899,15 @@ void wm_event_free_handler(wmEventHandler *handler)
MEM_freeN(handler);
}
-/* Only set context when area/region is part of screen. */
-static void wm_handler_op_context(bContext *C, wmEventHandler_Op *handler, const wmEvent *event)
+/**
+ * Check if the handler's area and/or region are actually part of the screen, and return them if
+ * so.
+ */
+static void wm_handler_op_context_get_if_valid(bContext *C,
+ wmEventHandler_Op *handler,
+ const wmEvent *event,
+ ScrArea **r_area,
+ ARegion **r_region)
{
wmWindow *win = handler->context.win ? handler->context.win : CTX_wm_window(C);
/* It's probably fine to always use #WM_window_get_active_screen() to get the screen. But this
@@ -1908,12 +1915,15 @@ static void wm_handler_op_context(bContext *C, wmEventHandler_Op *handler, const
* possible. */
bScreen *screen = handler->context.win ? WM_window_get_active_screen(win) : CTX_wm_screen(C);
+ *r_area = NULL;
+ *r_region = NULL;
+
if (screen == NULL || handler->op == NULL) {
return;
}
if (handler->context.area == NULL) {
- CTX_wm_area_set(C, NULL);
+ /* Pass */
}
else {
ScrArea *area = NULL;
@@ -1937,7 +1947,7 @@ static void wm_handler_op_context(bContext *C, wmEventHandler_Op *handler, const
else {
ARegion *region;
wmOperator *op = handler->op ? (handler->op->opm ? handler->op->opm : handler->op) : NULL;
- CTX_wm_area_set(C, area);
+ *r_area = area;
if (op && (op->flag & OP_IS_MODAL_CURSOR_REGION)) {
region = BKE_area_find_region_xy(area, handler->context.region_type, event->xy);
@@ -1960,12 +1970,21 @@ static void wm_handler_op_context(bContext *C, wmEventHandler_Op *handler, const
/* No warning print here, after full-area and back regions are remade. */
if (region) {
- CTX_wm_region_set(C, region);
+ *r_region = region;
}
}
}
}
+static void wm_handler_op_context(bContext *C, wmEventHandler_Op *handler, const wmEvent *event)
+{
+ ScrArea *area = NULL;
+ ARegion *region = NULL;
+ wm_handler_op_context_get_if_valid(C, handler, event, &area, ®ion);
+ CTX_wm_area_set(C, area);
+ CTX_wm_region_set(C, region);
+}
+
void WM_event_remove_handlers(bContext *C, ListBase *handlers)
{
wmWindowManager *wm = CTX_wm_manager(C);
@@ -2502,6 +2521,9 @@ static int wm_handler_fileselect_do(bContext *C,
case EVT_FILESELECT_CANCEL:
case EVT_FILESELECT_EXTERNAL_CANCEL: {
wmWindow *ctx_win = CTX_wm_window(C);
+ wmEvent *eventstate = ctx_win->eventstate;
+ /* The root window of the operation as determined in #WM_event_add_fileselect(). */
+ wmWindow *root_win = handler->context.win;
/* Remove link now, for load file case before removing. */
BLI_remlink(handlers, handler);
@@ -2537,21 +2559,17 @@ static int wm_handler_fileselect_do(bContext *C,
ED_fileselect_params_to_userdef(file_area->spacedata.first, win_size, is_maximized);
if (BLI_listbase_is_single(&file_area->spacedata)) {
- BLI_assert(ctx_win != win);
+ BLI_assert(root_win != win);
wm_window_close(C, wm, win);
- CTX_wm_window_set(C, ctx_win); /* #wm_window_close() NULLs. */
+ CTX_wm_window_set(C, root_win); /* #wm_window_close() NULLs. */
/* Some operators expect a drawable context (for #EVT_FILESELECT_EXEC). */
- wm_window_make_drawable(wm, ctx_win);
+ wm_window_make_drawable(wm, root_win);
/* Ensure correct cursor position, otherwise, popups may close immediately after
* opening (#UI_BLOCK_MOVEMOUSE_QUIT). */
- wm_cursor_position_get(
- ctx_win, &ctx_win->eventstate->xy[0], &ctx_win->eventstate->xy[1]);
- wm->winactive = ctx_win; /* Reports use this... */
- if (handler->context.win == win) {
- handler->context.win = NULL;
- }
+ wm_cursor_position_get(root_win, &eventstate->xy[0], &eventstate->xy[1]);
+ wm->winactive = root_win; /* Reports use this... */
}
else if (file_area->full) {
ED_screen_full_prevspace(C, file_area);
@@ -2570,7 +2588,13 @@ static int wm_handler_fileselect_do(bContext *C,
}
}
- wm_handler_op_context(C, handler, ctx_win->eventstate);
+ CTX_wm_window_set(C, root_win);
+ wm_handler_op_context(C, handler, eventstate);
+ /* At this point context is supposed to match the root context determined by
+ * #WM_event_add_fileselect(). */
+ BLI_assert(!CTX_wm_area(C) || (CTX_wm_area(C) == handler->context.area));
+ BLI_assert(!CTX_wm_region(C) || (CTX_wm_region(C) == handler->context.region));
+
ScrArea *handler_area = CTX_wm_area(C);
/* Make sure new context area is ready, the operator callback may operate on it. */
if (handler_area) {
@@ -3982,52 +4006,111 @@ void WM_event_fileselect_event(wmWindowManager *wm, void *ophandle, int eventval
}
}
+/**
+ * From the context window, try to find a window that is appropriate for use as root window of a
+ * modal File Browser (modal means: there is a #SpaceFile.op to execute). The root window will
+ * become the parent of the File Browser and provides a context to execute the file operator in,
+ * even after closing the File Browser.
+ *
+ * An appropriate window is either of the following:
+ * * A parent window that does not yet contain a modal File Browser. This is determined using
+ * #ED_fileselect_handler_area_find_any_with_op().
+ * * A parent window containing a modal File Browser, but in a maximized/fullscreen state. Users
+ * shouldn't be able to put a temporary screen like the modal File Browser into
+ * maximized/fullscreen state themselves. So this setup indicates that the File Browser was
+ * opened using #USER_TEMP_SPACE_DISPLAY_FULLSCREEN.
+ *
+ * If no appropriate parent window can be found from the context window, return the first
+ * registered window (which can be assumed to be a regular window, e.g. no modal File Browser; this
+ * is asserted).
+ */
+static wmWindow *wm_event_find_fileselect_root_window_from_context(const bContext *C)
+{
+ wmWindow *ctx_win = CTX_wm_window(C);
+
+ for (wmWindow *ctx_win_or_parent = ctx_win; ctx_win_or_parent;
+ ctx_win_or_parent = ctx_win_or_parent->parent) {
+ ScrArea *file_area = ED_fileselect_handler_area_find_any_with_op(ctx_win_or_parent);
+
+ if (!file_area) {
+ return ctx_win_or_parent;
+ }
+
+ if (file_area->full) {
+ return ctx_win_or_parent;
+ }
+ }
+
+ /* Fallback to the first window. */
+ const wmWindowManager *wm = CTX_wm_manager(C);
+ BLI_assert(!ED_fileselect_handler_area_find_any_with_op(wm->windows.first));
+ return wm->windows.first;
+}
+
/* Operator is supposed to have a filled "path" property. */
/* Optional property: file-type (XXX enum?) */
void WM_event_add_fileselect(bContext *C, wmOperator *op)
{
wmWindowManager *wm = CTX_wm_manager(C);
- wmWindow *win = CTX_wm_window(C);
- const bool is_temp_screen = WM_window_is_temp_screen(win);
+ wmWindow *ctx_win = CTX_wm_window(C);
+
+ /* The following vars define the root context. That is essentially the "parent" context of the
+ * File Browser operation, to be restored for eventually executing the file operation. */
+ wmWindow *root_win = wm_event_find_fileselect_root_window_from_context(C);
+ /* Determined later. */
+ ScrArea *root_area = NULL;
+ ARegion *root_region = NULL;
/* Close any popups, like when opening a file browser from the splash. */
- UI_popup_handlers_remove_all(C, &win->modalhandlers);
+ UI_popup_handlers_remove_all(C, &root_win->modalhandlers);
- if (!is_temp_screen) {
- /* Only allow 1 file selector open per window. */
- LISTBASE_FOREACH_MUTABLE (wmEventHandler *, handler_base, &win->modalhandlers) {
- if (handler_base->type == WM_HANDLER_TYPE_
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list