View Issue Details

IDProjectCategoryView StatusLast Update
0026358LazarusLCLpublic2014-10-07 17:54
ReporterMike ThompsonAssigned ToZeljan Rikalo 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformwin32OSWindowsOS Version8
Product VersionProduct BuildLazarus Trunk 
Target Version1.2.6Fixed in Version1.3 (SVN) 
Summary0026358: TTrackbar issues too many OnChanges
DescriptionWin32 TTrackbar issues extra OnChange events.

When a MouseUp occurs, two additional OnChange are sent.
When the user uses a keyboard, one additional OnChange is set per keypress.

Confirmed with both Lazarus 1.2.4 and Lazarus trunk.
Steps To ReproduceRun the attached Demo.

Modifying the Trackbar in code (top button) results in the expected number of OnChange

Use Mouse or keyboard and additional OnChanges are tracked and reported.
Additional Information(No Trunk option available under Product Version)

I'm not all that familiar with the LCL messaging, but from what I can make out, this is a Win32 issue, not an LCL. The OnChange is only issued in response to the TrackBar receiving a LM_CHANGE message.

At this time I cannot find where the WM_ messages are converted to LM_ messages. I will keep looking.

I've attached a patch that I admit workarounds the issue, not solves it. The OnChange is only fired when Position is actually changed. With the supplied patch, the control itself will still receive too many LM_CHANGE messages.
Tagspatch
Fixed in Revision46326
LazTarget1.2.6
WidgetsetWin32/Win64
Attached Files
  • TrackBar1Change - Demo.zip (130,503 bytes)
  • Trackbar.patch (1,066 bytes)
    Index: lcl/comctrls.pp
    ===================================================================
    --- lcl/comctrls.pp	(revision 45571)
    +++ lcl/comctrls.pp	(working copy)
    @@ -2485,6 +2485,7 @@
         FMax: Integer;
         FFrequency: Integer;
         FPosition: Integer;
    +    FLastPosition: Integer;
         FScalePos: TTrackBarScalePos;
         FScaleDigits: integer;
         FOnChange: TNotifyEvent;
    Index: lcl/include/trackbar.inc
    ===================================================================
    --- lcl/include/trackbar.inc	(revision 45571)
    +++ lcl/include/trackbar.inc	(working copy)
    @@ -56,6 +56,7 @@
       FMax := 10;
       FMin := 0;
       FPosition := 0;
    +  FLastPosition := -1;
       FPageSize := 2;
       FFrequency := 1;
       FOrientation := trHorizontal;
    @@ -320,7 +321,11 @@
     procedure TCustomTrackBar.Changed;
     begin
       if Assigned (FOnChange) then
    -    FOnChange(Self);
    +    if FLastPosition<>FPosition Then
    +    begin
    +      FOnChange(Self);
    +      FLastPosition := FPosition;
    +    end;
     end;
     
     {------------------------------------------------------------------------------
    
    Trackbar.patch (1,066 bytes)
  • Win32 Widget TrackBar.patch (711 bytes)
    Index: lcl/interfaces/win32/win32wscomctrls.pp
    ===================================================================
    --- lcl/interfaces/win32/win32wscomctrls.pp	(revision 46323)
    +++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
    @@ -825,7 +825,10 @@
             Message.wParam := 0;
             Message.lParam := 0;
             Message.Result := 0;
    -        DeliverMessage(Info^.WinControl, Message);
    +
    +        //debugln('LOWORD(WPARAM)=%d, HIWORD(WPARAM)=%d', [LOWORD(WParam), HIWORD(WPARAM)]);
    +        if TWin32WSTrackBar.GetPosition(TTrackBar(Info^.WinControl))<>TTrackBar(Info^.WinControl).Position then
    +          DeliverMessage(Info^.WinControl, Message);
           end;
           Result := True;
         end;
    
  • Win32 Widget TrackBar (updated).patch (723 bytes)
    Index: lcl/interfaces/win32/win32wscomctrls.pp
    ===================================================================
    --- lcl/interfaces/win32/win32wscomctrls.pp	(revision 46323)
    +++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
    @@ -825,7 +825,10 @@
             Message.wParam := 0;
             Message.lParam := 0;
             Message.Result := 0;
    -        DeliverMessage(Info^.WinControl, Message);
    +
    +        //debugln('LOWORD(WPARAM)=%d, HIWORD(WPARAM)=%d', [LOWORD(WParam), HIWORD(WPARAM)]);
    +        if TWin32WSTrackBar.GetPosition(TCustomTrackBar(Info^.WinControl))<>TCustomTrackBar(Info^.WinControl).Position then
    +          DeliverMessage(Info^.WinControl, Message);
           end;
           Result := True;
         end;
    

Activities

Mike Thompson

2014-06-18 18:14

developer  

TrackBar1Change - Demo.zip (130,503 bytes)

Mike Thompson

2014-06-18 18:14

developer  

Trackbar.patch (1,066 bytes)
Index: lcl/comctrls.pp
===================================================================
--- lcl/comctrls.pp	(revision 45571)
+++ lcl/comctrls.pp	(working copy)
@@ -2485,6 +2485,7 @@
     FMax: Integer;
     FFrequency: Integer;
     FPosition: Integer;
+    FLastPosition: Integer;
     FScalePos: TTrackBarScalePos;
     FScaleDigits: integer;
     FOnChange: TNotifyEvent;
Index: lcl/include/trackbar.inc
===================================================================
--- lcl/include/trackbar.inc	(revision 45571)
+++ lcl/include/trackbar.inc	(working copy)
@@ -56,6 +56,7 @@
   FMax := 10;
   FMin := 0;
   FPosition := 0;
+  FLastPosition := -1;
   FPageSize := 2;
   FFrequency := 1;
   FOrientation := trHorizontal;
@@ -320,7 +321,11 @@
 procedure TCustomTrackBar.Changed;
 begin
   if Assigned (FOnChange) then
-    FOnChange(Self);
+    if FLastPosition<>FPosition Then
+    begin
+      FOnChange(Self);
+      FLastPosition := FPosition;
+    end;
 end;
 
 {------------------------------------------------------------------------------
Trackbar.patch (1,066 bytes)

Mike Thompson

2014-06-18 18:15

developer   ~0075784

Last edited: 2014-06-18 19:39

View 3 revisions

See also
http://forum.lazarus.freepascal.org/index.php/topic,24900.msg150466.html

I've now found Win32WSComCtrls.pp and I can confirm that on mouseup, in TrackBarParentMsgHandler the control is receiving three WM_HSCROLL messages in a row.
This indeed looks like a Win32 issue, and suspect my workaround is our only solution.
Small typo in my "Additional information" above. The LCL message the control is receiving is LM_CHANGED, not LM_CHANGE

Update. I'm currently going through http://msdn.microsoft.com/en-us/library/windows/desktop/gg430017(v=vs.85).aspx We need to decode the LoWord of WParam which specifies why each of the different WM_HSCROLLs are being sent. I'm currently playing with this, but at the moment the control is *NOT* behaving as I understand the MSDN documentation says it should.

Sorry for posting this before doing further research, but at the time I honestly thought this would be beyond me...

Mike Thompson

2014-06-18 20:17

developer   ~0075786

Last edited: 2014-06-18 20:21

View 3 revisions

What an odd win32 control :-( WM_HSCROLL is issued each and every time the user interacts with the control, its got nothing to do with the thumb actually scrolling. In the demo app I've provided, give the trackbar focus, press and hold the right arrow key. Keep the key down. WM_HSCROLL messages continue to be sent even after the thumb has reached the far right. And the reason is that WM_SCROLL is sending TB_LINEDOWN to indicate the right key is being pressed.

I can solve the multiple mouse events by only broadcasting LM_CHANGED on receipt of TB_THUMBTRACK, but then we don't get a LM_CHANGED if the user uses the keyboard :-(

I can't find any messages that indicate the thumb has reached it's extents (MSDN documentation explicitly states its up to the end user to track this). Which means we're going to have use Position anyway, which means we might as well use the patch I added to the bug. At least other widgets will also benefit from the additional filtering of OnChange.

Juha Manninen

2014-09-14 21:14

developer   ~0077244

The problem is in LCL-Win bindings code. The fix should go there, too.

Mike Thompson

2014-09-15 22:08

developer   ~0077282

Last edited: 2014-09-15 22:09

View 2 revisions

Thanks for the feedback and I understand. Means duplicating functionality from the TTrackbar control inside Win32WSComCtrls.pp, but you're right - it'll be the more correct fix.
Be a few weeks before I can revisit this.

Juha Manninen

2014-09-15 22:34

developer   ~0077284

I don't think there is much duplication needed, although I have not studied the code.

Mike Thompson

2014-09-25 16:13

developer  

Win32 Widget TrackBar.patch (711 bytes)
Index: lcl/interfaces/win32/win32wscomctrls.pp
===================================================================
--- lcl/interfaces/win32/win32wscomctrls.pp	(revision 46323)
+++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
@@ -825,7 +825,10 @@
         Message.wParam := 0;
         Message.lParam := 0;
         Message.Result := 0;
-        DeliverMessage(Info^.WinControl, Message);
+
+        //debugln('LOWORD(WPARAM)=%d, HIWORD(WPARAM)=%d', [LOWORD(WParam), HIWORD(WPARAM)]);
+        if TWin32WSTrackBar.GetPosition(TTrackBar(Info^.WinControl))<>TTrackBar(Info^.WinControl).Position then
+          DeliverMessage(Info^.WinControl, Message);
       end;
       Result := True;
     end;

Mike Thompson

2014-09-25 16:16

developer   ~0077651

Last edited: 2014-09-25 16:19

View 2 revisions

You were correct, change at the widgetset level was less code than at the control level.

Please find attached "Win32 Widget TrackBar.patch" that only propagates OnChange if it is required.

Mike Thompson

2014-09-25 16:29

developer  

Win32 Widget TrackBar (updated).patch (723 bytes)
Index: lcl/interfaces/win32/win32wscomctrls.pp
===================================================================
--- lcl/interfaces/win32/win32wscomctrls.pp	(revision 46323)
+++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
@@ -825,7 +825,10 @@
         Message.wParam := 0;
         Message.lParam := 0;
         Message.Result := 0;
-        DeliverMessage(Info^.WinControl, Message);
+
+        //debugln('LOWORD(WPARAM)=%d, HIWORD(WPARAM)=%d', [LOWORD(WParam), HIWORD(WPARAM)]);
+        if TWin32WSTrackBar.GetPosition(TCustomTrackBar(Info^.WinControl))<>TCustomTrackBar(Info^.WinControl).Position then
+          DeliverMessage(Info^.WinControl, Message);
       end;
       Result := True;
     end;

Mike Thompson

2014-09-25 16:30

developer   ~0077652

Apologies - new patch "Win32 Widget TrackBar (updated).patch" uploaded.
In the original I cast the control as TTrackBar, should have used TCustomTrackBar (in case someone creates a TCustomTrackBar descendant)

Zeljan Rikalo

2014-09-25 18:05

developer   ~0077657

Thanks for the patch.Please test and close if ok.
Also, this commit will be mergedd into 1.2.6, so LazTarget = 1.2.6

Mike Thompson

2014-10-07 17:54

developer   ~0078050

Tested under both 1.2.5 & 1.3. All good. Many thanks

Issue History

Date Modified Username Field Change
2014-06-18 18:14 Mike Thompson New Issue
2014-06-18 18:14 Mike Thompson File Added: TrackBar1Change - Demo.zip
2014-06-18 18:14 Mike Thompson File Added: Trackbar.patch
2014-06-18 18:15 Mike Thompson Note Added: 0075784
2014-06-18 18:41 Mike Thompson Note Edited: 0075784 View Revisions
2014-06-18 19:39 Mike Thompson Note Edited: 0075784 View Revisions
2014-06-18 20:17 Mike Thompson Note Added: 0075786
2014-06-18 20:20 Mike Thompson Note Edited: 0075786 View Revisions
2014-06-18 20:21 Mike Thompson Note Edited: 0075786 View Revisions
2014-09-14 18:26 Mike Thompson Tag Attached: patch
2014-09-14 21:14 Juha Manninen Note Added: 0077244
2014-09-15 22:08 Mike Thompson Note Added: 0077282
2014-09-15 22:09 Mike Thompson Note Edited: 0077282 View Revisions
2014-09-15 22:34 Juha Manninen Note Added: 0077284
2014-09-25 16:13 Mike Thompson File Added: Win32 Widget TrackBar.patch
2014-09-25 16:16 Mike Thompson Note Added: 0077651
2014-09-25 16:19 Mike Thompson Note Edited: 0077651 View Revisions
2014-09-25 16:29 Mike Thompson File Added: Win32 Widget TrackBar (updated).patch
2014-09-25 16:30 Mike Thompson Note Added: 0077652
2014-09-25 18:05 Zeljan Rikalo Fixed in Revision => 46326
2014-09-25 18:05 Zeljan Rikalo LazTarget => 1.2.6
2014-09-25 18:05 Zeljan Rikalo Note Added: 0077657
2014-09-25 18:05 Zeljan Rikalo Status new => resolved
2014-09-25 18:05 Zeljan Rikalo Fixed in Version => 1.3 (SVN)
2014-09-25 18:05 Zeljan Rikalo Resolution open => fixed
2014-09-25 18:05 Zeljan Rikalo Assigned To => Zeljan Rikalo
2014-09-25 18:05 Zeljan Rikalo Target Version => 1.2.6
2014-10-07 17:54 Mike Thompson Note Added: 0078050
2014-10-07 17:54 Mike Thompson Status resolved => closed