From 568411347bb643c3ba4440ddb44ece6914e432d9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 25 Oct 2021 23:28:05 +0200 Subject: [PATCH 1/3] Make wxOSX wxBitmap::CopyFromIcon() work as in the other ports Just use the same object, as wxIcon is the same thing as wxBitmap anyhow, instead of re-creating another wxBitmap which can be subtly different from the original one. Notably, converting an icon with default scale factor of 1 to bitmap when using a high DPI display resulted in a bitmap with the same physical size but scale factor of 2, as wxOSXGetImageScaleFactor() returned 2 in this case, i.e. changed the logical bitmap size. This couldn't be anything other than a bug, so fix this and, at the same time, simplify the code and make it consistent with the other ports. --- src/osx/core/bitmap.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/osx/core/bitmap.cpp b/src/osx/core/bitmap.cpp index 2f82e5d9a2..3149731261 100644 --- a/src/osx/core/bitmap.cpp +++ b/src/osx/core/bitmap.cpp @@ -784,7 +784,8 @@ wxBitmapRefData::~wxBitmapRefData() bool wxBitmap::CopyFromIcon(const wxIcon& icon) { - return Create( icon.OSXGetImage() ); + *this = icon; + return IsOk(); } wxBitmap::wxBitmap(const char bits[], int the_width, int the_height, int no_bits) From 41b1ba3c9e4259f37c093fc1a004e7770a0821af Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 25 Oct 2021 23:32:23 +0200 Subject: [PATCH 2/3] Replace many identical wxBitmap::CopyFromIcon() with a single one Define CopyFromIcon() directly in wxBitmapBase for the non-MSW ports, as it was implemented exactly in the same way in all ports using this class anyhow. This means this function is not virtual any longer, but this shouldn't be a problem as it was never supposed to be overridden in application code and this couldn't be done with wxMSW, where it never was virtual in the first place, anyhow. No real changes, just a simplification. --- include/wx/bitmap.h | 2 +- include/wx/dfb/bitmap.h | 3 --- include/wx/gtk/bitmap.h | 3 --- include/wx/gtk1/bitmap.h | 3 --- include/wx/osx/bitmap.h | 3 --- include/wx/qt/bitmap.h | 3 --- include/wx/x11/bitmap.h | 3 --- src/common/bmpbase.cpp | 6 ++++++ src/dfb/bitmap.cpp | 6 ------ src/gtk/bitmap.cpp | 6 ------ src/gtk1/bitmap.cpp | 6 ------ src/osx/core/bitmap.cpp | 6 ------ src/qt/bitmap.cpp | 7 ------- src/x11/bitmap.cpp | 6 ------ 14 files changed, 7 insertions(+), 56 deletions(-) diff --git a/include/wx/bitmap.h b/include/wx/bitmap.h index 6b31c71c37..8713884001 100644 --- a/include/wx/bitmap.h +++ b/include/wx/bitmap.h @@ -223,7 +223,7 @@ public: #endif // wxUSE_PALETTE // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon) = 0; + bool CopyFromIcon(const wxIcon& icon); // implementation: #if WXWIN_COMPATIBILITY_3_0 diff --git a/include/wx/dfb/bitmap.h b/include/wx/dfb/bitmap.h index ed2976d68f..cb0e4f0551 100644 --- a/include/wx/dfb/bitmap.h +++ b/include/wx/dfb/bitmap.h @@ -63,9 +63,6 @@ public: virtual void SetPalette(const wxPalette& palette); #endif - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon); - static void InitStandardHandlers(); // raw bitmap access support functions diff --git a/include/wx/gtk/bitmap.h b/include/wx/gtk/bitmap.h index 2a6f9c62f5..f79c081393 100644 --- a/include/wx/gtk/bitmap.h +++ b/include/wx/gtk/bitmap.h @@ -103,9 +103,6 @@ public: wxImage ConvertToImage() const wxOVERRIDE; #endif // wxUSE_IMAGE - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon) wxOVERRIDE; - wxMask *GetMask() const wxOVERRIDE; void SetMask( wxMask *mask ) wxOVERRIDE; diff --git a/include/wx/gtk1/bitmap.h b/include/wx/gtk1/bitmap.h index dc335d9f14..00f592de92 100644 --- a/include/wx/gtk1/bitmap.h +++ b/include/wx/gtk1/bitmap.h @@ -88,9 +88,6 @@ public: wxImage ConvertToImage() const; - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon); - wxMask *GetMask() const; void SetMask( wxMask *mask ); diff --git a/include/wx/osx/bitmap.h b/include/wx/osx/bitmap.h index 4f85b77fcb..47c7e5fdc1 100644 --- a/include/wx/osx/bitmap.h +++ b/include/wx/osx/bitmap.h @@ -165,9 +165,6 @@ public: wxBitmapRefData *GetBitmapData() { return (wxBitmapRefData *)m_refData; } - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon) wxOVERRIDE; - int GetWidth() const wxOVERRIDE; int GetHeight() const wxOVERRIDE; int GetDepth() const wxOVERRIDE; diff --git a/include/wx/qt/bitmap.h b/include/wx/qt/bitmap.h index b8eeb57ad5..d9451ce4b8 100644 --- a/include/wx/qt/bitmap.h +++ b/include/wx/qt/bitmap.h @@ -61,9 +61,6 @@ public: virtual void SetPalette(const wxPalette& palette) wxOVERRIDE; #endif // wxUSE_PALETTE - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon) wxOVERRIDE; - // implementation: #if WXWIN_COMPATIBILITY_3_0 wxDEPRECATED(virtual void SetHeight(int height) wxOVERRIDE); diff --git a/include/wx/x11/bitmap.h b/include/wx/x11/bitmap.h index 25808e4e80..4c0e6ce3b6 100644 --- a/include/wx/x11/bitmap.h +++ b/include/wx/x11/bitmap.h @@ -98,9 +98,6 @@ public: bool CreateFromImage(const wxImage& image, int depth = -1); #endif // wxUSE_IMAGE - // copies the contents and mask of the given (colour) icon to the bitmap - virtual bool CopyFromIcon(const wxIcon& icon); - wxMask *GetMask() const; void SetMask( wxMask *mask ); diff --git a/src/common/bmpbase.cpp b/src/common/bmpbase.cpp index 1136d1f1d2..1b76bc3a1a 100644 --- a/src/common/bmpbase.cpp +++ b/src/common/bmpbase.cpp @@ -196,6 +196,12 @@ public: wxIMPLEMENT_DYNAMIC_CLASS(wxBitmapBaseModule, wxModule); +bool wxBitmapBase::CopyFromIcon(const wxIcon& icon) +{ + *this = icon; + return IsOk(); +} + // ---------------------------------------------------------------------------- // Trivial implementations of scale-factor related functions // ---------------------------------------------------------------------------- diff --git a/src/dfb/bitmap.cpp b/src/dfb/bitmap.cpp index 577daf67c2..98a0167eb4 100644 --- a/src/dfb/bitmap.cpp +++ b/src/dfb/bitmap.cpp @@ -603,12 +603,6 @@ void wxBitmap::SetMask(wxMask *mask) M_BITMAP->m_mask = mask; } -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = *((wxBitmap*)(&icon)); - return true; -} - wxBitmap wxBitmap::GetSubBitmap(const wxRect& rect) const { wxCHECK_MSG( IsOk() && diff --git a/src/gtk/bitmap.cpp b/src/gtk/bitmap.cpp index c6b229e211..5728f236b9 100644 --- a/src/gtk/bitmap.cpp +++ b/src/gtk/bitmap.cpp @@ -988,12 +988,6 @@ void wxBitmap::SetMask( wxMask *mask ) } } -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = icon; - return IsOk(); -} - #ifdef __WXGTK3__ bool wxBitmap::CreateScaled(int w, int h, int depth, double scale) { diff --git a/src/gtk1/bitmap.cpp b/src/gtk1/bitmap.cpp index 4bcf68f784..365cea5737 100644 --- a/src/gtk1/bitmap.cpp +++ b/src/gtk1/bitmap.cpp @@ -1192,12 +1192,6 @@ void wxBitmap::SetMask( wxMask *mask ) M_BMPDATA->m_mask = mask; } -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = icon; - return true; -} - wxBitmap wxBitmap::GetSubBitmap( const wxRect& rect) const { wxCHECK_MSG( IsOk() && diff --git a/src/osx/core/bitmap.cpp b/src/osx/core/bitmap.cpp index 3149731261..ec597f3f13 100644 --- a/src/osx/core/bitmap.cpp +++ b/src/osx/core/bitmap.cpp @@ -782,12 +782,6 @@ wxBitmapRefData::~wxBitmapRefData() // wxBitmap // ---------------------------------------------------------------------------- -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = icon; - return IsOk(); -} - wxBitmap::wxBitmap(const char bits[], int the_width, int the_height, int no_bits) { m_refData = new wxBitmapRefData( the_width , the_height , no_bits ) ; diff --git a/src/qt/bitmap.cpp b/src/qt/bitmap.cpp index 32fffeb47f..349b044504 100644 --- a/src/qt/bitmap.cpp +++ b/src/qt/bitmap.cpp @@ -372,13 +372,6 @@ void wxBitmap::SetPalette(const wxPalette& WXUNUSED(palette)) #endif // wxUSE_PALETTE -// copies the contents and mask of the given (colour) icon to the bitmap -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = icon; - return IsOk(); -} - #if WXWIN_COMPATIBILITY_3_0 void wxBitmap::SetHeight(int height) { diff --git a/src/x11/bitmap.cpp b/src/x11/bitmap.cpp index 99020ed1a7..1dfcd2f14d 100644 --- a/src/x11/bitmap.cpp +++ b/src/x11/bitmap.cpp @@ -993,12 +993,6 @@ void wxBitmap::SetMask( wxMask *mask ) M_BMPDATA->m_mask = mask; } -bool wxBitmap::CopyFromIcon(const wxIcon& icon) -{ - *this = icon; - return true; -} - wxBitmap wxBitmap::GetSubBitmap( const wxRect& rect) const { wxCHECK_MSG( IsOk() && From da73be0d7748aae8b69889e268b34997f6a1f15c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 25 Oct 2021 20:22:08 +0200 Subject: [PATCH 3/3] Upscale the biggest bitmap in the bundle if it's too small The changes of b20552116c (Allow wxBitmapBundle to specify its preferred bitmap size, 2021-10-19) resulted in never rescaling the bitmaps in standard size in high DPI at all, which isn't the right thing to do: by default, i.e. if just a single bitmap is specified, we should scale it up as necessary in order to show the UI elements in the correct sizes. --- src/common/bmpbndl.cpp | 9 ++++++++- tests/graphics/bmpbundle.cpp | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/common/bmpbndl.cpp b/src/common/bmpbndl.cpp index af6702d08a..7ed5ebf933 100644 --- a/src/common/bmpbndl.cpp +++ b/src/common/bmpbndl.cpp @@ -205,7 +205,14 @@ wxSize wxBitmapBundleImplSet::GetPreferredSizeAtScale(double scale) const // We only get here if the target size is bigger than all the available // sizes, in which case we have no choice but to use the biggest bitmap. - return m_entries.back().bitmap.GetSize(); + const wxSize sizeMax = m_entries.back().bitmap.GetSize(); + + // But check how far is it from the requested scale: if it's more than 1.5 + // times smaller, we should still scale it, notably to ensure that bitmaps + // of standard size are scaled when 2x DPI scaling is used. + return static_cast(sizeTarget.y) / sizeMax.y >= 1.5 + ? sizeTarget + : sizeMax; } wxBitmap wxBitmapBundleImplSet::GetBitmap(const wxSize& size) diff --git a/tests/graphics/bmpbundle.cpp b/tests/graphics/bmpbundle.cpp index b08fafc26a..0f0abfe913 100644 --- a/tests/graphics/bmpbundle.cpp +++ b/tests/graphics/bmpbundle.cpp @@ -57,6 +57,8 @@ TEST_CASE("BitmapBundle::GetPreferredSize", "[bmpbundle]") const wxBitmapBundle b = wxBitmapBundle::FromBitmaps(wxBitmap(normal), wxBitmap(bigger)); + // Check that the existing bitmaps are used without scaling for most of the + // typical scaling values. CHECK( b.GetPreferredSizeAtScale(0 ) == normal ); CHECK( b.GetPreferredSizeAtScale(1 ) == normal ); CHECK( b.GetPreferredSizeAtScale(1.25) == normal ); @@ -65,7 +67,10 @@ TEST_CASE("BitmapBundle::GetPreferredSize", "[bmpbundle]") CHECK( b.GetPreferredSizeAtScale(1.75) == bigger ); CHECK( b.GetPreferredSizeAtScale(2 ) == bigger ); CHECK( b.GetPreferredSizeAtScale(2.5 ) == bigger ); - CHECK( b.GetPreferredSizeAtScale(3 ) == bigger ); + + // This scale is too big to use any of the existing bitmaps, so they will + // be scaled. + CHECK( b.GetPreferredSizeAtScale(3 ) == 3*normal ); } #ifdef wxHAS_BITMAP_SCALE_FACTOR