From 7d92f321c7c8b0422f330c995f804a9639339f7b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 22:38:25 +0000 Subject: [PATCH 1/6] Add wxImageList::Init() to wxMSW version Initialize all fields in a single place, just as it's done in most of the other classes. No real changes. --- include/wx/msw/imaglist.h | 12 ++++++++++-- src/msw/imaglist.cpp | 6 ------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/wx/msw/imaglist.h b/include/wx/msw/imaglist.h index fde871c32e..404d8aadd0 100644 --- a/include/wx/msw/imaglist.h +++ b/include/wx/msw/imaglist.h @@ -24,7 +24,7 @@ public: * Public interface */ - wxImageList(); + wxImageList() { Init(); } // Creates an image list. // Specify the width and height of the images in the list, @@ -32,7 +32,7 @@ public: // from icons), and the initial size of the list. wxImageList(int width, int height, bool mask = true, int initialCount = 1) { - m_hImageList = NULL; + Init(); Create(width, height, mask, initialCount); } virtual ~wxImageList(); @@ -198,8 +198,16 @@ public: protected: WXHIMAGELIST m_hImageList; wxSize m_size; + +private: bool m_useMask; + void Init() + { + m_hImageList = NULL; + m_useMask = false; + } + wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxImageList); }; diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index de3b4fe564..c52c390065 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -64,12 +64,6 @@ static HBITMAP GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask); // wxImageList creation/destruction // ---------------------------------------------------------------------------- -wxImageList::wxImageList() - : m_hImageList(NULL) - , m_useMask(false) -{ -} - // Creates an image list bool wxImageList::Create(int width, int height, bool mask, int initial) { From 5fc5e5a734f1a45c8f5480dda586289f2c7279e6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 21:59:51 +0000 Subject: [PATCH 2/6] Simplify initializing wxImageList::m_useMask Set it to true at the same time as turning ILC_MASK on instead of checking for this flag separately later. No real changes. --- src/msw/imaglist.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index c52c390065..f76c3d4603 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -82,7 +82,10 @@ bool wxImageList::Create(int width, int height, bool mask, int initial) // For comctl32.dll < 6 always use masks as it doesn't support alpha. if ( mask || wxApp::GetComCtl32Version() < 600 ) + { + m_useMask = true; flags |= ILC_MASK; + } // Grow by 1, I guess this is reasonable behaviour most of the time m_hImageList = (WXHIMAGELIST) ImageList_Create(width, height, flags, @@ -92,7 +95,6 @@ bool wxImageList::Create(int width, int height, bool mask, int initial) wxLogLastError(wxT("ImageList_Create()")); } - m_useMask = (flags & ILC_MASK) != 0; return m_hImageList != 0; } From 436c92f8348a0e36670d90db54d6d02bfcf42399 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 22:00:50 +0000 Subject: [PATCH 3/6] Remove extraneous semicolon after namespace closing brace No real changes. --- src/msw/imaglist.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index f76c3d4603..3bc42acd4d 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -209,7 +209,7 @@ void GetImageListBitmaps(const wxBitmap& bitmap, const wxBitmap& mask, bool useM hbmp = GetHbitmapOf(bitmap); #endif // wxUSE_WXDIB && wxUSE_IMAGE } -}; +} // anonymous namespace // Adds a bitmap, and optionally a mask bitmap. // Note that wxImageList creates new bitmaps, so you may delete From 689ce0a1383e77ac4d5b18f56f3e597743dc1cdb Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 22:42:02 +0000 Subject: [PATCH 4/6] Always use alpha if it's defined in wxMSW wxImageList Use the alpha channel of bitmaps added to wxImageList even if it was created with "mask = true" ctor parameter (which is the default value). To avoid using both mask and alpha, which is not supported by the native image list, convert mask to alpha if we have it and either we also have alpha or we are not using mask at all. This restores the behaviour until the changes of 8f08233a13 (Fix adding wxBitmap with mask to wxImageList not supporting masks (wxMSW), 2021-01-12), fixes compatibility with the other ports and, last but not least, is the expected and actually useful behaviour: alpha shouldn't be ignored if it exists in the bitmap. Closes #22021. --- src/msw/imaglist.cpp | 92 ++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 59 deletions(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index 3bc42acd4d..7fb5ac72cf 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -142,72 +142,46 @@ void GetImageListBitmaps(const wxBitmap& bitmap, const wxBitmap& mask, bool useM AutoHBITMAP& hbmpRelease, AutoHBITMAP& hbmpMask, HBITMAP& hbmp) { #if wxUSE_WXDIB && wxUSE_IMAGE - // wxBitmap normally stores alpha in pre-multiplied format but - // ImageList_Draw() does pre-multiplication internally so we need to undo - // the pre-multiplication here. Converting back and forth like this is, of - // course, very inefficient but it's better than wrong appearance so we do - // this for now until a better way can be found. - if ( useMask ) + // We can only use directly bitmaps without alpha and without mask unless + // the image list uses masks and need to modify bitmap in all the other + // cases, so check if this is necessary. + if ( bitmap.HasAlpha() || (!useMask && (mask.IsOk() || bitmap.GetMask())) ) { - if ( bitmap.HasAlpha() ) + wxBitmap bmp(bitmap); + + if ( mask.IsOk() || bmp.GetMask() ) { - // Remove alpha channel from image to prevent - // possible interferences with the mask. - // The bitmap isn't drawn correctly if we use both. - wxImage img = bitmap.ConvertToImage(); - img.ClearAlpha(); - hbmp = wxDIB(img, wxDIB::PixelFormat_NotPreMultiplied).Detach(); - hbmpRelease.Init(hbmp); - } - else - { - hbmp = GetHbitmapOf(bitmap); + // Explicitly specified mask overrides the mask associated with the + // bitmap, if any. + if ( mask.IsOk() ) + bmp.SetMask(new wxMask(mask)); + + // Get rid of the mask by converting it to alpha. + if ( bmp.HasAlpha() ) + bmp.MSWBlendMaskWithAlpha(); } - hbmpMask.Init(GetMaskForImage(bitmap, mask)); + // wxBitmap normally stores alpha in pre-multiplied format but + // ImageList_Draw() does pre-multiplication internally so we need to undo + // the pre-multiplication here. Converting back and forth like this is, of + // course, very inefficient but it's better than wrong appearance so we do + // this for now until a better way can be found. + wxImage img = bmp.ConvertToImage(); + if ( !img.HasAlpha() ) + img.InitAlpha(); + hbmp = wxDIB(img, wxDIB::PixelFormat_NotPreMultiplied).Detach(); + hbmpRelease.Init(hbmp); + + // In any case we'll never use mask at the native image list level as + // it's incompatible with alpha and we need to use alpha. } else - { - if ( bitmap.HasAlpha() ) - { - wxBitmap bmp(bitmap); - if ( mask.IsOk() || bmp.GetMask() ) - { - // Blend mask with alpha channel. - if ( mask.IsOk() ) - { - bmp.SetMask(new wxMask(mask)); - } - bmp.MSWBlendMaskWithAlpha(); - } - wxImage img = bmp.ConvertToImage(); - hbmp = wxDIB(img, wxDIB::PixelFormat_NotPreMultiplied).Detach(); - hbmpRelease.Init(hbmp); - } - else - { - if ( mask.IsOk() || bitmap.GetMask() ) - { - // Convert mask to alpha channel. - wxBitmap bmp(bitmap); - if ( mask.IsOk() ) - { - bmp.SetMask(new wxMask(mask)); - } - wxImage img = bmp.ConvertToImage(); - img.InitAlpha(); - hbmp = wxDIB(img, wxDIB::PixelFormat_NotPreMultiplied).Detach(); - hbmpRelease.Init(hbmp); - } - else - { - hbmp = GetHbitmapOf(bitmap); - } - } - } -#else - hbmp = GetHbitmapOf(bitmap); #endif // wxUSE_WXDIB && wxUSE_IMAGE + { + hbmp = GetHbitmapOf(bitmap); + if ( useMask ) + hbmpMask.Init(GetMaskForImage(bitmap, mask)); + } } } // anonymous namespace From 9e683e01379f7c9d32a5caccc489bf1d1787b554 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 22:46:02 +0000 Subject: [PATCH 5/6] Remove code not used any more from GetMaskForImage() helper This function is never called for the bitmaps with alpha now, so don't bother converting it to mask in it. --- src/msw/imaglist.cpp | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index 7fb5ac72cf..9010d9d4d8 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -458,10 +458,6 @@ wxIcon wxImageList::GetIcon(int index) const static HBITMAP GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask) { -#if wxUSE_IMAGE - wxBitmap bitmapWithMask; -#endif // wxUSE_IMAGE - HBITMAP hbmpMask; wxMask *pMask; bool deleteMask = false; @@ -474,23 +470,6 @@ static HBITMAP GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask) else { pMask = bitmap.GetMask(); - -#if wxUSE_IMAGE - // check if we don't have alpha in this bitmap -- we can create a mask - // from it (and we need to do it for the older systems which don't - // support 32bpp bitmaps natively) - if ( !pMask ) - { - wxImage img(bitmap.ConvertToImage()); - if ( img.HasAlpha() ) - { - img.ConvertAlphaToMask(); - bitmapWithMask = wxBitmap(img); - pMask = bitmapWithMask.GetMask(); - } - } -#endif // wxUSE_IMAGE - if ( !pMask ) { // use the light grey count as transparent: the trouble here is From 4c0269b0d1820ee89de0292141c8119d878fad4a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 6 Feb 2022 22:50:04 +0000 Subject: [PATCH 6/6] Simplify code in GetMaskForImage() by using wxScopedPtr Use smart pointer to cleanup the temporary mask, if any, rather than tracking it manually. No real changes. --- src/msw/imaglist.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index 9010d9d4d8..3121f4ea20 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -36,6 +36,7 @@ #include "wx/imaglist.h" #include "wx/dc.h" +#include "wx/scopedptr.h" #include "wx/msw/dc.h" #include "wx/msw/dib.h" #include "wx/msw/private.h" @@ -459,17 +460,15 @@ wxIcon wxImageList::GetIcon(int index) const static HBITMAP GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask) { HBITMAP hbmpMask; - wxMask *pMask; - bool deleteMask = false; + wxScopedPtr maskDeleter; if ( mask.IsOk() ) { hbmpMask = GetHbitmapOf(mask); - pMask = NULL; } else { - pMask = bitmap.GetMask(); + wxMask* pMask = bitmap.GetMask(); if ( !pMask ) { // use the light grey count as transparent: the trouble here is @@ -481,19 +480,12 @@ static HBITMAP GetMaskForImage(const wxBitmap& bitmap, const wxBitmap& mask) pMask = new wxMask(bitmap, col); - deleteMask = true; + maskDeleter.reset(pMask); } hbmpMask = (HBITMAP)pMask->GetMaskBitmap(); } // windows mask convention is opposite to the wxWidgets one - HBITMAP hbmpMaskInv = wxInvertMask(hbmpMask); - - if ( deleteMask ) - { - delete pMask; - } - - return hbmpMaskInv; + return wxInvertMask(hbmpMask); }