From 6bf593b3ad5db43a1738e3d1dd164b7d98055b8f Mon Sep 17 00:00:00 2001 From: Harley Acheson Date: Mon, 21 Aug 2023 11:20:33 -0700 Subject: [PATCH 1/4] Fix #111295: Add Missing Win32 Platform-Specific Window functions Add platform-specific (Windows) versions of getOSWindow() and getWindowUnderCursor(). --- The base default behavior of `getOSWindow` always returns nullptr. This PR makes it return the Windows OS Handle as expected. This is needed for `getWindowAssociatedWithOSWindow` to work correctly, otherwise that always returns nullptr. The base default behavior of `getWindowUnderCursor` does a forward search through the ghost windows checking bounds. This does not consider z-depth. And because the search is forward it will always return the main window if a child is overlapping in front. This specific implementation gets the cursor location from the OS and then the Window from that. This PR does not use the passed cursor location because that will often be wrong. The best example is when we have a blender window on one monitor but the mouse cursor is on a different monitor that uses a different scale. In this case the cursor location will not be a nice offset from our blender window. Best to just ask the OS where the cursor actually is. For testing and how this relates to the referenced bug report. Opening the "Preferences" window and having it above the main window, using the color picker in the Themes area will select through to the main window. This is because the base getWindowUnderCursor returns the first window with bounds that match the cursor position. --- intern/ghost/intern/GHOST_SystemWin32.cc | 17 +++++++++++++++++ intern/ghost/intern/GHOST_SystemWin32.hh | 8 ++++++++ intern/ghost/intern/GHOST_WindowWin32.cc | 5 +++++ intern/ghost/intern/GHOST_WindowWin32.hh | 6 ++++++ 4 files changed, 36 insertions(+) diff --git a/intern/ghost/intern/GHOST_SystemWin32.cc b/intern/ghost/intern/GHOST_SystemWin32.cc index 9d13e73c28a..49d80094fa2 100644 --- a/intern/ghost/intern/GHOST_SystemWin32.cc +++ b/intern/ghost/intern/GHOST_SystemWin32.cc @@ -473,6 +473,23 @@ GHOST_TSuccess GHOST_SystemWin32::getPixelAtCursor(float r_color[3]) const return GHOST_kSuccess; } +GHOST_IWindow *GHOST_SystemWin32::getWindowUnderCursor(int32_t /*x*/, int32_t /*y*/) +{ + /* Get cursor position from OS. Do not use supplied positions as those could + * be incorrect, especially if using multiple windows of differing OS scale. */ + POINT point; + if (!GetCursorPos(&point)) { + return nullptr; + } + + HWND win = WindowFromPoint(point); + if (win == NULL) { + return nullptr; + } + + return m_windowManager->getWindowAssociatedWithOSWindow((void *)win); +} + GHOST_TSuccess GHOST_SystemWin32::getModifierKeys(GHOST_ModifierKeys &keys) const { /* `GetAsyncKeyState` returns the current interrupt-level state of the hardware, which is needed diff --git a/intern/ghost/intern/GHOST_SystemWin32.hh b/intern/ghost/intern/GHOST_SystemWin32.hh index 8fa578f7cd3..0222ea9c90f 100644 --- a/intern/ghost/intern/GHOST_SystemWin32.hh +++ b/intern/ghost/intern/GHOST_SystemWin32.hh @@ -151,6 +151,14 @@ class GHOST_SystemWin32 : public GHOST_System { */ static GHOST_TSuccess disposeContextD3D(GHOST_ContextD3D *context); + /** + * Get the Window under the cursor. + * \param x: The x-coordinate of the cursor. + * \param y: The y-coordinate of the cursor. + * \return The window under the cursor or nullptr if none. + */ + GHOST_IWindow *getWindowUnderCursor(int32_t x, int32_t y); + /*************************************************************************************** ** Event management functionality ***************************************************************************************/ diff --git a/intern/ghost/intern/GHOST_WindowWin32.cc b/intern/ghost/intern/GHOST_WindowWin32.cc index 882648e9da7..3ea3412717b 100644 --- a/intern/ghost/intern/GHOST_WindowWin32.cc +++ b/intern/ghost/intern/GHOST_WindowWin32.cc @@ -361,6 +361,11 @@ HWND GHOST_WindowWin32::getHWND() const return m_hWnd; } +void *GHOST_WindowWin32::getOSWindow() const +{ + return (void *)m_hWnd; +} + void GHOST_WindowWin32::setTitle(const char *title) { wchar_t *title_16 = alloc_utf16_from_8((char *)title, 0); diff --git a/intern/ghost/intern/GHOST_WindowWin32.hh b/intern/ghost/intern/GHOST_WindowWin32.hh index 665f3fbfc73..aeb64514784 100644 --- a/intern/ghost/intern/GHOST_WindowWin32.hh +++ b/intern/ghost/intern/GHOST_WindowWin32.hh @@ -107,6 +107,12 @@ class GHOST_WindowWin32 : public GHOST_Window { */ HWND getHWND() const; + /** + * Returns the handle of the window. + * \return The handle of the window. + */ + void *getOSWindow() const; + /** * Sets the title displayed in the title bar. * \param title: The title to display in the title bar. -- 2.30.2 From 16e664d14bcb9e083a00e69d2cd027ac7a63d39d Mon Sep 17 00:00:00 2001 From: Harley Acheson Date: Mon, 21 Aug 2023 13:43:33 -0700 Subject: [PATCH 2/4] Comment improvements --- intern/ghost/GHOST_C-api.h | 5 ++++- intern/ghost/GHOST_ISystem.hh | 5 ++++- intern/ghost/intern/GHOST_System.cc | 6 ++++-- intern/ghost/intern/GHOST_SystemWin32.cc | 4 ++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/intern/ghost/GHOST_C-api.h b/intern/ghost/GHOST_C-api.h index 028fa2cc263..bce666c9063 100644 --- a/intern/ghost/GHOST_C-api.h +++ b/intern/ghost/GHOST_C-api.h @@ -256,7 +256,10 @@ extern GHOST_TSuccess GHOST_EndFullScreen(GHOST_SystemHandle systemhandle); extern bool GHOST_GetFullScreen(GHOST_SystemHandle systemhandle); /** - * Get the Window under the cursor. + * Get the Window under the cursor. Although coordinates of the mouse are supplied, + * these could be incorrect on some platforms under some conditions, for example when + * using multiple monitors that vary in scale or DPI). It is best to ignore these + * and ask the OS for the position of the mouse instead. * \param x: The x-coordinate of the cursor. * \param y: The y-coordinate of the cursor. * \return The window under the cursor or nullptr in none. diff --git a/intern/ghost/GHOST_ISystem.hh b/intern/ghost/GHOST_ISystem.hh index 0e145c9eac6..2b165bc9e73 100644 --- a/intern/ghost/GHOST_ISystem.hh +++ b/intern/ghost/GHOST_ISystem.hh @@ -332,7 +332,10 @@ class GHOST_ISystem { virtual void setAutoFocus(const bool auto_focus) = 0; /** - * Get the Window under the cursor. + * Get the Window under the cursor. Although coordinates of the mouse are supplied, + * these could be incorrect on some platforms under some conditions, for example when + * using multiple monitors that vary in scale or DPI). It is best to ignore these + * and ask the OS for the position of the mouse instead. * \param x: The x-coordinate of the cursor. * \param y: The y-coordinate of the cursor. * \return The window under the cursor or nullptr if none. diff --git a/intern/ghost/intern/GHOST_System.cc b/intern/ghost/intern/GHOST_System.cc index 65d1323938b..ce62f6c8493 100644 --- a/intern/ghost/intern/GHOST_System.cc +++ b/intern/ghost/intern/GHOST_System.cc @@ -210,8 +210,10 @@ bool GHOST_System::getFullScreen() GHOST_IWindow *GHOST_System::getWindowUnderCursor(int32_t x, int32_t y) { - /* TODO: This solution should follow the order of the activated windows (Z-order). - * It is imperfect but usable in most cases. */ + /* TODO: The following does not consider z-order so only works for simple cases + * of windows that do not overlap. It iterates in natural order, which will result + * in parent windows returned when their children are in front of them. Reversed order + * would be better. However, each platform should use a specific version of this. */ for (GHOST_IWindow *iwindow : m_windowManager->getWindows()) { if (iwindow->getState() == GHOST_kWindowStateMinimized) { continue; diff --git a/intern/ghost/intern/GHOST_SystemWin32.cc b/intern/ghost/intern/GHOST_SystemWin32.cc index 49d80094fa2..bfaa3ac00ff 100644 --- a/intern/ghost/intern/GHOST_SystemWin32.cc +++ b/intern/ghost/intern/GHOST_SystemWin32.cc @@ -475,8 +475,8 @@ GHOST_TSuccess GHOST_SystemWin32::getPixelAtCursor(float r_color[3]) const GHOST_IWindow *GHOST_SystemWin32::getWindowUnderCursor(int32_t /*x*/, int32_t /*y*/) { - /* Get cursor position from OS. Do not use supplied positions as those could - * be incorrect, especially if using multiple windows of differing OS scale. */ + /* Get cursor position from the OS. Do not use the supplied positions as those + * could be incorrect, especially if using multiple windows of differing OS scale. */ POINT point; if (!GetCursorPos(&point)) { return nullptr; -- 2.30.2 From 87fae133fa4b0739a8f23eb4f5e1934b1019ccbc Mon Sep 17 00:00:00 2001 From: Harley Acheson Date: Fri, 25 Aug 2023 10:55:16 -0700 Subject: [PATCH 3/4] Comment Changes. Remove x,y arguments from Win32 getWindowUnderCursor --- intern/ghost/intern/GHOST_System.cc | 6 ++---- intern/ghost/intern/GHOST_SystemWin32.hh | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/intern/ghost/intern/GHOST_System.cc b/intern/ghost/intern/GHOST_System.cc index ce62f6c8493..65d1323938b 100644 --- a/intern/ghost/intern/GHOST_System.cc +++ b/intern/ghost/intern/GHOST_System.cc @@ -210,10 +210,8 @@ bool GHOST_System::getFullScreen() GHOST_IWindow *GHOST_System::getWindowUnderCursor(int32_t x, int32_t y) { - /* TODO: The following does not consider z-order so only works for simple cases - * of windows that do not overlap. It iterates in natural order, which will result - * in parent windows returned when their children are in front of them. Reversed order - * would be better. However, each platform should use a specific version of this. */ + /* TODO: This solution should follow the order of the activated windows (Z-order). + * It is imperfect but usable in most cases. */ for (GHOST_IWindow *iwindow : m_windowManager->getWindows()) { if (iwindow->getState() == GHOST_kWindowStateMinimized) { continue; diff --git a/intern/ghost/intern/GHOST_SystemWin32.hh b/intern/ghost/intern/GHOST_SystemWin32.hh index 0222ea9c90f..68672946145 100644 --- a/intern/ghost/intern/GHOST_SystemWin32.hh +++ b/intern/ghost/intern/GHOST_SystemWin32.hh @@ -152,12 +152,10 @@ class GHOST_SystemWin32 : public GHOST_System { static GHOST_TSuccess disposeContextD3D(GHOST_ContextD3D *context); /** - * Get the Window under the cursor. - * \param x: The x-coordinate of the cursor. - * \param y: The y-coordinate of the cursor. + * Get the Window under the mouse cursor. Location obtained from the OS. * \return The window under the cursor or nullptr if none. */ - GHOST_IWindow *getWindowUnderCursor(int32_t x, int32_t y); + GHOST_IWindow *getWindowUnderCursor(int32_t /*x*/, int32_t /*y*/); /*************************************************************************************** ** Event management functionality -- 2.30.2 From 10e545fb8bae4497213b4ffb04e54f2aadc154e4 Mon Sep 17 00:00:00 2001 From: Harley Acheson Date: Fri, 25 Aug 2023 12:50:24 -0700 Subject: [PATCH 4/4] Only comment changes. --- intern/ghost/GHOST_C-api.h | 8 ++++---- intern/ghost/GHOST_ISystem.hh | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/intern/ghost/GHOST_C-api.h b/intern/ghost/GHOST_C-api.h index bce666c9063..e767da45940 100644 --- a/intern/ghost/GHOST_C-api.h +++ b/intern/ghost/GHOST_C-api.h @@ -256,10 +256,10 @@ extern GHOST_TSuccess GHOST_EndFullScreen(GHOST_SystemHandle systemhandle); extern bool GHOST_GetFullScreen(GHOST_SystemHandle systemhandle); /** - * Get the Window under the cursor. Although coordinates of the mouse are supplied, - * these could be incorrect on some platforms under some conditions, for example when - * using multiple monitors that vary in scale or DPI). It is best to ignore these - * and ask the OS for the position of the mouse instead. + * Get the Window under the cursor. Although coordinates of the mouse are supplied, platform- + * specific implementations are free to ignore these and query the mouse location themselves, due + * to them possibly being incorrect under certain conditions, for example when using multiple + * monitors that vary in scale and/or DPI. * \param x: The x-coordinate of the cursor. * \param y: The y-coordinate of the cursor. * \return The window under the cursor or nullptr in none. diff --git a/intern/ghost/GHOST_ISystem.hh b/intern/ghost/GHOST_ISystem.hh index 2b165bc9e73..59df6208da6 100644 --- a/intern/ghost/GHOST_ISystem.hh +++ b/intern/ghost/GHOST_ISystem.hh @@ -332,10 +332,10 @@ class GHOST_ISystem { virtual void setAutoFocus(const bool auto_focus) = 0; /** - * Get the Window under the cursor. Although coordinates of the mouse are supplied, - * these could be incorrect on some platforms under some conditions, for example when - * using multiple monitors that vary in scale or DPI). It is best to ignore these - * and ask the OS for the position of the mouse instead. + * Get the Window under the cursor. Although coordinates of the mouse are supplied, platform- + * specific implementations are free to ignore these and query the mouse location themselves, due + * to them possibly being incorrect under certain conditions, for example when using multiple + * monitors that vary in scale and/or DPI. * \param x: The x-coordinate of the cursor. * \param y: The y-coordinate of the cursor. * \return The window under the cursor or nullptr if none. -- 2.30.2