From 41d62f36d525afa62008225c06b43bb100f99809 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 3 Oct 2023 15:14:39 +1100 Subject: [PATCH] Fix uninitialized variable use accessing the screen size under Wayland It's possible for there to be no outputs under Wayland (when unplugging monitors for e.g.) so this must be accounted for. Also avoid calculating the window position when the GHOST backend doesn't support window positions (which is the case for Wayland). Add checks for the SDL backend too, where accessing the screen & desktop size may fail. --- intern/ghost/GHOST_C-api.h | 23 +++--- intern/ghost/intern/GHOST_C-api.cc | 24 ++++--- intern/ghost/intern/GHOST_SystemSDL.cc | 10 ++- intern/ghost/intern/GHOST_SystemWayland.cc | 45 ++++++------ intern/ghost/test/multitest/MultiTest.c | 11 ++- .../windowmanager/intern/wm_playanim.cc | 10 +-- .../blender/windowmanager/intern/wm_window.cc | 71 ++++++++++++------- source/blender/windowmanager/wm_window.hh | 6 +- 8 files changed, 123 insertions(+), 77 deletions(-) diff --git a/intern/ghost/GHOST_C-api.h b/intern/ghost/GHOST_C-api.h index 83d9ce18644..5bfa96d4cb0 100644 --- a/intern/ghost/GHOST_C-api.h +++ b/intern/ghost/GHOST_C-api.h @@ -130,25 +130,26 @@ extern uint8_t GHOST_GetNumDisplays(GHOST_SystemHandle systemhandle); /** * Returns the dimensions of the main display on this system. * \param systemhandle: The handle to the system. - * \param width: A pointer the width gets put in. - * \param height: A pointer the height gets put in. + * \param r_width: A pointer the width gets put in. + * \param r_height: A pointer the height gets put in. + * \return success. */ -extern void GHOST_GetMainDisplayDimensions(GHOST_SystemHandle systemhandle, - uint32_t *width, - uint32_t *height); +extern GHOST_TSuccess GHOST_GetMainDisplayDimensions(GHOST_SystemHandle systemhandle, + uint32_t *r_width, + uint32_t *r_height); /** * Returns the dimensions of all displays combine * (the current workspace). * No need to worry about overlapping monitors. * \param systemhandle: The handle to the system. - * \param width: A pointer the width gets put in. - * \param height: A pointer the height gets put in. + * \param r_width: A pointer the width gets put in. + * \param r_height: A pointer the height gets put in. + * \return success. */ -extern void GHOST_GetAllDisplayDimensions(GHOST_SystemHandle systemhandle, - uint32_t *width, - uint32_t *height); - +extern GHOST_TSuccess GHOST_GetAllDisplayDimensions(GHOST_SystemHandle systemhandle, + uint32_t *r_width, + uint32_t *r_height); /** * Create a new window. * The new window is added to the list of windows managed. diff --git a/intern/ghost/intern/GHOST_C-api.cc b/intern/ghost/intern/GHOST_C-api.cc index 0c7348841d6..5dcc9bbe9db 100644 --- a/intern/ghost/intern/GHOST_C-api.cc +++ b/intern/ghost/intern/GHOST_C-api.cc @@ -118,22 +118,26 @@ uint8_t GHOST_GetNumDisplays(GHOST_SystemHandle systemhandle) return system->getNumDisplays(); } -void GHOST_GetMainDisplayDimensions(GHOST_SystemHandle systemhandle, - uint32_t *width, - uint32_t *height) +GHOST_TSuccess GHOST_GetMainDisplayDimensions(GHOST_SystemHandle systemhandle, + uint32_t *r_width, + uint32_t *r_height) { const GHOST_ISystem *system = (const GHOST_ISystem *)systemhandle; - - system->getMainDisplayDimensions(*width, *height); + *r_width = 0; + *r_height = 0; + system->getMainDisplayDimensions(*r_width, *r_height); + return (*r_width == 0 && *r_height == 0) ? GHOST_kFailure : GHOST_kSuccess; } -void GHOST_GetAllDisplayDimensions(GHOST_SystemHandle systemhandle, - uint32_t *width, - uint32_t *height) +GHOST_TSuccess GHOST_GetAllDisplayDimensions(GHOST_SystemHandle systemhandle, + uint32_t *r_width, + uint32_t *r_height) { const GHOST_ISystem *system = (const GHOST_ISystem *)systemhandle; - - system->getAllDisplayDimensions(*width, *height); + *r_width = 0; + *r_height = 0; + system->getAllDisplayDimensions(*r_width, *r_height); + return (*r_width == 0 && *r_height == 0) ? GHOST_kFailure : GHOST_kSuccess; } GHOST_ContextHandle GHOST_CreateGPUContext(GHOST_SystemHandle systemhandle, diff --git a/intern/ghost/intern/GHOST_SystemSDL.cc b/intern/ghost/intern/GHOST_SystemSDL.cc index 81172120ff1..16613751005 100644 --- a/intern/ghost/intern/GHOST_SystemSDL.cc +++ b/intern/ghost/intern/GHOST_SystemSDL.cc @@ -109,7 +109,10 @@ GHOST_TSuccess GHOST_SystemSDL::init() void GHOST_SystemSDL::getAllDisplayDimensions(uint32_t &width, uint32_t &height) const { SDL_DisplayMode mode; - SDL_GetDesktopDisplayMode(0, &mode); /* NOTE: always 0 display. */ + const int display_index = 0; /* NOTE: always 0 display. */ + if (SDL_GetDesktopDisplayMode(display_index, &mode) < 0) { + return; + } width = mode.w; height = mode.h; } @@ -117,7 +120,10 @@ void GHOST_SystemSDL::getAllDisplayDimensions(uint32_t &width, uint32_t &height) void GHOST_SystemSDL::getMainDisplayDimensions(uint32_t &width, uint32_t &height) const { SDL_DisplayMode mode; - SDL_GetCurrentDisplayMode(0, &mode); /* NOTE: always 0 display. */ + const int display_index = 0; /* NOTE: always 0 display. */ + if (SDL_GetCurrentDisplayMode(display_index, &mode) < 0) { + return; + } width = mode.w; height = mode.h; } diff --git a/intern/ghost/intern/GHOST_SystemWayland.cc b/intern/ghost/intern/GHOST_SystemWayland.cc index 95c043e93b3..0d26335c2b1 100644 --- a/intern/ghost/intern/GHOST_SystemWayland.cc +++ b/intern/ghost/intern/GHOST_SystemWayland.cc @@ -971,6 +971,11 @@ struct GWL_Display { #endif GWL_XDG_Decor_System *xdg_decor = nullptr; + /** + * NOTE(@ideasman42): Handling of outputs must account for this vector to be empty. + * While this seems like something which might not ever happen, it can occur when + * unplugging monitors (presumably from dodgy cables/connections too). + */ std::vector outputs; std::vector seats; /** @@ -6295,12 +6300,11 @@ void GHOST_SystemWayland::getMainDisplayDimensions(uint32_t &width, uint32_t &he std::lock_guard lock_server_guard{*server_mutex}; #endif - if (display_->outputs.empty()) { - return; + if (!display_->outputs.empty()) { + /* We assume first output as main. */ + width = uint32_t(display_->outputs[0]->size_native[0]); + height = uint32_t(display_->outputs[0]->size_native[1]); } - /* We assume first output as main. */ - width = uint32_t(display_->outputs[0]->size_native[0]); - height = uint32_t(display_->outputs[0]->size_native[1]); } void GHOST_SystemWayland::getAllDisplayDimensions(uint32_t &width, uint32_t &height) const @@ -6308,24 +6312,25 @@ void GHOST_SystemWayland::getAllDisplayDimensions(uint32_t &width, uint32_t &hei #ifdef USE_EVENT_BACKGROUND_THREAD std::lock_guard lock_server_guard{*server_mutex}; #endif + if (!display_->outputs.empty()) { + int32_t xy_min[2] = {INT32_MAX, INT32_MAX}; + int32_t xy_max[2] = {INT32_MIN, INT32_MIN}; - int32_t xy_min[2] = {INT32_MAX, INT32_MAX}; - int32_t xy_max[2] = {INT32_MIN, INT32_MIN}; - - for (const GWL_Output *output : display_->outputs) { - int32_t xy[2] = {0, 0}; - if (output->has_position_logical) { - xy[0] = output->position_logical[0]; - xy[1] = output->position_logical[1]; + for (const GWL_Output *output : display_->outputs) { + int32_t xy[2] = {0, 0}; + if (output->has_position_logical) { + xy[0] = output->position_logical[0]; + xy[1] = output->position_logical[1]; + } + xy_min[0] = std::min(xy_min[0], xy[0]); + xy_min[1] = std::min(xy_min[1], xy[1]); + xy_max[0] = std::max(xy_max[0], xy[0] + output->size_native[0]); + xy_max[1] = std::max(xy_max[1], xy[1] + output->size_native[1]); } - xy_min[0] = std::min(xy_min[0], xy[0]); - xy_min[1] = std::min(xy_min[1], xy[1]); - xy_max[0] = std::max(xy_max[0], xy[0] + output->size_native[0]); - xy_max[1] = std::max(xy_max[1], xy[1] + output->size_native[1]); - } - width = xy_max[0] - xy_min[0]; - height = xy_max[1] - xy_min[1]; + width = xy_max[0] - xy_min[0]; + height = xy_max[1] - xy_min[1]; + } } GHOST_IContext *GHOST_SystemWayland::createOffscreenContext(GHOST_GPUSettings gpuSettings) diff --git a/intern/ghost/test/multitest/MultiTest.c b/intern/ghost/test/multitest/MultiTest.c index 9dd8dbfe0b6..34b9b78425c 100644 --- a/intern/ghost/test/multitest/MultiTest.c +++ b/intern/ghost/test/multitest/MultiTest.c @@ -566,12 +566,17 @@ LoggerWindow *loggerwindow_new(MultiTestApp *app) uint32_t screensize[2]; GHOST_WindowHandle win; - GHOST_GetMainDisplayDimensions(sys, &screensize[0], &screensize[1]); + int posx = 40; + int posy = 0; + if (GHOST_GetMainDisplayDimensions(sys, &screensize[0], &screensize[1]) == GHOST_kSuccess) { + posy = screensize[1] - 432; + } + win = GHOST_CreateWindow(sys, NULL, "MultiTest:Logger", - 40, - screensize[1] - 432, + posx, + posy, 800, 300, GHOST_kWindowStateNormal, diff --git a/source/blender/windowmanager/intern/wm_playanim.cc b/source/blender/windowmanager/intern/wm_playanim.cc index 1223fa44ae2..def4de03d3c 100644 --- a/source/blender/windowmanager/intern/wm_playanim.cc +++ b/source/blender/windowmanager/intern/wm_playanim.cc @@ -1461,11 +1461,13 @@ static GHOST_WindowHandle playanim_window_open( GHOST_GPUSettings gpusettings = {0}; const eGPUBackendType gpu_backend = GPU_backend_type_selection_get(); gpusettings.context_type = wm_ghost_drawing_context_type(gpu_backend); - uint32_t scr_w, scr_h; - GHOST_GetMainDisplayDimensions(ghost_system, &scr_w, &scr_h); - - posy = (scr_h - posy - sizey); + if (GHOST_GetCapabilities() & GHOST_kCapabilityWindowPosition) { + uint32_t scr_w, scr_h; + if (GHOST_GetMainDisplayDimensions(ghost_system, &scr_w, &scr_h) == GHOST_kSuccess) { + posy = (scr_h - posy - sizey); + } + } return GHOST_CreateWindow(ghost_system, nullptr, diff --git a/source/blender/windowmanager/intern/wm_window.cc b/source/blender/windowmanager/intern/wm_window.cc index 7cf27da99a3..095a2e8b436 100644 --- a/source/blender/windowmanager/intern/wm_window.cc +++ b/source/blender/windowmanager/intern/wm_window.cc @@ -166,36 +166,39 @@ static void wm_window_set_drawable(wmWindowManager *wm, wmWindow *win, bool acti static bool wm_window_timers_process(const bContext *C, int *sleep_us); static uint8_t wm_ghost_modifier_query(const enum ModSide side); -void wm_get_screensize(int *r_width, int *r_height) +bool wm_get_screensize(int *r_width, int *r_height) { - uint uiwidth; - uint uiheight; - - GHOST_GetMainDisplayDimensions(g_system, &uiwidth, &uiheight); + uint32_t uiwidth, uiheight; + if (GHOST_GetMainDisplayDimensions(g_system, &uiwidth, &uiheight) == GHOST_kFailure) { + return false; + } *r_width = uiwidth; *r_height = uiheight; + return true; } -void wm_get_desktopsize(int *r_width, int *r_height) +bool wm_get_desktopsize(int *r_width, int *r_height) { - uint uiwidth; - uint uiheight; - - GHOST_GetAllDisplayDimensions(g_system, &uiwidth, &uiheight); + uint32_t uiwidth, uiheight; + if (GHOST_GetAllDisplayDimensions(g_system, &uiwidth, &uiheight) == GHOST_kFailure) { + return false; + } *r_width = uiwidth; *r_height = uiheight; + return true; } /* keeps size within monitor bounds */ static void wm_window_check_size(rcti *rect) { int width, height; - wm_get_screensize(&width, &height); - if (BLI_rcti_size_x(rect) > width) { - BLI_rcti_resize_x(rect, width); - } - if (BLI_rcti_size_y(rect) > height) { - BLI_rcti_resize_y(rect, height); + if (wm_get_screensize(&width, &height)) { + if (BLI_rcti_size_x(rect) > width) { + BLI_rcti_resize_x(rect, width); + } + if (BLI_rcti_size_y(rect) > height) { + BLI_rcti_resize_y(rect, height); + } } } @@ -686,9 +689,16 @@ static void wm_window_ghostwindow_add(wmWindowManager *wm, eGPUBackendType gpu_backend = GPU_backend_type_selection_get(); gpuSettings.context_type = wm_ghost_drawing_context_type(gpu_backend); - int scr_w, scr_h; - wm_get_desktopsize(&scr_w, &scr_h); - int posy = (scr_h - win->posy - win->sizey); + int posx = 0; + int posy = 0; + + if (WM_capabilities_flag() & WM_CAPABILITY_WINDOW_POSITION) { + int scr_w, scr_h; + if (wm_get_desktopsize(&scr_w, &scr_h)) { + posx = win->posx; + posy = (scr_h - win->posy - win->sizey); + } + } /* Clear drawable so we can set the new window. */ wmWindow *prev_windrawable = wm->windrawable; @@ -698,7 +708,7 @@ static void wm_window_ghostwindow_add(wmWindowManager *wm, g_system, static_cast((win->parent) ? win->parent->ghostwin : nullptr), title, - win->posx, + posx, posy, win->sizex, win->sizey, @@ -830,7 +840,11 @@ void wm_window_ghostwindows_ensure(wmWindowManager *wm) * when there is no startup.blend yet. */ if (wm_init_state.size_x == 0) { - wm_get_screensize(&wm_init_state.size_x, &wm_init_state.size_y); + if (UNLIKELY(!wm_get_screensize(&wm_init_state.size_x, &wm_init_state.size_y))) { + /* Use fallback values. */ + wm_init_state.size_x = 0; + wm_init_state.size_y = 0; + } /* NOTE: this isn't quite correct, active screen maybe offset 1000s if PX, * we'd need a #wm_get_screensize like function that gives offset, @@ -865,12 +879,19 @@ static bool wm_window_update_size_position(wmWindow *win) GHOST_DisposeRectangle(client_rect); - int scr_w, scr_h; - wm_get_desktopsize(&scr_w, &scr_h); int sizex = r - l; int sizey = b - t; - int posx = l; - int posy = scr_h - t - win->sizey; + + int posx = 0; + int posy = 0; + + if (WM_capabilities_flag() & WM_CAPABILITY_WINDOW_POSITION) { + int scr_w, scr_h; + if (wm_get_desktopsize(&scr_w, &scr_h)) { + posx = l; + posy = scr_h - t - win->sizey; + } + } if (win->sizex != sizex || win->sizey != sizey || win->posx != posx || win->posy != posy) { win->sizex = sizex; diff --git a/source/blender/windowmanager/wm_window.hh b/source/blender/windowmanager/wm_window.hh index d7f0eb04207..8dbe5fcdebc 100644 --- a/source/blender/windowmanager/wm_window.hh +++ b/source/blender/windowmanager/wm_window.hh @@ -31,12 +31,14 @@ void wm_ghost_exit(); /** * This one should correctly check for apple top header... * done for Cocoa: returns window contents (and not frame) max size. + * \return true on success. */ -void wm_get_screensize(int *r_width, int *r_height); +bool wm_get_screensize(int *r_width, int *r_height) ATTR_NONNULL(1, 2) ATTR_WARN_UNUSED_RESULT; /** * Size of all screens (desktop), useful since the mouse is bound by this. + * \return true on success. */ -void wm_get_desktopsize(int *r_width, int *r_height); +bool wm_get_desktopsize(int *r_width, int *r_height) ATTR_NONNULL(1, 2) ATTR_WARN_UNUSED_RESULT; /** * Don't change context itself.