From f9f58cca546e411514e5b5a1a29e789e389c0115 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 15 Jul 2019 18:13:29 +0200 Subject: [PATCH 1/4] Mention that Iconize() and Maximize() don't have immediate effect At least in wxGTK, the change of the window state is asynchronous. --- interface/wx/toplevel.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/interface/wx/toplevel.h b/interface/wx/toplevel.h index 5ad74b5912..361a5794c9 100644 --- a/interface/wx/toplevel.h +++ b/interface/wx/toplevel.h @@ -228,6 +228,12 @@ public: /** Iconizes or restores the window. + Note that in wxGTK the change to the window state is not immediate, + i.e. IsIconized() will typically return @false right after a call to + Iconize() and its return value will only change after the control flow + returns to the event loop and the notification about the window being + really iconized is received. + @param iconize If @true, iconizes the window; if @false, shows and restores it. @@ -285,6 +291,9 @@ public: /** Maximizes or restores the window. + Note that, just as with Iconize(), the change to the window state is + not immediate in at least wxGTK port. + @param maximize If @true, maximizes the window, otherwise it restores it. From 8cdd20667fc941c7d54bee87c667586555fcc578 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 15 Jul 2019 21:41:27 +0200 Subject: [PATCH 2/4] Initialize wxTopLevelWindow::DecorSize in wxGTK Add default ctor for this struct as it was too easy to forget to initialize it otherwise, ending up with bogus values in it, as it happened with wxTLWGeometry::m_decorSize, which resulted in a failure in wxPersistTLW unit test and, probably, real code too. --- include/wx/gtk/toplevel.h | 8 ++++++++ src/gtk/minifram.cpp | 1 - src/gtk/toplevel.cpp | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/wx/gtk/toplevel.h b/include/wx/gtk/toplevel.h index a95358d88e..602a232aba 100644 --- a/include/wx/gtk/toplevel.h +++ b/include/wx/gtk/toplevel.h @@ -112,6 +112,14 @@ public: // size of WM decorations struct DecorSize { + DecorSize() + { + left = + right = + top = + bottom = 0; + } + int left, right, top, bottom; }; DecorSize m_decorSize; diff --git a/src/gtk/minifram.cpp b/src/gtk/minifram.cpp index 923b8f4b89..2cf63de9c0 100644 --- a/src/gtk/minifram.cpp +++ b/src/gtk/minifram.cpp @@ -307,7 +307,6 @@ bool wxMiniFrame::Create( wxWindow *parent, wxWindowID id, const wxString &title if (style & wxRESIZE_BORDER) m_gdkFunc |= GDK_FUNC_RESIZE; gtk_window_set_default_size(GTK_WINDOW(m_widget), m_width, m_height); - memset(&m_decorSize, 0, sizeof(m_decorSize)); m_deferShow = false; if (m_parent && (GTK_IS_WINDOW(m_parent->m_widget))) diff --git a/src/gtk/toplevel.cpp b/src/gtk/toplevel.cpp index 57a457a418..c21d0e427c 100644 --- a/src/gtk/toplevel.cpp +++ b/src/gtk/toplevel.cpp @@ -565,7 +565,6 @@ void wxTopLevelWindowGTK::Init() m_updateDecorSize = true; m_netFrameExtentsTimerId = 0; m_incWidth = m_incHeight = 0; - memset(&m_decorSize, 0, sizeof(m_decorSize)); m_urgency_hint = -2; } From 8d405b80e43f0b314429fd1f8f9eaaeca8590022 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 15 Jul 2019 21:44:08 +0200 Subject: [PATCH 3/4] Wait for the TLW to become iconized under GTK in the test Iconize() doesn't take effect immediately, so wait until it does for up to some reasonable amount of time. --- tests/persistence/tlw.cpp | 41 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/tests/persistence/tlw.cpp b/tests/persistence/tlw.cpp index e78d7e36ac..47a346d4af 100644 --- a/tests/persistence/tlw.cpp +++ b/tests/persistence/tlw.cpp @@ -20,6 +20,10 @@ #ifndef WX_PRECOMP #include "wx/frame.h" + + #ifdef __WXGTK__ + #include "wx/stopwatch.h" + #endif // __WXGTK__ #endif // WX_PRECOMP #include "wx/persist/toplevel.h" @@ -88,6 +92,7 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]") } // Now try recreating the frame using the restored values. + bool checkIconized = true; { wxFrame* const frame = CreatePersistenceTestFrame(); @@ -106,6 +111,21 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]") frame->Iconize(); frame->Show(); +#ifdef __WXGTK__ + wxStopWatch sw; + while ( !frame->IsIconized() ) + { + wxYield(); + if ( sw.Time() > 500 ) + { + // 500ms should be enough for the window to end up iconized. + WARN("Frame wasn't iconized as expected"); + checkIconized = false; + break; + } + } +#endif // __WXGTK__ + delete frame; } @@ -115,8 +135,27 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]") CHECK(wxPersistenceManager::Get().RegisterAndRestore(frame)); + // As above, we need to show the frame for it to be actually iconized. + frame->Show(); + CHECK(!frame->IsMaximized()); - CHECK(frame->IsIconized()); + if ( checkIconized ) + { +#ifdef __WXGTK__ + wxStopWatch sw; + while ( !frame->IsIconized() ) + { + wxYield(); + if ( sw.Time() > 500 ) + { + INFO("Abandoning wait after " << sw.Time() << "ms"); + break; + } + } +#endif // __WXGTK__ + + CHECK(frame->IsIconized()); + } frame->Restore(); From 70f40e2f6aa33e9bfa3295615abe32ea1fba317b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 16 Jul 2019 02:22:02 +0200 Subject: [PATCH 4/4] Fix position returned for iconized windows in wxGTK Don't lose window position when the window is iconized, as this prevents it from being correctly saved by wxPersitentTLW, for example, resulting in failures in the corresponding unit test. Unfortunately there doesn't seem to be any simple way to just ignore the bogus (0, 0) configure events that we get from GTK when the window is iconized, as explained in the comment, so we're reduced to remembering the last position and restoring it when we realize that the window got minimized and not moved, after all. This is obviously not ideal, as there is still a lapse of time when (0, 0) is returned, but there just doesn't seem to be anything better to do. --- include/wx/gtk/toplevel.h | 3 ++ src/gtk/toplevel.cpp | 58 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/include/wx/gtk/toplevel.h b/include/wx/gtk/toplevel.h index 602a232aba..9a2c705f1d 100644 --- a/include/wx/gtk/toplevel.h +++ b/include/wx/gtk/toplevel.h @@ -164,6 +164,9 @@ private: // size hint increments int m_incWidth, m_incHeight; + // position before it last changed + wxPoint m_lastPos; + // is the frame currently iconized? bool m_isIconized; diff --git a/src/gtk/toplevel.cpp b/src/gtk/toplevel.cpp index c21d0e427c..81d210a865 100644 --- a/src/gtk/toplevel.cpp +++ b/src/gtk/toplevel.cpp @@ -326,6 +326,8 @@ void wxTopLevelWindowGTK::GTKConfigureEvent(int x, int y) if (m_x != point.x || m_y != point.y) { + m_lastPos = wxPoint(m_x, m_y); + m_x = point.x; m_y = point.y; wxMoveEvent event(point, GetId()); @@ -1495,6 +1497,62 @@ void wxTopLevelWindowGTK::SetIconizeState(bool iconize) { if ( iconize != m_isIconized ) { + /* + We get a configure-event _before_ the window is iconized with dummy + (0, 0) coordinates. Unfortunately we can't just ignore this event + when we get it, because we can't know if we're going to be iconized + or not: even remembering that we should be soon in Iconize() itself + doesn't help due to the crazy sequence of events generated by GTK, + e.g. under X11 for the sequence of Iconize() and Show() calls we + get the following events with GTK2: + + - window-state changes to GDK_WINDOW_STATE_ICONIFIED | WITHDRAWN + - window-state changes to just GDK_WINDOW_STATE_ICONIFIED + - map event + - window-state changes to normal + - configure event with normal size + - window-state changes to GDK_WINDOW_STATE_ICONIFIED + - configure event with normal size + - configure event with (0, 0) size + - window-state changes to normal (yes, again) + - configure event with (0, 0) size + - window-state changes to GDK_WINDOW_STATE_ICONIFIED + + So even though we could ignore the first (0, 0) configure event, we + still wouldn't be able to ignore the second one, happening after + the inexplicable switch to normal state. + + For completeness, with GTK3 the sequence of events is simpler, but + still very unhelpful for our purposes: + + - window-state changes to GDK_WINDOW_STATE_ICONIFIED + - window-state changes to normal + - map event + - configure event with normal size + - configure event with normal size (yes, because one is not enough) + - configure event with (0, 0) size + - configure event with (0, 0) size (because why not have 2 of them) + - window-state changes to GDK_WINDOW_STATE_ICONIFIED + + Here again we have (0, 0) configure events happening _after_ the + window-state switch to normal, that would reset our flag. + + In conclusion, we have no choice but to assume that the window got + really moved, but at least we can remember its previous position in + order to restore it here if it turns out that it was just iconized. + */ + if ( iconize ) + { + if ( wxPoint(m_x, m_y) == wxPoint(0, 0) ) + { + m_x = m_lastPos.x; + m_y = m_lastPos.y; + + // This is not really necessary, but seems tidier. + m_lastPos = wxPoint(); + } + } + m_isIconized = iconize; (void)SendIconizeEvent(iconize); }