Fix handling of bitmaps with alpha channel in wxMSW wxImageList.

Don't use mask and alpha together, this results in visual artefacts and masks
are unnecessary with RGBA bitmaps anyhow.

The only potentially problematic remaining case is mixing bitmaps with alpha
and mask inside the same image list (as we need to indicate whether we use the
mask or not when creating it), but this should probably be rare and in the
meanwhile we can at least RGBA bitmaps with image lists, which includes doing
this implicitly when they are used as button bitmaps.

Also refactor wxBitmap code to extract part of CopyFromIconOrCursor() to allow
reusing it in the newly added MSWUpdateAlpha().

See #11476.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@75567 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/msw/bitmap.h

@@ -180,6 +180,10 @@ public:
     // implementation only from now on
     // -------------------------------
 
+    // Set alpha flag to true if this is a 32bpp bitmap which has any non-0
+    // values in its alpha channel.
+    void MSWUpdateAlpha();
+
 public:
     void SetHBITMAP(WXHBITMAP bmp) { SetHandle((WXHANDLE)bmp); }
     WXHBITMAP GetHBITMAP() const { return (WXHBITMAP)GetHandle(); }

src/msw/anybutton.cpp

@@ -215,7 +215,8 @@ public:
     // we must be constructed with the size of our images as we need to create
     // the image list
     wxXPButtonImageData(wxAnyButton *btn, const wxBitmap& bitmap)
-        : m_iml(bitmap.GetWidth(), bitmap.GetHeight(), true /* use mask */,
+        : m_iml(bitmap.GetWidth(), bitmap.GetHeight(),
+                !bitmap.HasAlpha() /* use mask only if no alpha */,
                 wxAnyButton::State_Max + 1 /* see "pulse" comment below */),
           m_hwndBtn(GetHwndOf(btn))
     {

src/msw/bitmap.cpp

@@ -292,6 +292,60 @@ wxGDIRefData *wxBitmap::CloneGDIRefData(const wxGDIRefData *data) const
     return new wxBitmapRefData(*static_cast<const wxBitmapRefData *>(data));
 }
 
+// Premultiply the values of all RGBA pixels in the given range.
+static void PremultiplyPixels(unsigned char* begin, unsigned char* end)
+{
+    for ( unsigned char* pixels = begin; pixels < end; pixels += 4 )
+    {
+        const unsigned char a = pixels[3];
+
+        pixels[0] = ((pixels[0]*a) + 127)/255;
+        pixels[1] = ((pixels[1]*a) + 127)/255;
+        pixels[2] = ((pixels[2]*a) + 127)/255;
+    }
+}
+
+// Helper which examines the alpha channel for any non-0 values and also
+// possibly returns the DIB with premultiplied values if it does have alpha
+// (i.e. this DIB is only filled if the function returns true).
+//
+// The function semantics is complicated but necessary to avoid converting to
+// DIB twice, which is expensive for large bitmaps, yet avoid code duplication
+// between CopyFromIconOrCursor() and MSWUpdateAlpha().
+static bool CheckAlpha(HBITMAP hbmp, HBITMAP* hdib = NULL)
+{
+    BITMAP bm;
+    if ( !::GetObject(hbmp, sizeof(bm), &bm) || (bm.bmBitsPixel != 32) )
+        return false;
+
+    wxDIB dib(hbmp);
+    if ( !dib.IsOk() )
+        return false;
+
+    unsigned char* pixels = dib.GetData();
+    unsigned char* const end = pixels + 4*dib.GetWidth()*dib.GetHeight();
+    for ( ; pixels < end; pixels += 4 )
+    {
+        if ( pixels[3] != 0 )
+        {
+            if ( hdib )
+            {
+                // If we do have alpha, ensure we use premultiplied data for
+                // our pixels as this is what the bitmaps created in other ways
+                // do and this is necessary for e.g. AlphaBlend() to work with
+                // this bitmap.
+                PremultiplyPixels(dib.GetData(), end);
+
+                *hdib = dib.Detach();
+            }
+
+            return true;
+        }
+    }
+
+    return false;
+}
+
 bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon,
                                     wxBitmapTransparency transp)
 {
@@ -332,48 +386,14 @@ bool wxBitmap::CopyFromIconOrCursor(const wxGDIImage& icon,
 #if wxUSE_WXDIB
             // If the icon is 32 bits per pixel then it may have alpha channel
             // data, although there are some icons that are 32 bpp but have no
-            // alpha... So convert to a DIB and manually check the 4th byte for
+            // alpha, so check for this.
-            // each pixel.
             {
-                BITMAP bm;
+                HBITMAP hdib = 0;
-                if ( ::GetObject(iconInfo.hbmColor, sizeof(bm), &bm) &&
+                if ( CheckAlpha(iconInfo.hbmColor, &hdib) )
-                        (bm.bmBitsPixel == 32) )
                 {
-                    wxDIB dib(iconInfo.hbmColor);
+                    refData->m_hasAlpha = true;
-                    if (dib.IsOk())
+                    ::DeleteObject(refData->m_hBitmap);
-                    {
+                    refData->m_hBitmap = hdib;
-                        unsigned char* const pixels = dib.GetData();
-                        int idx;
-                        for ( idx = 0; idx < w*h*4; idx += 4 )
-                        {
-                            if (pixels[idx+3] != 0)
-                            {
-                                // If there is an alpha byte that is non-zero
-                                // then set the alpha flag and stop checking
-                                refData->m_hasAlpha = true;
-                                break;
-                            }
-                        }
-
-                        if ( refData->m_hasAlpha )
-                        {
-                            // If we do have alpha, ensure we use premultiplied
-                            // data for our pixels as this is what the bitmaps
-                            // created in other ways do and this is necessary
-                            // for e.g. AlphaBlend() to work with this bitmap.
-                            for ( idx = 0; idx < w*h*4; idx += 4 )
-                            {
-                                const unsigned char a = pixels[idx+3];
-
-                                pixels[idx]   = ((pixels[idx]  *a) + 127)/255;
-                                pixels[idx+1] = ((pixels[idx+1]*a) + 127)/255;
-                                pixels[idx+2] = ((pixels[idx+2]*a) + 127)/255;
-                            }
-
-                            ::DeleteObject(refData->m_hBitmap);
-                            refData->m_hBitmap = dib.Detach();
-                        }
-                    }
                 }
             }
             break;
@@ -1194,6 +1214,12 @@ bool wxBitmap::HasAlpha() const
     return GetBitmapData() && GetBitmapData()->m_hasAlpha;
 }
 
+void wxBitmap::MSWUpdateAlpha()
+{
+    if ( CheckAlpha(GetHbitmap()) )
+        GetBitmapData()->m_hasAlpha = true;
+}
+
 // ----------------------------------------------------------------------------
 // wxBitmap setters
 // ----------------------------------------------------------------------------

src/msw/imaglist.cpp

@@ -158,7 +158,11 @@ int wxImageList::Add(const wxBitmap& bitmap, const wxBitmap& mask)
 #endif // wxUSE_WXDIB && wxUSE_IMAGE
         hbmp = GetHbitmapOf(bitmap);
 
-    AutoHBITMAP hbmpMask(GetMaskForImage(bitmap, mask));
+    // Use mask only if we don't have alpha, the bitmap isn't drawn correctly
+    // if we use both.
+    AutoHBITMAP hbmpMask;
+    if ( !bitmap.HasAlpha() )
+        hbmpMask.Init(GetMaskForImage(bitmap, mask));
 
     int index = ImageList_Add(GetHImageList(), hbmp, hbmpMask);
     if ( index == -1 )
@@ -328,7 +332,6 @@ bool wxImageList::Draw(int index,
 // Get the bitmap
 wxBitmap wxImageList::GetBitmap(int index) const
 {
-#if wxUSE_WXDIB && wxUSE_IMAGE
     int bmp_width = 0, bmp_height = 0;
     GetSize(index, bmp_width, bmp_height);
 
@@ -336,29 +339,50 @@ wxBitmap wxImageList::GetBitmap(int index) const
     wxMemoryDC dc;
     dc.SelectObject(bitmap);
 
-    // draw it the first time to find a suitable mask colour
+#if wxUSE_WXDIB && wxUSE_IMAGE
-    ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT);
+    IMAGEINFO ii;
-    dc.SelectObject(wxNullBitmap);
+    ImageList_GetImageInfo(GetHImageList(), index, &ii);
+    if ( ii.hbmMask )
+    {
+        // draw it the first time to find a suitable mask colour
+        ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT);
+        dc.SelectObject(wxNullBitmap);
 
-    // find the suitable mask colour
+        // find the suitable mask colour
-    wxImage image = bitmap.ConvertToImage();
+        wxImage image = bitmap.ConvertToImage();
-    unsigned char r = 0, g = 0, b = 0;
+        unsigned char r = 0, g = 0, b = 0;
-    image.FindFirstUnusedColour(&r, &g, &b);
+        image.FindFirstUnusedColour(&r, &g, &b);
 
-    // redraw whole image and bitmap in the mask colour
+        // redraw whole image and bitmap in the mask colour
-    image.Create(bmp_width, bmp_height);
+        image.Create(bmp_width, bmp_height);
-    image.Replace(0, 0, 0, r, g, b);
+        image.Replace(0, 0, 0, r, g, b);
-    bitmap = wxBitmap(image);
+        bitmap = wxBitmap(image);
 
-    // redraw icon over the mask colour to actually draw it
+        // redraw icon over the mask colour to actually draw it
-    dc.SelectObject(bitmap);
+        dc.SelectObject(bitmap);
-    ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT);
+        ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT);
-    dc.SelectObject(wxNullBitmap);
+        dc.SelectObject(wxNullBitmap);
 
-    // get the image, set the mask colour and convert back to get transparent bitmap
+        // get the image, set the mask colour and convert back to get transparent bitmap
-    image = bitmap.ConvertToImage();
+        image = bitmap.ConvertToImage();
-    image.SetMaskColour(r, g, b);
+        image.SetMaskColour(r, g, b);
-    bitmap = wxBitmap(image);
+        bitmap = wxBitmap(image);
+    }
+    else // no mask
+    {
+        // Just draw it normally.
+        ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_NORMAL);
+        dc.SelectObject(wxNullBitmap);
+
+        // And adjust its alpha flag as the destination bitmap would get it if
+        // the source one had it.
+        //
+        // Note that perhaps we could just call UseAlpha() which would set the
+        // "has alpha" flag unconditionally as it doesn't seem to do any harm,
+        // but for now only do it if necessary, just to be on the safe side,
+        // even if it requires more work (and takes more time).
+        bitmap.MSWUpdateAlpha();
+    }
 #else
     wxBitmap bitmap;
 #endif