Items inside a wxStaticBox on a wxNotebook page are not visible anymore (Issue #23239)
taler21
Description
In the current master, items inside a wxStaticBox on a wxNotebook page are not visible if the static box is not their parent.
This can be reproduced in the notebook sample with these changes.
diff --git "a/samples/notebook/notebook.cpp" "b/samples/notebook/notebook.cpp" index 57500b8c74..45676d06f3 100644 --- "a/samples/notebook/notebook.cpp" +++ "b/samples/notebook/notebook.cpp" @@ -98,9 +98,17 @@ wxPanel *CreateRadioButtonsPage(wxBookCtrlBase *parent) "Choose your favourite", wxDefaultPosition, wxDefaultSize, 4, computers, 0, wxRA_SPECIFY_COLS); + wxStaticBox* stbox = new wxStaticBox(panel,wxID_ANY," wxStaticBox "); + wxStaticBoxSizer* sbs = new wxStaticBoxSizer(stbox, wxVERTICAL); + wxStaticText* stx1 = new wxStaticText(panel, wxID_ANY, "wxStaticText 1"); + wxStaticText* stx2 = new wxStaticText(stbox, wxID_ANY, "wxStaticText 2"); + sbs->Add( stx1 ); + sbs->Add( stx2 ); + wxBoxSizer *sizerPanel = new wxBoxSizer(wxVERTICAL); sizerPanel->Add(radiobox1, 2, wxEXPAND); sizerPanel->Add(radiobox2, 1, wxEXPAND); + sizerPanel->Add(sbs, 1, wxEXPAND); panel->SetSizer(sizerPanel); return panel;
The first text is not visible, its parent is the panel.
The second text is visible, its parent is the static box.
I know that since wxWidgets 2.9.1 it is recommended to use the wxStaticBox as parent of the included items.
But is it really intentional that such code no longer works with wxWidgets 3.3.0?
As the combo sample shows, it basically still works if items in a wxStaticBox don't have the static box as parent.
https://github.com/wxWidgets/wxWidgets/blob/d8edc746e24b28f9222a51658624db6f6827c99a/samples/combo/combo.cpp#L974-L979
But on a wxNotebook page, this doesn't work anymore.
Is that intentional or a bug?
Before I have to revise a lot of old code, I want to make sure if this is really necessary.
Thanks in advance for any clarification.
Platform and version information
- wxWidgets version you use: current master for upcoming 3.3.0
- wxWidgets port you use: wxMSW
- OS and its version: Windows 10
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
This seems to be the same as #23182, isn't it?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
Yes indeed. Sorry, I hadn't seen that this issue is already known.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #23239 as completed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
So it's not quite the same because disabling compositing doesn't help here: the first item is now visible but it appears at 0,0 instead of its correct location. Looking...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Reopened #23239.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
So this issue accidentally discovered another problem: using items with either the box itself or the parent window as parent in wxStaticBoxSizer works, but mixing them does not. I.e. your example works with my PR if stx2 is changed to use panel as parent as well.
AFAICS mixing different parents has never worked, and I don't think it's worth making it work, even if it could be possible, so I'm just going to document it and leave it at that.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
The updated PR #23259 adds an assert which will be triggered by using mismatched parents as in the example above. IMO this is warranted here as it's better than silently getting wrong layout.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
What still doesn't work is when the panel is used as parent for the items in a wxGridSizer in a wxStaticBox in a wxNotebook page.
Here is a reproducer for the combo sample:
diff --git "a/samples/combo/combo.cpp" "b/samples/combo/combo.cpp" index d97c2f275c..4d7dacedcf 100644 --- "a/samples/combo/combo.cpp" +++ "b/samples/combo/combo.cpp" @@ -34,6 +34,7 @@ #include "wx/combo.h" #include "wx/odcombo.h" +#include "wx/notebook.h" // ---------------------------------------------------------------------------- // resources @@ -94,6 +95,65 @@ private: wxDECLARE_EVENT_TABLE(); }; +class MyTestDlg : public wxDialog +{ +public: + MyTestDlg( wxWindow* parent, wxWindowID id ); + +private: + void CreateControls(); +}; + +MyTestDlg::MyTestDlg( wxWindow* parent, wxWindowID id ) +{ + wxDialog::Create( parent, id, "Test dialog" ); + + CreateControls(); + if (GetSizer()) + { + GetSizer()->SetSizeHints(this); + } + Centre(); +} + +void MyTestDlg::CreateControls() +{ + wxBoxSizer* topSizer = new wxBoxSizer(wxVERTICAL); + SetSizer(topSizer); + + wxNotebook* nb = new wxNotebook( this, wxID_ANY ); + + wxPanel* panel = new wxPanel( nb, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL ); + wxBoxSizer* pageSizer = new wxBoxSizer(wxVERTICAL); + panel->SetSizer(pageSizer); + + wxStaticBox* staticBox = new wxStaticBox(panel, wxID_ANY, _("StaticBox")); + wxStaticBoxSizer* staticBoxSizer = new wxStaticBoxSizer(staticBox, wxVERTICAL); + pageSizer->Add(staticBoxSizer, 0, wxGROW|wxALL, 5); + wxFlexGridSizer* gridSizer = new wxFlexGridSizer(0, 2, 0, 0); + staticBoxSizer->Add(gridSizer, 0, wxALIGN_LEFT|wxALL, 5); + wxStaticText* staticText = new wxStaticText( panel, wxID_STATIC, _("Static text"), wxDefaultPosition, wxDefaultSize, 0 ); + gridSizer->Add(staticText, 0, wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL|wxALL, 5); + + wxTextCtrl* textCtrl = new wxTextCtrl( panel, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0 ); + gridSizer->Add(textCtrl, 0, wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL|wxALL, 5); + + nb->AddPage(panel, _("Tab")); + + topSizer->Add(nb, 1, wxGROW|wxALL, 5); + + wxStdDialogButtonSizer* sbs = new wxStdDialogButtonSizer; + + topSizer->Add(sbs, 0, wxGROW|wxALL, 5); + wxButton* okBtn = new wxButton( this, wxID_OK, _("&OK"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(okBtn); + + wxButton* cancelBtn = new wxButton( this, wxID_CANCEL, _("&Cancel"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(cancelBtn); + + sbs->Realize(); +} + // ---------------------------------------------------------------------------- // constants // ---------------------------------------------------------------------------- @@ -949,6 +1009,13 @@ void MyFrame::OnComboBoxUpdate( wxCommandEvent& event ) void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) ) { +#if 1 + + MyTestDlg dlg( this, wxID_ANY ); + dlg.ShowModal(); + +#else + // // Show some wxOwnerDrawComboBoxes for comparison // @@ -1121,6 +1188,7 @@ void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) ) dlg->SetSize(60,240); dlg->Centre(); dlg->ShowModal(); +#endif } MyFrame::~MyFrame()
Open the test dialog via File > Compare against wxComboBox...
With wxWidgets-3.2 all items are visible.
But with wxWidgets-3.3 (the PR) the items inside the grid sizer are not visible.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks, I see why this happens (we don't disable compositing when sibling windows are not added directly to the static box sizer) and even think that I see how to fix it (we need to check the children of the sizers being added and not just the windows added directly) and will try to fix this too soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Ugh, except it's more difficult to do when the windows are added to the inner sizer after it had been added to the static box sizer itself as this means that we can't do this check in wxStaticBoxSizer::DoInsert() but are going to have to do it in wxStaticBoxSizer::RepositionChildren() itself, which means that we'd have to do it on every resize, which is not ideal.
But I don't see any other way to keep supporting this...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I've updated #23259 with the fix for the latest problem too now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
What still doesn't work is when controls are added later to a sizer in a dynamically constructed window.
Here is a reproducer for the combo sample:
diff --git "a/samples/combo/combo.cpp" "b/samples/combo/combo.cpp"
index d97c2f275c..3d72ceea35 100644
--- "a/samples/combo/combo.cpp" +++ "b/samples/combo/combo.cpp" @@ -34,6 +34,7 @@ #include "wx/combo.h" #include "wx/odcombo.h" +#include "wx/notebook.h" // ---------------------------------------------------------------------------- // resources
@@ -94,6 +95,135 @@ private: wxDECLARE_EVENT_TABLE(); }; +class MyStaticBoxSizer : public wxStaticBoxSizer +{ +public: + MyStaticBoxSizer(wxWindow *parent); + + void SetItemsNumber(int n); + +protected: + void AddItems(int n); + +protected: + wxFlexGridSizer *m_Grid; + + static const int DsplMax = 10; + wxStaticText *m_Label[DsplMax]; + wxTextCtrl *m_Text[DsplMax]; +}; + +MyStaticBoxSizer::MyStaticBoxSizer(wxWindow *parent) + : wxStaticBoxSizer(wxVERTICAL, parent, _("MyStaticBoxSizer")) +{ + m_Grid = new wxFlexGridSizer(5, 5, 5); + Add(m_Grid, 0, wxALIGN_CENTER_HORIZONTAL|wxALL, 5); + + for (int i = 0; i < DsplMax; i++) + { + m_Label[i] = new wxStaticText(parent, wxID_STATIC, wxString::Format(_("Label %d"), i+1)); + m_Text[i] = new wxTextCtrl(parent, wxID_ANY); + } + + // AddItems(DsplMax); +} + +void MyStaticBoxSizer::AddItems(int n) +{ + m_Grid->Clear(); + int cols = n > 5 ? 2 : 1; + m_Grid->SetCols(5 * cols / 2); + + for (int i = 0; i < n; i++) + { + int j = cols == 1 ? i : ((i % 2) * ((n + 1) / 2)) + (i / 2); + m_Grid->Add(m_Label[j], 0, wxALIGN_CENTER_VERTICAL, 0); + m_Grid->Add(m_Text[j], 0, wxALIGN_CENTER_VERTICAL, 0); + if ((cols == 2) && (i % 2 == 0)) + m_Grid->AddSpacer(5); + } +} + +void MyStaticBoxSizer::SetItemsNumber(int n) +{ + for (int i = 0; i < DsplMax; i++) + { + m_Label[i]->Show(i < n); + m_Text[i]->Show(i < n); + } + + AddItems(n); + GetContainingWindow()->Layout(); +} +
+class MyTestDlg : public wxDialog +{ +public: + MyTestDlg( wxWindow* parent, wxWindowID id ); +
+ void SetMyBoxItemsNumber(int n) + { m_myBoxSizer->SetItemsNumber(n); }
+ +private: + void CreateControls(); +
+ MyStaticBoxSizer* m_myBoxSizer;+}; + +MyTestDlg::MyTestDlg( wxWindow* parent, wxWindowID id ) +{ + wxDialog::Create( parent, id, "Test dialog" ); + + CreateControls(); + if (GetSizer()) + { + GetSizer()->SetSizeHints(this); + }
+ SetClientSize( wxDLG_UNIT(this, wxSize(250, 150)) );+ m_myBoxSizer = new MyStaticBoxSizer(panel); + pageSizer->Add(m_myBoxSizer, 0, wxGROW|wxALL, 5); +
+ nb->AddPage(panel, _("Tab")); + + topSizer->Add(nb, 1, wxGROW|wxALL, 5); + + wxStdDialogButtonSizer* sbs = new wxStdDialogButtonSizer; + + topSizer->Add(sbs, 0, wxGROW|wxALL, 5); + wxButton* okBtn = new wxButton( this, wxID_OK, _("&OK"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(okBtn); + + wxButton* cancelBtn = new wxButton( this, wxID_CANCEL, _("&Cancel"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(cancelBtn); + + sbs->Realize(); +} + // ---------------------------------------------------------------------------- // constants // ----------------------------------------------------------------------------
@@ -949,6 +1079,14 @@ void MyFrame::OnComboBoxUpdate( wxCommandEvent& event )
void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) )
{
+#if 1 + + MyTestDlg dlg( this, wxID_ANY );
+ dlg.SetMyBoxItemsNumber(6);+ dlg.ShowModal(); + +#else + // // Show some wxOwnerDrawComboBoxes for comparison //
@@ -1121,6 +1259,7 @@ void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) ) dlg->SetSize(60,240); dlg->Centre(); dlg->ShowModal(); +#endif } MyFrame::~MyFrame()
Open the test dialog via File > Compare against wxComboBox...
With wxWidgets-3.2 all items are displayed correctly.
But with the current PR, the items are displayed at wrong position.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
There is also an issue when using wxOwnerDrawnComboBox.
Here is a reproducer for the combo sample:
Repro for combo samplediff --git "a/samples/combo/combo.cpp" "b/samples/combo/combo.cpp"
index d97c2f275c..4b0316cadc 100644
--- "a/samples/combo/combo.cpp" +++ "b/samples/combo/combo.cpp" @@ -34,6 +34,7 @@ #include "wx/combo.h" #include "wx/odcombo.h" +#include "wx/notebook.h" // ---------------------------------------------------------------------------- // resources
@@ -94,6 +95,64 @@ private:
wxDECLARE_EVENT_TABLE();
};
+class MyTestDlg : public wxDialog +{ +public: + MyTestDlg( wxWindow* parent, wxWindowID id ); +
+private: + void CreateControls(); +}; +
+MyTestDlg::MyTestDlg( wxWindow* parent, wxWindowID id ) +{ + wxDialog::Create( parent, id, "Test dialog" ); + + CreateControls(); + if (GetSizer()) + { + GetSizer()->SetSizeHints(this); + }
+ Centre(); +} + +void MyTestDlg::CreateControls() +{ + wxBoxSizer* topSizer = new wxBoxSizer(wxVERTICAL); + SetSizer(topSizer); + + wxNotebook* nb = new wxNotebook( this, wxID_ANY ); + + wxPanel* panel = new wxPanel( nb, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL ); + wxBoxSizer* pageSizer = new wxBoxSizer(wxVERTICAL); + panel->SetSizer(pageSizer); + + wxStaticBox* staticBox = new wxStaticBox(panel, wxID_ANY, _("StaticBox")); + wxStaticBoxSizer* staticBoxSizer = new wxStaticBoxSizer(staticBox, wxVERTICAL); + pageSizer->Add(staticBoxSizer, 0, wxGROW|wxALL, 5); +
+ wxStaticText* staticText = new wxStaticText( panel, wxID_STATIC, _("Static text"), wxDefaultPosition, wxDefaultSize, 0 );
+ staticBoxSizer->Add(staticText, 0, wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL|wxALL, 5); + + wxOwnerDrawnComboBox* odcombo = new wxOwnerDrawnComboBox( panel, wxID_ANY ); + staticBoxSizer->Add(odcombo, 0, wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL|wxALL, 5); +
+ nb->AddPage(panel, _("Tab")); + + topSizer->Add(nb, 1, wxGROW|wxALL, 5); + + wxStdDialogButtonSizer* sbs = new wxStdDialogButtonSizer; + + topSizer->Add(sbs, 0, wxGROW|wxALL, 5); + wxButton* okBtn = new wxButton( this, wxID_OK, _("&OK"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(okBtn); + + wxButton* cancelBtn = new wxButton( this, wxID_CANCEL, _("&Cancel"), wxDefaultPosition, wxDefaultSize, 0 ); + sbs->AddButton(cancelBtn); + + sbs->Realize(); +} + // ---------------------------------------------------------------------------- // constants // ----------------------------------------------------------------------------
@@ -949,6 +1008,13 @@ void MyFrame::OnComboBoxUpdate( wxCommandEvent& event )
void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) )
{
+#if 1 + + MyTestDlg dlg( this, wxID_ANY );
+ dlg.ShowModal(); + +#else + // // Show some wxOwnerDrawComboBoxes for comparison //
@@ -1121,6 +1187,7 @@ void MyFrame::OnShowComparison( wxCommandEvent& WXUNUSED(event) ) dlg->SetSize(60,240); dlg->Centre(); dlg->ShowModal(); +#endif } MyFrame::~MyFrame()
Open the test dialog via File > Compare against wxComboBox...
With wxWidgets-3.2 all items are displayed correctly.
But with the current PR, the wxStaticBox, the wxOwnerDrawnComboBox and the buttons in the wxStdDialogButtonSizer are initially not visible.
They only become visible when the dialog is moved on the screen.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Concerning the first problem, and unless I'm missing something, it is the same one I mentioned in the commit message and I don't see how to make the sizer adapt to the new items without performing a check whenever adding any new item to any sizer, which would impose extra overhead on all wx code using sizers. So we need to either disable compositing whenever wxStaticBoxSizer is used or live with this regression. Or just abandon the attempts to use compositing completely, which seems more and more appealing, to be honest.
The second problem is less clear, I'll try to look at it, thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
OK, the second problem is due to the fact that we need to disable WS_EX_COMPOSITED for wxODCombo itself when it's inside the static box, and this is only done if we call MSWDisableComposited() after adding it -- which means that we have to call CheckIfNonBoxChild() for each new child and not just the first one.
I also realized that we can only check for non-box children if the box doesn't have any real children as even if this will miss mixing up both kinds of children in some cases, it's still going to work correctly when there is only a single type of them and doesn't impose any overhead on the normal case, so this fixes the first problem too -- and actually simplifies the code. So the updated PR should work in these cases too now. Thank you for your testing and hopefully it should be finally really good now!
P.S. BTW, wxNotebook is a red herring, nothing here is specific to it and the behaviour is exactly the same if it's not used at all. And so is wxStdDialogButtonSizer.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
I am sorry, but the second problem is still reproducible.
Only when the dialog is moved on the screen all controls become visible.
I also noticed a painting issue which can be seen in the widgets sample.
The red area is not drawn initially if the 'Button' page is the start page.
This problem does not exist in the master but only in the PR branch.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Oops, I've somehow lost the change fixing the second problem while rebasing my changes. Restored in 674bfb5 now, thanks.
The background problem in the sample is worse as it seems like background handling is just completely broken with compositing and even when we turn it off later, i.e. just setting the page background makes almost all controls invisible on all widgets sample pages. I don't know why yet.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
OK, I've finally got it, this change uncovered a preexisting problem with wrong wxStaticBox redrawing if WS_EX_COMPOSITING is turned off for its parent after it is created (which could already happen if you happened to have a wxListCtrl in the same window, for example...).
I've fixed this one in 2baf34f
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
taler21
Looks good now, many thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks a lot for your testing!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.