diff --git a/docs/doxygen/overviews/high_dpi.md b/docs/doxygen/overviews/high_dpi.md index ffc6e8fed0..58960a1a09 100644 --- a/docs/doxygen/overviews/high_dpi.md +++ b/docs/doxygen/overviews/high_dpi.md @@ -299,7 +299,8 @@ programs involves doing at least the following: wxArtProvider::CreateBitmapBundle() (and, of course, return multiple bitmaps from it) instead of wxArtProvider::CreateBitmap(). 4. Removing any calls to wxToolBar::SetToolBitmapSize() or, equivalently, - `` attributes from the XRC, as this forces unwanted scaling. + `` attributes from the XRC, as this overrides the best size + determination and may result in suboptimal appearance. diff --git a/include/wx/bmpbndl.h b/include/wx/bmpbndl.h index 1b137e588a..dabbfa8d2d 100644 --- a/include/wx/bmpbndl.h +++ b/include/wx/bmpbndl.h @@ -152,11 +152,11 @@ public: // Implementation only from now on. // Get the bitmap size preferred by the majority of the elements of the - // bundles at the scale appropriate for the given scale. + // bundles at the given scale or the scale appropriate for the given window. static wxSize - GetConsensusSizeFor(wxWindow* win, - const wxVector& bundles, - const wxSize& sizeDefault); + GetConsensusSizeFor(double scale, const wxVector& bundles); + static wxSize + GetConsensusSizeFor(wxWindow* win, const wxVector& bundles); // Create wxImageList and fill it with the images from the given bundles in // the sizes appropriate for the DPI scaling used for the specified window. diff --git a/interface/wx/toolbar.h b/interface/wx/toolbar.h index c021c1ec56..d67c92b178 100644 --- a/interface/wx/toolbar.h +++ b/interface/wx/toolbar.h @@ -882,15 +882,15 @@ public: It is usually unnecessary to call this function, as the tools will always be made big enough to fit the size of the bitmaps used in them. - Moreover, calling it may force wxToolBar to scale its images, even - using non-integer scaling factor, which will usually look bad, instead - of adapting the image size to the current DPI scaling in order to avoid - doing this. + Moreover, calling it forces wxToolBar to scale its images in high DPI + using the provided size, instead of letting wxBitmapBundle used for the + tool bitmaps determine the best suitable bitmap size, which may result + in suboptimal appearance. If you do call it, it must be done before toolbar is Realize()'d. Example of using this function to force the bitmaps to be at least - 32 pixels wide and tall: + 32 pixels wide and tall (at normal DPI): @code toolbar->SetToolBitmapSize(FromDIP(wxSize(32, 32))); toolbar->AddTool(wxID_NEW, "New", wxBitmapBundle::FromXXX(...)); @@ -898,9 +898,6 @@ public: toolbar->Realize(); @endcode - Note that this example would scale bitmaps to 48 pixels when using 150% - DPI scaling, which wouldn't happen without calling SetToolBitmapSize(). - @param size The size of the bitmaps in the toolbar in logical pixels. diff --git a/src/common/bmpbndl.cpp b/src/common/bmpbndl.cpp index 3316c0b14e..a15b004b65 100644 --- a/src/common/bmpbndl.cpp +++ b/src/common/bmpbndl.cpp @@ -593,12 +593,16 @@ void RecordSizePref(SizePrefs& prefs, const wxSize& size) /* static */ wxSize wxBitmapBundle::GetConsensusSizeFor(wxWindow* win, - const wxVector& bundles, - const wxSize& sizeDefault) + const wxVector& bundles) { - const double scale = win->GetDPIScaleFactor(); - const wxSize sizeIdeal = sizeDefault*scale; + return GetConsensusSizeFor(win->GetDPIScaleFactor(), bundles); +} +/* static */ +wxSize +wxBitmapBundle::GetConsensusSizeFor(double scale, + const wxVector& bundles) +{ // We want to use preferred bitmap size, but the preferred sizes can be // different for different bitmap bundles, so record all their preferences // first. @@ -623,25 +627,13 @@ wxBitmapBundle::GetConsensusSizeFor(wxWindow* win, } else if ( countThis == countMax ) { - // We have a tie between different sizes, choose the one - // corresponding to the current scale factor, if possible, as this - // is the ideal bitmap size that should be consistent with all the - // other bitmaps. - if ( sizePreferred != sizeIdeal ) - { - if ( sizeThis == sizeIdeal ) - { - sizePreferred = sizeThis; - } - else // Neither of the sizes is the ideal one. - { - // Choose the larger one as like this some bitmaps will be - // downscaled, which should look better than upscaling some - // (other) ones. - if ( sizeThis.y > sizePreferred.y ) - sizePreferred = sizeThis; - } - } + // We have a tie between different sizes. + + // Choose the larger one as like this some bitmaps will be + // downscaled, which should look better than upscaling some + // (other) ones. + if ( sizeThis.y > sizePreferred.y ) + sizePreferred = sizeThis; } } @@ -656,12 +648,7 @@ wxBitmapBundle::CreateImageList(wxWindow* win, wxCHECK_MSG( win, NULL, "must have a valid window" ); wxCHECK_MSG( !bundles.empty(), NULL, "should have some images" ); - // We arbitrarily choose the default size of the first bundle as the - // default size for the image list too, as it's not clear what else could - // we do here. Note that this size is only used to break the tie in case - // the same number of bundles prefer two different sizes, so it's not going - // to matter at all in most cases. - wxSize size = GetConsensusSizeFor(win, bundles, bundles[0].GetDefaultSize()); + wxSize size = GetConsensusSizeFor(win, bundles); // wxImageList wants the logical size for the platforms where logical and // physical pixels are different. diff --git a/src/common/bmpcboxcmn.cpp b/src/common/bmpcboxcmn.cpp index db4e608e7d..73023ddd79 100644 --- a/src/common/bmpcboxcmn.cpp +++ b/src/common/bmpcboxcmn.cpp @@ -72,8 +72,7 @@ void wxBitmapComboBoxBase::UpdateInternals() m_usedImgSize = wxBitmapBundle::GetConsensusSizeFor ( GetControl(), - m_bitmapbundles, - wxSize(0, 0) + m_bitmapbundles ); } } diff --git a/src/common/tbarbase.cpp b/src/common/tbarbase.cpp index 1aaf478555..72b022812c 100644 --- a/src/common/tbarbase.cpp +++ b/src/common/tbarbase.cpp @@ -475,14 +475,26 @@ void wxToolBarBase::AdjustToolBitmapSize() bundles.push_back(bmp); } - if ( !bundles.empty() ) + if ( bundles.empty() ) + return; + + wxSize sizeNeeded; + + if ( m_requestedBitmapSize != wxSize(0, 0) ) { - wxSize sizePreferred = wxBitmapBundle::GetConsensusSizeFor - ( - this, - bundles, - ToDIP(sizeOrig) - ); + // If we have a fixed requested bitmap size, use it, but scale it by + // integer factor only, as otherwise we'd force fractional (and hence + // ugly looking) scaling here whenever fractional DPI scaling is used. + + // We want to round 1.5 down to 1, but 1.75 up to 2. + int scaleFactorRoundedDown = + static_cast(ceil(2*GetDPIScaleFactor())) / 2; + sizeNeeded = m_requestedBitmapSize*scaleFactorRoundedDown; + } + else // Determine the best size to use from the bitmaps we have. + { + const wxSize + sizePreferred = wxBitmapBundle::GetConsensusSizeFor(this, bundles); // GetConsensusSizeFor() returns physical size, but we want to operate // with logical pixels as everything else is expressed in them. @@ -492,22 +504,17 @@ void wxToolBarBase::AdjustToolBitmapSize() // scale factor may be different from 1) use this size at all // currently, so it shouldn't matter. But if/when they are modified to // use the size computed here, this would need to be revisited. - sizePreferred = FromPhys(sizePreferred); - - // Don't decrease the bitmap below the size requested by the application - // as using larger bitmaps shouldn't shrink them to the small default - // size. - sizePreferred.IncTo(FromDIP(m_requestedBitmapSize)); - - // No need to change the bitmaps size if it doesn't really change. - if ( sizePreferred == sizeOrig ) - return; + sizeNeeded = FromPhys(sizePreferred); + } + // No need to change the bitmaps size if it doesn't really change. + if ( sizeNeeded != sizeOrig ) + { // Call DoSetToolBitmapSize() and not SetToolBitmapSize() to avoid // changing the requested bitmap size: if we set our own adjusted size // as the preferred one, we wouldn't decrease it later even if we ought // to, as when moving from a monitor with higher DPI to a lower-DPI one. - DoSetToolBitmapSize(sizePreferred); + DoSetToolBitmapSize(sizeNeeded); } } diff --git a/tests/graphics/bmpbundle.cpp b/tests/graphics/bmpbundle.cpp index c90e022e6d..810ed9f61d 100644 --- a/tests/graphics/bmpbundle.cpp +++ b/tests/graphics/bmpbundle.cpp @@ -460,3 +460,34 @@ TEST_CASE("BitmapBundle::Scale", "[bmpbundle][scale]") } #endif // ports with scaled bitmaps support + +TEST_CASE("BitmapBundle::GetConsensusSize", "[bmpbundle]") +{ + // Just a trivial helper to make writing the tests below simpler. + struct Bundles + { + wxVector vec; + + void Add(int size) + { + vec.push_back(wxBitmapBundle::FromBitmap(wxSize(size, size))); + } + + int GetConsensusSize(double scale) const + { + return wxBitmapBundle::GetConsensusSizeFor(scale, vec).y; + } + } bundles; + + // When there is a tie, a larger size is chosen by default. + bundles.Add(16); + bundles.Add(24); + CHECK( bundles.GetConsensusSize(2) == 48 ); + + // Breaking the tie results in the smaller size winning now. + bundles.Add(16); + CHECK( bundles.GetConsensusSize(2) == 32 ); + + // Integer scaling factors should be preferred. + CHECK( bundles.GetConsensusSize(1.5) == 16 ); +}