From 33cda12b74fe5a1eb0a574b6ec3b30bf544ef565 Mon Sep 17 00:00:00 2001 From: PB Date: Tue, 5 May 2020 20:22:11 +0200 Subject: [PATCH 1/4] Handle wxToolBars created with wxTB_HORIZONTAL | wxTB_BOTTOM style correctly wxToolBar created with wxTB_HORIZONTAL | wxTB_BOTTOM had two issues: (1) There was a toolbar-high gap below the menu bar. (2) The status bar was not positioned correctly. Fix both issues by taking into account that wxTB_TOP is an alias for wxTB_HORIZONTAL and so a wxToolBar can have quite counter-intuitively set both wxTB_TOP and wxTB_BOTTOM. In such cases it means that the toolbar is horizontal and on the frame bottom, this needs to be accounted for when computing the origin of the frame client area as well as the status bar position. Closes #18760 --- src/msw/frame.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/msw/frame.cpp b/src/msw/frame.cpp index fe8c35fce7..9b957056be 100644 --- a/src/msw/frame.cpp +++ b/src/msw/frame.cpp @@ -315,7 +315,7 @@ void wxFrame::PositionStatusBar() int x = 0; #if wxUSE_TOOLBAR wxToolBar * const toolbar = GetToolBar(); - if ( toolbar && !toolbar->HasFlag(wxTB_TOP) ) + if ( toolbar ) { const wxSize sizeTB = toolbar->GetSize(); @@ -326,13 +326,14 @@ void wxFrame::PositionStatusBar() w += sizeTB.x; } - else // wxTB_BOTTOM + else + if ( toolbar->HasFlag(wxTB_BOTTOM) ) { // we need to position the status bar below the toolbar h += sizeTB.y; } + //else: no adjustments necessary for the toolbar on top } - //else: no adjustments necessary for the toolbar on top #endif // wxUSE_TOOLBAR // GetSize returns the height of the clientSize in which the statusbar @@ -933,7 +934,12 @@ wxPoint wxFrame::GetClientAreaOrigin() const { const wxSize sizeTB = toolbar->GetSize(); - if ( toolbar->HasFlag(wxTB_TOP) ) + // wxTB_TOP has the same value as wxTB_HORIZONTAL so HasFlag(wxTB_TOP) + // returns true for a toolbar created with wxTB_HORIZONTAL | wxTB_BOTTOM + // style despite the toolbar being on the frame bottom and not affecting + // the client area origin. We therefore need to check here not only for + // presence of wxTB_TOP but also for absence of wxTB_BOTTOM. + if ( toolbar->HasFlag(wxTB_TOP) && !toolbar->HasFlag(wxTB_BOTTOM) ) { pt.y += sizeTB.y; } From 101f1354451c42ba906e6e5cecde8d5cfc031bf7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 6 May 2020 02:09:34 +0200 Subject: [PATCH 2/4] Fix problem with wxTB_VERTICAL | wxTB_RIGHT too This is similar to the previous commit, but for the other direction. --- src/msw/frame.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/msw/frame.cpp b/src/msw/frame.cpp index 9b957056be..c77d8335e8 100644 --- a/src/msw/frame.cpp +++ b/src/msw/frame.cpp @@ -321,13 +321,12 @@ void wxFrame::PositionStatusBar() if ( toolbar->HasFlag(wxTB_LEFT | wxTB_RIGHT) ) { - if ( toolbar->HasFlag(wxTB_LEFT) ) + if ( !toolbar->HasFlag(wxTB_RIGHT) ) x -= sizeTB.x; w += sizeTB.x; } - else - if ( toolbar->HasFlag(wxTB_BOTTOM) ) + else if ( toolbar->HasFlag(wxTB_BOTTOM) ) { // we need to position the status bar below the toolbar h += sizeTB.y; @@ -943,7 +942,7 @@ wxPoint wxFrame::GetClientAreaOrigin() const { pt.y += sizeTB.y; } - else if ( toolbar->HasFlag(wxTB_LEFT) ) + else if ( toolbar->HasFlag(wxTB_LEFT) && !toolbar->HasFlag(wxTB_RIGHT) ) { pt.x += sizeTB.x; } From 46dfc54e8dcbe07f348e8d5f4e0fc0c4fe154a61 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 6 May 2020 02:14:02 +0200 Subject: [PATCH 3/4] Generalize changes of the previous commits to wxMac too Handle "impossible" flag combinations such as wxTB_LEFT|wxTB_RIGHT and wxTB_TOP|wxTB_BOTTOM that are actually possible under Mac too. --- src/osx/carbon/frame.cpp | 25 +++++++++++++++---------- src/osx/cocoa/toolbar.mm | 10 +++++++--- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/osx/carbon/frame.cpp b/src/osx/carbon/frame.cpp index d0033a3d5d..5d6a0bd47d 100644 --- a/src/osx/carbon/frame.cpp +++ b/src/osx/carbon/frame.cpp @@ -62,11 +62,11 @@ wxPoint wxFrame::GetClientAreaOrigin() const int w, h; toolbar->GetSize(&w, &h); - if ( toolbar->HasFlag(wxTB_LEFT) ) + if ( toolbar->HasFlag(wxTB_LEFT) && !toolbar->HasFlag(wxTB_RIGHT) ) { pt.x += w; } - else if ( toolbar->HasFlag(wxTB_TOP) ) + else if ( toolbar->HasFlag(wxTB_TOP) && !toolbar->HasFlag(wxTB_BOTTOM) ) { #if !wxOSX_USE_NATIVE_TOOLBAR pt.y += h; @@ -342,14 +342,12 @@ void wxFrame::PositionToolBar() tx = ty = 0 ; GetToolBar()->GetSize(&tw, &th); - if (GetToolBar()->HasFlag(wxTB_LEFT)) - { - // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS - // means, pretend we don't have toolbar/status bar, so we - // have the original client size. - GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); - } - else if (GetToolBar()->HasFlag(wxTB_RIGHT)) + + // Note: we need to test for wxTB_RIGHT before wxTB_LEFT, as the latter + // is the same as wxTB_VERTICAL and it's possible to combine wxTB_RIGHT + // with wxTB_VERTICAL, so testing for wxTB_LEFT could be true for a + // right-aligned toolbar (while the reverse is, luckily, impossible). + if (GetToolBar()->HasFlag(wxTB_RIGHT)) { // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS // means, pretend we don't have toolbar/status bar, so we @@ -357,6 +355,13 @@ void wxFrame::PositionToolBar() tx = cw - tw; GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); } + else if (GetToolBar()->HasFlag(wxTB_LEFT)) + { + // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS + // means, pretend we don't have toolbar/status bar, so we + // have the original client size. + GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); + } else if (GetToolBar()->HasFlag(wxTB_BOTTOM)) { tx = 0; diff --git a/src/osx/cocoa/toolbar.mm b/src/osx/cocoa/toolbar.mm index a0bb728b03..89bbbf200e 100644 --- a/src/osx/cocoa/toolbar.mm +++ b/src/osx/cocoa/toolbar.mm @@ -1688,10 +1688,14 @@ void wxToolBar::OnPaint(wxPaintEvent& event) } dc.SetPen( wxPen( wxColour( 0x51,0x51,0x51 ) ) ); - if ( HasFlag(wxTB_LEFT) ) - dc.DrawLine(w-1, 0, w-1, h); - else if ( HasFlag(wxTB_RIGHT) ) + + // Note that testing this is in the "reverse" order, i.e. right before + // left and bottom before top because these flags can actually be + // combined because left/top are the same as vertical/horizontal + if ( HasFlag(wxTB_RIGHT) ) dc.DrawLine(0, 0, 0, h); + else if ( HasFlag(wxTB_LEFT) ) + dc.DrawLine(w-1, 0, w-1, h); else if ( HasFlag(wxTB_BOTTOM) ) dc.DrawLine(0, 0, w, 0); else if ( HasFlag(wxTB_TOP) ) From 13b15fecc075d678cccce911e7b78c59dfd0dc6e Mon Sep 17 00:00:00 2001 From: PB Date: Tue, 26 May 2020 23:43:54 +0200 Subject: [PATCH 4/4] Simplify code dealing with toolbar position in associated frame --- include/wx/tbarbase.h | 4 ++++ src/common/tbarbase.cpp | 18 ++++++++++++++++++ src/msw/frame.cpp | 17 +++++++---------- src/osx/carbon/frame.cpp | 28 +++++++++++++--------------- src/osx/cocoa/toolbar.mm | 12 +++++------- 5 files changed, 47 insertions(+), 32 deletions(-) diff --git a/include/wx/tbarbase.h b/include/wx/tbarbase.h index 35d1700841..6d1b423015 100644 --- a/include/wx/tbarbase.h +++ b/include/wx/tbarbase.h @@ -482,6 +482,10 @@ public: // return true if this is a vertical toolbar, otherwise false bool IsVertical() const; + // returns one of wxTB_TOP, wxTB_BOTTOM, wxTB_LEFT, wxTB_RIGHT + // indicating where the toolbar is placed in the associated frame + int GetDirection() const; + // these methods allow to access tools by their index in the toolbar size_t GetToolsCount() const { return m_tools.GetCount(); } wxToolBarToolBase *GetToolByPos(int pos) { return m_tools[pos]; } diff --git a/src/common/tbarbase.cpp b/src/common/tbarbase.cpp index 5272c298f3..3c2a6e99a7 100644 --- a/src/common/tbarbase.cpp +++ b/src/common/tbarbase.cpp @@ -628,6 +628,24 @@ bool wxToolBarBase::IsVertical() const return HasFlag(wxTB_LEFT | wxTB_RIGHT); } +// wxTB_HORIZONTAL is same as wxTB_TOP and wxTB_VERTICAL is same as wxTB_LEFT, +// so a toolbar created with wxTB_HORIZONTAL | wxTB_BOTTOM style can have set both +// wxTB_TOP and wxTB_BOTTOM, similarly wxTB_VERTICAL | wxTB_RIGHT == wxTB_LEFT | wxTB_RIGHT. +// GetDirection() makes things less confusing and returns just one of wxTB_TOP, wxTB_BOTTOM, +// wxTB_LEFT, wxTB_RIGHT indicating where the toolbar is placed in the associated frame. +int wxToolBarBase::GetDirection() const +{ + if ( HasFlag(wxTB_BOTTOM) ) + return wxTB_BOTTOM; + + if ( HasFlag(wxTB_RIGHT) ) + return wxTB_RIGHT; + + if ( HasFlag(wxTB_LEFT) ) + return wxTB_LEFT; + + return wxTB_TOP; +} // ---------------------------------------------------------------------------- // event processing diff --git a/src/msw/frame.cpp b/src/msw/frame.cpp index c77d8335e8..90c88702f4 100644 --- a/src/msw/frame.cpp +++ b/src/msw/frame.cpp @@ -318,15 +318,16 @@ void wxFrame::PositionStatusBar() if ( toolbar ) { const wxSize sizeTB = toolbar->GetSize(); + const int directionTB = toolbar->GetDirection(); - if ( toolbar->HasFlag(wxTB_LEFT | wxTB_RIGHT) ) + if ( toolbar->IsVertical() ) { - if ( !toolbar->HasFlag(wxTB_RIGHT) ) + if ( directionTB == wxTB_LEFT ) x -= sizeTB.x; w += sizeTB.x; } - else if ( toolbar->HasFlag(wxTB_BOTTOM) ) + else if ( directionTB == wxTB_BOTTOM ) { // we need to position the status bar below the toolbar h += sizeTB.y; @@ -932,17 +933,13 @@ wxPoint wxFrame::GetClientAreaOrigin() const if ( toolbar && toolbar->IsShown() ) { const wxSize sizeTB = toolbar->GetSize(); + const int directionTB = toolbar->GetDirection(); - // wxTB_TOP has the same value as wxTB_HORIZONTAL so HasFlag(wxTB_TOP) - // returns true for a toolbar created with wxTB_HORIZONTAL | wxTB_BOTTOM - // style despite the toolbar being on the frame bottom and not affecting - // the client area origin. We therefore need to check here not only for - // presence of wxTB_TOP but also for absence of wxTB_BOTTOM. - if ( toolbar->HasFlag(wxTB_TOP) && !toolbar->HasFlag(wxTB_BOTTOM) ) + if ( directionTB == wxTB_TOP ) { pt.y += sizeTB.y; } - else if ( toolbar->HasFlag(wxTB_LEFT) && !toolbar->HasFlag(wxTB_RIGHT) ) + else if ( directionTB == wxTB_LEFT ) { pt.x += sizeTB.x; } diff --git a/src/osx/carbon/frame.cpp b/src/osx/carbon/frame.cpp index 5d6a0bd47d..626157f6ea 100644 --- a/src/osx/carbon/frame.cpp +++ b/src/osx/carbon/frame.cpp @@ -59,14 +59,15 @@ wxPoint wxFrame::GetClientAreaOrigin() const wxToolBar *toolbar = GetToolBar(); if ( toolbar && toolbar->IsShown() ) { + const int direction = toolbar->GetDirection(); int w, h; toolbar->GetSize(&w, &h); - if ( toolbar->HasFlag(wxTB_LEFT) && !toolbar->HasFlag(wxTB_RIGHT) ) + if ( direction == wxTB_LEFT ) { pt.x += w; } - else if ( toolbar->HasFlag(wxTB_TOP) && !toolbar->HasFlag(wxTB_BOTTOM) ) + else if ( direction == wxTB_TOP ) { #if !wxOSX_USE_NATIVE_TOOLBAR pt.y += h; @@ -338,16 +339,20 @@ void wxFrame::PositionToolBar() if (GetToolBar()) { + const int direction = GetToolBar()->GetDirection(); int tx, ty, tw, th; tx = ty = 0 ; GetToolBar()->GetSize(&tw, &th); - // Note: we need to test for wxTB_RIGHT before wxTB_LEFT, as the latter - // is the same as wxTB_VERTICAL and it's possible to combine wxTB_RIGHT - // with wxTB_VERTICAL, so testing for wxTB_LEFT could be true for a - // right-aligned toolbar (while the reverse is, luckily, impossible). - if (GetToolBar()->HasFlag(wxTB_RIGHT)) + if (direction == wxTB_LEFT) + { + // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS + // means, pretend we don't have toolbar/status bar, so we + // have the original client size. + GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); + } + else if (direction == wxTB_RIGHT) { // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS // means, pretend we don't have toolbar/status bar, so we @@ -355,14 +360,7 @@ void wxFrame::PositionToolBar() tx = cw - tw; GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); } - else if (GetToolBar()->HasFlag(wxTB_LEFT)) - { - // Use the 'real' position. wxSIZE_NO_ADJUSTMENTS - // means, pretend we don't have toolbar/status bar, so we - // have the original client size. - GetToolBar()->SetSize(tx , ty , tw, ch , wxSIZE_NO_ADJUSTMENTS ); - } - else if (GetToolBar()->HasFlag(wxTB_BOTTOM)) + else if (direction == wxTB_BOTTOM) { tx = 0; ty = ch - th; diff --git a/src/osx/cocoa/toolbar.mm b/src/osx/cocoa/toolbar.mm index 89bbbf200e..d1d13bd333 100644 --- a/src/osx/cocoa/toolbar.mm +++ b/src/osx/cocoa/toolbar.mm @@ -1670,6 +1670,7 @@ void wxToolBar::OnPaint(wxPaintEvent& event) else #endif { + const int direction = GetDirection(); int w, h; GetSize( &w, &h ); @@ -1689,16 +1690,13 @@ void wxToolBar::OnPaint(wxPaintEvent& event) dc.SetPen( wxPen( wxColour( 0x51,0x51,0x51 ) ) ); - // Note that testing this is in the "reverse" order, i.e. right before - // left and bottom before top because these flags can actually be - // combined because left/top are the same as vertical/horizontal - if ( HasFlag(wxTB_RIGHT) ) + if ( direction == wxTB_RIGHT ) dc.DrawLine(0, 0, 0, h); - else if ( HasFlag(wxTB_LEFT) ) + else if ( direction == wxTB_LEFT ) dc.DrawLine(w-1, 0, w-1, h); - else if ( HasFlag(wxTB_BOTTOM) ) + else if ( direction == wxTB_BOTTOM ) dc.DrawLine(0, 0, w, 0); - else if ( HasFlag(wxTB_TOP) ) + else if ( direction == wxTB_TOP ) dc.DrawLine(0, h-1, w, h-1); } event.Skip();