From 054972e56ba84b36d47139189e56251257f1e118 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 4 Jun 2022 23:02:49 +0100 Subject: [PATCH 1/2] Fix adding icons with mask to wxMSW wxImageList The changes of 6feeed9fe9 (Handle transparency to the best of our ability in wxImageList, 2022-05-05) didn't handle the icons with mask (and without alpha) correctly, as the native image list doesn't handle the icon masks without ILC_MASK, so the masks were just ignored. Fix this by handling icons as bitmaps: for them everything already works and even it means extra unneeded conversions, it's much simpler than the alternative of reimplementing GetImageListBitmaps() logic for the icons. See #22349. --- src/msw/imaglist.cpp | 48 ++++++++------------------------------------ 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/src/msw/imaglist.cpp b/src/msw/imaglist.cpp index 21a32958b1..bccbbd1527 100644 --- a/src/msw/imaglist.cpp +++ b/src/msw/imaglist.cpp @@ -328,26 +328,11 @@ int wxImageList::Add(const wxBitmap& bitmap, const wxColour& maskColour) // Adds a bitmap and mask from an icon. int wxImageList::Add(const wxIcon& icon) { - // ComCtl32 prior 6.0 doesn't support images with alpha - // channel so if we have 32-bit icon with transparency - // we need to add it as a wxBitmap via dedicated method - // where alpha channel will be converted to the mask. - if ( wxApp::GetComCtl32Version() < 600 ) - { - wxBitmap bmp(icon); - if ( bmp.HasAlpha() ) - { - return Add(bmp); - } - } - - int index = ImageList_AddIcon(GetHImageList(), GetHiconOf(icon)); - if ( index == -1 ) - { - wxLogError(_("Couldn't add an image to the image list.")); - } - - return index; + // We don't use ImageList_AddIcon() here as this only works for icons with + // masks when using ILC_MASK, which we usually don't do, so reuse the + // bitmap function instead -- even if it's slightly less efficient due to + // extra conversions, it's simpler than handling all the various cases here. + return Add(wxBitmap(icon)); } // Replaces a bitmap, optionally passing a mask bitmap. @@ -372,26 +357,9 @@ bool wxImageList::Replace(int index, // Replaces a bitmap and mask from an icon. bool wxImageList::Replace(int i, const wxIcon& icon) { - // ComCtl32 prior 6.0 doesn't support images with alpha - // channel so if we have 32-bit icon with transparency - // we need to replace it as a wxBitmap via dedicated method - // where alpha channel will be converted to the mask. - if ( wxApp::GetComCtl32Version() < 600 ) - { - wxBitmap bmp(icon); - if ( bmp.HasAlpha() ) - { - return Replace(i, bmp); - } - } - - bool ok = ImageList_ReplaceIcon(GetHImageList(), i, GetHiconOf(icon)) != -1; - if ( !ok ) - { - wxLogLastError(wxT("ImageList_ReplaceIcon()")); - } - - return ok; + // Same as in Add() above, just reuse the existing function for simplicity + // even if it means an extra conversion. + return Replace(i, wxBitmap(icon)); } // Removes the image at the given index. From e1b3d93f5a4cbd9399c93a7b9a51e08aff1fe258 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 4 Jun 2022 23:10:10 +0100 Subject: [PATCH 2/2] Allow upscaling 16x15 art bitmaps to multiples of 16x16 too The default art provider has many icons of 16x15 size, suitable for use in the standard-sized MSW toolbars (which nobody uses any more, but this is not the point), so handle them specially and not only resize them to 16x16, which was already done as part of the normal logic here (in the past this used to be handled specially too, but bd0db4a5f9 (Still resize (16, 15) bitmaps to (16, 16) ones in wxArtProvider, 2022-02-05) changed this), but also upscale them to 32x32 and other multiple-of-16 sizes by resizing them to 16x16 first and then upscaling. This makes the appearance of 16x15 and 16x16 icons consistent with each other when showing them in bigger sizes in the resource browser dialog of the artprov sample, which seems to be the right thing to do. This commit is best viewed with Git --color-moved --color-moved-ws=ignore-all-space options. --- src/common/artprov.cpp | 67 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/src/common/artprov.cpp b/src/common/artprov.cpp index 1cde929a62..6b88efb91f 100644 --- a/src/common/artprov.cpp +++ b/src/common/artprov.cpp @@ -304,6 +304,30 @@ void wxArtProvider::RescaleBitmap(wxBitmap& bmp, const wxSize& sizeNeeded) } #endif // WXWIN_COMPATIBILITY_3_0 +#if wxUSE_IMAGE + +namespace +{ + +bool CanUpscaleByInt(int w, int h, const wxSize& sizeNeeded) +{ + return !(sizeNeeded.x % w) && !(sizeNeeded.y % h); +} + +void ExtendBitmap(wxBitmap& bmp, const wxSize& sizeNeeded) +{ + const wxSize size = bmp.GetSize(); + + wxPoint offset((sizeNeeded.x - size.x)/2, (sizeNeeded.y - size.y)/2); + wxImage img = bmp.ConvertToImage(); + img.Resize(sizeNeeded, offset); + bmp = wxBitmap(img); +} + +} // anonymous namespace + +#endif // wxUSE_IMAGE + void wxArtProvider::RescaleOrResizeIfNeeded(wxBitmap& bmp, const wxSize& sizeNeeded) { @@ -317,25 +341,42 @@ wxArtProvider::RescaleOrResizeIfNeeded(wxBitmap& bmp, const wxSize& sizeNeeded) return; #if wxUSE_IMAGE - // Allow upscaling by an integer factor: this looks not too horribly and is - // needed to use reasonably-sized bitmaps in the code not yet updated to - // use wxBitmapBundle but using custom art providers. - if ((bmp_w <= sizeNeeded.x) && (bmp_h <= sizeNeeded.y) && - (sizeNeeded.x % bmp_w || sizeNeeded.y % bmp_h)) + // Check if we need to increase or decrease the image size (mixed case is + // handled as decreasing). + if ((bmp_w <= sizeNeeded.x) && (bmp_h <= sizeNeeded.y)) { + // Allow upscaling by an integer factor: this looks not too horribly and is + // needed to use reasonably-sized bitmaps in the code not yet updated to + // use wxBitmapBundle but using custom art providers. + bool shouldUpscale = CanUpscaleByInt(bmp_w, bmp_h, sizeNeeded); + + // And account for the common case of 16x15 bitmaps used for many wxMSW + // images: those can be resized to 16x16 and then upscaled if possible + // (and if 16x16 is the required size, there is no need to upscale, so + // don't handle this sub-case specially at all). + if (!shouldUpscale && bmp_w == 16 && bmp_h == 15 && sizeNeeded.y != 16) + { + // If we can't upscale it with its current height, perhaps we can + // if we resize it to 16 first? + if (CanUpscaleByInt(bmp_w, 16, sizeNeeded)) + { + ExtendBitmap(bmp, wxSize(16, 16)); + shouldUpscale = true; + } + } + // the caller wants default size, which is larger than // the image we have; to avoid degrading it visually by // scaling it up, paste it into transparent image instead: - wxPoint offset((sizeNeeded.x - bmp_w)/2, (sizeNeeded.y - bmp_h)/2); - wxImage img = bmp.ConvertToImage(); - img.Resize(sizeNeeded, offset); - bmp = wxBitmap(img); + if (!shouldUpscale) + { + ExtendBitmap(bmp, sizeNeeded); + return; + } } - else // scale (down or mixed, but not up, or at least not by an int factor) #endif // wxUSE_IMAGE - { - wxBitmap::Rescale(bmp, sizeNeeded); - } + + wxBitmap::Rescale(bmp, sizeNeeded); } /*static*/ wxBitmap wxArtProvider::GetBitmap(const wxArtID& id,