View Issue Details

IDProjectCategoryView StatusLast Update
0037455LazarusLCLpublic2020-08-01 09:36
ReporterJoeny Ang Assigned Towp  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version2.1 (SVN) 
Summary0037455: [Patch] TButtonPanel cannot hide glyphs
DescriptionProblems:
1. Using the ObjectInspector, clicking on TButtonPanel.ShowGlyphs' members does nothing.
2. Assigning values to ShowGlyphs in code multiple times will eventually cause all buttons to lose their glyphs.

Patch:
- Call DoShowGlyphs() when control is showing/hiding to update glyph show status.
- Remove code that re-assign glyphs to internal FGlyphs variable. This will cause a button's empty glyph (hidden state) to be assigned to FGlyph[].
- Win32: In TCustomBitBtn.RealizeKind(), when glyph is determined using LCL (LCLGlyphName), we need to have a valid Glyph. This is needed by TButtonPanel when hiding the buttons' glyph. It does so by calling Glyph.Assign(nil), and if Glyph is already nil, no code is executed and glyph is not hidden.

Tested under GTK2, QT5, Win32
Steps To ReproduceTest project.
1. Using the ObjectInspector, try hiding a button's glyph via the ShowGlyphs property.
2. Upon launch, the glyphs should all be hidden.
3. Clicking randomly on the 4 buttons multiple times will eventually cause all buttons to lose their glyphs.
TagsNo tags attached.
Fixed in Revision63672
LazTarget-
Widgetset
Attached Files

Activities

Joeny Ang

2020-07-31 10:58

reporter  

tbuttonpanel-cannot-hide-glyphs.patch (1,435 bytes)   
--- lcl/buttonpanel.pas.63671
+++ lcl/buttonpanel.pas
@@ -232,13 +232,8 @@
     if FButtons[btn] = nil then Continue;
 
     if btn in FShowGlyphs
-    then begin
-      FButtons[btn].Glyph.Assign(FGlyphs[btn]);
-    end
-    else begin
-      FGlyphs[btn].Assign(FButtons[btn].Glyph);
-      FButtons[btn].Glyph.Assign(nil);
-    end;
+    then FButtons[btn].Glyph.Assign(FGlyphs[btn])
+    else FButtons[btn].Glyph.Assign(nil);
   end;
   EnableAutoSizing{$IFDEF DebugDisableAutoSizing}('TCustomButtonPanel.DoShowGlyphs'){$ENDIF};
 end;
@@ -429,6 +424,7 @@
   inherited;
 
   UpdateButtonSize;
+  DoShowGlyphs;
 end;
 
 procedure TCustomButtonPanel.SetButtonOrder(Value: TButtonOrder);
--- lcl/include/bitbtn.inc.63671
+++ lcl/include/bitbtn.inc
@@ -321,7 +321,17 @@
         idButton := BitBtnImages[Kind];
         if (idButton >= Low(BitBtnResNames)) and (idButton <= High(BitBtnResNames))
         and (BitBtnResNames[idButton] <> '') then
-          FButtonGlyph.LCLGlyphName := BitBtnResNames[idButton]
+        begin
+          FButtonGlyph.LCLGlyphName := BitBtnResNames[idButton];
+          CustomGlyph := GetLCLDefaultBtnGlyph(Kind);
+          try
+            if not Assigned(Glyph) then
+              Glyph := TBitmap.Create;
+            Glyph.Assign(CustomGlyph);
+          finally
+            CustomGlyph.Free;
+          end;
+        end
         else
           ImageIndex := -1;
         GlyphValid := True;

Joeny Ang

2020-07-31 12:22

reporter   ~0124428

wp

2020-07-31 12:47

developer   ~0124429

Last edited: 2020-07-31 13:51

View 3 revisions

I can reproduce the issue of glyphs not disappearing on Win10. Seems to be have been introduced somewhere in the Laz 2.x series because the demo program behaves correctly with Laz 1.8.4 while I see the error in Laz 2.0.2 (the oldest v2.x on my disk).

I cannot confirm step # 3 on Win 10.

The code in TCustomButtonPanel.DoShowGlyphs of v1.8.4 seems to be exactly the same as in Laz trunk. So, is this part of your patch really needed?

With complete patch applied, dlicking on the [ ] button as first button after the test program starts does not remove the glyphs of the buttons. But when another button is clicked first, then the [ ] does remove the glyphs.

Joeny Ang

2020-07-31 15:27

reporter   ~0124433

Last edited: 2020-07-31 15:35

View 3 revisions

The above third problem is present under GTK2. The following sequence will make the buttons lose their glyphs: [], [pbClose], [], [pbOk, pbCancel]. Pressing any button afterwards does not bring back the glyphs.

I've retested it under WinXP. Indeed, pressing any button changes nothing. I think this is because clearing TBitBtn's Glyph does not work in Win32 (Kind <> bkCustom).

I've tried applying only the bitbtn.inc part of the patch, and clearing Glyph now works and will hide the glyph, but the above third problem will now be present under Win32.

The patch to TCustomButtonPanel.DoShowGlyphs() is to prevent re-assigning the FGlyph local variable. For example if DoShowGlyphs() is called while the OK button has its glyph hidden, FGlyph[pbOK] will be assigned nil. The next time it is shown, FGlyph[pbOK] = nil, will be assigned to Glyph.

> With complete patch applied, dlicking on the [ ] button as first button after the test program starts does not remove the glyphs of the buttons. But when another button is clicked first, then the [ ] does remove the glyphs.
Upon program start, the value of ShowGlyphs is [], so assigning it [] does nothing. It is supposed to start with all glyphs turned off. I've tried the complete patch again under WinXP and it started without any glyphs.

Joeny Ang

2020-07-31 15:33

reporter   ~0124434

There is actually a simpler approach... for hiding, instead of assigning nil to Glyph, we can set the button's GlyphShowMode to gsmNever, and for showing, set it to gsmApplication or gsmAlways.

wp

2020-07-31 17:01

developer   ~0124436

Last edited: 2020-07-31 17:33

View 2 revisions

> Upon program start, the value of ShowGlyphs is [], so assigning it [] does nothing. It is supposed to start with all glyphs turned off. I've tried the complete
> patch again under WinXP and it started without any glyphs.

Well, on Win 10 at least this is not the case, the demo starts with glyphs visible although they should be turned off according to the lfm. So we have a new problem here. Or maybe the same? Because the real root cause is not fixed by your patch?

Without having debugged too much into this issue I think that the patch should not touch TBitBtn. The basic requirements regarding glyphs are:

- assign a bitmap to the BitBtn
- clear the bitmap from the BitBtn.

That's all what the TButtonGroup needs here, and the attached demo shows that TBitBtn alone seeoms to fulfill these requirements (I am adding to the demo also the ImageList way of assigning a glyph). This tells me that the bug must be inside the TButtonPanel alone. Fixing the bug in the TButtonPanel logics by changes in TBitBtn has a high risk that the fix will break something else.

[EDIT]
When BitBtn.Kind is involved - and this happens here - , however, i do agree that the TBitBtn begins to act strange...
bitbtn_demo.zip (5,982 bytes)

wp

2020-07-31 18:40

developer   ~0124439

Last edited: 2020-07-31 18:54

View 2 revisions

Here is modificed version of the bitbtn_demo in which the BitBtn.Kind can be set. It can be seen that the glyph cannot be reset when Kind <> bkCustom.

Application of your bitbtn patch makes this work. So, in contrast to my previous post I think that you really fixed a BitBtn bug.

I still cannot confirm issue # 3. What's left for me is the failure to adjust glyph visibility to the values found in the lfm. I can fix this by adding DoShowGlyphs to the Loaded method:

procedure TCustomButtonPanel.Loaded;
begin
  inherited;
  DoShowGlyphs;
end;
bitbtn_demo-v2.zip (6,112 bytes)

wp

2020-07-31 22:03

developer   ~0124444

Found a way to reproduce issue # 3 (click [bkClose] first). Your patch fixes the issue, thanks. Applied it together with the "Loaded". Please test and close if ok.

Joeny Ang

2020-08-01 09:36

reporter   ~0124449

Tested. Ok. Thanks :)

Issue History

Date Modified Username Field Change
2020-07-31 10:58 Joeny Ang New Issue
2020-07-31 10:58 Joeny Ang File Added: tbuttonpanel-cannot-hide-glyphs.patch
2020-07-31 12:22 Joeny Ang Note Added: 0124428
2020-07-31 12:22 Joeny Ang File Added: tbuttonpanel-cannot-hide-glyphs-test01.zip
2020-07-31 12:47 wp Note Added: 0124429
2020-07-31 13:50 wp Note Edited: 0124429 View Revisions
2020-07-31 13:51 wp Note Edited: 0124429 View Revisions
2020-07-31 15:27 Joeny Ang Note Added: 0124433
2020-07-31 15:28 Joeny Ang Note Edited: 0124433 View Revisions
2020-07-31 15:33 Joeny Ang Note Added: 0124434
2020-07-31 15:35 Joeny Ang Note Edited: 0124433 View Revisions
2020-07-31 17:01 wp Note Added: 0124436
2020-07-31 17:01 wp File Added: bitbtn_demo.zip
2020-07-31 17:33 wp Note Edited: 0124436 View Revisions
2020-07-31 18:40 wp Note Added: 0124439
2020-07-31 18:40 wp File Added: bitbtn_demo-v2.zip
2020-07-31 18:54 wp Note Edited: 0124439 View Revisions
2020-07-31 18:58 wp Assigned To => wp
2020-07-31 18:58 wp Status new => assigned
2020-07-31 22:03 wp Note Added: 0124444
2020-07-31 22:04 wp Status assigned => resolved
2020-07-31 22:04 wp Resolution open => fixed
2020-07-31 22:04 wp Fixed in Revision => 63672
2020-07-31 22:04 wp LazTarget => -
2020-08-01 09:36 Joeny Ang Status resolved => closed
2020-08-01 09:36 Joeny Ang Note Added: 0124449