View Issue Details

IDProjectCategoryView StatusLast Update
0036490LazarusWidgetsetpublic2020-03-31 13:57
ReporterDenis Kozlov Assigned ToOndrej Pokorny  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.1 (SVN) 
Summary0036490: Bitmap transparency (mask) not respected by TMenuItem
DescriptionThe 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 InformationThis issue was introduced in revision 57227:
https://github.com/graemeg/lazarus/commit/4ae869ad6ca2b50d032f6f005d073610c35d405d
Tagstransparent
Fixed in Revision62551
LazTarget-
WidgetsetWin32/Win64
Attached Files

Relationships

related to 0033099 closedOndrej Pokorny Menu item Bitmap issues 
related to 0024241 new TBitBtn do not display bitmap correctly 

Activities

Denis Kozlov

2019-12-28 18:15

reporter  

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)   

jamie philbrook

2019-12-28 19:40

reporter   ~0120114

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

Denis Kozlov

2019-12-29 01:01

reporter   ~0120120

@jamie

> 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?

jamie philbrook

2019-12-29 03:19

reporter   ~0120121

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

Denis Kozlov

2019-12-29 11:14

reporter   ~0120124

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)   

Juha Manninen

2020-01-11 11:29

developer   ~0120323

Ondrej, what do you think about the latest patch?

Ondrej Pokorny

2020-01-13 11:28

developer   ~0120397

Yes. I'll take a look. I don't remember exactly the original issue anymore. But I will decide somehow.

Ondrej Pokorny

2020-01-14 15:57

developer   ~0120435

Thank you for the patch. My revision 57227 was wrong.

Issue History

Date Modified Username Field Change
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
2020-03-31 10:29 Juha Manninen Relationship added related to 0024241
2020-03-31 13:57 Juha Manninen Tag Attached: transparent