From 331840edb30e6d2ed55db51543fb06433784baf4 Mon Sep 17 00:00:00 2001 From: Artur Wieczorek Date: Wed, 13 Jan 2021 18:38:57 +0100 Subject: [PATCH] Fix replacing images in generic wxImageList Bitmaps stored in the list as a result of replacing existing ones should conform to the same constraints as bitmaps directly added to the list. These constraints are applied in the shared GetImageListBitmap() function called both on adding and replacing the images. --- src/generic/imaglist.cpp | 85 +++++++++++++++++-------------- tests/graphics/imagelist.cpp | 96 ++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 37 deletions(-) diff --git a/src/generic/imaglist.cpp b/src/generic/imaglist.cpp index 80b1c7b00a..f7b34cea71 100644 --- a/src/generic/imaglist.cpp +++ b/src/generic/imaglist.cpp @@ -52,41 +52,12 @@ bool wxGenericImageList::Create( int width, int height, bool mask, int WXUNUSED( return true; } -int wxGenericImageList::Add( const wxBitmap &bitmap ) +namespace +{ +wxBitmap GetImageListBitmap(const wxBitmap& bitmap, bool useMask) { - // We use the scaled, i.e. logical, size here as image list images size is - // specified in logical pixels, just as window coordinates and sizes are. - const wxSize bitmapSize = bitmap.GetScaledSize(); - - if ( m_size == wxSize(0, 0) ) - { - // This is the first time Add() is called and we hadn't had any fixed - // size: adopt the size of our first bitmap as image size. - m_size = bitmapSize; - } - else // We already have a fixed size, check that the bitmap conforms to it. - { - // There is a special case: a bitmap may contain more than one image, - // in which case we're supposed to chop it in parts, just as Windows - // ImageList_Add() does. - if ( bitmapSize.x > m_size.x && (bitmapSize.x % m_size.x == 0) ) - { - const int numImages = bitmapSize.x / m_size.x; - for (int subIndex = 0; subIndex < numImages; subIndex++) - { - wxRect rect(m_size.x * subIndex, 0, m_size.x, m_size.y); - Add(bitmap.GetSubBitmap(rect)); - } - - return GetImageCount() - 1; - } - - wxASSERT_MSG( bitmapSize == m_size, - "All bitmaps in wxImageList must have the same size" ); - } - wxBitmap bmp(bitmap); - if ( m_useMask ) + if ( useMask ) { if ( bmp.GetMask() ) { @@ -96,7 +67,7 @@ int wxGenericImageList::Add( const wxBitmap &bitmap ) // native-based wxMSW wxImageList where stored images are not allowed // to have both mask and alpha channel. #if wxUSE_IMAGE - wxImage img = bitmap.ConvertToImage(); + wxImage img = bmp.ConvertToImage(); img.ClearAlpha(); bmp = img; #endif // wxUSE_IMAGE @@ -143,6 +114,45 @@ int wxGenericImageList::Add( const wxBitmap &bitmap ) } } } + + return bmp; +} +}; + +int wxGenericImageList::Add( const wxBitmap &bitmap ) +{ + // We use the scaled, i.e. logical, size here as image list images size is + // specified in logical pixels, just as window coordinates and sizes are. + const wxSize bitmapSize = bitmap.GetScaledSize(); + + if ( m_size == wxSize(0, 0) ) + { + // This is the first time Add() is called and we hadn't had any fixed + // size: adopt the size of our first bitmap as image size. + m_size = bitmapSize; + } + else // We already have a fixed size, check that the bitmap conforms to it. + { + // There is a special case: a bitmap may contain more than one image, + // in which case we're supposed to chop it in parts, just as Windows + // ImageList_Add() does. + if ( bitmapSize.x > m_size.x && (bitmapSize.x % m_size.x == 0) ) + { + const int numImages = bitmapSize.x / m_size.x; + for (int subIndex = 0; subIndex < numImages; subIndex++) + { + wxRect rect(m_size.x * subIndex, 0, m_size.x, m_size.y); + Add(bitmap.GetSubBitmap(rect)); + } + + return GetImageCount() - 1; + } + + wxASSERT_MSG( bitmapSize == m_size, + "All bitmaps in wxImageList must have the same size" ); + } + + wxBitmap bmp = GetImageListBitmap(bitmap, m_useMask); m_images.push_back(bmp); return GetImageCount() - 1; @@ -202,10 +212,11 @@ wxGenericImageList::Replace(int index, if ( !DoGetPtr(index) ) return false; - m_images[index] = bitmap; - + wxBitmap bmp(bitmap); if ( mask.IsOk() ) - m_images[index].SetMask(new wxMask(mask)); + bmp.SetMask(new wxMask(mask)); + + m_images[index] = GetImageListBitmap(bmp, m_useMask); return true; } diff --git a/tests/graphics/imagelist.cpp b/tests/graphics/imagelist.cpp index 57bdefbf14..03828d94de 100644 --- a/tests/graphics/imagelist.cpp +++ b/tests/graphics/imagelist.cpp @@ -131,6 +131,54 @@ TEST_CASE("ImageList:WithMask", "[imagelist][withmask]") CHECK(icon1.GetWidth() == 32); CHECK(icon1.GetHeight() == 32); } + + SECTION("Replace with RGB image") + { + il.RemoveAll(); + int idx1 = il.Add(bmpRGBA); + CHECK(il.GetImageCount() == 1); + int idx2 = il.Add(bmpRGBAWithMask); + CHECK(il.GetImageCount() == 2); + + il.Replace(idx1, bmpRGB); + il.Replace(idx2, bmpRGBWithMask); + + wxBitmap bmp1 = il.GetBitmap(idx1); + CHECK(bmp1.HasAlpha() == false); + CHECK(bmp1.GetMask() != NULL); + CHECK(bmp1.GetWidth() == 32); + CHECK(bmp1.GetHeight() == 32); + + wxBitmap bmp2 = il.GetBitmap(idx2); + CHECK(bmp2.HasAlpha() == false); + CHECK(bmp2.GetMask() != NULL); + CHECK(bmp2.GetWidth() == 32); + CHECK(bmp2.GetHeight() == 32); + } + + SECTION("Replace with RGBA image") + { + il.RemoveAll(); + int idx1 = il.Add(bmpRGB); + CHECK(il.GetImageCount() == 1); + int idx2 = il.Add(bmpRGBWithMask); + CHECK(il.GetImageCount() == 2); + + il.Replace(idx1, bmpRGBA); + il.Replace(idx2, bmpRGBAWithMask); + + wxBitmap bmp1 = il.GetBitmap(idx1); + CHECK(bmp1.HasAlpha() == false); + CHECK(bmp1.GetMask() != NULL); + CHECK(bmp1.GetWidth() == 32); + CHECK(bmp1.GetHeight() == 32); + + wxBitmap bmp2 = il.GetBitmap(idx2); + CHECK(bmp2.HasAlpha() == false); + CHECK(bmp2.GetMask() != NULL); + CHECK(bmp2.GetWidth() == 32); + CHECK(bmp2.GetHeight() == 32); + } } TEST_CASE("ImageList:NoMask", "[imagelist][nomask]") @@ -240,4 +288,52 @@ TEST_CASE("ImageList:NoMask", "[imagelist][nomask]") CHECK(icon1.GetWidth() == 32); CHECK(icon1.GetHeight() == 32); } + + SECTION("Replace with RGB image") + { + il.RemoveAll(); + int idx1 = il.Add(bmpRGBA); + CHECK(il.GetImageCount() == 1); + int idx2 = il.Add(bmpRGBAWithMask); + CHECK(il.GetImageCount() == 2); + + il.Replace(idx1, bmpRGB); + il.Replace(idx2, bmpRGBWithMask); + + wxBitmap bmp1 = il.GetBitmap(idx1); + CHECK(bmp1.HasAlpha() == false); + CHECK(bmp1.GetMask() == NULL); + CHECK(bmp1.GetWidth() == 32); + CHECK(bmp1.GetHeight() == 32); + + wxBitmap bmp2 = il.GetBitmap(idx2); + CHECK(bmp2.HasAlpha() == true); + CHECK(bmp2.GetMask() == NULL); + CHECK(bmp2.GetWidth() == 32); + CHECK(bmp2.GetHeight() == 32); + } + + SECTION("Replace with RGBA image") + { + il.RemoveAll(); + int idx1 = il.Add(bmpRGB); + CHECK(il.GetImageCount() == 1); + int idx2 = il.Add(bmpRGBWithMask); + CHECK(il.GetImageCount() == 2); + + il.Replace(idx1, bmpRGBA); + il.Replace(idx2, bmpRGBAWithMask); + + wxBitmap bmp1 = il.GetBitmap(idx1); + CHECK(bmp1.HasAlpha() == true); + CHECK(bmp1.GetMask() == NULL); + CHECK(bmp1.GetWidth() == 32); + CHECK(bmp1.GetHeight() == 32); + + wxBitmap bmp2 = il.GetBitmap(idx2); + CHECK(bmp2.HasAlpha() == true); + CHECK(bmp2.GetMask() == NULL); + CHECK(bmp2.GetWidth() == 32); + CHECK(bmp2.GetHeight() == 32); + } }