From 2d782443a4ab27fa98e9a7ac867d52485b708cdf Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Wed, 1 Jun 2022 23:09:10 +0100
Subject: [PATCH] Choose the best bitmap to rescale to the target size

It's not necessarily the largest bitmap, but one which may be scaled
using an integer factor.

Fix the tests to expect the correct results, now that they actually
pass.
---
 src/common/bmpbndl.cpp       | 35 ++++++++++++++++++++++++++---------
 src/msw/bmpbndl.cpp          | 25 ++++++++++++++++++-------
 tests/graphics/bmpbundle.cpp | 14 +++++++++++---
 3 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/src/common/bmpbndl.cpp b/src/common/bmpbndl.cpp
index 9b376a230c..6809b3ab4f 100644
--- a/src/common/bmpbndl.cpp
+++ b/src/common/bmpbndl.cpp
@@ -292,18 +292,35 @@ wxBitmap wxBitmapBundleImplSet::GetBitmap(const wxSize& size)
 
     // We only get here if the requested size is larger than the size of all
     // the bitmaps we have, in which case we have no choice but to upscale one
-    // of the bitmaps, so find the largest available non-generated bitmap.
-    for ( size_t i = n; n > 0; --i )
+    // of the bitmaps, so find the largest available non-generated bitmap which
+    // can be scaled using an integer factor or, failing that, just the largest
+    // non-generated bitmap.
+    const Entry* entryToRescale = NULL;
+    const Entry* entryLastNotGen = NULL;
+    for ( size_t i = 0; i < n; ++i )
     {
-        const Entry& entry = m_entries[i - 1];
-        if ( !entry.generated )
-        {
-            const Entry entryNew(entry, size);
+        const Entry& entry = m_entries[i];
+        if ( entry.generated )
+            continue;
 
-            m_entries.push_back(entryNew);
+        entryLastNotGen = &entry;
 
-            return entryNew.bitmap;
-        }
+        const double
+            scale = static_cast<double>(size.y) / entry.bitmap.GetSize().y;
+        if ( scale == wxRound(scale) )
+            entryToRescale = &entry;
+    }
+
+    if ( !entryToRescale )
+        entryToRescale = entryLastNotGen;
+
+    if ( entryToRescale )
+    {
+        const Entry entryNew(*entryToRescale, size);
+
+        m_entries.push_back(entryNew);
+
+        return entryNew.bitmap;
     }
 
     // We should have at least one non-generated bitmap.
diff --git a/src/msw/bmpbndl.cpp b/src/msw/bmpbndl.cpp
index 6ecd42a78e..fbd707f040 100644
--- a/src/msw/bmpbndl.cpp
+++ b/src/msw/bmpbndl.cpp
@@ -269,17 +269,28 @@ wxBitmap wxBitmapBundleImplRC::GetBitmap(const wxSize& size)
 
         const wxSize sizeThis = sizeDef*info.scale;
 
-        // Use this bitmap if it's the first one bigger than the requested size
-        // or if it's the last item as in this case we're not going to find any
-        // bitmap bigger than the given one anyhow and we don't have any choice
-        // but to upscale the largest one we have.
-        if ( sizeThis.y >= size.y || i + 1 == m_resourceInfos.size() )
+        // Use this bitmap if it's the first one bigger than the requested size.
+        if ( sizeThis.y >= size.y )
             return AddBitmap(info, sizeThis, size);
     }
 
-    wxFAIL_MSG( wxS("unreachable") );
+    // We have to upscale some bitmap because we don't have any bitmaps larger
+    // than the requested size. Try to find one which can be upscaled using an
+    // integer factor.
+    const ResourceInfo* infoToRescale = NULL;
+    for ( size_t i = 0; i < m_resourceInfos.size(); ++i )
+    {
+        const ResourceInfo& info = m_resourceInfos[i];
 
-    return wxBitmap();
+        const double scale = size.y / sizeDef.y*info.scale;
+        if ( scale == wxRound(scale) )
+            infoToRescale = &info;
+    }
+
+    if ( !infoToRescale )
+        infoToRescale = &m_resourceInfos.back();
+
+    return AddBitmap(*infoToRescale, sizeDef*infoToRescale->scale, size);
 }
 
 // ============================================================================
diff --git a/tests/graphics/bmpbundle.cpp b/tests/graphics/bmpbundle.cpp
index c9a179174a..174c9085ab 100644
--- a/tests/graphics/bmpbundle.cpp
+++ b/tests/graphics/bmpbundle.cpp
@@ -272,10 +272,14 @@ TEST_CASE("BitmapBundle::GetPreferredSize", "[bmpbundle]")
     CHECK_THAT( BitmapAtScale(b, 3   ), SameAs(2) );
 
     // This scale is too big to use any of the existing bitmaps, so they will
-    // be scaled, but use integer factors.
-    CHECK_THAT( BitmapAtScale(b, 3.33), SameAs(3, 2) );
+    // be scaled, but use integer factors and, importantly, scale the correct
+    // bitmap using them: we need to scale the small bitmap by a factor of 3,
+    // rather than scaling the larger bitmap by a factor of 1.5 here, but we
+    // must also scale the larger one by a factor of 2 rather than scaling the
+    // small one by a factor of 4.
+    CHECK_THAT( BitmapAtScale(b, 3.33), SameAs(3, 1) );
     CHECK_THAT( BitmapAtScale(b, 4   ), SameAs(4, 2) );
-    CHECK_THAT( BitmapAtScale(b, 5   ), SameAs(5, 2) );
+    CHECK_THAT( BitmapAtScale(b, 5   ), SameAs(5, 1) );
 
 
     // Finally check that things work as expected when we have 3 versions.
@@ -294,6 +298,10 @@ TEST_CASE("BitmapBundle::GetPreferredSize", "[bmpbundle]")
     CHECK_THAT( BitmapAtScale(b, 2   ), SameAs(2.0) );
     CHECK_THAT( BitmapAtScale(b, 2.5 ), SameAs(2.0) );
     CHECK_THAT( BitmapAtScale(b, 3   ), SameAs(2.0) );
+
+    CHECK_THAT( BitmapAtScale(b, 3.33), SameAs(3.0, 1.5) );
+    CHECK_THAT( BitmapAtScale(b, 4.25), SameAs(4.0, 2.0) );
+    CHECK_THAT( BitmapAtScale(b, 5   ), SameAs(5.0, 1.0) );
 }
 
 #ifdef wxHAS_DPI_INDEPENDENT_PIXELS