Wrong sizing of TToolBar buttons when Parent is reassigned
Original Reporter info from Mantis: dezlov
-
Reporter name: Denis Kozlov
Original Reporter info from Mantis: dezlov
- Reporter name: Denis Kozlov
Description:
Changing TToolBar.Parent causes the controls inside TToolBar to be resized incorrectly exactly every 3rd time the Parent is assigned.
ToolBar1.Parent := Form1; // 1st time - sizing OK
ToolBar1.Parent := nil;
ToolBar1.Parent := Form1; // 2nd time - sizing OK
ToolBar1.Parent := nil;
ToolBar1.Parent := Form1; // 3rd time - sizing WRONG!
ToolBar1.Parent := nil;
(keeps repeating...)
I have narrowed it down to incorrect preferred size being calculated, but then I got lost in the sizing maze:
TToolBar.WrapButtons -> (inlined) CalculatePosition -> TControl.GetPreferredSize
Additional information:
If the source of the problem is indeed in GetPreferredSize, then it may be affecting other controls too, not just TToolBar, and not only when Parent is reassigned.
Affects all versions of Lazarus: 1.4.4, 1.6, 1.7.
Mantis conversion info:
Activity
added CategoryLCL Version1.7 (SVN) WS: Win32/64 labels
Converted from Mantis. Note by Zeljan Rikalo @zeljan1
- Mantis submitter username: zeljko@holobit.net @zeljan1
- Mantis submitter real name: Zeljan Rikalo
Cannot reproduce here, but under wine (linux). Also, qt and gtk2 works fine in this case. Lazarus 1.7 r52174 FPC 2.6.4 x86_64-linux-qt
-
Converted from Mantis. Note by Denis Kozlov
- Mantis submitter username: dezlov
- Mantis submitter real name: Denis Kozlov
Tested on Windows 7 (win32 widgetset) with 1.4.4, 1.6, 1.7 - problem occurs in all.
Originally, I've discovered it by having a toolbar in a frame, and every 3rd time the frame is shown on a form the issue occurs.
Toggling captions of TToolBar buttons just after showing it seems to workaround the problem, i.e.:
ToolBar1.ShowCaptions := False; ToolBar1.ShowCaptions := True; -
I've debuggt this case a while. I'm now 100 percent sure what happens.
TCanvas is controlled on demand and the handle (a Device Context) is saved in Canvas.FHandle and in TControlCanvas.FDeviceContext.
On Windows it could happen, that the Canvas.FHandle is setted to 0 by the system (e.g., if the parent is freed) and the TControlCanvas.FDeviceContext still holds the old (now invalid) handle. A check TCanvas.HandleAllocated would fail.
There is no way to detect, if a handle, stored back from TControlCanvas.FDeviceContext, is valid or not.
So what happens here:
-
The ToolBar is created with ToolButtons with released Canvas.Handles.
-
Click -> parent is Nil -> Canvas.FHandle is 0 (by Windows and TControlCanvas).
-
Click -> Canvas.Handle is created on demand -> the size of the ToolButoons are correct calculated (no error on Windows.GetTextExtentPoint32W) -> the size of the ToolButtons is the old size, no ChangeBounds needed -> no ChangeBounds, so TControlCanvas.FreeHandle isn't called -> TControlCanvas.FDeviceContext has the old handle.
-
Click -> parent is Nil -> Canvas.FHandle is 0 (by Windows), TControlCanvas.FDeviceContext has the old handle.
-
Click -> Canvas.Handle get the old invalid TControlCanvas.FDeviceContext -> the size of the ToolButoons are wrong calculated (error on Windows.GetTextExtentPoint32W) -> the size of the ToolButtons are the default sizes, ChangeBounds needed -> ChangeBounds, so TControlCanvas.FreeHandle is called -> TControlCanvas.FDeviceContext get a 0.
-
Click -> parent is Nil -> Canvas.FHandle is 0 (by Windows and TControlCanvas).
-
Seventh click -> Canvas.Handle is created on demand -> the size of the ToolButoons are correct calculated (no error on Windows.GetTextExtentPoint32W) -> ChangeBounds needed -> ChangeBounds, so TControlCanvas.FreeHandle is called -> TControlCanvas.FDeviceContext get a 0.
-
Click -> back to 2.
First, I would fix it only for Win32Widgetset, to implement the TWSGraphicControl and TWin32WSGraphicControl, but I think, there is nothing wrong, to free the Canvas.Handle as default, if the control is freed by its parent.
I add the patch and a other example to show, all child TGraphicControls are effected.
-
-
Converted from Mantis. Note by Denis Kozlov
- Mantis submitter username: dezlov
- Mantis submitter real name: Denis Kozlov
On Windows it could happen, that the Canvas.FHandle is setted to 0 by the system (e.g., if the parent is freed) and the TControlCanvas.FDeviceContext still holds the old (now invalid) handle. A check TCanvas.HandleAllocated would fail.
Sounds like the problem is in TControlCanvas.FDeviceContext getting out of sync with reality. Can it be updated (nil'ed) when Canvas.FHandle is freed?
I think, there is nothing wrong to free the Canvas.Handle as default, if the control is freed by its parent.
I agree, there should be nothing wrong with that.
-
Sounds like the problem is in TControlCanvas.FDeviceContext getting out of sync with reality. Can it be updated (nil'ed) when Canvas.FHandle is freed?
After some more debugging, I know a little bit more.
The FHandle is set to 0 by TGraphicControl.WMPaint. Which is normal. But before the painting, the old handle is overwritten by a new one, so the old FHandle is in TControlCanvas.FDeviceContext and wouldn't be freed by default.
Please wait and don't apply one of the patches, maybe there is a better solution.
-
But before the painting, the old handle is overwritten by a new one, so the old FHandle is in TControlCanvas.FDeviceContext and wouldn't be freed by default.
I can't see any wrong behaviours to set TControlCanvas.FDeviceContext in the same moment to 0.
Patch added (which is imho a better choice as the others before).
[Edit]
I've tested the patch graphiccontrol.inc.patch for Windows 7 32bit and 64bit and Linux Mint KDE GTK2 32bit and 64bit. Seems to work fine. -
After some more debugging, I know a little bit more.
The FHandle is set to 0 by TGraphicControl.WMPaint. Which is normal. But before the painting, the old handle is overwritten by a new one, so the old FHandle is in TControlCanvas.FDeviceContext and wouldn't be freed by default.
Thank you for this information. It helped a lot. Unfortunately I couldn't apply your patches because they didn't solve the source of the issue but were more or less workarounds.
Please test. If everything is OK, I'll merge it to 1.6.2.
-
Thank you for this information. It helped a lot.
Nice, if I could give you a hand.
Please test. If everything is OK, I'll merge it to 1.6.2.
I've tested Lazarus 1.7 r52449 FPC 3.1.1 i386-win32-win32/win64 but there is only the first bug fixed. With my workarounds both bugs were gone. Please test SizingBugProject.zip and click multiple times on the left bottom ToggleBox to see the bug.
-
Correct. Thanks. Fixed.
+ Your 1st patch was the best one :) (Unfortunately I somehow overlooked it before.)
Actually my modifications do the same, but IMO at better places (e.g. you forgot about TGraphicControl.DoOnChangeBounds -> there's HandleAllocated/FreeHandle as well). I solved the parent change bug at the same place where it's done for TWinControl (r52500) - but this is also a matter of taste.
Thank you!
-
-
Attached files:
-
(Size: 11046 bytes, user: dezlov (Denis Kozlov), date: 2016-04-13) -
(Size: 11451 bytes, user: dezlov (Denis Kozlov), date: 2016-04-13) - ToolBarSizingBug.zip (Size: 805881 bytes, user: dezlov (Denis Kozlov), date: 2016-04-13)
- graphiccontrol.inc.patch (Size: 476 bytes, user: Michl (Michl), date: 2016-06-11)
- Win32GraphicControl.patch (Size: 3826 bytes, user: Michl (Michl), date: 2016-06-09)
- GraphicControl.patch (Size: 1174 bytes, user: Michl (Michl), date: 2016-06-09)
- SizingBugProject.zip (Size: 2661 bytes, user: Michl (Michl), date: 2016-06-09)
-