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

@@ -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_

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_

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);
+    image = wxOSXImageFromBitmap(m_entries[0].bitmap);
-        const size_t n = m_entries.size();
+    const size_t n = m_entries.size();
-        for ( size_t i = 1; i < n; ++i )
+    for ( size_t i = 1; i < n; ++i )
-        {
+    {
-            const Entry& entry = m_entries[i];
+        const Entry& entry = m_entries[i];
-            wxOSXAddBitmapToImage(m_nsImage, entry.bitmap);
+        wxOSXAddBitmapToImage(image, 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);
     }
-    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
+}

src/osx/core/bmpbndl.mm

@@ -28,11 +28,76 @@
 #include "wx/osx/private.h"
 
 #include <algorithm>
+#include <unordered_map>
 
 // ----------------------------------------------------------------------------
 // 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<wxBitmapBundleImpl*>(impl)->GetBitmap(sz);
+        image = wxOSXImageFromBitmap(bmp);
+
+        // unconditionally try to add a 2x version
+        double scale = 2.0;
+        wxSize doublesz = sz * scale;
+        bmp = const_cast<wxBitmapBundleImpl*>(impl)->GetBitmap(doublesz);
+        if ( bmp.IsOk() && bmp.GetSize() != sz )
+            wxOSXAddBitmapToImage(image, bmp);
+#else
+        double scale = wxOSXGetMainScreenContentScaleFactor();
+        wxSize scaledSize = sz * scale;
+        wxBitmap bmp = const_cast<wxBitmapBundleImpl*>(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<wxBitmapBundleImpl*>(impl)->GetBitmap(sz);
+            if ( bmp.IsOk() )
+                image = wxOSXImageFromBitmap(bmp);
+        }
+#endif
+        if ( image )
+            wxOSXSetImageForBundleImpl(impl, image);
+    }
 
     return image;
 }