Opened 5 years ago

Last modified 8 weeks ago

#11476 portneeded defect (port to stable)

wxMSW: owner drawn wxButtons with bitmap mangle alpha channel

Reported by: pete_b Owned by:
Priority: low Milestone:
Component: wxMSW Version: 2.9.0
Keywords: wxButton bitmap alpha Cc:
Blocked By: Blocking:
Patch: yes

Description

Using a wxButton with text AND a wxBitmap label on MSW causes bitmap to lose its alpha channel.

Button is fine if only the bitmap is displayed without text.

If you always use owner draw when the button has a bitmap then its fine.

Attachments (3)

icon_cross.png download (510 bytes) - added by pete_b 5 years ago.
xp.png download (4.3 KB) - added by vadz 4 years ago.
Screenshot under XP
buttons_owner_drawn_alpa_bitmap.patch download (4.0 KB) - added by awi 11 months ago.
Fix to drawing bitmaps with alpha channel for owner drawn buttons

Download all attachments as: .zip

Change History (17)

comment:1 in reply to: ↑ description Changed 5 years ago by vadz

Replying to pete_b:

Using a wxButton with text AND a wxBitmap label on MSW causes bitmap to lose its alpha channel.

Do you have an example bitmap showing this (even if this happens with any bitmaps with alpha it would still be nice to have one for testing)?

If you always use owner draw when the button has a bitmap then its fine.

Sorry, what do you mean? I.e. how do you "always use owner draw"?

Also, what Windows version do you use? Are themes enabled or not?

Changed 5 years ago by pete_b

comment:2 Changed 5 years ago by pete_b

Attached image ends up with black outline in areas where alpha channel is a partial value (where the edges are antialiased).

I've copied wxButtonBase and just removed the usage of wxXPButtonImageData in DoGetBitmap() and it works.

I'm using winxp sp2.

comment:3 follow-up: Changed 4 years ago by vadz

  • Status changed from new to infoneeded_new

I don't see the black outline with the following patch to the minimal sample under XP:

  • samples/minimal/minimal.cpp

    diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp
    index fb8f721..277d95e 100644
    a b MyFrame::MyFrame(const wxString& title) 
    172172    CreateStatusBar(2); 
    173173    SetStatusText("Welcome to wxWidgets!"); 
    174174#endif // wxUSE_STATUSBAR 
     175 
     176    new wxStaticText(this, -1, "", wxPoint(100, 100)); 
     177    wxButton *b = new wxButton(this, wxID_OK); 
     178    wxImage::AddHandler(new wxPNGHandler); 
     179    b->SetBitmap(wxImage("icon_cross.png")); 
    175180} 
    176181 
    177182 

There are some grey pixels, see the attached (zoomed) screenshot but nothing obviously wrong. Do you see the same thing as I do?

OTOH I see something really weird under Win7: the image pulses on its own, i.e. it keeps appearing and disappearing. I have really no idea why does this happen though...

Changed 4 years ago by vadz

Screenshot under XP

comment:4 in reply to: ↑ 3 Changed 22 months ago by catalin

  • Status changed from infoneeded_new to new

This does not happen under Win7 with r73482.

Replying to vadz:

OTOH I see something really weird under Win7: the image pulses on its own, i.e. it keeps appearing and disappearing. I have really no idea why does this happen though...

See #15034

comment:5 Changed 22 months ago by vadz

  • Resolution set to worksforme
  • Status changed from new to closed

comment:6 Changed 11 months ago by HansR

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I've found an easy way to reproduce this problem: call SetLabelMarkup(). This implicitly calls MakeOwnerDrawn(), causing the ugly borders where nicely alpha-blended edges should be.

So, add the following after line 179 in vadz's minimal.cpp test patch above:
b->SetLabelMarkup("");

comment:7 Changed 11 months ago by vadz

  • Priority changed from normal to low
  • Status changed from reopened to confirmed
  • Summary changed from wxMSW: wxButton with bitmap and text, bitmap loses alpha channel. to wxMSW: owner drawn wxButtons with bitmap mangle alpha channel

This is another problem due to bad interaction between image lists and alpha... we basically lose alpha right now when adding the bitmap to the image list.

A simple fix would be

  • src/msw/anybutton.cpp

    diff --git a/src/msw/anybutton.cpp b/src/msw/anybutton.cpp
    index b99e2f6..875993f 100644
    a b void wxAnyButton::DoSetBitmap(const wxBitmap& bitmap, State which) 
    707707        // (even if we use BUTTON_IMAGELIST_ALIGN_CENTER alignment and 
    708708        // BS_BITMAP style), at least under Windows 2003 so use owner drawn 
    709709        // strategy for bitmap-only buttons 
    710         if ( ShowsLabel() && wxUxThemeEngine::GetIfActive() ) 
     710        if ( ShowsLabel() && wxUxThemeEngine::GetIfActive() && 
     711                !bitmap.HasAlpha() ) 
    711712        { 
    712713            m_imageData = new wxXPButtonImageData(this, bitmap); 
    713714 

but this is not ideal as we sacrifice the native appearance.

I don't know what's going on with the image lists... We already try to adjust to ImageList_Draw() way of doing things in wxImageList::Add() but apparently we still don't understand how does it really work.

Changed 11 months ago by awi

Fix to drawing bitmaps with alpha channel for owner drawn buttons

comment:8 Changed 11 months ago by awi

  • Patch set

The problem with improper drawing of custom bitmaps having transparency channel is a result of applying masks when such a bitmaps are drawn.

Masks stored in image list are intended to simulate transparency for RGB images without transparency data:
"When a masked image is drawn, the bits of the image are combined with the bits of the mask, typically producing transparent areas in the bitmap where the background color of the target device context shows through."
http://msdn.microsoft.com/en-us/library/windows/desktop/bb761389%28v=vs.85%29.aspx

If bitmap already has a transparency data there is no need to simulate transparency effect with separate mask. Masks are useful for bitmaps without alpha channel.

Changes made:

  1. Mask is determined and stored in the image list only if bitmap has no alpha channel.
  2. When owner drawn process for button (wxAnyButton::MSWOnDraw) is executed the bitmap with alpha channel (having no mask) is simply drawn onto background (in wxImageList::GetBitmap).

Patch file attached.

Tested on 3.0 (trunk).

comment:9 follow-up: Changed 11 months ago by vadz

This seems to work, thanks, but I still don't understand something here: why do we need

        // get the image
        wxImage image = bitmap.ConvertToImage();
        bitmap = wxBitmap(image);

in wxImageList::GetBitmap()? What does this do exactly?

Also, could we perhaps avoid drawing the bitmap at all and just copy the hbmImage we get from ImageList_GetImageInfo()?

comment:10 in reply to: ↑ 9 ; follow-up: Changed 11 months ago by awi

Replying to vadz:

This seems to work, thanks, but I still don't understand something here: why do we need

        // get the image
        wxImage image = bitmap.ConvertToImage();
        bitmap = wxBitmap(image);

in wxImageList::GetBitmap()? What does this do exactly?

Also, could we perhaps avoid drawing the bitmap at all and just copy the hbmImage we get from ImageList_GetImageInfo()?

This conversion back and forth between wxBitmap and wxImage is used to determine in a comprehensive way if bitmap has a real alpha channel (including checks if all alpha bytes are 00 or FF) so we can be sure that "Has Alpha" flag for this bitmap is set correctly.
I don't know any other/better/faster way to do so.

Regarding using hbmImage handle directly: I think we should let the image list to draw an image in the native way (using ImageList_Draw) in order to preserve its native quirks (maybe there are any).

comment:11 in reply to: ↑ 10 Changed 11 months ago by vadz

Replying to awi:

This conversion back and forth between wxBitmap and wxImage is used to determine in a comprehensive way if bitmap has a real alpha channel (including checks if all alpha bytes are 00 or FF) so we can be sure that "Has Alpha" flag for this bitmap is set correctly.
I don't know any other/better/faster way to do so.

We can at least use wxDIB here, which only requires one conversion, rather than 3 (if I count correctly) that using wxImage does. Of course, image list elements are typically small, so it's perhaps not such a big deal, but I'll change this just in case.

BTW, I've also noticed that conversion to wxImage and back slightly changed the values of some pixels (by 1, so I suspect a rounding error when premultiplying somewhere...). This looks like yet another bug in this code :-(

Regarding using hbmImage handle directly: I think we should let the image list to draw an image in the native way (using ImageList_Draw) in order to preserve its native quirks (maybe there are any).

I don't see any difference compared to using CopyImage() but then it probably just does a BitBlt() internally anyhow, so let's keep your version, I agree that it might behave differently in some cases I didn't test.

Thanks again for looking at this!

comment:12 Changed 11 months ago by VZ

(In [75567]) 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.

comment:13 Changed 11 months ago by vadz

  • Resolution set to port to stable
  • Status changed from confirmed to portneeded

If no problems are found with this, it should be backported to 3.0 branch as the changes are ABI compatible (we just need wxABI_VERSION check around MSWUpdateAlpha()).

comment:14 Changed 8 weeks ago by VZ

In 77947:

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.]

Note: See TracTickets for help on using tickets.