View Issue Details

IDProjectCategoryView StatusLast Update
0026096LazarusLCLpublic2014-05-10 18:44
ReporterVojtech Cihak Assigned ToJuha Manninen  
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionfixed 
Platformamd64OSLinux 
Product Version1.3 (SVN) 
Summary0026096: AlignControls for TCoolBar [Patch]
DescriptionWith this patch are controls on bands aligned more solidly.

I was considering to enable BorderSpacing.Around for controls but only place where I can hook is CoolBar.Resize. Unfortunately, LCL triggers many Resizes so it would lead to many unnecessary recalculation with only small gain. Therefore I decided for KISS principle and BorderSpacing.Around is now restricted for controls on bands.
Steps To ReproducePut TCoolBar to the Form and drop any control on it (TEdit is good choice because it has published Align property) - it will create a new band.
Try to change property Align or Anchors or BorderSpacing properties. With this patch it is restricted.
Additional InformationLazarus 1.3 r44847M FPC 2.7.1 x86_64-linux-qt
TagsNo tags attached.
Fixed in Revisionr44861, r44993, r44994
LazTarget-
Widgetset
Attached Files

Activities

Vojtech Cihak

2014-04-30 09:05

reporter  

comctrls.diff (835 bytes)   
Index: comctrls.pp
===================================================================
--- comctrls.pp	(revision 44847)
+++ comctrls.pp	(working copy)
@@ -2272,6 +2272,8 @@
     cHorSpacing = 7;
     cVertSpacing = 3;
   protected
+    FControlLeft: Integer;
+    FControlTop: Integer;
     function CalcPreferredHeight: Integer;
     function CalcPrefferedWidth: Integer;
     function GetDisplayName: string; override;
@@ -2361,6 +2363,7 @@
     FPrevHeight: Integer;
     FPrevWidth: Integer;
     FTextHeight: Integer;
+    procedure AlignControls(AControl: TControl; var RemainingClientRect: TRect); override;
     procedure BitmapOrImageListChange(Sender: TObject);
     procedure CalculatePreferredSize(var PreferredWidth, PreferredHeight: integer;
                                      {%H-}WithThemeSpace: Boolean); override;
comctrls.diff (835 bytes)   

Vojtech Cihak

2014-04-30 09:05

reporter  

coolbar.diff (1,375 bytes)   
Index: coolbar.inc
===================================================================
--- coolbar.inc	(revision 44847)
+++ coolbar.inc	(working copy)
@@ -409,6 +409,21 @@
   Invalidate;
 end;
 
+procedure TCustomCoolBar.AlignControls(AControl: TControl; var RemainingClientRect: TRect);
+var i: Integer;
+begin 
+  //DebugLn('AlignControls');
+  for i:=0 to Bands.Count-1 do
+    if assigned(Bands[i].FControl) then begin
+      Bands[i].FControl.Align:=alNone;
+      Bands[i].FControl.BorderSpacing.Around:=0;
+      Bands[i].FControl.Anchors:=[akLeft, akTop];
+      Bands[i].FControl.AnchorParallel(akLeft, Bands[i].FControlLeft, self);
+      Bands[i].FControl.AnchorParallel(akTop, Bands[i].FControlTop, self);
+    end;
+  inherited AlignControls(AControl, RemainingClientRect);
+end;
+
 procedure TCustomCoolBar.BitmapOrImageListChange(Sender: TObject);
 begin
   Invalidate;
@@ -470,8 +485,9 @@
       inc(x, aLeft);
       y := aTop+(FVisiBands[i].FHeight-FVisiBands[i].Control.Height) div 2;
       FVisiBands[i].Control.Width:=aWidth;
-      FVisiBands[i].Control.AnchorParallel(akLeft, x-cBorderWidth, self);
-      FVisiBands[i].Control.AnchorParallel(akTop, y-cBorderWidth, self);
+      FVisiBands[i].FControlLeft:=x-cBorderWidth;
+      FVisiBands[i].FControlTop:=y-cBorderWidth;
+      ReAlign;
     end;
     x := FVisiBands[i].Width;
     inc(aLeft, x);
coolbar.diff (1,375 bytes)   

Vojtech Cihak

2014-04-30 13:16

reporter   ~0074686

BTW, I expected that AlignControls(AControl: TControl; var RemainingClientRect: TRect); is triggered with currently aligned control, but AControl is always nil (unassigned) here.

Juha Manninen

2014-04-30 22:49

developer   ~0074697

I applied your patches in r44861. Thanks.
It works pretty well now, although IMO the code is not optimal. You could do all position adjustment in AlignControls instead of calling CalculateAndAlign from many places.
Anyway, after your changes the component is good, no complaints with that.

About AControl being always Nil, you again have the same questions that I had when designing the first rudimentary version. :)
See
 http://free-pascal-lazarus.989080.n3.nabble.com/Lazarus-AlignControls-td4028973.html
and
 http://wiki.lazarus.freepascal.org/Autosize_/_Layout#TWinControl.AlignControls

Vojtech Cihak

2014-05-01 12:42

reporter   ~0074705

Last edited: 2014-05-01 13:37

View 2 revisions

@ It works pretty well now, although IMO the code is not optimal.

I tested CoolBar with 3 bands now and when I try to move or resize bands, LCL triggers 6-12 AlignControls on one CalculateAndAlign (just uncomment DebugLn() in these methods).
I agree that it would be cleaner code but not more optimal. I would not change it now.

I already observed TControlBar in Delphi7. It has no (published) bands. Components can be in multiple rows and height of higher controls is always rounded to integer-multiple of the first row. Control are movable at design time.

I think it is not so hard task. I try to move common code of TCoolBar and TControlBar to some TCustomBar (or TBaseBar - if there'll be some abstract methods) - as you wrote on ML.

EDIT: I tested with r. 44865.

Vojtech Cihak

2014-05-06 21:34

reporter  

comctrls3.diff (1,094 bytes)   
Index: comctrls.pp
===================================================================
--- comctrls.pp	(revision 44921)
+++ comctrls.pp	(working copy)
@@ -2360,8 +2360,6 @@
     FDragBand: TDragBand;
     FDraggedBandIndex: Integer;  // -1 .. space below the last row; other negative .. invalid area
     FDragInitPos: Integer;       // Initial mouse X - position (for resizing Bands)
-    FPrevHeight: Integer;
-    FPrevWidth: Integer;
     FTextHeight: Integer;
     procedure AlignControls(AControl: TControl; var RemainingClientRect: TRect); override;
     procedure BitmapOrImageListChange(Sender: TObject);
@@ -2382,8 +2380,8 @@
     procedure MouseUp(Button: TMouseButton; Shift: TShiftState; X, Y: Integer); override;
     procedure Notification(AComponent: TComponent; Operation: TOperation); override;
     procedure Paint; override;
-    procedure Resize; override;
     procedure SetAlign(aValue: TAlign); reintroduce;
+    procedure WMSize(var Message: TLMSize); message LM_SIZE;
   public
     constructor Create(AOwner: TComponent); override;
     destructor Destroy; override;
comctrls3.diff (1,094 bytes)   

Vojtech Cihak

2014-05-06 21:34

reporter  

coolbar3.diff (1,462 bytes)   
Index: coolbar.inc
===================================================================
--- coolbar.inc	(revision 44921)
+++ coolbar.inc	(working copy)
@@ -459,7 +459,7 @@
     inc(aLeft, FVisiBands[i].Width);
     aRowEnd := (i = aCountM1) or ((i < aCountM1)
       and ((FVisiBands[i+1].Break or Vertical)
-      or ((aLeft+FVisiBands[i+1].Width) > (ClientWidth-2*cBorderWidth))));
+      or ((aLeft+FVisiBands[i+1].Width) > ClientWidth)));  //condition must be same as in IsRowEnd 
     //Set all Bands in row to uniform height
     if aRowEnd then begin
       for y := aStartIndex to i do
@@ -506,8 +506,6 @@
     if aCountM1 >= 0 then EnableAutoSizing;
     dec(FUpdateCount);
   end;
-  FPrevWidth := Width;
-  FPrevHeight := Height;
 end;
 
 procedure TCustomCoolBar.CalculatePreferredSize(var PreferredWidth, PreferredHeight: Integer;
@@ -955,19 +953,11 @@
   end;
 end;
 
-procedure TCustomCoolBar.Resize;
-var aWidth, aHeight: Integer;
+procedure TCustomCoolBar.WMSize(var Message: TLMSize);
 begin
-  //DebugLn('Resize');
-  inherited Resize;
-  aWidth := Width;
-  aHeight := Height;
-  if ((aWidth <> FPrevWidth) or (aHeight <> FPrevHeight))
-    and (aWidth*aHeight > 0) and HandleAllocated then
-    begin
-      //DebugLn('Resize calls CalcAndAlign');
-      CalculateAndAlign;
-      Invalidate;  //Required by GTK2
-    end;
+  //DebugLn('WMSize');
+  inherited WMSize(Message);
+  CalculateAndAlign;
+  Invalidate;  //Required by GTK2
 end;
 
coolbar3.diff (1,462 bytes)   

Vojtech Cihak

2014-05-06 21:41

reporter   ~0074815

I attached one small patch (files comctrls3.diff and coolbar3.diff).

It mainly replaces method Resize with WMSize. The reason is that LCL triggers many Resizes (also before Handle is allocated) while WMSize is triggered only once and only when the size of component is actually changed (and only when Handle is allcoated).

It also solves one small optical issue - bands in a row are aligned to the same height exactly in the same moment when overlaying band is moved to the next row.

Juha Manninen

2014-05-10 16:49

developer   ~0074882

I applied the changes (divided into more commits). Thanks.
BTW, you can also include changes of many files into one patch, using command like
 $ cd lazarus_root_dir
 $ svn diff lcl/comctrls.pp lcl/include/coolbar.inc > coolbar3.patch
Then it is easier to inspect and apply.

I will write to mailing list about the architechture.

Vojtech Cihak

2014-05-10 18:44

reporter   ~0074888

I tested with r. 44995. Next patches will come in one *.diff. Thanks.

Issue History

Date Modified Username Field Change
2014-04-30 09:05 Vojtech Cihak New Issue
2014-04-30 09:05 Vojtech Cihak File Added: comctrls.diff
2014-04-30 09:05 Vojtech Cihak File Added: coolbar.diff
2014-04-30 13:16 Vojtech Cihak Note Added: 0074686
2014-04-30 21:42 Juha Manninen Assigned To => Juha Manninen
2014-04-30 21:42 Juha Manninen Status new => assigned
2014-04-30 22:49 Juha Manninen Note Added: 0074697
2014-05-01 12:42 Vojtech Cihak Note Added: 0074705
2014-05-01 13:37 Vojtech Cihak Note Edited: 0074705 View Revisions
2014-05-06 21:34 Vojtech Cihak File Added: comctrls3.diff
2014-05-06 21:34 Vojtech Cihak File Added: coolbar3.diff
2014-05-06 21:41 Vojtech Cihak Note Added: 0074815
2014-05-10 16:49 Juha Manninen Fixed in Revision => r44861, r44993, r44994
2014-05-10 16:49 Juha Manninen LazTarget => -
2014-05-10 16:49 Juha Manninen Note Added: 0074882
2014-05-10 16:49 Juha Manninen Status assigned => resolved
2014-05-10 16:49 Juha Manninen Resolution open => fixed
2014-05-10 18:44 Vojtech Cihak Note Added: 0074888
2014-05-10 18:44 Vojtech Cihak Status resolved => closed