View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0036490||Lazarus||Widgetset||public||2019-12-28 18:15||2020-01-14 15:57|
|Reporter||Denis Kozlov||Assigned To||Ondrej Pokorny|
|Product Version||2.1 (SVN)||Product Build|
|Target Version||Fixed in Version|
|Summary||0036490: Bitmap transparency (mask) not respected by TMenuItem|
|Description||The drawing of the menu item icon on Windows platform does not respect the bitmap transparency (mask) properties.|
For example, assigning `TMenuItem.Bitmap` to an image with `Transparent=False` (i.e. no mask) will draw an image with the bottom left pixel used as mask, which is logically backwards and breaks backwards compatibility too.
Affects both 2.0 and TRUNK branches.
My suggestion is to respect the `Transparent` and `TransparentColor` properties of the bitmap.
The attached patch achieves exactly that.
|Additional Information||This issue was introduced in revision 57227:|
|Tags||No tags attached.|
|Fixed in Revision||62551|
Win32WSMenus.diff (841 bytes)
Index: lcl/interfaces/win32/win32wsmenus.pp =================================================================== --- lcl/interfaces/win32/win32wsmenus.pp (revision 62451) +++ lcl/interfaces/win32/win32wsmenus.pp (working copy) @@ -1122,8 +1122,8 @@ AImageList := TImageList.Create(nil); AImageList.Width := AMenuItem.Bitmap.Width; // maybe height to prevent too wide bitmaps? AImageList.Height := AMenuItem.Bitmap.Height; - if not AMenuItem.Bitmap.Transparent then - AImageIndex := AImageList.AddMasked(AMenuItem.Bitmap, AMenuItem.Bitmap.Canvas.Pixels[0, AImageList.Height-1]) + if AMenuItem.Bitmap.Transparent then + AImageIndex := AImageList.AddMasked(AMenuItem.Bitmap, AMenuItem.Bitmap.TransparentColor) else AImageIndex := AImageList.Add(AMenuItem.Bitmap, nil); FreeImageList := True;
Win32WSMenus.diff (841 bytes)
You are forgetting about TransparentMode……..
That makes your difference...
Auto will reference the lower corner, while manual will use the specific TransparentColor specified..
Transparent just simply indicates that it is to be drawn transparently
What you are purposing will most likely make a mess of things because for one, I don't see where you specified the TransparentColor..
I do believe and I maybe wrong but the default value of TransparentMode = tmAuto;
that gets the color from the lower left color..
So I think you should rethink your steps and try again, the original code is working as designed..
> You are forgetting about TransparentMode
Not exactly. There is overlap and conflict between TransparentMode and TransparentColor, where if the latter is set to clDefault it will too resolve to the bottom left pixel. An extra check can be added for handling TransparentMode=tmAuto, but it is not clear which of the two conflicting properties should have precedence (TransparentMode vs TransparentColor).
> I don't see where you specified the TransparentColor
Perhaps you should take a look at the patch more carefully.
> the original code is working as designed
Wrong. The original code uses only 1 of the 3 related properties, and I believe that 1 property is also used illogically. The older code forces transparency when and only when transparency was explicitly disabled (hence, illogical), also completely ignoring both TransparentMode and TransparentColor properties.
P.S. Do you know the difference between constructive and destructive criticism?
"P.S. Do you know the difference between constructive and destructive criticism? "
Do you know the difference between something working for everyone else? And for how long now ?
Since the code you provided didn't even look at the fact what the Transparentmode was doing its obvious you had a better idea. I think we already have too many better ideas flying around here lately.., many of them not really going anywhere. Others causing death and destruction as being witness to them here.
Its been a long time standard using the lower left corner for the mask color. If you are having issues then maybe you should dig a littler deeper..
Btw, If these images are 32 bit formats, they won't draw transparently anyways due to an issue in the Tbitmap alpha mode that never got completed so currently 32 bit images are a no go for transparent drawing unless you do it yourself, like I did.
I just hope someone here comes to their senses, because I am starting to lose mine.
An updated patch is attached which respects the TransparentMode property, where it takes precedence over the TransparentColor property.
Win32WSMenus.2.diff (1,363 bytes)
Index: lcl/interfaces/win32/win32wsmenus.pp =================================================================== --- lcl/interfaces/win32/win32wsmenus.pp (revision 62451) +++ lcl/interfaces/win32/win32wsmenus.pp (working copy) @@ -1114,6 +1114,7 @@ AImageList: TCustomImageList; FreeImageList: Boolean; AImageIndex, AImagesWidth: Integer; + ATransparentColor: TColor; APPI: longint; begin AMenuItem.GetImageList(AImageList, AImagesWidth); @@ -1122,10 +1123,18 @@ AImageList := TImageList.Create(nil); AImageList.Width := AMenuItem.Bitmap.Width; // maybe height to prevent too wide bitmaps? AImageList.Height := AMenuItem.Bitmap.Height; - if not AMenuItem.Bitmap.Transparent then - AImageIndex := AImageList.AddMasked(AMenuItem.Bitmap, AMenuItem.Bitmap.Canvas.Pixels[0, AImageList.Height-1]) + if AMenuItem.Bitmap.Transparent then + begin + if AMenuItem.Bitmap.TransparentMode = tmAuto then + ATransparentColor := AMenuItem.Bitmap.Canvas.Pixels[0, AImageList.Height-1] + else + ATransparentColor := AMenuItem.Bitmap.TransparentColor; + AImageIndex := AImageList.AddMasked(AMenuItem.Bitmap, ATransparentColor); + end else + begin AImageIndex := AImageList.Add(AMenuItem.Bitmap, nil); + end; FreeImageList := True; end else // using icon from ImageList
Win32WSMenus.2.diff (1,363 bytes)
||Ondrej, what do you think about the latest patch?|
||Yes. I'll take a look. I don't remember exactly the original issue anymore. But I will decide somehow.|
||Thank you for the patch. My revision 57227 was wrong.|
|2019-12-28 18:15||Denis Kozlov||New Issue|
|2019-12-28 18:15||Denis Kozlov||File Added: Win32WSMenus.diff|
|2019-12-28 19:40||jamie philbrook||Note Added: 0120114|
|2019-12-28 22:24||Juha Manninen||Relationship added||related to 0033099|
|2019-12-29 01:01||Denis Kozlov||Note Added: 0120120|
|2019-12-29 03:19||jamie philbrook||Note Added: 0120121|
|2019-12-29 11:14||Denis Kozlov||File Added: Win32WSMenus.2.diff|
|2019-12-29 11:14||Denis Kozlov||Note Added: 0120124|
|2020-01-11 11:29||Juha Manninen||Assigned To||=> Ondrej Pokorny|
|2020-01-11 11:29||Juha Manninen||Status||new => assigned|
|2020-01-11 11:29||Juha Manninen||Note Added: 0120323|
|2020-01-13 11:28||Ondrej Pokorny||Note Added: 0120397|
|2020-01-14 15:57||Ondrej Pokorny||Status||assigned => resolved|
|2020-01-14 15:57||Ondrej Pokorny||Resolution||open => fixed|
|2020-01-14 15:57||Ondrej Pokorny||Fixed in Revision||=> 62551|
|2020-01-14 15:57||Ondrej Pokorny||LazTarget||=> -|
|2020-01-14 15:57||Ondrej Pokorny||Widgetset||Win32/Win64 => Win32/Win64|
|2020-01-14 15:57||Ondrej Pokorny||Note Added: 0120435|