Opened 4 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#19146 closed defect (fixed)

Windows monochrome icons/cursors may not be handled properly

Reported by: pb101 Owned by: Artur Wieczorek <artwik@…>
Priority: normal Milestone:
Component: wxMSW Version: dev-latest
Keywords: wxBitmap wxIcon wxCursor monochrome conversion Cc:
Blocked By: Blocking:
Patch: no

Description

(this ticket is based on a recent conversation on wxForum)

The Microsoft documentation states that monochrome icons are stored like this:

The icon bitmask bitmap. If this structure defines a black and white icon, this bitmask is formatted so that the upper half is the icon AND bitmask and the lower half is the icon XOR bitmask. Under this condition, the height should be an even multiple of two.

wxWidgets accounts for this in its wxBitmap::CopyFromIconOrCursor(); however, the result is different from what I would expect and what is drawn by ::DrawIconEx().

When I create a wxIcon from a monochrome HCURSOR (e.g., obtained by ::LoadCursor(NULL, MAKEINTRESOURCE(IDC_IBEAM))) and then create a wxBitmap from such a wxIcon and draw the two, the output looks incorrect to me. wxDC::DrawIcon() draws the icon stretched by two in height; wxDC::DrawBitmap() also draws the bitmap double in height but the upper half is transparent and the bottom half contains the inverted cursor on black.

I believe that this screenshot generated by the patched minimal sample (attached) shows the difference clearly, unlike the IBeam cursor the Arrow cursor is in color and thus drawn correctly in all cases:
https://i.imgur.com/vGOucxL.png

See also #16512.

BTW, when I tried to create a wxCursor from an HCURSOR and then creating a wxBitmap from it (wxBitmap::CopyFromCursor() and wxBitmap::IsOK() both returning true), the wxBitmap was not drawn by wxDC::DrawBitmap(). When trying to use such a wxBitmap with wxStaticBitmap, the app crashed when the control attempted to create a disabled bitmap.

Attachments (3)

mono-icon-minimal.diff download (2.9 KB) - added by pb101 4 weeks ago.
Patched minimal sample demonstrating the issue
mono-icon-drawing.png download (5.8 KB) - added by pb101 4 weeks ago.
Screenshot for the modified minimal sample
cursors.png download (7.0 KB) - added by awi 4 weeks ago.
Drawing cursors

Download all attachments as: .zip

Change History (11)

Changed 4 weeks ago by pb101

Patched minimal sample demonstrating the issue

Changed 4 weeks ago by pb101

Screenshot for the modified minimal sample

comment:1 in reply to: ↑ description Changed 4 weeks ago by pb101

  • Cc pbfordev@… added

I forgot to write that wxBitmap::CopyFromIconOrCursor() comments (and code) for handling monochrome icons for some reason wrongly assume that h is the actual height of the icon.

However, as described in the docs, it is actually double of that, as iconInfo.hbmMask contains two bitmasks stacked vertically, for example h is 64 for 32x32 icon.

I am not sure if monochrome icons can even be properly used in a wxBitmap, as a wxBitmap can store a transparent mask but not the cursor shape that is supposed to be drawn over XORed, i.e., inverted against its background, so it is always visible. However, perhaps a bit better job could be done, by accounting for doubled h.

And perhaps wxDC::DrawIcon() could be fixed entirely.

Last edited 4 weeks ago by pb101 (previous) (diff)

comment:2 Changed 4 weeks ago by vadz

  • Cc wsu@… added

Sorry, really no time to look at this right now, so I'm just cc'ing Bill Su who was the last person to work on this (or at least closely related) code, see 47078992c674e912c4f314ccaddbca659d81b218, in case he might have some ideas about it (and sorry for bothering you, Bill, if you don't).

All I can say right now is that wxBitmap could, in principle, support this because nothing prevents us from adding more fields to wxBitmapRefData, but this could require quite some work...

comment:3 Changed 4 weeks ago by awi

  • Cc wsu@… removed

The catch is that wxIcon::GetSize() for wxIcon created from monochrome cursor (iconBeam in your sample) returns 32x64 but the icon is actually of size 32x32.
The problem seems to be in the function wxGetHiconSize().

comment:4 Changed 4 weeks ago by Artur Wieczorek <artwik@…>

  • Owner set to Artur Wieczorek <artwik@…>
  • Resolution set to fixed
  • Status changed from new to closed

In b889f6897/git-wxWidgets:

Fix determining height of monochrome HICON in wxGetHiconSize()

For monochrome icons height reported by GetObject() is doubled because
icon bitmap contains both AND and XOR masks. To get actual icon height
we need to divide the value by 2.

Closes #19146.

comment:5 Changed 4 weeks ago by awi

And now it looks like this:
Drawing cursors

Changed 4 weeks ago by awi

Drawing cursors

comment:6 Changed 4 weeks ago by vadz

Thanks a lot for fixing this, Artur!

Do you think it merits a change log entry?

comment:7 follow-up: Changed 4 weeks ago by pb101

  • Cc pbfordev@… removed

Thanks for the replies and quick fix!

BTW, I assume there is a reason why a wxBitmap created from a monochrome HICON has the display depth?

comment:8 in reply to: ↑ 7 Changed 4 weeks ago by awi

Replying to pb101:

BTW, I assume there is a reason why a wxBitmap created from a monochrome HICON has the display depth?

At the time this code was developed there was a paradigm that wxBitmap can be only 24- or 32-bpp. Support for 1-bpp wxBitmaps was introduced recently in
7e9afad53a ("Add real support for monochrome bitmaps to wxMSW", 2020-10-19).

Note: See TracTickets for help on using tickets.