From 62c3d921b21a1fe8faccd3b0dbe942b129f5f1f4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Oct 2021 23:38:17 +0100 Subject: [PATCH 1/2] Check that all windows in a sizer use associated window as parent Using a wrong parent for the window managed by the sizer of the given window or, alternatively, inserting a window into a wrong sizer, seems to be a common mistake when creating sizer-based layouts and results in hard to understand visual problems (as typically the window with the mismatched parent/sizer just remains stuck at some wrong location). So try to help detect it sooner and provide more information about the problem by checking the parent of the window either when it is added to the sizer -- if we already know which sizer the window is associated with at that moment -- or when associating the sizer with the window later. --- src/common/sizer.cpp | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 3b18ac910f..55d0ca40bb 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -187,6 +187,21 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) ); } +wxString MakeExpectedParentMessage(wxWindow* w, wxWindow* expectedParent) +{ + return wxString::Format + ( + "Windows managed by the sizer associated with the given " + "window must have this window as parent, otherwise they " + "will not be repositioned correctly.\n" + "\n" + "Please use the window %s with which this sizer is associated, " + "as the parent when creating the window %s managed by it.", + wxDumpWindow(expectedParent), + wxDumpWindow(w) + ); +} + } // anonymous namespace #endif // wxDEBUG_LEVEL @@ -225,6 +240,18 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) ASSERT_INCOMPATIBLE_NOT_USED(f, wxALIGN_CENTRE_HORIZONTAL, wxALIGN_RIGHT); \ ASSERT_INCOMPATIBLE_NOT_USED(f, wxALIGN_CENTRE_VERTICAL, wxALIGN_BOTTOM) +// Verify that the given window has the expected parent. +// +// Both pointers must be non-null. +// +// Note that this is a serious error and that, unlike for benign sizer flag +// checks, it can't be disabled by setting some environment variable. +#define ASSERT_WINDOW_PARENT_IS(w, expectedParent) \ + wxASSERT_MSG \ + ( \ + w->GetParent() == expectedParent, \ + MakeExpectedParentMessage(w, expectedParent) \ + ) /* static */ void wxSizerFlags::DisableConsistencyChecks() @@ -779,8 +806,18 @@ wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item ) ContainingSizerGuard guard( item ); - if ( item->GetWindow() ) - item->GetWindow()->SetContainingSizer( this ); + if ( wxWindow* const w = item->GetWindow() ) + { + w->SetContainingSizer( this ); + + // If possible, detect adding windows with a wrong parent to the sizer + // as early as possible, as this allows to see where exactly it happens + // (otherwise this will be checked when the containing window is set + // later, but by this time the stack trace at the moment of assertion + // won't point out the culprit any longer). + if ( m_containingWindow ) + ASSERT_WINDOW_PARENT_IS(w, m_containingWindow); + } if ( item->GetSizer() ) item->GetSizer()->SetContainingWindow( m_containingWindow ); @@ -810,6 +847,9 @@ void wxSizer::SetContainingWindow(wxWindow *win) { sizer->SetContainingWindow(win); } + + if ( wxWindow* const w = item->GetWindow() ) + ASSERT_WINDOW_PARENT_IS(w, m_containingWindow); } } From b88f472291748d129ccf098703d087789a4b401c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Oct 2021 15:33:49 +0100 Subject: [PATCH 2/2] Relax the sizer parent check to allow for null parent too Normally non-toplevel windows must have non-null parents, as creating them with null parent would fail, but our own wxTabFrame used in AUI is an example of a window which can have null parent because it's actually never created at all. This does look like a pretty bad hack, but it's very unlikely to happen accidentally, so relax the assertion check added in the previous commit to allow for it. --- src/common/sizer.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 55d0ca40bb..beb1bbf243 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -187,6 +187,17 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) ); } +bool CheckExpectedParentIs(wxWindow* w, wxWindow* expectedParent) +{ + // We specifically exclude the case of a window with a null parent as it + // typically doesn't happen accidentally, but does happen intentionally in + // our own wxTabFrame which is a hack used by AUI for whatever reason, and + // could presumably be also done on purpose in application code. + wxWindow* const parent = w->GetParent(); + + return !parent || parent == expectedParent; +} + wxString MakeExpectedParentMessage(wxWindow* w, wxWindow* expectedParent) { return wxString::Format @@ -249,7 +260,7 @@ wxString MakeExpectedParentMessage(wxWindow* w, wxWindow* expectedParent) #define ASSERT_WINDOW_PARENT_IS(w, expectedParent) \ wxASSERT_MSG \ ( \ - w->GetParent() == expectedParent, \ + CheckExpectedParentIs(w, expectedParent), \ MakeExpectedParentMessage(w, expectedParent) \ )