Refactor wxMSW wxStaticBitmap to separate bitmaps and icons

Use different members for storing wxBitmap and wxIcon instead of using a
single pointer for storing a heap-allocated copy of either of them
because this is simpler, more efficient due to avoiding unnecessary
conversions and RTTI, and will be simpler to generalize to using
wxBitmapBundle instead of wxBitmap in the upcoming commits.

As a side effect, don't assert in GetIcon() or GetBitmap() if there is
no valid icon or bitmap, as this is inconsistent with the other ports
and wasn't even consistent between these functions in wxMSW itself.

There should be no other changes in behaviour.

include/wx/msw/statbmp.h

@@ -25,7 +25,20 @@ public:
 
     wxStaticBitmap(wxWindow *parent,
                    wxWindowID id,
-                   const wxGDIImage& label,
+                   const wxBitmap& label,
+                   const wxPoint& pos = wxDefaultPosition,
+                   const wxSize& size = wxDefaultSize,
+                   long style = 0,
+                   const wxString& name = wxASCII_STR(wxStaticBitmapNameStr))
+    {
+        Init();
+
+        Create(parent, id, label, pos, size, style, name);
+    }
+
+    wxStaticBitmap(wxWindow *parent,
+                   wxWindowID id,
+                   const wxIcon& label,
                    const wxPoint& pos = wxDefaultPosition,
                    const wxSize& size = wxDefaultSize,
                    long style = 0,
@@ -38,16 +51,34 @@ public:
 
     bool Create(wxWindow *parent,
                 wxWindowID id,
-                const wxGDIImage& label,
+                const wxBitmap& label,
                 const wxPoint& pos = wxDefaultPosition,
                 const wxSize& size = wxDefaultSize,
                 long style = 0,
-                const wxString& name = wxASCII_STR(wxStaticBitmapNameStr));
+                const wxString& name = wxASCII_STR(wxStaticBitmapNameStr))
+    {
+        m_bitmap = label;
+
+        return DoCreate(parent, id, pos, size, style, name);
+    }
+
+    bool Create(wxWindow *parent,
+                wxWindowID id,
+                const wxIcon& label,
+                const wxPoint& pos = wxDefaultPosition,
+                const wxSize& size = wxDefaultSize,
+                long style = 0,
+                const wxString& name = wxASCII_STR(wxStaticBitmapNameStr))
+    {
+        m_icon = label;
+
+        return DoCreate(parent, id, pos, size, style, name);
+    }
 
     virtual ~wxStaticBitmap() { Free(); }
 
-    virtual void SetIcon(const wxIcon& icon) wxOVERRIDE { SetImage(&icon); }
+    virtual void SetIcon(const wxIcon& icon) wxOVERRIDE;
-    virtual void SetBitmap(const wxBitmap& bitmap) wxOVERRIDE { SetImage(&bitmap); }
+    virtual void SetBitmap(const wxBitmap& bitmap) wxOVERRIDE;
     virtual wxBitmap GetBitmap() const wxOVERRIDE;
     virtual wxIcon GetIcon() const wxOVERRIDE;
 
@@ -59,15 +90,23 @@ public:
 protected:
     virtual wxSize DoGetBestClientSize() const wxOVERRIDE;
 
+private:
     // ctor/dtor helpers
     void Init();
     void Free();
 
-    // true if icon/bitmap is valid
+    // common part of both Create() overloads
-    bool ImageIsOk() const;
+    bool DoCreate(wxWindow *parent,
+                  wxWindowID id,
+                  const wxPoint& pos,
+                  const wxSize& size,
+                  long style,
+                  const wxString& name);
 
-    void SetImage(const wxGDIImage* image);
+    // update the image to correspond to the current m_icon or m_bitmap value,
-    void SetImageNoCopy( wxGDIImage* image );
+    // resize the control if the new size is different from the old one and
+    // update its style if the new "is icon" value differs from the old one
+    void DoUpdateImage(const wxSize& sizeOld, bool wasIcon);
 
     // draw the bitmap ourselves here if the OS can't do it correctly (if it
     // can we leave it to it)
@@ -75,14 +114,19 @@ protected:
 
     void WXHandleSize(wxSizeEvent& event);
 
-    // we can have either an icon or a bitmap
+    // return the either m_icon or m_bitmap (may still be invalid of both of
-    bool m_isIcon;
+    // them are)
-    wxGDIImage *m_image;
+    const wxGDIImage& GetImage() const;
+
+
+    // we can have either an icon or a bitmap: if m_icon is valid, it is used,
+    // otherwise m_bitmap is used if it is valid
+    wxIcon m_icon;
+    wxBitmap m_bitmap;
 
     // handle used in last call to STM_SETIMAGE
     WXHANDLE m_currentHandle;
 
-private:
     // Flag indicating whether we own m_currentHandle, i.e. should delete it.
     bool m_ownsCurrentHandle;
 

src/msw/statbmp.cpp

@@ -55,41 +55,8 @@ wxEND_EVENT_TABLE()
 // wxStaticBitmap
 // ---------------------------------------------------------------------------
 
-// we may have either bitmap or icon: if a bitmap with mask is passed, we
+bool wxStaticBitmap::DoCreate(wxWindow *parent,
-// will transform it to an icon ourselves because otherwise the mask will
-// be ignored by Windows
-// note that this function will create a new object every time
-// it is called even if the image needs no conversion
-
-static wxGDIImage* ConvertImage( const wxGDIImage& bitmap )
-{
-    bool isIcon = bitmap.IsKindOf( wxCLASSINFO(wxIcon) );
-
-    if( !isIcon )
-    {
-        wxASSERT_MSG( wxDynamicCast(&bitmap, wxBitmap),
-                      wxT("not an icon and not a bitmap?") );
-
-        const wxBitmap& bmp = (const wxBitmap&)bitmap;
-        wxMask *mask = bmp.GetMask();
-        if ( mask && mask->GetMaskBitmap() )
-        {
-            wxIcon* icon = new wxIcon;
-            icon->CopyFromBitmap(bmp);
-
-            return icon;
-        }
-
-        return new wxBitmap( bmp );
-    }
-
-    // copying a bitmap is a cheap operation
-    return new wxIcon( (const wxIcon&)bitmap );
-}
-
-bool wxStaticBitmap::Create(wxWindow *parent,
                             wxWindowID id,
-                            const wxGDIImage& bitmap,
                             const wxPoint& pos,
                             const wxSize& size,
                             long style,
@@ -98,17 +65,6 @@ bool wxStaticBitmap::Create(wxWindow *parent,
     if ( !CreateControl(parent, id, pos, size, style, wxDefaultValidator, name) )
         return false;
 
-    // we may have either bitmap or icon: if a bitmap with mask is passed, we
-    // will transform it to an icon ourselves because otherwise the mask will
-    // be ignored by Windows
-    wxGDIImage *image = ConvertImage( bitmap );
-
-    // Note that m_isIcon must be set before calling MSWCreateControl() so that
-    // it creates the control with the correct style, as returned by
-    // MSWGetStyle(), which uses m_isIcon to determine whether to use SS_ICON
-    // or SS_BITMAP.
-    m_isIcon = image->IsKindOf( wxCLASSINFO(wxIcon) );
-
     // create the native control
     if ( !MSWCreateControl(wxT("STATIC"), wxEmptyString, pos, size) )
     {
@@ -116,8 +72,7 @@ bool wxStaticBitmap::Create(wxWindow *parent,
         return false;
     }
 
-    // no need to delete the new image
+    DoUpdateImage(wxSize(), m_icon.IsOk());
-    SetImageNoCopy(image);
 
     // GetBestSize will work properly now, so set the best size if needed
     SetInitialSize(size);
@@ -139,7 +94,7 @@ WXDWORD wxStaticBitmap::MSWGetStyle(long style, WXDWORD *exstyle) const
     WXDWORD msStyle = wxControl::MSWGetStyle(style, exstyle);
 
     // what kind of control are we?
-    msStyle |= m_isIcon ? SS_ICON : SS_BITMAP;
+    msStyle |= m_icon.IsOk() ? SS_ICON : SS_BITMAP;
 
     // we use SS_CENTERIMAGE to prevent the control from resizing the bitmap to
     // fit to its size -- this is unexpected and doesn't happen in other ports
@@ -150,43 +105,58 @@ WXDWORD wxStaticBitmap::MSWGetStyle(long style, WXDWORD *exstyle) const
     return msStyle;
 }
 
-bool wxStaticBitmap::ImageIsOk() const
+const wxGDIImage& wxStaticBitmap::GetImage() const
 {
-    return m_image && m_image->IsOk();
+    // Note: do not use ternary operator here to avoid unwanted conversions
+    // from wxIcon to wxBitmap.
+    if ( m_icon.IsOk() )
+        return m_icon;
+
+    return m_bitmap;
+}
+
+void wxStaticBitmap::SetIcon(const wxIcon& icon)
+{
+    const wxSize sizeOld = GetImage().GetSize();
+    const bool wasIcon = m_icon.IsOk();
+
+    m_icon = icon;
+    m_bitmap = wxBitmap();
+
+    DoUpdateImage(sizeOld, wasIcon);
+}
+
+void wxStaticBitmap::SetBitmap(const wxBitmap& bitmap)
+{
+    const wxSize sizeOld = GetImage().GetSize();
+    const bool wasIcon = m_icon.IsOk();
+
+    m_icon = wxIcon();
+    m_bitmap = bitmap;
+
+    DoUpdateImage(sizeOld, wasIcon);
 }
 
 wxIcon wxStaticBitmap::GetIcon() const
 {
-    wxCHECK_MSG( m_image, wxIcon(), wxT("no image in wxStaticBitmap") );
+    wxIcon icon = m_icon;
+    if ( !icon.IsOk() && m_bitmap.IsOk() )
+        icon.CopyFromBitmap(m_bitmap);
 
-    // we can't ask for an icon if all we have is a bitmap
+    return icon;
-    wxCHECK_MSG( m_isIcon, wxIcon(), wxT("no icon in this wxStaticBitmap") );
-
-    return *(wxIcon *)m_image;
 }
 
 wxBitmap wxStaticBitmap::GetBitmap() const
 {
-    if ( m_isIcon )
+    wxBitmap bitmap = m_bitmap;
-    {
+    if ( !bitmap.IsOk() && m_icon.IsOk() )
-        // don't fail because we might have replaced the bitmap with icon
+        bitmap.CopyFromIcon(m_icon);
-        // ourselves internally in ConvertImage() to keep the transparency but
-        // the user code doesn't know about it so it still can use GetBitmap()
-        // to retrieve the bitmap
-        return wxBitmap(GetIcon());
-    }
-    else // we have a bitmap
-    {
-        wxCHECK_MSG( m_image, wxBitmap(), wxT("no image in wxStaticBitmap") );
 
-        return *(wxBitmap *)m_image;
+    return bitmap;
-    }
 }
 
 void wxStaticBitmap::Init()
 {
-    m_isIcon = true;
-    m_image = NULL;
     m_currentHandle = 0;
     m_ownsCurrentHandle = false;
 }
@@ -200,16 +170,15 @@ void wxStaticBitmap::Free()
         ::DeleteObject(m_currentHandle);
         m_ownsCurrentHandle = false;
     }
-
-    wxDELETE(m_image);
 }
 
 wxSize wxStaticBitmap::DoGetBestClientSize() const
 {
     wxSize size;
-    if ( ImageIsOk() )
+    const wxGDIImage& image = GetImage();
+    if ( image.IsOk() )
     {
-        size = m_image->GetSize();
+        size = image.GetSize();
     }
     else // No image yet
     {
@@ -252,16 +221,10 @@ void wxStaticBitmap::DoPaintManually(wxPaintEvent& WXUNUSED(event))
                   true /* use mask */);
 }
 
-void wxStaticBitmap::SetImage( const wxGDIImage* image )
-{
-    wxGDIImage* convertedImage = ConvertImage( *image );
-    SetImageNoCopy( convertedImage );
-}
-
 void wxStaticBitmap::MSWReplaceImageHandle(WXLPARAM handle)
 {
     HGDIOBJ oldHandle = (HGDIOBJ)::SendMessage(GetHwnd(), STM_SETIMAGE,
-                  m_isIcon ? IMAGE_ICON : IMAGE_BITMAP, (LPARAM)handle);
+                  m_icon.IsOk() ? IMAGE_ICON : IMAGE_BITMAP, (LPARAM)handle);
     // detect if this is still the handle we passed before or
     // if the static-control made a copy of the bitmap!
     if (oldHandle != 0 && oldHandle != (HGDIOBJ) m_currentHandle)
@@ -271,53 +234,41 @@ void wxStaticBitmap::MSWReplaceImageHandle(WXLPARAM handle)
     }
 }
 
-void wxStaticBitmap::SetImageNoCopy( wxGDIImage* image)
+void wxStaticBitmap::DoUpdateImage(const wxSize& sizeOld, bool wasIcon)
 {
-    wxSize sizeOld;
+    const wxSize sizeNew = GetImage().GetSize();
-    if ( m_image )
+    const bool isIcon = m_icon.IsOk();
-        sizeOld = m_image->GetSize();
-
-    wxSize sizeNew;
-    if ( image )
-        sizeNew = image->GetSize();
-
-    const bool wasIcon = m_isIcon;
 
     Free();
 
-    m_isIcon = image->IsKindOf( wxCLASSINFO(wxIcon) );
-    // the image has already been copied
-    m_image = image;
-
     // Normally we just use the handle of provided image but in some cases we
     // create our own temporary bitmap, so the actual handle may end up being
     // different from the original one.
-    const HANDLE handleOrig = (HANDLE)m_image->GetHandle();
+    const HANDLE handleOrig = (HANDLE)GetImage().GetHandle();
     HANDLE handle = handleOrig;
 
 #if wxUSE_WXDIB
-    if ( !m_isIcon )
+    if ( !isIcon )
     {
         // wxBitmap normally stores alpha in pre-multiplied format but
         // apparently STM_SETIMAGE message handler does pre-multiplication
         // internally so we need to undo the pre-multiplication here for a
         // while (this is similar to what we do in ImageList::Add()).
-        const wxBitmap& bmp = static_cast<wxBitmap&>(*image);
+        if ( m_bitmap.HasAlpha() )
-        if ( bmp.HasAlpha() )
         {
             // For bitmap with alpha channel create temporary DIB with
             // not-premultiplied alpha values.
-            handle = wxDIB(bmp.ConvertToImage(),
+            handle = wxDIB(m_bitmap.ConvertToImage(),
                            wxDIB::PixelFormat_NotPreMultiplied).Detach();
         }
     }
 #endif // wxUSE_WXDIB
 
-    if ( m_isIcon != wasIcon )
+    if ( isIcon != wasIcon )
     {
         wxMSWWinStyleUpdater(GetHwnd())
             .TurnOff(SS_BITMAP | SS_ICON)
-            .TurnOn(m_isIcon ? SS_ICON : SS_BITMAP);
+            .TurnOn(isIcon ? SS_ICON : SS_BITMAP);
     }
 
     MSWReplaceImageHandle((WXLPARAM)handle);