From a8c1ae705818425435231498e7c729449418eb39 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 22 Jun 2018 02:43:23 +0200 Subject: [PATCH 1/4] Fix TLW activation unit test Ensure that another TLW is active before calling ShowWithoutActivating() as otherwise the newly shown window would be considered active, even if it actually isn't, because it contains the current focus. --- tests/toplevel/toplevel.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/toplevel/toplevel.cpp b/tests/toplevel/toplevel.cpp index 60722f0ce2..799f35fd10 100644 --- a/tests/toplevel/toplevel.cpp +++ b/tests/toplevel/toplevel.cpp @@ -17,6 +17,7 @@ #endif #ifndef WX_PRECOMP + #include "wx/app.h" #include "wx/dialog.h" #include "wx/frame.h" #include "wx/textctrl.h" @@ -32,6 +33,7 @@ static void TopLevelWindowShowTest(wxTopLevelWindow* tlw) // only run this test on platforms where ShowWithoutActivating is implemented. #if defined(__WXMSW__) || defined(__WXMAC__) + wxTheApp->GetTopWindow()->SetFocus(); tlw->ShowWithoutActivating(); CHECK(tlw->IsShown()); CHECK(!tlw->IsActive()); @@ -53,7 +55,7 @@ static void TopLevelWindowShowTest(wxTopLevelWindow* tlw) tlw->Hide(); CHECK(!tlw->IsShown()); #ifndef __WXGTK__ - CHECK(tlw->IsActive()); + CHECK(!tlw->IsActive()); #endif } From f03d655b1af50a9d86dc16072fa40426bc132351 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 22 Jun 2018 02:44:40 +0200 Subject: [PATCH 2/4] Enable running TLW activation unit tests in CI environments Hopefully the previous commit fixed it under AppVeyor. --- tests/toplevel/toplevel.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/toplevel/toplevel.cpp b/tests/toplevel/toplevel.cpp index 799f35fd10..5e62a1b05c 100644 --- a/tests/toplevel/toplevel.cpp +++ b/tests/toplevel/toplevel.cpp @@ -61,13 +61,6 @@ static void TopLevelWindowShowTest(wxTopLevelWindow* tlw) TEST_CASE("wxTopLevel::Show", "[tlw][show]") { - if ( IsAutomaticTest() ) - { - // For some reason, activation test doesn't work when running under - // AppVeyor, so skip it to avoid spurious failures. - return; - } - SECTION("Dialog") { wxDialog* dialog = new wxDialog(NULL, -1, "Dialog Test"); From 3518f1a7d8b27877183b0aa2338a59dec836ae3c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 22 Jun 2018 03:23:31 +0200 Subject: [PATCH 3/4] Use a single wxTopLevelWindow::m_showCmd flag in wxMSW This single field replaces m_iconized and m_maximizeOnShow which were not really independent and will make it simpler to schedule either maximizing or maximizing the window later, when it can't be done immediately because the window is hidden, in the following commit. --- include/wx/msw/toplevel.h | 17 ++++---- src/msw/dialog.cpp | 6 +-- src/msw/frame.cpp | 8 ++-- src/msw/toplevel.cpp | 90 ++++++++++++++++++++++----------------- 4 files changed, 67 insertions(+), 54 deletions(-) diff --git a/include/wx/msw/toplevel.h b/include/wx/msw/toplevel.h index c79754f722..3652cbbf0a 100644 --- a/include/wx/msw/toplevel.h +++ b/include/wx/msw/toplevel.h @@ -132,9 +132,12 @@ protected: const wxPoint& pos, const wxSize& size); - // common part of Iconize(), Maximize() and Restore() + // Just a wrapper around MSW ShowWindow(). void DoShowWindow(int nShowCmd); + // Return true if the window is iconized at MSW level, ignoring m_showCmd. + bool MSWIsIconized() const; + // override those to return the normal window coordinates even when the // window is minimized virtual void DoGetPosition(int *x, int *y) const wxOVERRIDE; @@ -156,13 +159,11 @@ protected: int& x, int& y, int& w, int& h) const wxOVERRIDE; - - // is the window currently iconized? - bool m_iconized; - - // should the frame be maximized when it will be shown? set by Maximize() - // when it is called while the frame is hidden - bool m_maximizeOnShow; + // This field contains the show command to use when showing the window the + // next time and also indicates whether the window should be considered + // being iconized or maximized (which may be different from whether it's + // actually iconized or maximized at MSW level). + WXUINT m_showCmd; // Data to save/restore when calling ShowFullScreen long m_fsStyle; // Passed to ShowFullScreen diff --git a/src/msw/dialog.cpp b/src/msw/dialog.cpp index 45478e34a3..c6281f583d 100644 --- a/src/msw/dialog.cpp +++ b/src/msw/dialog.cpp @@ -321,7 +321,7 @@ WXLRESULT wxDialog::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM lPar switch ( wParam ) { case SIZE_MINIMIZED: - m_iconized = true; + m_showCmd = SW_MINIMIZE; break; case SIZE_MAXIMIZED: @@ -331,9 +331,9 @@ WXLRESULT wxDialog::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM lPar if ( m_hGripper ) ShowGripper( wParam == SIZE_RESTORED ); - if ( m_iconized ) + if ( m_showCmd == SW_MINIMIZE ) (void)SendIconizeEvent(false); - m_iconized = false; + m_showCmd = SW_RESTORE; break; } diff --git a/src/msw/frame.cpp b/src/msw/frame.cpp index 91b917667b..742d928850 100644 --- a/src/msw/frame.cpp +++ b/src/msw/frame.cpp @@ -261,7 +261,7 @@ void wxFrame::DoGetClientSize(int *x, int *y) const // generate an artificial resize event void wxFrame::SendSizeEvent(int flags) { - if ( !m_iconized ) + if ( !MSWIsIconized() ) { RECT r = wxGetWindowRect(GetHwnd()); @@ -667,7 +667,7 @@ void wxFrame::PositionToolBar() // on the desktop, but are iconized/restored with it void wxFrame::IconizeChildFrames(bool bIconize) { - m_iconized = bIconize; + m_showCmd = bIconize ? SW_MINIMIZE : SW_RESTORE; for ( wxWindowList::compatibility_iterator node = GetChildren().GetFirst(); node; @@ -750,7 +750,7 @@ bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id) // only do it it if we were iconized before, otherwise resizing the // parent frame has a curious side effect of bringing it under it's // children - if ( !m_iconized ) + if ( m_showCmd != SW_MINIMIZE ) break; // restore all child frames too @@ -765,7 +765,7 @@ bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id) break; } - if ( !m_iconized ) + if ( m_showCmd != SW_MINIMIZE ) { #if wxUSE_STATUSBAR PositionStatusBar(); diff --git a/src/msw/toplevel.cpp b/src/msw/toplevel.cpp index 2d24bf92a5..8ad7391d09 100644 --- a/src/msw/toplevel.cpp +++ b/src/msw/toplevel.cpp @@ -95,8 +95,7 @@ wxEND_EVENT_TABLE() void wxTopLevelWindowMSW::Init() { - m_iconized = - m_maximizeOnShow = false; + m_showCmd = SW_SHOWNORMAL; // Data to save/restore when calling ShowFullScreen m_fsStyle = 0; @@ -522,14 +521,6 @@ void wxTopLevelWindowMSW::DoShowWindow(int nShowCmd) { ::ShowWindow(GetHwnd(), nShowCmd); - // Hiding the window doesn't change its iconized state. - if ( nShowCmd != SW_HIDE ) - { - // Otherwise restoring, maximizing or showing the window normally also - // makes it not iconized and only minimizing it does make it iconized. - m_iconized = nShowCmd == SW_MINIMIZE; - } - #if wxUSE_TOOLTIPS // Don't leave a tooltip hanging around if TLW is hidden now. wxToolTip::UpdateVisibility(); @@ -541,7 +532,12 @@ void wxTopLevelWindowMSW::ShowWithoutActivating() if ( !wxWindowBase::Show(true) ) return; - DoShowWindow(SW_SHOWNA); + // We can't show the window in a maximized state without activating it, so + // the sequence of hiding the window, calling Maximize() and this function + // will end up with the window not being maximized -- but this is arguably + // better than activating it and is compatible with the historical + // behaviour of this function. + DoShowWindow(m_showCmd == SW_MINIMIZE ? SW_SHOWMINNOACTIVE : SW_SHOWNA); } bool wxTopLevelWindowMSW::Show(bool show) @@ -553,18 +549,10 @@ bool wxTopLevelWindowMSW::Show(bool show) int nShowCmd; if ( show ) { - if ( m_maximizeOnShow ) + // If we need to minimize or maximize the window, do it now. + if ( m_showCmd == SW_MAXIMIZE || m_showCmd == SW_MINIMIZE ) { - // show and maximize - nShowCmd = SW_MAXIMIZE; - - m_maximizeOnShow = false; - } - else if ( m_iconized ) - { - // We were iconized while we were hidden, so now we need to show - // the window in iconized state. - nShowCmd = SW_MINIMIZE; + nShowCmd = m_showCmd; } else if ( ::IsIconic(GetHwnd()) ) { @@ -622,16 +610,19 @@ void wxTopLevelWindowMSW::Raise() void wxTopLevelWindowMSW::Maximize(bool maximize) { + // Update m_showCmd to ensure that the window is maximized when it's shown + // later even if it's currently hidden. + m_showCmd = maximize ? SW_MAXIMIZE : SW_RESTORE; + if ( IsShown() ) { // just maximize it directly - DoShowWindow(maximize ? SW_MAXIMIZE : SW_RESTORE); + DoShowWindow(m_showCmd); } else // hidden { // we can't maximize the hidden frame because it shows it as well, - // so just remember that we should do it later in this case - m_maximizeOnShow = maximize; + // so don't do anything other than updating m_showCmd for now #if wxUSE_DEFERRED_SIZING // after calling Maximize() the client code expects to get the frame @@ -659,45 +650,66 @@ bool wxTopLevelWindowMSW::IsMaximized() const { return IsAlwaysMaximized() || (::IsZoomed(GetHwnd()) != 0) || - m_maximizeOnShow; + m_showCmd == SW_MAXIMIZE; } void wxTopLevelWindowMSW::Iconize(bool iconize) { - if ( iconize == m_iconized ) + if ( iconize == MSWIsIconized() ) { // Do nothing, in particular don't restore non-iconized windows when // Iconize(false) is called as this would wrongly un-maximize them. return; } + // Note that we can't change m_showCmd yet as wxFrame WM_SIZE handler uses + // its value to determine whether the frame had been iconized before or not + // and this handler will be called from inside DoShowWindow() below. + const UINT showCmd = iconize ? SW_MINIMIZE : SW_RESTORE; + + // We can't actually iconize the window if it's currently hidden, as this + // would also show it unexpectedly. if ( IsShown() ) { - // change the window state immediately - DoShowWindow(iconize ? SW_MINIMIZE : SW_RESTORE); - } - else // hidden - { - // iconizing the window shouldn't show it so just update the internal - // state (otherwise it's done by DoShowWindow() itself) - m_iconized = iconize; + DoShowWindow(showCmd); } + + // Update the internal flag in any case, so that IsIconized() returns the + // correct value, for example. And if the window is currently hidden, this + // also ensures that the next call to Show() will show it in an iconized + // state instead of showing it normally. + m_showCmd = showCmd; } bool wxTopLevelWindowMSW::IsIconized() const { if ( !IsShown() ) - return m_iconized; + { + // Hidden windows are never actually iconized at MSW level, but can be + // in wx, so use m_showCmd to determine our status. + return m_showCmd == SW_MINIMIZE; + } - // don't use m_iconized, it may be briefly out of sync with the real state + // don't use m_showCmd, it may be briefly out of sync with the real state // as it's only modified when we receive a WM_SIZE and we could be called // from an event handler from one of the messages we receive before it, // such as WM_MOVE + return MSWIsIconized(); +} + +bool wxTopLevelWindowMSW::MSWIsIconized() const +{ return ::IsIconic(GetHwnd()) != 0; } void wxTopLevelWindowMSW::Restore() { + // Forget any previous minimized/maximized status. + m_showCmd = SW_SHOW; + + // And actually restore the window to its normal state. Note that here, + // unlike in Maximize() and Iconize(), we do it even if the window is + // currently hidden, i.e. Restore() is supposed to show it in this case. DoShowWindow(SW_RESTORE); } @@ -1181,7 +1193,7 @@ void wxTopLevelWindowMSW::DoThaw() void wxTopLevelWindowMSW::DoSaveLastFocus() { - if ( m_iconized ) + if ( MSWIsIconized() ) return; // remember the last focused child if it is our child @@ -1211,7 +1223,7 @@ void wxTopLevelWindowMSW::OnActivate(wxActivateEvent& event) // We get WM_ACTIVATE before being restored from iconized state, so we // can be still iconized here. In this case, avoid restoring the focus // as it doesn't work anyhow and we will do when we're really restored. - if ( m_iconized ) + if ( MSWIsIconized() ) { event.Skip(); return; From cf20a9ced531d502433b5acc25a3bd84505f5fe5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 22 Jun 2018 03:25:43 +0200 Subject: [PATCH 4/4] Fix bug with showing TLW in wxMSW when restoring its geometry Since the changes of 6ae7aa444358cd2618a57e9ab29826f1714dbb4b, the windows were shown when their geometry was restored as a side effect of calling ::SetWindowPlacement(). This was unexpected and resulted in flicker on startup, so fix this by explicitly passing SW_HIDE to SetWindowPlacement() if the window is currently hidden and storing the real show command inside wxTLW itself, where it will be used when it's finally shown. --- include/wx/msw/private/tlwgeom.h | 24 ++++++++++++++++++++++-- include/wx/msw/toplevel.h | 3 +++ src/msw/frame.cpp | 6 ++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/include/wx/msw/private/tlwgeom.h b/include/wx/msw/private/tlwgeom.h index 1142f94c6a..40e6a799bd 100644 --- a/include/wx/msw/private/tlwgeom.h +++ b/include/wx/msw/private/tlwgeom.h @@ -77,12 +77,20 @@ public: wxCopyRectToRECT(r, m_placement.rcNormalPosition); // Maximized/minimized state. + // + // Note the special case of SW_MINIMIZE: while GetWindowPlacement() + // returns SW_SHOWMINIMIZED when the window is iconized, we restore it + // as SW_MINIMIZE as this is what the code in wxTLW checks to determine + // whether the window is supposed to be iconized or not. + // + // Just to confuse matters further, note that SW_MAXIMIZE is exactly + // the same thing as SW_SHOWMAXIMIZED. int tmp; UINT& show = m_placement.showCmd; if ( ser.RestoreField(wxPERSIST_TLW_MAXIMIZED, &tmp) && tmp ) - show = SW_SHOWMAXIMIZED; + show = SW_MAXIMIZE; else if ( ser.RestoreField(wxPERSIST_TLW_ICONIZED, &tmp) && tmp ) - show = SW_SHOWMINIMIZED; + show = SW_MINIMIZE; else show = SW_SHOWNORMAL; @@ -110,6 +118,18 @@ public: virtual bool ApplyTo(wxTopLevelWindow* tlw) wxOVERRIDE { + // There is a subtlety here: if the window is currently hidden, + // restoring its geometry shouldn't show it, so we must use SW_HIDE as + // show command, but showing it later should restore it to the correct + // state, so we need to remember it in wxTLW itself. And even if it's + // currently shown, we still need to update its show command, so that + // it matches the real window state after SetWindowPlacement() call. + tlw->MSWSetShowCommand(m_placement.showCmd); + if ( !tlw->IsShown() ) + { + m_placement.showCmd = SW_HIDE; + } + if ( !::SetWindowPlacement(GetHwndOf(tlw), &m_placement) ) { wxLogLastError(wxS("SetWindowPlacement")); diff --git a/include/wx/msw/toplevel.h b/include/wx/msw/toplevel.h index 3652cbbf0a..8e79013a5e 100644 --- a/include/wx/msw/toplevel.h +++ b/include/wx/msw/toplevel.h @@ -116,6 +116,9 @@ public: // returns true if the platform should explicitly apply a theme border virtual bool CanApplyThemeBorder() const wxOVERRIDE { return false; } + // This function is only for internal use. + void MSWSetShowCommand(WXUINT showCmd) { m_showCmd = showCmd; } + protected: // common part of all ctors void Init(); diff --git a/src/msw/frame.cpp b/src/msw/frame.cpp index 742d928850..de176504f0 100644 --- a/src/msw/frame.cpp +++ b/src/msw/frame.cpp @@ -743,6 +743,12 @@ bool wxFrame::MSWDoTranslateMessage(wxFrame *frame, WXMSG *pMsg) bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id) { + // We can get a WM_SIZE when restoring a hidden window using + // SetWindowPlacement(), don't do anything in this case as our state will + // be really updated later, when (and if) we're shown. + if ( !IsShown() ) + return true; + switch ( id ) { case SIZE_RESTORED: