#19170: SetSizeHints regression

[wxTrac]

#19170: SetSizeHints regression
--------------------------------+-------------------------
Reporter: ericj | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: GUI-all | Version: dev-latest
Keywords: wxSizer regression | Blocked By:
Blocking: | Patch: 0
--------------------------------+-------------------------
The following code used to work since 3.1.4


{{{
wxPanel *panel = new wxPanel(this, wxID_ANY);
wxButton *btn = new wxButton(panel, wxID_ANY, "Button");
wxBoxSizer *sizer = new wxBoxSizer(wxVERTICAL);
sizer->Add(btn,
wxSizerFlags(0).Align(wxALIGN_CENTER_HORIZONTAL).Border());

panel->SetSizer(sizer);
sizer->SetSizeHints(this);
}}}

With 3.1.5 and the latest HEAD, SetSizeHints() exits early, because there
is no sizer assigned to the window (this).

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------

Reporter: ericj | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------
Changes (by ericj):

* Attachment "minimal_setsizehints.patch" added.

Patch to the minimal sample demonstrating the issue

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest
Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------
Changes (by vadz):

* status: new => confirmed
* milestone: => 3.1.6


Comment:

Thanks and sorry for the delay!

We can fix the regression by just passing the sizer itself as (optional)
parameter to `WXSetInitialFittingClientSize()`, but this is (probably?)
not going to help with the GTK-specific problems from #16088. For this
we'd probably need to check for the special case of a window with its
sizer in `wxTopLevelWindowGTK::GTKUpdateClientSizeIfNecessary()` too.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:1>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:

Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------
Changes (by ericj):

* cc: ericj (added)


Comment:

As it is right now the method '''wxSizer::SetSizeHints( wxWindow *window
)''' loses its purpose, because the sizer is not used at all. It basically
turns into '''wxWindow::SetSizeHints()'''.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:2>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:

Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------

Comment (by vadz):

Just to be clear, this definitely needs to be addressed, I am just not
sure how exactly yet...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:3>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:

Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------

Comment (by vadz):

Actually I don't really understand what did I write in comment:1 any
longer, i.e. why wouldn't a patch like this be enough:
{{{
#!diff
diff --git a/include/wx/window.h b/include/wx/window.h
index 794d7a0a15..393d71ba49 100644
--- a/include/wx/window.h
+++ b/include/wx/window.h
@@ -1571,7 +1571,11 @@ class WXDLLIMPEXP_CORE wxWindowBase : public
wxEvtHandler
// that we really need to use is not known until the window is
actually
// shown, as is the case for TLWs with recent GTK versions, as it
will
// update the size again when it does become known, if necessary.
- virtual void WXSetInitialFittingClientSize(int flags);
+ //
+ // The optional sizer argument can be passed to use the given sizer
for
+ // laying out the window, which is useful if this function is called
before
+ // SetSizer(). By default the window sizer is used.
+ virtual void WXSetInitialFittingClientSize(int flags, wxSizer* sizer
= NULL);

// get the handle of the window for the underlying window system:
this
// is only used for wxWin itself or for user code which wants to
call
diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp
index 755ba7fd3e..3b18ac910f 100644
--- a/src/common/sizer.cpp
+++ b/src/common/sizer.cpp
@@ -1079,7 +1079,7 @@ wxSize wxSizer::Fit( wxWindow *window )
wxCHECK_MSG( window, wxDefaultSize, "window can't be NULL" );

// set client size
- window->WXSetInitialFittingClientSize(wxSIZE_SET_CURRENT);
+ window->WXSetInitialFittingClientSize(wxSIZE_SET_CURRENT, this);

// return entire size
return window->GetSize();
@@ -1119,7 +1119,8 @@ void wxSizer::SetSizeHints( wxWindow *window )
{
// Preserve the window's max size hints, but set the
// lower bound according to the sizer calculations.
- window->WXSetInitialFittingClientSize(wxSIZE_SET_CURRENT |
wxSIZE_SET_MIN);
+ window->WXSetInitialFittingClientSize(wxSIZE_SET_CURRENT |
wxSIZE_SET_MIN,
+ this);
}

#if WXWIN_COMPATIBILITY_2_8
diff --git a/src/common/wincmn.cpp b/src/common/wincmn.cpp
index bb8127670c..a851b371ce 100644
--- a/src/common/wincmn.cpp
+++ b/src/common/wincmn.cpp
@@ -999,11 +999,17 @@ wxSize wxWindowBase::WindowToClientSize(const
wxSize& size) const
size.y == -1 ? -1 : size.y - diff.y);
}

-void wxWindowBase::WXSetInitialFittingClientSize(int flags)
+void wxWindowBase::WXSetInitialFittingClientSize(int flags, wxSizer*
sizer)
{
- wxSizer* const sizer = GetSizer();
+ // Use the window sizer by default.
if ( !sizer )
- return;
+ {
+ sizer = GetSizer();
+
+ // If there is none, we can't compute the fitting size.
+ if ( !sizer )
+ return;
+ }

const wxSize
size = sizer->ComputeFittingClientSize(static_cast<wxWindow
*>(this));
}}}

This shouldn't change anything for wxGTK and, in particular, shouldn't
break anything.

Am I missing something here?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:4>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:

Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------
Changes (by ericj):

* cc: ericj (removed)


Comment:

Yes, i think that should work. As i don't work under GTK, it was your
comment about "GTK-specific problems" that confused me, i thought there
was more to it that i don't know about.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:5>

[wxTrac]

#19170: SetSizeHints regression
----------------------+--------------------------------
Reporter: ericj | Owner:

Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+--------------------------------

Comment (by vadz):

Try as I might, I don't manage to remember what kind of problem I had in
mind when writing that comment, so let's commit this simple fix.

Unfortunately I also still don't know how to write a unit test for the
problem fixed by `WXSetInitialFittingClientSize()`, so we're limited to
manual testing here.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:6>

[wxTrac]

#19170: SetSizeHints regression
----------------------+-------------------------------------
Reporter: ericj | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed

Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: fixed | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+-------------------------------------
Changes (by Vadim Zeitlin <vadim@…>):

* owner: => Vadim Zeitlin <vadim@…>
* status: confirmed => closed
* resolution: => fixed


Comment:

In [changeset:"136574b1e0ca3a7165d379261952dfb3fb2f5ca6/git-wxWidgets"
136574b1e/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="136574b1e0ca3a7165d379261952dfb3fb2f5ca6"
Make wxSizer::SetSizeHints() work again

This function was broken when it was called for a window which was not
the window the sizer was associated with since the recent (pre-3.1.5)
changes trying to work around the problem with the initial windows size
when using GTK 3, see 9c0a8be1dc (Merge branch 'gtk-initial-size',
2021-04-13).

Fix it by passing the sizer to use for calculating the size explicitly
to WXSetInitialFittingClientSize() when we have it, and only falling
back on the window's own sizer if we don't.

Closes #19170.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:7>

[wxTrac]

#19170: SetSizeHints regression
----------------------+-------------------------------------
Reporter: ericj | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.6
Component: GUI-all | Version: dev-latest

Resolution: fixed | Keywords: wxSizer regression

Blocked By: | Blocking:
Patch: 0 |

----------------------+-------------------------------------

Comment (by Vadim Zeitlin <vadim@…>):

In [changeset:"912f4b76ac42a79ff772cc9ea5cf8cb5c3f09960/git-wxWidgets"
912f4b76a/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="912f4b76ac42a79ff772cc9ea5cf8cb5c3f09960"
Fix wxGTK build after WXSetInitialFittingClientSize() change

This should have been part of 136574b1e0 (Make wxSizer::SetSizeHints()
work again, 2021-07-24).

See #19170.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19170#comment:8>