From 44eb96b993354a9b17b0085e96956e5d2370c736 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 23 Feb 2019 18:38:52 +0100 Subject: [PATCH 01/19] Get rid of unnecessary temporary variable in wxMSW wxToolBar No real changes, just remove a variable only used once. --- src/msw/toolbar.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 2c4ae26eb1..8e8a2ea4b9 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1854,8 +1854,7 @@ void wxToolBar::OnEraseBackground(wxEraseEvent& event) bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM lParam) { // wait until we have some tools - const int toolsCount = GetToolsCount(); - if ( toolsCount == 0 ) + if ( !GetToolsCount() ) return false; // calculate our minor dimension ourselves - we're confusing the standard From 4a02b73a6afe6b9903d4de59bc658e04ca857423 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 23 Feb 2019 19:03:51 +0100 Subject: [PATCH 02/19] Use a symbolic constant instead of hardcoded 3 No real changes, just give a name to the constant used for the number of pixels to leave between an embedded control and its label. --- src/msw/toolbar.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 8e8a2ea4b9..dbe67831e9 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -94,6 +94,9 @@ #define TB_GETMAXSIZE (WM_USER + 83) #endif +// Margin between the control and its label. +static const int MARGIN_CONTROL_LABEL = 3; + // ---------------------------------------------------------------------------- // wxWin macros // ---------------------------------------------------------------------------- @@ -1227,7 +1230,7 @@ bool wxToolBar::Realize() if ( staticText && staticText->IsShown() ) { staticTextSize = staticText->GetSize(); - staticTextSize.y += 3; // margin between control and its label + staticTextSize.y += MARGIN_CONTROL_LABEL; } // position the control itself correctly vertically centering it on the From 82857cfa9d3cd5e0b6bef0879581b848796e5940 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 02:03:23 +0100 Subject: [PATCH 03/19] Add small helper function to wxMSW wxToolBar code Encapsulate the logic for deciding whether we should show the labels for the embedded controls or not in a small helper function, to make it simpler to change it in the future if needed and also to have a single place to explain why do we do what we do now. No real changes. --- include/wx/msw/toolbar.h | 10 ++++++++++ src/msw/toolbar.cpp | 6 +----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/include/wx/msw/toolbar.h b/include/wx/msw/toolbar.h index 5423f771f1..ff5da800c5 100644 --- a/include/wx/msw/toolbar.h +++ b/include/wx/msw/toolbar.h @@ -174,6 +174,16 @@ private: WXHBRUSH MSWGetToolbarBgBrush(); #endif // wxHAS_MSW_BACKGROUND_ERASE_HOOK + // Return true if we're showing the labels for the embedded controls: we + // only do it if text is enabled and, somewhat less expectedly, if icons + // are enabled too because showing both the control and its label when only + // text is shown for the other buttons is too inconsistent to be useful. + bool AreControlLabelsShown() const + { + return HasFlag(wxTB_TEXT) && !HasFlag(wxTB_NOICONS); + } + + wxDECLARE_EVENT_TABLE(); wxDECLARE_DYNAMIC_CLASS(wxToolBar); wxDECLARE_NO_COPY_CLASS(wxToolBar); diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index dbe67831e9..8158d1cd9f 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1050,11 +1050,7 @@ bool wxToolBar::Realize() case wxTOOL_STYLE_CONTROL: if ( wxStaticText *staticText = tool->GetStaticText() ) { - // Display control and its label only if buttons have icons - // and texts as otherwise there is not enough room on the - // toolbar to fit the label. - staticText-> - Show(HasFlag(wxTB_TEXT) && !HasFlag(wxTB_NOICONS)); + staticText->Show(AreControlLabelsShown()); } // Set separator width/height to fit the control width/height From 44d732b8a5399e6a181d5b0a86912bc5639d7c9d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 03:38:52 +0100 Subject: [PATCH 04/19] Use TB_SETBUTTONINFO when updating stretchable toolbar separators This is simpler than TB_DELETEBUTTON followed by TB_INSERTBUTTON and avoids weird side effects of using the old messages which affect the toolbar size. There should be no changes in behaviour with this change. --- src/msw/toolbar.cpp | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 8158d1cd9f..5a28b58295 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1387,31 +1387,20 @@ void wxToolBar::UpdateStretchableSpacersSize() const int newSize = --numSpaces ? sizeSpacer : sizeLastSpacer; if ( newSize != oldSize) { - if ( !::SendMessage(GetHwnd(), TB_DELETEBUTTON, toolIndex, 0) ) + WinStruct tbbi; + tbbi.dwMask = TBIF_BYINDEX | TBIF_SIZE; + tbbi.cx = newSize; + if ( !::SendMessage(GetHwnd(), TB_SETBUTTONINFO, + toolIndex, (LPARAM)&tbbi) ) { - wxLogLastError(wxT("TB_DELETEBUTTON (separator)")); + wxLogLastError(wxT("TB_SETBUTTONINFO (separator)")); } else { - TBBUTTON button; - wxZeroMemory(button); - - button.idCommand = tool->GetId(); - button.iBitmap = newSize; // set separator width/height - button.fsState = TBSTATE_ENABLED; - button.fsStyle = TBSTYLE_SEP; - if ( IsVertical() ) - button.fsState |= TBSTATE_WRAP; - if ( !::SendMessage(GetHwnd(), TB_INSERTBUTTON, toolIndex, (LPARAM)&button) ) - { - wxLogLastError(wxT("TB_INSERTBUTTON (separator)")); - } - else - { - // We successfully replaced this seprator, move all the controls after it - // by the corresponding amount (may be positive or negative) - offset += newSize - oldSize; - } + // We successfully updated the separator width, move all the + // controls appearing after it by the corresponding amount + // (which may be positive or negative) + offset += newSize - oldSize; } } From c7e8aac70f319c1224594d9733069adb30d0a9e1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 03:49:56 +0100 Subject: [PATCH 05/19] Explicitly skip expanding stretchable separators in MSW toolbars When computing the fixed size of MSW toolbars, don't take stretchable separators into account as this doesn't make sense and so, even if this doesn't matter currently because the separators still have their initial, fixed size when total fixed size is computed, but would start mattering if UpdateStretchableSpacersSize() is ever called before this is done and it also just makes more sense to skip them here. --- src/msw/toolbar.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 5a28b58295..9f60b5b773 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1200,10 +1200,15 @@ bool wxToolBar::Realize() if ( !tool->IsControl() ) { - if ( IsVertical() ) - m_totalFixedSize += r.bottom - r.top; - else - m_totalFixedSize += r.right - r.left; + // Stretchable space don't have any fixed size and their current + // size shouldn't count at all. + if ( !tool->IsStretchableSpace() ) + { + if ( IsVertical() ) + m_totalFixedSize += r.bottom - r.top; + else + m_totalFixedSize += r.right - r.left; + } continue; } From 7c8ba45705353ca23cd490f14ce74594a360753e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 18:19:49 +0100 Subject: [PATCH 06/19] Reuse helper MSWGetFittingtSizeForControl() in a couple of places Refactor the code to reuse the same function for determining the size needed by an embedded control in the toolbar, including its label. --- include/wx/msw/toolbar.h | 4 ++++ src/msw/toolbar.cpp | 35 +++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/include/wx/msw/toolbar.h b/include/wx/msw/toolbar.h index ff5da800c5..ba3581515b 100644 --- a/include/wx/msw/toolbar.h +++ b/include/wx/msw/toolbar.h @@ -183,6 +183,10 @@ private: return HasFlag(wxTB_TEXT) && !HasFlag(wxTB_NOICONS); } + // Return the size required to accommodate the given tool which must be of + // "control" type. + wxSize MSWGetFittingtSizeForControl(class wxToolBarTool* tool) const; + wxDECLARE_EVENT_TABLE(); wxDECLARE_DYNAMIC_CLASS(wxToolBar); diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 9f60b5b773..a9f9dfb6b3 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -530,6 +530,31 @@ wxToolBar::~wxToolBar() delete m_disabledImgList; } +wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const +{ + wxSize size = tool->GetControl()->GetBestSize(); + + // Account for the label, if any. + if ( wxStaticText * const staticText = tool->GetStaticText() ) + { + if ( AreControlLabelsShown() ) + { + const wxSize sizeLabel = staticText->GetSize(); + + if ( size.x < sizeLabel.x ) + size.x = sizeLabel.x; + + size.y += sizeLabel.y; + size.y += MARGIN_CONTROL_LABEL; + } + } + + // Also account for the tool padding value. + size += wxSize(m_toolPacking, m_toolPacking); + + return size; +} + wxSize wxToolBar::DoGetBestSize() const { wxSize sizeBest; @@ -578,10 +603,8 @@ wxSize wxToolBar::DoGetBestSize() const tool = static_cast(node->GetData()); if (tool->IsControl()) { - int y = tool->GetControl()->GetSize().y; - // Approximate border size - if (y > (sizeBest.y - 4)) - sizeBest.y = y + 4; + // Ensure we're tall enough for the embedded controls. + sizeBest.IncTo(wxSize(-1, MSWGetFittingtSizeForControl(tool).y)); } } @@ -1057,8 +1080,8 @@ bool wxToolBar::Realize() // taking into account tool padding value. // (height is not used but it is set for the sake of consistency). { - const wxSize sizeControl = tool->GetControl()->GetSize(); - button.iBitmap = m_toolPacking + (IsVertical() ? sizeControl.y : sizeControl.x); + const wxSize size = MSWGetFittingtSizeForControl(tool); + button.iBitmap = size.x; } wxFALLTHROUGH; From 1a7961036195a961ec220d420a3015e335613753 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 18:20:47 +0100 Subject: [PATCH 07/19] Rewrite wxToolBar resizing logic in wxMSW wxToolBar handled its size in a very unique way as it waited to be resized and then set its size correctly from its own WM_SIZE handler. This was very confusing and, worse, broke a very natural assumption that after calling SetSize(size) on a window this window does have the specified size -- which wasn't necessarily the case for wxToolBar, resulting in problems such as the one in #18294. The reason for doing it in such weird way is that the native toolbar control is weird itself and uses a specific message (TB_AUTOSIZE) to update its size and, moreover, doesn't always do it correctly (notably for the vertical toolbars). It seems that we can make it work more or less as wanted if we use TB_SETBUTTONSIZE _after_ TB_AUTOSIZE (why does it need to be done in this order remains a complete mystery) and if we correct the width of vertical toolbars in UpdateSize() (which is not called from WM_SIZE handler but only from Realize() and other methods modifying the toolbar, so it's not a problem for it to change the size). The only known problem with this commit known at this time is that stretchable separators in vertical don't work any longer, it seems that the size passed to TB_SETBUTTONSIZE is just ignored in this case. However stretchable toolbar separators are pretty rare nowadays and even more so in vertical toolbars, so, arguably, this is not a big loss. Also tweak the layout of the labels for the embedded controls to make them more similar for the labels used for the normal tools and, notably, allocate enough space if the label is longer than the control itself. Closes #18294. --- src/msw/toolbar.cpp | 283 ++++++++++++++++++-------------------------- 1 file changed, 116 insertions(+), 167 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index a9f9dfb6b3..2cdf24f9b1 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -165,17 +165,7 @@ public: if ( IsControl() && !m_label.empty() ) { // Create a control to render the control's label. - // It has the same witdh as the control. - wxSize size(control->GetSize().GetWidth(), wxDefaultCoord); - m_staticText = new wxStaticText - ( - m_tbar, - wxID_ANY, - m_label, - wxDefaultPosition, - size, - wxALIGN_CENTRE | wxST_NO_AUTORESIZE - ); + m_staticText = new wxStaticText(m_tbar, wxID_ANY, m_label); } else // no label { @@ -1057,6 +1047,7 @@ bool wxToolBar::Realize() // this array will hold the indices of all controls in the toolbar wxArrayInt controlIds; + int minReqHeight = 0; bool lastWasRadio = false; int i = 0; for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) @@ -1071,16 +1062,11 @@ bool wxToolBar::Realize() switch ( tool->GetStyle() ) { case wxTOOL_STYLE_CONTROL: - if ( wxStaticText *staticText = tool->GetStaticText() ) - { - staticText->Show(AreControlLabelsShown()); - } - - // Set separator width/height to fit the control width/height - // taking into account tool padding value. - // (height is not used but it is set for the sake of consistency). + if ( !IsVertical() ) { const wxSize size = MSWGetFittingtSizeForControl(tool); + if ( minReqHeight < size.y ) + minReqHeight = size.y; button.iBitmap = size.x; } @@ -1197,6 +1183,15 @@ bool wxToolBar::Realize() break; } + if ( IsVertical() ) + { + // MSDN says that TBSTATE_WRAP should be used for all buttons in + // vertical toolbars, so do it even if it doesn't seem to actually + // change anything in practice (including the problem with + // TB_AUTOSIZE mentioned in UpdateSize()). + button.fsState |= TBSTATE_WRAP; + } + lastWasRadio = isRadio; i++; @@ -1208,6 +1203,24 @@ bool wxToolBar::Realize() } + // Make sure the toolbar is tall enough to fit the embedded controls, which + // may be taller than the buttons. + wxSize toolSize = GetToolSize(); + if ( toolSize.y < minReqHeight ) + { + toolSize.y = minReqHeight; + + // Surprisingly, we need to send TB_AUTOSIZE before TB_SETBUTTONSIZE + // and not after it, as might be expected: otherwise, the button size + // remains unchanged (doing TB_AUTOSIZE later doesn't seem to do any + // harm but doesn't change the button size neither). + ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); + + ::SendMessage(GetHwnd(), TB_SETBUTTONSIZE, + 0, MAKELPARAM(toolSize.x, toolSize.y)); + } + + // Adjust controls and stretchable spaces // -------------------------------------- @@ -1243,58 +1256,52 @@ bool wxToolBar::Realize() // good and wxGTK doesn't do it neither (and the code below can't // deal with this case) control->Hide(); + if ( wxStaticText * const staticText = tool->GetStaticText() ) + staticText->Hide(); continue; } control->Show(); - wxStaticText * const staticText = tool->GetStaticText(); - wxSize size = control->GetSize(); - wxSize staticTextSize; - if ( staticText && staticText->IsShown() ) - { - staticTextSize = staticText->GetSize(); - staticTextSize.y += MARGIN_CONTROL_LABEL; - } - - // position the control itself correctly vertically centering it on the - // icon area of the toolbar - int height = r.bottom - r.top - staticTextSize.y; - - int diff = height - size.y; - if ( diff < 0 || !HasFlag(wxTB_TEXT) ) - { - // not enough room for the static text - if ( staticText ) - staticText->Hide(); - - // recalculate height & diff without the staticText control - height = r.bottom - r.top; - diff = height - size.y; - if ( diff < 0 ) - { - // the control is too high, resize to fit - // Actually don't set the size, otherwise we can never fit - // the toolbar around the controls. - // control->SetSize(wxDefaultCoord, height - 2); - - diff = 2; - } - } - else // enough space for both the control and the label - { - if ( staticText ) - staticText->Show(); - } + const wxSize controlSize = control->GetSize(); // Take also into account tool padding value. - control->Move(r.left + m_toolPacking/2, r.top + (diff + 1) / 2); - if ( staticText ) + const int x = r.left + m_toolPacking/2; + const int height = r.bottom - r.top; + + // Greater of control and its label widths. + int totalWidth = controlSize.x; + + // Height of control and its label, if any, including the margin + // between them. + int totalHeight = controlSize.y; + + if ( wxStaticText * const staticText = tool->GetStaticText() ) { - staticText->Move(r.left + m_toolPacking/2 + (size.x - staticTextSize.x)/2, - r.bottom - staticTextSize.y); + const bool shown = AreControlLabelsShown(); + staticText->Show(shown); + + if ( shown ) + { + const wxSize staticTextSize = staticText->GetSize(); + + if ( staticTextSize.x > totalWidth ) + totalWidth = staticTextSize.x; + + // Center the static text horizontally for consistency with the + // button labels and position it below the control vertically. + staticText->Move(x + (totalWidth - staticTextSize.x)/2, + r.top + (height + controlSize.y + - staticTextSize.y + + MARGIN_CONTROL_LABEL)/2); + + totalHeight += staticTextSize.y + MARGIN_CONTROL_LABEL; + } } + control->Move(x + (totalWidth - controlSize.x)/2, + r.top + (height - totalHeight)/2); + m_totalFixedSize += r.right - r.left; } @@ -1638,14 +1645,25 @@ void wxToolBar::SetRows(int nRows) const bool enable = (!IsVertical() && m_maxRows == 1) || (IsVertical() && (size_t)m_maxRows == m_nButtons); - const LPARAM state = MAKELONG(enable ? TBSTATE_ENABLED : TBSTATE_HIDDEN, 0); + LPARAM state = enable ? TBSTATE_ENABLED : TBSTATE_HIDDEN; + + if ( IsVertical() ) + { + // As in Realize(), ensure that TBSTATE_WRAP is used for all the + // tools, including separators, in vertical toolbar, and here it does + // make a difference: without it, the following tools wouldn't be + // visible because they would be on the same row as the separator. + state |= TBSTATE_WRAP; + } + wxToolBarToolsList::compatibility_iterator node; for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) { wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); if ( tool->IsStretchableSpace() ) { - if ( !::SendMessage(GetHwnd(), TB_SETSTATE, tool->GetId(), state) ) + if ( !::SendMessage(GetHwnd(), TB_SETSTATE, + tool->GetId(), MAKELONG(state, 0)) ) { wxLogLastError(wxT("TB_SETSTATE (stretchable spacer)")); } @@ -1683,6 +1701,40 @@ void wxToolBar::UpdateSize() { wxPoint pos = GetPosition(); ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); + + // TB_AUTOSIZE doesn't seem to work for vertical toolbars which it resizes + // to have the same width as a horizontal toolbar would have, even though + // we do use TBSTATE_WRAP which should make it start a new row after each + // tool. So override its width determination explicitly. + if ( IsVertical() ) + { + // Find the widest tool in the toolbar. + int maxWidth = 0; + + int i = 0; + for ( wxToolBarToolsList::compatibility_iterator node = m_tools.GetFirst(); + node; + node = node->GetNext(), ++i ) + { + wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); + if ( tool->IsSeparator() ) + { + // Somehow, separators get resized to have huge width, which + // probably explains why TB_AUTOSIZE doesn't work correctly for + // us in the first place. Because of this, don't take them into + // account: they can't be wider than any normal button, anyhow. + continue; + } + + const RECT rc = wxGetTBItemRect(GetHwnd(), i); + const int width = rc.right - rc.left; + if ( width > maxWidth ) + maxWidth = width; + } + + SetSize(maxWidth, GetSize().y); + } + if (pos != GetPosition()) Move(pos); @@ -1867,115 +1919,12 @@ void wxToolBar::OnEraseBackground(wxEraseEvent& event) #endif // wxHAS_MSW_BACKGROUND_ERASE_HOOK } -bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM lParam) +bool wxToolBar::HandleSize(WXWPARAM WXUNUSED(wParam), WXLPARAM WXUNUSED(lParam)) { // wait until we have some tools if ( !GetToolsCount() ) return false; - // calculate our minor dimension ourselves - we're confusing the standard - // logic (TB_AUTOSIZE) with our horizontal toolbars and other hacks - // Find bounding box for all rows. - RECT r; - ::SetRectEmpty(&r); - // Bounding box for single (current) row - RECT rcRow; - ::SetRectEmpty(&rcRow); - int rowPosX = INT_MIN; - wxToolBarToolsList::compatibility_iterator node; - int i = 0; - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(node->GetData()); - if ( tool->IsToBeDeleted() ) - continue; - - // Skip hidden buttons - const RECT rcItem = wxGetTBItemRect(GetHwnd(), i); - if ( ::IsRectEmpty(&rcItem) ) - { - i++; - continue; - } - - if ( rcItem.top > rowPosX ) - { - // We have the next row. - rowPosX = rcItem.top; - - // Shift origin to (0, 0) to make it the same as for the total rect. - ::OffsetRect(&rcRow, -rcRow.left, -rcRow.top); - - // And update the bounding box for all rows. - ::UnionRect(&r, &r, &rcRow); - - // Reset the current row bounding box for the next row. - ::SetRectEmpty(&rcRow); - } - - // Separators shouldn't be taken into account as they are sometimes - // reported to have the width of the entire client area by the toolbar. - // And we know that they are not the biggest items in the toolbar in - // any case, so just skip them. - if( !tool->IsSeparator() ) - { - // Update bounding box of current row - ::UnionRect(&rcRow, &rcRow, &rcItem); - } - - i++; - } - - // Take into account the last row rectangle too. - ::OffsetRect(&rcRow, -rcRow.left, -rcRow.top); - ::UnionRect(&r, &r, &rcRow); - - if ( !r.right ) - return false; - - int w, h; - - if ( IsVertical() ) - { - w = r.right - r.left; - h = HIWORD(lParam); - } - else - { - w = LOWORD(lParam); - if (HasFlag( wxTB_FLAT )) - h = r.bottom - r.top - 3; - else - h = r.bottom - r.top; - - // Take control height into account - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(node->GetData()); - if (tool->IsControl()) - { - int y = (tool->GetControl()->GetSize().y - 2); // -2 since otherwise control height + 4 (below) is too much - if (y > h) - h = y; - } - } - - if ( m_maxRows ) - { - // FIXME: hardcoded separator line height... - h += HasFlag(wxTB_NODIVIDER) ? 4 : 6; - h *= m_maxRows; - } - } - - if ( MAKELPARAM(w, h) != lParam ) - { - // size really changed - SetSize(w, h); - } - UpdateStretchableSpacersSize(); // message processed From 62b4974bf0b2cf38e00126586e6e9c45fb2c0929 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 22:12:43 +0100 Subject: [PATCH 08/19] Indentation fix in the toolbar sample No real changes. --- samples/toolbar/toolbar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/toolbar/toolbar.cpp b/samples/toolbar/toolbar.cpp index 63ba471f6d..e3745b9669 100644 --- a/samples/toolbar/toolbar.cpp +++ b/samples/toolbar/toolbar.cpp @@ -350,7 +350,7 @@ void MyFrame::RecreateToolbar() style |= wxTB_RIGHT; break; case TOOLBAR_BOTTOM: - style |= wxTB_BOTTOM; + style |= wxTB_BOTTOM; break; } From 23dee36ec3df4edb91e86f47bc4aef67516427d0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 22:13:54 +0100 Subject: [PATCH 09/19] Add accelerators for toolbar positioning menu items in the sample This makes it much more convenient to quickly test different toolbar orientations. --- samples/toolbar/toolbar.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/toolbar/toolbar.cpp b/samples/toolbar/toolbar.cpp index e3745b9669..f787d739d7 100644 --- a/samples/toolbar/toolbar.cpp +++ b/samples/toolbar/toolbar.cpp @@ -574,16 +574,16 @@ MyFrame::MyFrame(wxFrame* parent, tbarMenu->AppendSeparator(); tbarMenu->AppendRadioItem(IDM_TOOLBAR_TOP_ORIENTATION, - "Set toolbar at the top of the window", + "Set toolbar at the top of the window\tCtrl-Up", "Set toolbar at the top of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_LEFT_ORIENTATION, - "Set toolbar at the left of the window", + "Set toolbar at the left of the window\tCtrl-Left", "Set toolbar at the left of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_BOTTOM_ORIENTATION, - "Set toolbar at the bottom of the window", + "Set toolbar at the bottom of the window\tCtrl-Down", "Set toolbar at the bottom of the window"); tbarMenu->AppendRadioItem(IDM_TOOLBAR_RIGHT_ORIENTATION, - "Set toolbar at the right edge of the window", + "Set toolbar at the right edge of the window\tCtrl-Right", "Set toolbar at the right edge of the window"); tbarMenu->AppendSeparator(); From dc0586aad6fac787ab10e0d07f784ebd90e41ef9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 22:33:45 +0100 Subject: [PATCH 10/19] Remove a now unnecessary hack for vertical toolbars Don't recompute their total fixed size and update them once again in Realize() as the button heights seem to be just fine even without doing this. --- src/msw/toolbar.cpp | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 2cdf24f9b1..eb7f54b08e 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1325,29 +1325,6 @@ bool wxToolBar::Realize() InvalidateBestSize(); UpdateSize(); - if ( IsVertical() ) - { - // For vertical toolbar heights of buttons are incorrect - // unless TB_AUTOSIZE in invoked. - // We need to recalculate fixed elements size again. - m_totalFixedSize = 0; - toolIndex = 0; - for ( node = m_tools.GetFirst(); node; node = node->GetNext(), toolIndex++ ) - { - wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); - if ( !tool->IsStretchableSpace() ) - { - const RECT r = wxGetTBItemRect(GetHwnd(), toolIndex); - if ( !IsVertical() ) - m_totalFixedSize += r.right - r.left; - else if ( !tool->IsControl() ) - m_totalFixedSize += r.bottom - r.top; - } - } - // Enforce invoking UpdateStretchableSpacersSize() with correct value of fixed elements size. - UpdateSize(); - } - return true; } From 385ebf2f4a485787602197928b4e31ed48697b54 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Feb 2019 22:34:32 +0100 Subject: [PATCH 11/19] Don't call UpdateSize() unnecessarily at the end of Realize() SetRows(), called just above in most case, already calls UpdateSize(), so there is no need to do it again. --- src/msw/toolbar.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index eb7f54b08e..e868bb6668 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1312,8 +1312,16 @@ bool wxToolBar::Realize() if ( !IsVertical() ) { if ( m_maxRows == 0 ) + { // if not set yet, only one row SetRows(1); + } + else + { + // In all the other cases, UpdateSize() is called by SetRows(), but + // when we don't call it here, call it directly instead. + UpdateSize(); + } } else if ( m_nButtons > 0 ) // vertical non empty toolbar { @@ -1323,7 +1331,6 @@ bool wxToolBar::Realize() } InvalidateBestSize(); - UpdateSize(); return true; } From 1661f277d546e816fda51f98394f1c9bb58cc285 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 28 Feb 2019 23:23:15 +0100 Subject: [PATCH 12/19] Only use packing in horizontal direction for toolbar controls Tool packing seems to be relevant only in the major toolbar direction, i.e. horizontally when the controls are shown, and adding it in vertical direction too made the toolbar too tall. --- src/msw/toolbar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index e868bb6668..a9e436f25d 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -540,7 +540,7 @@ wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const } // Also account for the tool padding value. - size += wxSize(m_toolPacking, m_toolPacking); + size.x += m_toolPacking; return size; } From fa16db08c1b371645d46b3938977a2ffa9c12496 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 28 Feb 2019 23:24:40 +0100 Subject: [PATCH 13/19] Remove the extra height hack from wxToolBar::DoGetBestSize() There doesn't seem to be any good reason to add 3px to the vertical component of the toolbar best size and it's not clear how can the original problem be reproduced. This basically reverts c118d8b06e867235a050add7cf89564923de3387. --- src/msw/toolbar.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index a9e436f25d..7654ab8106 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -597,14 +597,6 @@ wxSize wxToolBar::DoGetBestSize() const sizeBest.IncTo(wxSize(-1, MSWGetFittingtSizeForControl(tool).y)); } } - - // Without the extra height, DoGetBestSize can report a size that's - // smaller than the actual window, causing windows to overlap slightly - // in some circumstances, leading to missing borders (especially noticeable - // in AUI layouts). - if (!(GetWindowStyle() & wxTB_NODIVIDER)) - sizeBest.y += 2; - sizeBest.y ++; } return sizeBest; From acfd7147439d44d7133d89a9040cc8616d4df032 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 28 Feb 2019 23:31:49 +0100 Subject: [PATCH 14/19] Make toolbar height compatible with previous wx version Restore the hack with making the toolbars with wxTB_FLAT style 3px less tall than the value given to them by TB_AUTOSIZE. There is no real explanation for doing this, this adjustment was added by 98b9643647468b3670c87e2d15dcba5c0fe32d11 back in 2002 with the rationale "make flat toolbars look slightly better", but after doing it for 17 years during which nobody complained about this, we probably should keep doing it now. --- src/msw/toolbar.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 7654ab8106..63b1b44c79 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -1678,6 +1678,8 @@ void wxToolBar::UpdateSize() wxPoint pos = GetPosition(); ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); + wxSize size = GetSize(); + // TB_AUTOSIZE doesn't seem to work for vertical toolbars which it resizes // to have the same width as a horizontal toolbar would have, even though // we do use TBSTATE_WRAP which should make it start a new row after each @@ -1708,11 +1710,19 @@ void wxToolBar::UpdateSize() maxWidth = width; } - SetSize(maxWidth, GetSize().y); + size.x = maxWidth; + } + else // horizontal + { + // For (flat) horizontal toolbars we used to make the toolbar 3px less + // tall in the previous wx versions, for some reason. It's not obvious + // at all if this is the right thing to do, but continue doing it now + // for compatibility. + if ( HasFlag(wxTB_FLAT) ) + size.y -= 3; } - if (pos != GetPosition()) - Move(pos); + SetSize(wxRect(pos, size)); // In case Realize is called after the initial display (IOW the programmer // may have rebuilt the toolbar) give the frame the option of resizing the From b8a5276bc8769248e3d5da9e5b391ece6b63dd46 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 28 Feb 2019 23:50:41 +0100 Subject: [PATCH 15/19] Use a symbolic constant for toolbar height adjustment hack value No real changes, just avoid hard-coding this 3 as it's going to be used in another place in the upcoming commit. --- src/msw/toolbar.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 63b1b44c79..2076c97e53 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -97,6 +97,13 @@ // Margin between the control and its label. static const int MARGIN_CONTROL_LABEL = 3; +// We make (flat-only) toolbars smaller by this amount than the value that +// would be given to them by TB_AUTOSIZE. This is done for cosmetic reasons and +// also to decrease the vertical space consumed by the toolbar and, finally and +// perhaps most significantly, for compatibility, as people dislike their +// toolbars changing height when updating to a new wxWidgets version. +static const int AUTOSIZE_HEIGHT_ADJUSTMENT = 3; + // ---------------------------------------------------------------------------- // wxWin macros // ---------------------------------------------------------------------------- @@ -1719,7 +1726,7 @@ void wxToolBar::UpdateSize() // at all if this is the right thing to do, but continue doing it now // for compatibility. if ( HasFlag(wxTB_FLAT) ) - size.y -= 3; + size.y -= AUTOSIZE_HEIGHT_ADJUSTMENT; } SetSize(wxRect(pos, size)); From 50bbfedb314cf2b5280eaeffb075c7fe7549fafd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 9 Mar 2019 16:17:53 +0100 Subject: [PATCH 16/19] Rewrite wxToolBar sizing code to avoid TB_AUTOSIZE This message just seems too weird and unreliable, so get rid of it and compute the toolbar size entirely on our own, which at the very least gives predictable and reproducible results and makes GetSize() consistent with GetBestSize(). The toolbar height doesn't remain exactly the same as before, with 1px differences here and there, but now the height is the same initially and after changing the toolbar styles, while previously the height changed when doing this. --- src/msw/toolbar.cpp | 198 +++++++++++++++++--------------------------- 1 file changed, 76 insertions(+), 122 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 2076c97e53..fac4b9f28f 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -97,13 +97,6 @@ // Margin between the control and its label. static const int MARGIN_CONTROL_LABEL = 3; -// We make (flat-only) toolbars smaller by this amount than the value that -// would be given to them by TB_AUTOSIZE. This is done for cosmetic reasons and -// also to decrease the vertical space consumed by the toolbar and, finally and -// perhaps most significantly, for compatibility, as people dislike their -// toolbars changing height when updating to a new wxWidgets version. -static const int AUTOSIZE_HEIGHT_ADJUSTMENT = 3; - // ---------------------------------------------------------------------------- // wxWin macros // ---------------------------------------------------------------------------- @@ -475,6 +468,14 @@ void wxToolBar::Recreate() const wxPoint pos = GetPosition(); const wxSize size = GetSize(); + // Note that MSWCreateToolbar() will set the current size as the initial + // and minimal size of the toolbar, which is unwanted both because it loses + // any actual min size set from user code and because even if SetMinSize() + // is never called, we're going to be stuck with the bigger than necessary + // min size when we're switching from text+icons to text or icons-only + // modes, so preserve the current min size here. + const wxSize minSizeOrig = GetMinSize(); + UnsubclassWin(); if ( !MSWCreateToolbar(pos, size) ) @@ -485,6 +486,8 @@ void wxToolBar::Recreate() return; } + SetMinSize(minSizeOrig); + // reparent all our children under the new toolbar for ( wxWindowList::compatibility_iterator node = m_children.GetFirst(); node; @@ -531,6 +534,10 @@ wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const { wxSize size = tool->GetControl()->GetBestSize(); + // This is arbitrary, but we want to leave at least 1px around the control + // vertically, otherwise it really looks too cramped. + size.y += 2*1; + // Account for the label, if any. if ( wxStaticText * const staticText = tool->GetStaticText() ) { @@ -554,56 +561,62 @@ wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const wxSize wxToolBar::DoGetBestSize() const { + const wxSize sizeTool = GetToolSize(); + wxSize sizeBest; - - SIZE size; - if ( !::SendMessage(GetHwnd(), TB_GETMAXSIZE, 0, (LPARAM)&size) ) + if ( IsVertical() ) { - // maybe an old (< 0x400) Windows version? try to approximate the - // toolbar size ourselves - sizeBest = GetToolSize(); - sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); // Add borders - sizeBest.x *= GetToolsCount(); + sizeBest.x = sizeTool.x + 2 * ::GetSystemMetrics(SM_CXBORDER); + } + else + { + sizeBest.y = sizeTool.y + 2 * ::GetSystemMetrics(SM_CYBORDER); + } + + wxToolBarToolsList::compatibility_iterator node; + for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) + { + wxToolBarTool * const + tool = static_cast(node->GetData()); - // reverse horz and vertical components if necessary if ( IsVertical() ) { - wxSwap(sizeBest.x, sizeBest.y); - } - } - else // TB_GETMAXSIZE succeeded - { - // but it could still return an incorrect result due to what appears to - // be a bug in old comctl32.dll versions which don't handle controls in - // the toolbar correctly, so work around it (see SF patch 1902358) - if ( !IsVertical() && wxApp::GetComCtl32Version() < 600 ) - { - // calculate the toolbar width in alternative way - const RECT rcFirst = wxGetTBItemRect(GetHwnd(), 0); - const RECT rcLast = wxGetTBItemRect(GetHwnd(), GetToolsCount() - 1); - - const int widthAlt = rcLast.right - rcFirst.left; - if ( widthAlt > size.cx ) - size.cx = widthAlt; - } - - sizeBest.x = size.cx; - sizeBest.y = size.cy; - } - - if ( !IsVertical() ) - { - wxToolBarToolsList::compatibility_iterator node; - for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) - { - wxToolBarTool * const - tool = static_cast(node->GetData()); - if (tool->IsControl()) + if ( !tool->IsControl() ) { - // Ensure we're tall enough for the embedded controls. - sizeBest.IncTo(wxSize(-1, MSWGetFittingtSizeForControl(tool).y)); + sizeBest.y += sizeTool.y + m_toolPacking; } + //else: Controls are not shown in vertical toolbars at all. } + else + { + if ( tool->IsControl() ) + { + const wxSize sizeControl = MSWGetFittingtSizeForControl(tool); + + // Ensure we're tall enough for the embedded controls. + sizeBest.IncTo(wxSize(-1, sizeControl.y)); + + sizeBest.x += sizeControl.x; + } + else + { + sizeBest.x += sizeTool.x; + } + + sizeBest.x += m_toolPacking; + } + } + + // Note that this needs to be done after the loop to account for controls + // too high to fit into the toolbar without the border size but that could + // fit if we had added the border beforehand. + if ( IsVertical() ) + { + sizeBest.x += 2 * ::GetSystemMetrics(SM_CXBORDER); + } + else + { + sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); } return sizeBest; @@ -750,6 +763,8 @@ bool wxToolBar::Realize() if ( !wxToolBarBase::Realize() ) return false; + InvalidateBestSize(); + const size_t nTools = GetToolsCount(); // don't change the values of these constants, they can be set from the @@ -1046,7 +1061,6 @@ bool wxToolBar::Realize() // this array will hold the indices of all controls in the toolbar wxArrayInt controlIds; - int minReqHeight = 0; bool lastWasRadio = false; int i = 0; for ( node = m_tools.GetFirst(); node; node = node->GetNext() ) @@ -1063,10 +1077,7 @@ bool wxToolBar::Realize() case wxTOOL_STYLE_CONTROL: if ( !IsVertical() ) { - const wxSize size = MSWGetFittingtSizeForControl(tool); - if ( minReqHeight < size.y ) - minReqHeight = size.y; - button.iBitmap = size.x; + button.iBitmap = MSWGetFittingtSizeForControl(tool).x; } wxFALLTHROUGH; @@ -1202,27 +1213,14 @@ bool wxToolBar::Realize() } - // Make sure the toolbar is tall enough to fit the embedded controls, which - // may be taller than the buttons. - wxSize toolSize = GetToolSize(); - if ( toolSize.y < minReqHeight ) - { - toolSize.y = minReqHeight; - - // Surprisingly, we need to send TB_AUTOSIZE before TB_SETBUTTONSIZE - // and not after it, as might be expected: otherwise, the button size - // remains unchanged (doing TB_AUTOSIZE later doesn't seem to do any - // harm but doesn't change the button size neither). - ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); - - ::SendMessage(GetHwnd(), TB_SETBUTTONSIZE, - 0, MAKELPARAM(toolSize.x, toolSize.y)); - } - - // Adjust controls and stretchable spaces // -------------------------------------- + // We don't trust the height returned by wxGetTBItemRect() as it may not + // have been updated yet, use the height that the toolbar will actually + // have instead. Also, account for 2px toolbar border here. + const int height = GetBestSize().y - 2 * ::GetSystemMetrics(SM_CYBORDER); + // adjust the controls size to fit nicely in the toolbar and compute its // total size while doing it m_totalFixedSize = 0; @@ -1266,7 +1264,6 @@ bool wxToolBar::Realize() // Take also into account tool padding value. const int x = r.left + m_toolPacking/2; - const int height = r.bottom - r.top; // Greater of control and its label widths. int totalWidth = controlSize.x; @@ -1329,8 +1326,6 @@ bool wxToolBar::Realize() SetRows(m_nButtons); } - InvalidateBestSize(); - return true; } @@ -1682,54 +1677,13 @@ wxToolBarToolBase *wxToolBar::FindToolForPosition(wxCoord x, wxCoord y) const void wxToolBar::UpdateSize() { - wxPoint pos = GetPosition(); - ::SendMessage(GetHwnd(), TB_AUTOSIZE, 0, 0); + // We used to use TB_AUTOSIZE here, but it didn't work at all for vertical + // toolbars and was more trouble than it was worth for horizontal one as it + // added some unwanted margins that we had to remove later. So now we just + // compute our own size and use it. + SetSize(GetBestSize()); - wxSize size = GetSize(); - - // TB_AUTOSIZE doesn't seem to work for vertical toolbars which it resizes - // to have the same width as a horizontal toolbar would have, even though - // we do use TBSTATE_WRAP which should make it start a new row after each - // tool. So override its width determination explicitly. - if ( IsVertical() ) - { - // Find the widest tool in the toolbar. - int maxWidth = 0; - - int i = 0; - for ( wxToolBarToolsList::compatibility_iterator node = m_tools.GetFirst(); - node; - node = node->GetNext(), ++i ) - { - wxToolBarTool * const tool = (wxToolBarTool*)node->GetData(); - if ( tool->IsSeparator() ) - { - // Somehow, separators get resized to have huge width, which - // probably explains why TB_AUTOSIZE doesn't work correctly for - // us in the first place. Because of this, don't take them into - // account: they can't be wider than any normal button, anyhow. - continue; - } - - const RECT rc = wxGetTBItemRect(GetHwnd(), i); - const int width = rc.right - rc.left; - if ( width > maxWidth ) - maxWidth = width; - } - - size.x = maxWidth; - } - else // horizontal - { - // For (flat) horizontal toolbars we used to make the toolbar 3px less - // tall in the previous wx versions, for some reason. It's not obvious - // at all if this is the right thing to do, but continue doing it now - // for compatibility. - if ( HasFlag(wxTB_FLAT) ) - size.y -= AUTOSIZE_HEIGHT_ADJUSTMENT; - } - - SetSize(wxRect(pos, size)); + UpdateStretchableSpacersSize(); // In case Realize is called after the initial display (IOW the programmer // may have rebuilt the toolbar) give the frame the option of resizing the From 07673fdb97f82e22962c10a11a69c7685e5f6767 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Mar 2019 04:45:49 +0100 Subject: [PATCH 17/19] Fix toolbar size calculations in wxTB_NODIVIDER case There is no 2px border separating the toolbar from the rest of the window in this case, so don't overcompensate by accounting for it. --- src/msw/toolbar.cpp | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index fac4b9f28f..1c792f209e 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -610,13 +610,16 @@ wxSize wxToolBar::DoGetBestSize() const // Note that this needs to be done after the loop to account for controls // too high to fit into the toolbar without the border size but that could // fit if we had added the border beforehand. - if ( IsVertical() ) + if ( !HasFlag(wxTB_NODIVIDER) ) { - sizeBest.x += 2 * ::GetSystemMetrics(SM_CXBORDER); - } - else - { - sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); + if ( IsVertical() ) + { + sizeBest.x += 2 * ::GetSystemMetrics(SM_CXBORDER); + } + else + { + sizeBest.y += 2 * ::GetSystemMetrics(SM_CYBORDER); + } } return sizeBest; @@ -1218,8 +1221,14 @@ bool wxToolBar::Realize() // We don't trust the height returned by wxGetTBItemRect() as it may not // have been updated yet, use the height that the toolbar will actually - // have instead. Also, account for 2px toolbar border here. - const int height = GetBestSize().y - 2 * ::GetSystemMetrics(SM_CYBORDER); + // have instead. + int height = GetBestSize().y; + if ( !HasFlag(wxTB_NODIVIDER) ) + { + // We want just the usable height, so remove the space taken by the + // border/divider. + height -= 2 * ::GetSystemMetrics(SM_CYBORDER); + } // adjust the controls size to fit nicely in the toolbar and compute its // total size while doing it From d24f6f79164699be21823d69f499c41a152be5c6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Mar 2019 14:20:48 +0100 Subject: [PATCH 18/19] Reduce the margins around toolbar controls by half This wastes less space in the toolbar and looks better and is more compatible with 3.0 (although not totally so, but this is intentional as 3.0 didn't use any margins at all, which looked bad). --- src/msw/toolbar.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/msw/toolbar.cpp b/src/msw/toolbar.cpp index 1c792f209e..5044242339 100644 --- a/src/msw/toolbar.cpp +++ b/src/msw/toolbar.cpp @@ -553,8 +553,11 @@ wxSize wxToolBar::MSWGetFittingtSizeForControl(wxToolBarTool* tool) const } } - // Also account for the tool padding value. - size.x += m_toolPacking; + // Also account for the tool padding value: note that we only have to add + // half of it to each tool, as the total amount of packing is the sum of + // the right margin of the previous tool and the left margin of the next + // one. + size.x += m_toolPacking / 2; return size; } @@ -603,7 +606,9 @@ wxSize wxToolBar::DoGetBestSize() const sizeBest.x += sizeTool.x; } - sizeBest.x += m_toolPacking; + // As explained in MSWGetFittingtSizeForControl() above, the actual + // margin used for a single tool is one half of the total packing. + sizeBest.x += m_toolPacking / 2; } } @@ -1271,8 +1276,10 @@ bool wxToolBar::Realize() const wxSize controlSize = control->GetSize(); - // Take also into account tool padding value. - const int x = r.left + m_toolPacking/2; + // Take also into account tool padding value: the amount of padding + // used for each tool is half of m_toolPacking, so the margin on each + // side is a half of that. + const int x = r.left + m_toolPacking / 4; // Greater of control and its label widths. int totalWidth = controlSize.x; From 6ad5aee8ac663b86d3bafdf86136524cca7e5f3f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Mar 2019 14:27:53 +0100 Subject: [PATCH 19/19] Document the change in wxMSW wxToolBar height determination Since 0185d61a2c6f91cf47932fce0d68d70b7e578b17 wxToolBar height is increased if the controls don't fit in it rather than decreasing the size of the controls, which results in different appearance than in the previous versions, so document this as well as the advice for restoring the old behaviour. --- docs/changes.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changes.txt b/docs/changes.txt index 3a288f2ca3..af29fa7cf9 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -65,6 +65,11 @@ Changes in behaviour not resulting in compilation errors - wxGTK wxNotebook::AddPage() doesn't generate any events any more for the first page being added, for consistency with the other ports. +- wxMSW wxToolBar height now adapts to the height of embedded controls, making + the toolbar taller if necessary, rather than making the controls smaller. To + return to the previous behaviour, you need to explicitly create controls of + smaller size. + Changes in behaviour which may result in build errors -----------------------------------------------------