From f751f15b9fa13f0617055e7792605e323e9083c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Thu, 12 Apr 2018 05:22:55 +0300 Subject: [PATCH 1/3] Shuffle code a bit to make it easier to skip deferring window position --- src/msw/window.cpp | 67 ++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/msw/window.cpp b/src/msw/window.cpp index 113b3b715d..ab5f008a14 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -1935,38 +1935,6 @@ void wxWindowMSW::DoClientToScreen(int *x, int *y) const bool wxWindowMSW::DoMoveSibling(WXHWND hwnd, int x, int y, int width, int height) { -#if wxUSE_DEFERRED_SIZING - // if our parent had prepared a defer window handle for us, use it (unless - // we are a top level window) - wxWindowMSW * const parent = IsTopLevel() ? NULL : GetParent(); - - HDWP hdwp = parent ? (HDWP)parent->m_hDWP : NULL; - if ( hdwp ) - { - hdwp = ::DeferWindowPos(hdwp, (HWND)hwnd, NULL, x, y, width, height, - SWP_NOZORDER | SWP_NOOWNERZORDER | SWP_NOACTIVATE); - if ( !hdwp ) - { - wxLogLastError(wxT("DeferWindowPos")); - } - } - - if ( parent ) - { - // hdwp must be updated as it may have been changed - parent->m_hDWP = (WXHANDLE)hdwp; - } - - if ( hdwp ) - { - // did deferred move, remember new coordinates of the window as they're - // different from what Windows would return for it - return true; - } - - // otherwise (or if deferring failed) move the window in place immediately -#endif // wxUSE_DEFERRED_SIZING - // toplevel window's coordinates are mirrored if the TLW is a child of another // RTL window and changing width without moving the position would enlarge the // window in the wrong direction, so we need to adjust for it @@ -1986,6 +1954,41 @@ wxWindowMSW::DoMoveSibling(WXHWND hwnd, int x, int y, int width, int height) } } +#if wxUSE_DEFERRED_SIZING + else + { + // if our parent had prepared a defer window handle for us, use it (unless + // we are a top level window) + wxWindowMSW * const parent = GetParent(); + + HDWP hdwp = parent ? (HDWP)parent->m_hDWP : NULL; + if ( hdwp ) + { + hdwp = ::DeferWindowPos(hdwp, (HWND)hwnd, NULL, x, y, width, height, + SWP_NOZORDER | SWP_NOOWNERZORDER | SWP_NOACTIVATE); + if ( !hdwp ) + { + wxLogLastError(wxT("DeferWindowPos")); + } + } + + if ( parent ) + { + // hdwp must be updated as it may have been changed + parent->m_hDWP = (WXHANDLE)hdwp; + } + + if ( hdwp ) + { + // did deferred move, remember new coordinates of the window as they're + // different from what Windows would return for it + return true; + } + + // otherwise (or if deferring failed) move the window in place immediately + } +#endif // wxUSE_DEFERRED_SIZING + if ( !::MoveWindow((HWND)hwnd, x, y, width, height, IsShown()) ) { wxLogLastError(wxT("MoveWindow")); From 578474526f068b445db40c2f21ac54e6ecf9f79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 18 Apr 2018 20:19:45 +0300 Subject: [PATCH 2/3] fix window position under MSW when limited by `short` range --- include/wx/msw/window.h | 21 ++++++++- src/msw/control.cpp | 28 +++++------- src/msw/spinctrl.cpp | 24 +++++------ src/msw/statbmp.cpp | 2 +- src/msw/window.cpp | 95 ++++++++++++++++++++++++++++------------- 5 files changed, 107 insertions(+), 63 deletions(-) diff --git a/include/wx/msw/window.h b/include/wx/msw/window.h index 1a6af48f5b..8ea5f8725d 100644 --- a/include/wx/msw/window.h +++ b/include/wx/msw/window.h @@ -729,8 +729,27 @@ private: bool MSWSafeIsDialogMessage(WXMSG* msg); #endif // __WXUNIVERSAL__ -#if wxUSE_DEFERRED_SIZING + static inline bool MSWIsPositionDirectlySupported(int x, int y) + { + // The supported coordinate intervals for various functions are: + // - MoveWindow, DeferWindowPos: [-32768, 32767] a.k.a. [SHRT_MIN, SHRT_MAX]; + // - CreateWindow, CreateWindowEx: [-32768, 32554]. + // CreateXXX will _sometimes_ manage to create the window at higher coordinates + // like 32580, 32684, 32710, but that was not consistent and the lowest common + // limit was 32554 (so far at least). + return (x >= SHRT_MIN && x <= 32554 && y >= SHRT_MIN && y <= 32554); + } + protected: + WXHWND MSWCreateWindowAtAnyPosition(WXDWORD exStyle, const wxChar* clName, + const wxChar* title, WXDWORD style, + int x, int y, int width, int height, + WXHWND parent, wxWindowID id); + + void MSWMoveWindowToAnyPosition(WXHWND hwnd, int x, int y, + int width, int height, bool bRepaint); + +#if wxUSE_DEFERRED_SIZING // this function is called after the window was resized to its new size virtual void MSWEndDeferWindowPos() { diff --git a/src/msw/control.cpp b/src/msw/control.cpp index e253935c23..f3be7cd0be 100644 --- a/src/msw/control.cpp +++ b/src/msw/control.cpp @@ -132,27 +132,19 @@ bool wxControl::MSWCreateControl(const wxChar *classname, // ... and adjust it to account for a possible parent frames toolbar AdjustForParentClientOrigin(x, y); - m_hWnd = (WXHWND)::CreateWindowEx - ( - exstyle, // extended style - classname, // the kind of control to create - label.t_str(), // the window name - style, // the window style - x, y, w, h, // the window position and size - GetHwndOf(GetParent()), // parent - (HMENU)wxUIntToPtr(GetId()), // child id - wxGetInstance(), // app instance - NULL // creation parameters - ); + m_hWnd = MSWCreateWindowAtAnyPosition + ( + exstyle, // extended style + classname, // the kind of control to create + label.t_str(), // the window name + style, // the window style + x, y, w, h, // the window position and size + GetHwndOf(GetParent()), // parent + GetId() // child id + ); if ( !m_hWnd ) { - wxLogLastError(wxString::Format - ( - wxT("CreateWindowEx(\"%s\", flags=%08lx, ex=%08lx)"), - classname, style, exstyle - )); - return false; } diff --git a/src/msw/spinctrl.cpp b/src/msw/spinctrl.cpp index a692a5044e..adc534b01a 100644 --- a/src/msw/spinctrl.cpp +++ b/src/msw/spinctrl.cpp @@ -298,19 +298,17 @@ bool wxSpinCtrl::Create(wxWindow *parent, // create the text window - m_hwndBuddy = (WXHWND)::CreateWindowEx - ( - exStyle, // sunken border - wxT("EDIT"), // window class - NULL, // no window title - msStyle, // style (will be shown later) - pos.x, pos.y, // position - 0, 0, // size (will be set later) - GetHwndOf(parent), // parent - (HMENU)-1, // control id - wxGetInstance(), // app instance - NULL // unused client data - ); + m_hwndBuddy = MSWCreateWindowAtAnyPosition + ( + exStyle, // sunken border + wxT("EDIT"), // window class + NULL, // no window title + msStyle, // style (will be shown later) + pos.x, pos.y, // position + 0, 0, // size (will be set later) + GetHwndOf(parent), // parent + -1 // control id + ); if ( !m_hwndBuddy ) { diff --git a/src/msw/statbmp.cpp b/src/msw/statbmp.cpp index 4add948108..66d2f37e02 100644 --- a/src/msw/statbmp.cpp +++ b/src/msw/statbmp.cpp @@ -333,7 +333,7 @@ void wxStaticBitmap::SetImageNoCopy( wxGDIImage* image) w = width; h = height; - ::MoveWindow(GetHwnd(), x, y, width, height, FALSE); + MSWMoveWindowToAnyPosition(GetHwnd(), x, y, width, height, false); } } diff --git a/src/msw/window.cpp b/src/msw/window.cpp index ab5f008a14..f55b8be586 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -1955,10 +1955,9 @@ wxWindowMSW::DoMoveSibling(WXHWND hwnd, int x, int y, int width, int height) } #if wxUSE_DEFERRED_SIZING - else + else if ( MSWIsPositionDirectlySupported(x, y) ) { - // if our parent had prepared a defer window handle for us, use it (unless - // we are a top level window) + // if our parent had prepared a defer window handle for us, use it wxWindowMSW * const parent = GetParent(); HDWP hdwp = parent ? (HDWP)parent->m_hDWP : NULL; @@ -1989,16 +1988,36 @@ wxWindowMSW::DoMoveSibling(WXHWND hwnd, int x, int y, int width, int height) } #endif // wxUSE_DEFERRED_SIZING - if ( !::MoveWindow((HWND)hwnd, x, y, width, height, IsShown()) ) - { - wxLogLastError(wxT("MoveWindow")); - } + MSWMoveWindowToAnyPosition(hwnd, x, y, width, height, IsShown()); // if wxUSE_DEFERRED_SIZING, indicates that we didn't use deferred move, // ignored otherwise return false; } +void wxWindowMSW::MSWMoveWindowToAnyPosition(WXHWND hwnd, int x, int y, int width, int height, bool bRepaint) +{ + bool scroll = GetParent() && !MSWIsPositionDirectlySupported(x, y); + + if ( scroll ) + { + // scroll to the actual position (looks like there is no need to Freeze() the parent) + ::ScrollWindow(GetHwndOf(GetParent()), -x, -y, NULL, NULL); + } + + // move to relative coordinates + if ( !::MoveWindow(hwnd, (scroll ? 0 : x), (scroll ? 0 : y), width, height, bRepaint) ) + { + wxLogLastError(wxT("MoveWindow")); + } + + if ( scroll ) + { + // scroll back + ::ScrollWindow(GetHwndOf(GetParent()), x, y, NULL, NULL); + } +} + void wxWindowMSW::DoMoveWindow(int x, int y, int width, int height) { // TODO: is this consistent with other platforms? @@ -2171,15 +2190,9 @@ void wxWindowMSW::DoSetClientSize(int width, int height) // and not defer it here as otherwise the value returned by // GetClient/WindowRect() wouldn't change as the window wouldn't be // really resized - if ( !::MoveWindow(GetHwnd(), - rectWin.left, - rectWin.top, - width + widthWin - rectClient.right, - height + heightWin - rectClient.bottom, - TRUE) ) - { - wxLogLastError(wxT("MoveWindow")); - } + MSWMoveWindowToAnyPosition(GetHwnd(), rectWin.left, rectWin.top, + width + widthWin - rectClient.right, + height + heightWin - rectClient.bottom, true); } } @@ -3900,23 +3913,19 @@ bool wxWindowMSW::MSWCreate(const wxChar *wclass, // do create the window wxWindowCreationHook hook(this); - m_hWnd = (WXHWND)::CreateWindowEx - ( - extendedStyle, - wclass, - title ? title : m_windowName.t_str(), - style, - x, y, w, h, - (HWND)MSWGetParent(), - (HMENU)wxUIntToPtr(controlId), - wxGetInstance(), - NULL // no extra data - ); + m_hWnd = MSWCreateWindowAtAnyPosition + ( + extendedStyle, + wclass, + title ? title : m_windowName.t_str(), + style, + x, y, w, h, + MSWGetParent(), + controlId + ); if ( !m_hWnd ) { - wxLogSysError(_("Can't create window of class %s"), wclass); - return false; } @@ -3925,6 +3934,32 @@ bool wxWindowMSW::MSWCreate(const wxChar *wclass, return true; } +WXHWND wxWindowMSW::MSWCreateWindowAtAnyPosition(WXDWORD exStyle, const wxChar* clName, + const wxChar* title, WXDWORD style, + int x, int y, int width, int height, + WXHWND parent, wxWindowID id) +{ + WXHWND hWnd = ::CreateWindowEx(exStyle, clName, title, style, x, y, width, height, + parent, (HMENU)wxUIntToPtr(id), wxGetInstance(), + NULL); // no extra data + + if ( !hWnd ) + { + wxLogLastError(wxString::Format + ( + wxT("CreateWindowEx(\"%s\", flags=%08lx, ex=%08lx)"), + clName, style, exStyle + )); + } + else if ( !IsTopLevel() && !MSWIsPositionDirectlySupported(x, y) ) + { + // fix position if limited by Short range + MSWMoveWindowToAnyPosition(hWnd, x, y, width, height, IsShown()); + } + + return hWnd; +} + // =========================================================================== // MSW message handlers // =========================================================================== From bb29b87a45a6ed6a8058b3d59d4cb9b2f7d1524d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Mon, 30 Apr 2018 00:58:39 +0300 Subject: [PATCH 3/3] Add test case for window positioning beyond Short limit --- tests/controls/windowtest.cpp | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/controls/windowtest.cpp b/tests/controls/windowtest.cpp index d99f8aaf36..c7933832eb 100644 --- a/tests/controls/windowtest.cpp +++ b/tests/controls/windowtest.cpp @@ -16,6 +16,7 @@ #include "wx/app.h" #include "wx/window.h" #include "wx/button.h" + #include "wx/sizer.h" #endif // WX_PRECOMP #include "asserthelper.h" @@ -49,6 +50,7 @@ private: CPPUNIT_TEST( Children ); CPPUNIT_TEST( Focus ); CPPUNIT_TEST( Positioning ); + CPPUNIT_TEST( PositioningBeyondShortLimit ); CPPUNIT_TEST( Show ); CPPUNIT_TEST( Enable ); CPPUNIT_TEST( FindWindowBy ); @@ -68,6 +70,7 @@ private: void Children(); void Focus(); void Positioning(); + void PositioningBeyondShortLimit(); void Show(); void Enable(); void FindWindowBy(); @@ -327,6 +330,47 @@ void WindowTestCase::Positioning() m_window->GetScreenRect().GetTopLeft()); } +void WindowTestCase::PositioningBeyondShortLimit() +{ +#ifdef __WXMSW__ + //Positioning under MSW is limited to short relative coordinates + + // + //Test window creation beyond SHRT_MAX + int commonDim = 10; + wxWindow* w = new wxWindow(m_window, wxID_ANY, + wxPoint(0, SHRT_MAX + commonDim), + wxSize(commonDim, commonDim)); + CPPUNIT_ASSERT_EQUAL(SHRT_MAX + commonDim, w->GetPosition().y); + + w->Move(0, 0); + + // + //Test window moving beyond SHRT_MAX + w->Move(0, SHRT_MAX + commonDim); + CPPUNIT_ASSERT_EQUAL(SHRT_MAX + commonDim, w->GetPosition().y); + + // + //Test window moving below SHRT_MIN + w->Move(0, SHRT_MIN - commonDim); + CPPUNIT_ASSERT_EQUAL(SHRT_MIN - commonDim, w->GetPosition().y); + + // + //Test deferred move beyond SHRT_MAX + m_window->SetVirtualSize(-1, SHRT_MAX + 2 * commonDim); + wxWindow* bigWin = new wxWindow(m_window, wxID_ANY, wxDefaultPosition, + //size is also limited by SHRT_MAX + wxSize(commonDim, SHRT_MAX)); + wxSizer *sizer = new wxBoxSizer(wxVERTICAL); + sizer->Add(bigWin); + sizer->AddSpacer(commonDim); //add some space to go beyond SHRT_MAX + sizer->Add(w); + m_window->SetSizer(sizer); + m_window->Layout(); + CPPUNIT_ASSERT_EQUAL(SHRT_MAX + commonDim, w->GetPosition().y); +#endif +} + void WindowTestCase::Show() { CPPUNIT_ASSERT(m_window->IsShown());