View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0026096 | Lazarus | LCL | public | 2014-04-30 09:05 | 2014-05-10 18:44 |
Reporter | Vojtech Cihak | Assigned To | Juha Manninen | ||
Priority | normal | Severity | minor | Reproducibility | N/A |
Status | closed | Resolution | fixed | ||
Platform | amd64 | OS | Linux | ||
Product Version | 1.3 (SVN) | ||||
Summary | 0026096: AlignControls for TCoolBar [Patch] | ||||
Description | With 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 Reproduce | Put 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 Information | Lazarus 1.3 r44847M FPC 2.7.1 x86_64-linux-qt | ||||
Tags | No tags attached. | ||||
Fixed in Revision | r44861, r44993, r44994 | ||||
LazTarget | - | ||||
Widgetset | |||||
Attached Files |
|
|
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; |
|
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); |
|
BTW, I expected that AlignControls(AControl: TControl; var RemainingClientRect: TRect); is triggered with currently aligned control, but AControl is always nil (unassigned) here. |
|
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 |
|
@ 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. |
|
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; |
|
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; |
|
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. |
|
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. |
|
I tested with r. 44995. Next patches will come in one *.diff. Thanks. |
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 |