griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix logical error in window creation with an empty space

Browse Source 
Resolve a crash in !104831, which exposed an error in window creation
with an empty space type.

Creating a new window with an empty space type would do the following:

- Create a new screen with a single empty area.
- Refresh the screen,
  - Initialize the area.
    - Convert the empty area into a 3D viewport.
    - Run SpaceType::init()

This doesn't cause any problems at the moment because `view3d_init`
isn't doing anything however !104831 accesses the View3D from
`area->spacedata.first` causing SCREEN_OT_area_dupli to crash.

Resolve by supporting an area setup callback for WM_window_open
so the area-data can be set before it's initialized.

The issue remains where an unknown/empty ScrArea::spacetype results in
a crash although it seems unlikely users run into this in practice.
Whatever the case, that can be resolved separately.
This commit is contained in:
Campbell Barton
2023-07-20 16:42:59 +10:00
parent ecd729433d
commit 486086d349
5 changed files with 63 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
source/blender/editors/render/render_view.cc
View File

@@ -161,7 +161,9 @@ ScrArea *render_view_open(bContext *C, int mx, int my, ReportList *reports)
true,
false,
true,
WIN_ALIGN_LOCATION_CENTER) == nullptr)
WIN_ALIGN_LOCATION_CENTER,
nullptr,
nullptr) == nullptr)
{
BKE_report(reports, RPT_ERROR, "Failed to open window!");
return nullptr;

4
source/blender/editors/screen/screen_edit.cc
View File

@@ -1647,7 +1647,9 @@ ScrArea *ED_screen_temp_space_open(bContext *C,
false,
dialog,
true,
WIN_ALIGN_LOCATION_CENTER))
WIN_ALIGN_LOCATION_CENTER,
nullptr,
nullptr))
{
area = CTX_wm_area(C);
}

37
source/blender/editors/screen/screen_ops.cc
View File

@@ -1428,6 +1428,13 @@ static void SCREEN_OT_area_swap(wmOperatorType *ot)
* Create new window from area.
* \{ */
/** Callback for #WM_window_open to setup the area's data. */
static void area_dupli_fn(bScreen * /*screen*/, ScrArea *area, void *user_data)
{
ScrArea *area_src = static_cast<ScrArea *>(user_data);
ED_area_data_copy(area, area_src, true);
};
/* operator callback */
static int area_dupli_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
@@ -1449,15 +1456,19 @@ static int area_dupli_invoke(bContext *C, wmOperator *op, const wmEvent *event)
};
/* Create new window. No need to set space_type since it will be copied over. */
wmWindow *newwin = WM_window_open(
C, "Blender", &window_rect, SPACE_EMPTY, false, false, false, WIN_ALIGN_ABSOLUTE);
wmWindow *newwin = WM_window_open(C,
"Blender",
&window_rect,
SPACE_EMPTY,
false,
false,
false,
WIN_ALIGN_ABSOLUTE,
/* Initialize area from callback. */
area_dupli_fn,
(void *)area);
if (newwin) {
/* copy area to new screen */
bScreen *newsc = WM_window_get_active_screen(newwin);
ED_area_data_copy((ScrArea *)newsc->areabase.first, area, true);
ED_area_tag_redraw((ScrArea *)newsc->areabase.first);
/* screen, areas init */
WM_event_add_notifier(C, NC_SCREEN | NA_EDITED, nullptr);
}
@@ -5161,7 +5172,9 @@ static int userpref_show_exec(bContext *C, wmOperator *op)
false,
false,
true,
WIN_ALIGN_LOCATION_CENTER) != nullptr)
WIN_ALIGN_LOCATION_CENTER,
nullptr,
nullptr) != nullptr)
{
/* The header only contains the editor switcher and looks empty.
* So hiding in the temp window makes sense. */
@@ -5238,7 +5251,9 @@ static int drivers_editor_show_exec(bContext *C, wmOperator *op)
false,
false,
true,
WIN_ALIGN_LOCATION_CENTER) != nullptr)
WIN_ALIGN_LOCATION_CENTER,
nullptr,
nullptr) != nullptr)
{
ED_drivers_editor_init(C, CTX_wm_area(C));
@@ -5319,7 +5334,9 @@ static int info_log_show_exec(bContext *C, wmOperator *op)
false,
false,
true,
WIN_ALIGN_LOCATION_CENTER) != nullptr)
WIN_ALIGN_LOCATION_CENTER,
nullptr,
nullptr) != nullptr)
{
return OPERATOR_FINISHED;
}

10
source/blender/windowmanager/WM_api.h
View File

@@ -336,6 +336,10 @@ typedef enum eWindowAlignment {
* \param dialog: whether this should be made as a dialog-style window
* \param temp: whether this is considered a short-lived window
* \param alignment: how this window is positioned relative to its parent
* \param area_setup_fn: An optional callback which can be used to initialize the area
* before it's initialized. When set, `space_type` should be #SPACE_EMTPY,
* so the setup function can take a blank area and initialize it.
* \param area_setup_user_data: User data argument passed to `area_setup_fn`.
* \return the window or NULL in case of failure.
*/
struct wmWindow *WM_window_open(struct bContext *C,
@@ -345,7 +349,11 @@ struct wmWindow *WM_window_open(struct bContext *C,
bool toplevel,
bool dialog,
bool temp,
eWindowAlignment alignment) ATTR_NONNULL(1, 2, 3);
eWindowAlignment alignment,
void (*area_setup_fn)(bScreen *screen,
ScrArea *area,
void *user_data),
void *area_setup_user_data) ATTR_NONNULL(1, 2, 3);
void WM_window_set_dpi(const wmWindow *win);

25
source/blender/windowmanager/intern/wm_window.c
View File

@@ -880,7 +880,9 @@ wmWindow *WM_window_open(bContext *C,
bool toplevel,
bool dialog,
bool temp,
eWindowAlignment alignment)
eWindowAlignment alignment,
void (*area_setup_fn)(bScreen *screen, ScrArea *area, void *user_data),
void *area_setup_user_data)
{
Main *bmain = CTX_data_main(C);
wmWindowManager *wm = CTX_wm_manager(C);
@@ -984,8 +986,21 @@ wmWindow *WM_window_open(bContext *C,
* to avoid having to take into account a partially-created window.
*/
/* ensure it shows the right spacetype editor */
if (space_type != SPACE_EMPTY) {
if (area_setup_fn) {
/* When the caller is setting up the area, it should always be empty
* because it's expected the callback sets the type. */
BLI_assert(space_type == SPACE_EMPTY);
/* NOTE(@ideasman42): passing in a callback to setup the `area` is admittedly awkward.
* This is done so #ED_screen_refresh has a valid area to initialize,
* otherwise it will attempt to make the empty area usable via #ED_area_init.
* While refreshing the window could be postponed this makes the state of the
* window less predictable to the caller. */
ScrArea *area = screen->areabase.first;
area_setup_fn(screen, area, area_setup_user_data);
CTX_wm_area_set(C, area);
}
else if (space_type != SPACE_EMPTY) {
/* Ensure it shows the right space-type editor. */
ScrArea *area = screen->areabase.first;
CTX_wm_area_set(C, area);
ED_area_newspace(C, area, space_type, false);
@@ -1048,7 +1063,9 @@ int wm_window_new_exec(bContext *C, wmOperator *op)
false,
false,
false,
WIN_ALIGN_PARENT_CENTER) != NULL);
WIN_ALIGN_PARENT_CENTER,
NULL,
NULL) != NULL);
if (!ok) {
BKE_report(op->reports, RPT_ERROR, "Failed to create window");