From 4eb4c1706fee6d3a7d0c853abb5aa135463d1d28 Mon Sep 17 00:00:00 2001 From: Stefan Csomor Date: Wed, 20 Oct 2021 15:18:37 +0200 Subject: [PATCH] Remove NSImage from public API of wxBitmapBundleImpl Hide wxOSX implementation details by storing NSImage associated with the bundle in a separate global map instead of making it part of wxBitmapBundleImpl itself. See https://github.com/wxWidgets/wxWidgets/pull/2555 --- include/wx/bmpbndl.h | 10 ++- include/wx/private/bmpbndl.h | 6 ++ src/common/bmpbndl.cpp | 62 ++++++++++--------- src/osx/core/bmpbndl.mm | 116 ++++++++++++++++++++++++++++++----- 4 files changed, 145 insertions(+), 49 deletions(-) diff --git a/include/wx/bmpbndl.h b/include/wx/bmpbndl.h index 4024e3154c..6609e649a0 100644 --- a/include/wx/bmpbndl.h +++ b/include/wx/bmpbndl.h @@ -175,8 +175,11 @@ wxBitmapBundle wxBitmapBundle::FromImage(const wxImage& image) // // It doesn't need to be used directly, but may be inherited from in order to // implement custom bitmap bundles. -class wxBitmapBundleImpl : public wxRefCounter +class WXDLLIMPEXP_CORE wxBitmapBundleImpl : public wxRefCounter { +protected: + virtual ~wxBitmapBundleImpl(); + public: // Return the size of the bitmaps represented by this bundle in the default // DPI (a.k.a. 100% resolution). @@ -194,11 +197,6 @@ public: // Note that this function is non-const because it may generate the bitmap // on demand and cache it. virtual wxBitmap GetBitmap(const wxSize& size) = 0; - -#ifdef __WXOSX__ - // returns the native representation of the bitmap bundle - virtual WXImage OSXGetImage() const { return NULL; } -#endif }; #endif // _WX_BMPBNDL_H_ diff --git a/include/wx/private/bmpbndl.h b/include/wx/private/bmpbndl.h index 6c25a51e62..53bd2f3843 100644 --- a/include/wx/private/bmpbndl.h +++ b/include/wx/private/bmpbndl.h @@ -23,6 +23,12 @@ WXImage WXDLLIMPEXP_CORE wxOSXImageFromBitmap(const wxBitmap& bmp); void WXDLLIMPEXP_CORE wxOSXAddBitmapToImage(WXImage image, const wxBitmap& bmp); #endif +// for hiding the storage of the NSImage with wxBitmapBundleImpls from public API + +WXImage WXDLLIMPEXP_CORE wxOSXGetImageFromBundleImpl(const wxBitmapBundleImpl* impl); +void WXDLLIMPEXP_CORE wxOSXSetImageForBundleImpl(const wxBitmapBundleImpl* impl, WXImage image); +void WXDLLIMPEXP_CORE wxOSXBundleImplDestroyed(const wxBitmapBundleImpl* impl); + #endif #endif // _WX_PRIVATE_BMPBNDL_H_ diff --git a/src/common/bmpbndl.cpp b/src/common/bmpbndl.cpp index 401a74b523..becd229c9c 100644 --- a/src/common/bmpbndl.cpp +++ b/src/common/bmpbndl.cpp @@ -58,23 +58,12 @@ public: ~wxBitmapBundleImplSet() { -#ifdef __WXOSX__ - if (m_nsImage) - wxMacCocoaRelease(m_nsImage); -#endif } virtual wxSize GetDefaultSize() const wxOVERRIDE; virtual wxSize GetPreferredSizeAtScale(double scale) const wxOVERRIDE; virtual wxBitmap GetBitmap(const wxSize& size) wxOVERRIDE; -#ifdef __WXOSX__ - virtual WXImage OSXGetImage() const wxOVERRIDE; - -private: - mutable WXImage m_nsImage = NULL; -#endif - private: // Struct containing bitmap itself as well as a flag indicating whether we // generated it by rescaling the existing bitmap or not. @@ -125,6 +114,10 @@ private: // Common implementation of all ctors. void Init(const wxBitmap* bitmaps, size_t n); +#ifdef __WXOSX__ + void OSXCreateNSImage(); +#endif + wxDECLARE_NO_COPY_CLASS(wxBitmapBundleImplSet); }; @@ -151,6 +144,10 @@ void wxBitmapBundleImplSet::Init(const wxBitmap* bitmaps, size_t n) // Should we check that all bitmaps really have unique sizes here? For now, // don't bother with this, but we might want to do it later if it really // turns out to be a problem in practice. + +#ifdef __WXOSX__ + OSXCreateNSImage(); +#endif } wxSize wxBitmapBundleImplSet::GetDefaultSize() const @@ -260,26 +257,24 @@ wxBitmap wxBitmapBundleImplSet::GetBitmap(const wxSize& size) } #ifdef __WXOSX__ -WXImage wxBitmapBundleImplSet::OSXGetImage() const +void wxBitmapBundleImplSet::OSXCreateNSImage() { - if ( m_nsImage == NULL ) - { + WXImage image = NULL; #if wxOSX_USE_COCOA - m_nsImage = wxOSXImageFromBitmap(m_entries[0].bitmap); - const size_t n = m_entries.size(); - for ( size_t i = 1; i < n; ++i ) - { - const Entry& entry = m_entries[i]; - wxOSXAddBitmapToImage(m_nsImage, entry.bitmap); - } -#else - // TODO determine best bitmap for device scale factor, and use that - // with wxOSXImageFromBitmap as on iOS there is only one bitmap in a UIImage -#endif - if ( m_nsImage ) - wxMacCocoaRetain(m_nsImage); + image = wxOSXImageFromBitmap(m_entries[0].bitmap); + const size_t n = m_entries.size(); + for ( size_t i = 1; i < n; ++i ) + { + const Entry& entry = m_entries[i]; + wxOSXAddBitmapToImage(image, entry.bitmap); } - return m_nsImage; +#else + image = wxOSXImageFromBitmap(m_entries[0].bitmap); + // TODO determine best bitmap for device scale factor, and use that + // with wxOSXImageFromBitmap as on iOS there is only one bitmap in a UIImage +#endif + if ( image ) + wxOSXSetImageForBundleImpl(this, image); } #endif @@ -391,3 +386,14 @@ wxBitmap wxBitmapBundle::GetBitmap(const wxSize& size) const return m_impl->GetBitmap(size == wxDefaultSize ? GetDefaultSize() : size); } + +// ============================================================================ +// wxBitmapBundleImpl implementation +// ============================================================================ + +wxBitmapBundleImpl::~wxBitmapBundleImpl() +{ +#ifdef __WXOSX__ + wxOSXBundleImplDestroyed(this); +#endif +} diff --git a/src/osx/core/bmpbndl.mm b/src/osx/core/bmpbndl.mm index 10663232de..362d2ac367 100644 --- a/src/osx/core/bmpbndl.mm +++ b/src/osx/core/bmpbndl.mm @@ -28,11 +28,76 @@ #include "wx/osx/private.h" #include +#include // ---------------------------------------------------------------------------- // private helpers // ---------------------------------------------------------------------------- +namespace { + +class wxOSXImageHolder +{ +public: + wxOSXImageHolder() : m_nsImage(NULL) + { + } + + explicit wxOSXImageHolder( WXImage image) : m_nsImage(image) + { + [m_nsImage retain]; + } + + wxOSXImageHolder( const wxOSXImageHolder& other ) : m_nsImage(other.m_nsImage) + { + [m_nsImage retain];; + } + + ~wxOSXImageHolder() + { + [m_nsImage release]; + } + + wxOSXImageHolder& operator=(const wxOSXImageHolder& other) + { + if ( other.m_nsImage != m_nsImage ) + { + [m_nsImage release]; + m_nsImage = other.m_nsImage; + [m_nsImage retain]; + } + return *this; + } + + WXImage GetImage() const { return m_nsImage; } +private: + WXImage m_nsImage; +}; + +} // anonymouse namespace + +std::unordered_map< const wxBitmapBundleImpl*, wxOSXImageHolder> gs_nativeImages; + +WXImage WXDLLIMPEXP_CORE wxOSXGetImageFromBundleImpl(const wxBitmapBundleImpl* impl) +{ + auto image = gs_nativeImages.find(impl); + if (image != gs_nativeImages.end()) + return image->second.GetImage(); + else + return NULL; +} + +void WXDLLIMPEXP_CORE wxOSXSetImageForBundleImpl(const wxBitmapBundleImpl* impl, WXImage image) +{ + gs_nativeImages[impl] = wxOSXImageHolder(image); +} + +void WXDLLIMPEXP_CORE wxOSXBundleImplDestroyed(const wxBitmapBundleImpl* impl) +{ + gs_nativeImages.erase(impl); +} + + namespace { // Bundle implementation using PNG bitmaps from Windows resources. @@ -48,11 +113,6 @@ public: virtual wxSize GetDefaultSize() const wxOVERRIDE; virtual wxSize GetPreferredSizeAtScale(double scale) const wxOVERRIDE; virtual wxBitmap GetBitmap(const wxSize& size) wxOVERRIDE; - - virtual WXImage OSXGetImage() const wxOVERRIDE; - -private: - WXImage m_nsImage; }; } // anonymouse namespace @@ -63,17 +123,16 @@ private: wxOSXImageBundleImpl::wxOSXImageBundleImpl(WXImage image) { - m_nsImage = (WXImage) wxMacCocoaRetain(image); + wxOSXSetImageForBundleImpl(this, image); } wxOSXImageBundleImpl::~wxOSXImageBundleImpl() { - wxMacCocoaRelease(m_nsImage); } wxSize wxOSXImageBundleImpl::GetDefaultSize() const { - CGSize sz = wxOSXGetImageSize(m_nsImage); + CGSize sz = wxOSXGetImageSize(wxOSXGetImageFromBundleImpl(this)); return wxSize(sz.width, sz.height); } @@ -90,11 +149,6 @@ wxBitmap wxOSXImageBundleImpl::GetBitmap(const wxSize& size) return wxBitmap(); } -WXImage wxOSXImageBundleImpl::OSXGetImage() const -{ - return m_nsImage; -} - wxBitmapBundle wxOSXMakeBundleFromImage( WXImage img) { return wxBitmapBundle::FromImpl( new wxOSXImageBundleImpl(img) ); @@ -183,10 +237,42 @@ WXImage wxOSXGetImageFromBundle(const wxBitmapBundle& bundle) if (!bundle.IsOk()) return NULL; - WXImage image = bundle.GetImpl()->OSXGetImage(); + wxBitmapBundleImpl* impl = bundle.GetImpl(); + + WXImage image = wxOSXGetImageFromBundleImpl(impl); if (image == 0) - image = bundle.GetBitmap(bundle.GetDefaultSize()).OSXGetImage(); + { + wxSize sz = impl->GetDefaultSize(); + +#if wxOSX_USE_COCOA + wxBitmap bmp = const_cast(impl)->GetBitmap(sz); + image = wxOSXImageFromBitmap(bmp); + + // unconditionally try to add a 2x version + double scale = 2.0; + wxSize doublesz = sz * scale; + bmp = const_cast(impl)->GetBitmap(doublesz); + if ( bmp.IsOk() && bmp.GetSize() != sz ) + wxOSXAddBitmapToImage(image, bmp); +#else + double scale = wxOSXGetMainScreenContentScaleFactor(); + wxSize scaledSize = sz * scale; + wxBitmap bmp = const_cast(impl)->GetBitmap(scaledSize); + if ( bmp.IsOk() ) + image = wxOSXImageFromBitmap(bmp); + else if ( scale > 1.9 ) + { + // if we are on a high dpi device and no matching bitmap is available + // use scale 1x + bmp = const_cast(impl)->GetBitmap(sz); + if ( bmp.IsOk() ) + image = wxOSXImageFromBitmap(bmp); + } +#endif + if ( image ) + wxOSXSetImageForBundleImpl(impl, image); + } return image; }