From 7d94e9fafd9f6eecc66f266eb889a36b49c42bd0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 3 Oct 2014 01:52:10 +0000 Subject: [PATCH] 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. [This is the backport of r75567 from trunk.] git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/branches/WX_3_0_BRANCH@77947 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/changes.txt | 1 + include/wx/msw/bitmap.h | 4 ++ src/msw/anybutton.cpp | 3 +- src/msw/bitmap.cpp | 106 +++++++++++++++++++++++++--------------- src/msw/imaglist.cpp | 60 +++++++++++++++-------- 5 files changed, 113 insertions(+), 61 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index cd3c7fa1a7..e35fe467f8 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -610,6 +610,7 @@ wxMSW: - Fix drawing on wxDC when using right-to-left layout (Artur Wieczorek). - Fix wxGrid appearance and behaviour in RTL (Artur Wieczorek). - Fix creating wxBitmap from monochrome wxIcon or wxCursor (Artur Wieczorek). +- Fix handling of bitmaps with alpha in wxImageList (Artur Wieczorek). - Add paragraph spacing attributes support to wxTextCtrl (dannchr). - Show new style directory selector even for non existent paths (raychow). - Fix order of radial gradient stops (Alexandru Pana). diff --git a/include/wx/msw/bitmap.h b/include/wx/msw/bitmap.h index c677af8ca3..47d638a1dd 100644 --- a/include/wx/msw/bitmap.h +++ b/include/wx/msw/bitmap.h @@ -183,6 +183,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(); } diff --git a/src/msw/anybutton.cpp b/src/msw/anybutton.cpp index 189742d9e7..b5d6dc84c0 100644 --- a/src/msw/anybutton.cpp +++ b/src/msw/anybutton.cpp @@ -211,7 +211,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)) { diff --git a/src/msw/bitmap.cpp b/src/msw/bitmap.cpp index fe8bac5024..a0abcad045 100644 --- a/src/msw/bitmap.cpp +++ b/src/msw/bitmap.cpp @@ -339,6 +339,60 @@ wxGDIRefData *wxBitmap::CloneGDIRefData(const wxGDIRefData *data) const return new wxBitmapRefData(*static_cast(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) { @@ -392,48 +446,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 - // each pixel. + // alpha, so check for this. { - BITMAP bm; - if ( ::GetObject(iconInfo.hbmColor, sizeof(bm), &bm) && - (bm.bmBitsPixel == 32) ) + HBITMAP hdib = 0; + if ( CheckAlpha(iconInfo.hbmColor, &hdib) ) { - wxDIB dib(iconInfo.hbmColor); - if (dib.IsOk()) - { - 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(); - } - } + refData->m_hasAlpha = true; + ::DeleteObject(refData->m_hBitmap); + refData->m_hBitmap = hdib; } } break; @@ -1280,6 +1300,12 @@ bool wxBitmap::HasAlpha() const return GetBitmapData() && GetBitmapData()->m_hasAlpha; } +void wxBitmap::MSWUpdateAlpha() +{ + if ( CheckAlpha(GetHbitmap()) ) + GetBitmapData()->m_hasAlpha = true; +} + // ---------------------------------------------------------------------------- // wxBitmap setters // ---------------------------------------------------------------------------- diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index 7e100c32e0..37b308c7fd 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -374,7 +374,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); @@ -382,29 +381,50 @@ wxBitmap wxImageList::GetBitmap(int index) const wxMemoryDC dc; dc.SelectObject(bitmap); - // draw it the first time to find a suitable mask colour - ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT); - dc.SelectObject(wxNullBitmap); +#if wxUSE_WXDIB && wxUSE_IMAGE + IMAGEINFO ii; + 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 - wxImage image = bitmap.ConvertToImage(); - unsigned char r = 0, g = 0, b = 0; - image.FindFirstUnusedColour(&r, &g, &b); + // find the suitable mask colour + wxImage image = bitmap.ConvertToImage(); + unsigned char r = 0, g = 0, b = 0; + image.FindFirstUnusedColour(&r, &g, &b); - // redraw whole image and bitmap in the mask colour - image.Create(bmp_width, bmp_height); - image.Replace(0, 0, 0, r, g, b); - bitmap = wxBitmap(image); + // redraw whole image and bitmap in the mask colour + image.Create(bmp_width, bmp_height); + image.Replace(0, 0, 0, r, g, b); + bitmap = wxBitmap(image); - // redraw icon over the mask colour to actually draw it - dc.SelectObject(bitmap); - ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT); - dc.SelectObject(wxNullBitmap); + // redraw icon over the mask colour to actually draw it + dc.SelectObject(bitmap); + ((wxImageList*)this)->Draw(index, dc, 0, 0, wxIMAGELIST_DRAW_TRANSPARENT); + dc.SelectObject(wxNullBitmap); - // get the image, set the mask colour and convert back to get transparent bitmap - image = bitmap.ConvertToImage(); - image.SetMaskColour(r, g, b); - bitmap = wxBitmap(image); + // get the image, set the mask colour and convert back to get transparent bitmap + image = bitmap.ConvertToImage(); + image.SetMaskColour(r, g, b); + 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