Don't force fractional scale when toolbar bitmap size is given

The old code in wxToolBarBase::AdjustToolBitmapSize() forced the use of
the exact value of the requested bitmap size multiplied by the current
scale factor, which resulted in ugly bitmaps whenever fractional scaling
factor was used. It also used not immediately clear IncTo() call.

Simplify and improve it by handling the cases when we have a requested
bitmap size and we don't have it differently: if we do have it, just use
it directly, but only with an integer scale factor. And if we don't,
then simply use the bitmap size suitable for the current scale factor.

This seems to result in the most expected behaviour and, notably,
doesn't break the toolbar sample where the bitmap size can still be
toggled between small and big bitmaps on both normal and high DPI
monitors.

Also update the documentation: still recommend not to use
SetToolBitmapSize() at all, but don't claim that it forces fractional
scaling, as this is not the case any longer.

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,
-   `<bitmapsize>` attributes from the XRC, as this forces unwanted scaling.
+   `<bitmapsize>` attributes from the XRC, as this overrides the best size
+   determination and may result in suboptimal appearance.
 
 
 

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.
 

src/common/tbarbase.cpp

@@ -475,7 +475,23 @@ void wxToolBarBase::AdjustToolBitmapSize()
             bundles.push_back(bmp);
     }
 
-    if ( !bundles.empty() )
+    if ( bundles.empty() )
+        return;
+
+    wxSize sizeNeeded;
+
+    if ( m_requestedBitmapSize != wxSize(0, 0) )
+    {
+        // 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<int>(ceil(2*GetDPIScaleFactor())) / 2;
+        sizeNeeded = m_requestedBitmapSize*scaleFactorRoundedDown;
+    }
+    else // Determine the best size to use from the bitmaps we have.
     {
         wxSize sizePreferred = wxBitmapBundle::GetConsensusSizeFor
                                (
@@ -492,22 +508,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);
     }
 }