View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005996JEDI VCL00 JVCL Componentspublic2012-10-01 20:542015-09-14 13:20
ReporterjkelleyAssigned ToAHUser 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.45 
Target VersionFixed in Version3.48 
Summary0005996: Problems with scrolling JvPrvwDoc (TJvPreviewControl)
DescriptionTJvPreviewControl's scrolling has several shortcomings:

* Scrolling increments are based on the printed page size, so scrolling while zoomed in results in very large jumps in the visible page.
* Mouse wheel support doesn't honor the user's chosen settings for how fast the wheel scrolls.
* The scroll bar's thumb buttons aren't proportional to the visible area.
* There's no distinction between scrolling a page at a time and scrolling a line at a time.
Additional InformationAttached is a patch to fix these problems. A description of the patch's logic:

Set SCROLLINFO.nPage so that thumb button size is proportional.
Adjust values for nMax. The new values for nMax and nPage are based on the "Remarks" section of MSDN's SetScrollInfo documentation (http://msdn.microsoft.com/en-us/library/windows/desktop/bb787595%28v=vs.85%29.aspx).
Use FMaxHeight - 1 and FMaxWidth - 1 for SCROLLINFO.nMax. (Since scrolling is in the range [ nMin, nMax ], and we use 0 for nMin, I believe we need to subtract 1 for nMax.)
Comment out ScrollBy call; it's redundant with SetScrollInfo.
Old code scrolled by 1/3 of an on-paper page, always. New code scrolls by 50 pixels for line at a time (value chosen because that's what Mozilla Firefox uses) or one screenful at a time, as expected.
Mouse wheel logic is based on http://msdn.microsoft.com/en-us/library/ms997498.aspx.

To do:
120 (mouse wheel delta) should be a constant somewhere, but I'm not sure where the JVCL keeps constants like this.
The scroll increment for horizontal scrolling should be updated to be similar to the scroll increment for vertical scrolling. My app doesn't make much use of horizontal scrolling of TJvPreviewControl, so I haven't yet done this.
TagsNo tags attached.

Activities

2012-10-01 20:54

 

jvprvwdocscrolling.diff (5,408 bytes) 
--- c:\src\JVCL3_45\run\JvPrvwDoc.pas	2011-10-12 08:42:22.514938000 -0400
+++ JvPrvwDoc.pas	2012-10-01 14:42:08.863987600 -0400
@@ -276,6 +276,7 @@
     FOnOptionsChange: TNotifyEvent;
     FOnScrollHint: TJvScrollHintEvent;
     FSelection: TJvPreviewSelection;
+    FAccumulatedWheelDelta: Integer;
     procedure DoOptionsChange(Sender: TObject);
     procedure DoDeviceInfoChange(Sender: TObject);
     procedure DoScaleModeChange(Sender: TObject);
@@ -961,8 +962,8 @@
     Inc(SI.fMask, SIF_DISABLENOSCROLL);
   GetScrollInfo(Handle, SB_HORZ, SI);
 
-  SI.nMax := FMaxWidth - ClientWidth;
-  SI.nPage := 0;
+  SI.nMax := FMaxWidth - 1;
+  SI.nPage := Min(ClientWidth, SI.nMax - SI.nMin + 1);
   ShowScrollBar(Handle, SB_HORZ, not HideScrollBars and (ScrollBars in [ssHorizontal, ssBoth]));
   SetScrollInfo(Handle, SB_HORZ, SI, True);
   // update scroll pos if it has changed
@@ -987,8 +988,8 @@
   end
   else
   begin
-    SI.nMax := FMaxHeight - ClientHeight;
-    SI.nPage := 0; // FMaxHeight div TotalRows;
+    SI.nMax := FMaxHeight - 1;
+    SI.nPage := Min(ClientHeight, SI.nMax - SI.nMin + 1);
   end;
   ShowScrollBar(Handle, SB_VERT, not HideScrollBars and (ScrollBars in [ssVertical, ssBoth]));
   SetScrollInfo(Handle, SB_VERT, SI, True);
@@ -1319,11 +1320,12 @@
     SB_ENDSCROLL:
       Exit;
   end;
-  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax);
+  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax - Max(SI.nPage - 1, 0));
   if Assigned(FOnHorzScroll) then
     FOnHorzScroll(Self, TScrollCode(Msg.ScrollCode), NewPos);
-  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax);
-  ScrollBy(-FScrollPos.X + NewPos, 0);
+  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax - Max(SI.nPage - 1, 0));
+  // Old versions of JVCL called ScrollBy, but it's redundant with SetScrollInfo.
+  //ScrollBy(-FScrollPos.X + NewPos, 0);
   FScrollPos.X := NewPos;
   SI.nPos := NewPos;
   SetScrollInfo(Handle, SB_HORZ, SI, True);
@@ -1335,11 +1337,18 @@
 procedure TJvCustomPreviewControl.WMVScroll(var Msg: TWMVScroll);
 var
   SI: TScrollInfo;
-  NewPos, Increment: Integer;
+  NewPos, Increment, PageIncrement: Integer;
 begin
-  Increment := FPageHeight + Integer(Options.VertSpacing);
-  if not IsPageMode then
-    Increment := Increment div 3;
+  if IsPageMode then
+  Begin
+    Increment := FPageHeight + Integer(Options.VertSpacing);
+    PageIncrement := Increment;
+  End
+  Else
+  Begin
+    Increment := 50;
+    PageIncrement := ClientHeight - Increment;
+  End;
   if Increment < 1 then
     Increment := 1;
 
@@ -1352,10 +1361,14 @@
       NewPos := 0;
     SB_BOTTOM:
       NewPos := FMaxHeight;
-    SB_LINEDOWN, SB_PAGEDOWN:
+    SB_LINEDOWN:
       NewPos := FScrollPos.Y + Increment;
-    SB_LINEUP, SB_PAGEUP:
+    SB_PAGEDOWN:
+      NewPos := FScrollPos.Y + PageIncrement;
+    SB_LINEUP:
       NewPos := FScrollPos.Y - Increment;
+    SB_PAGEUP:
+      NewPos := FScrollPos.Y - PageIncrement;
     SB_THUMBPOSITION, SB_THUMBTRACK:
       begin
         NewPos := SI.nTrackPos;
@@ -1371,11 +1384,12 @@
         Exit;
       end;
   end;
-  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax);
+  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax - Max(SI.nPage - 1, 0));
   if Assigned(FOnVertScroll) then
     FOnVertScroll(Self, TScrollCode(Msg.ScrollCode), NewPos);
-  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax);
-  ScrollBy(0, -FScrollPos.Y + NewPos);
+  NewPos := EnsureRange(NewPos, SI.nMin, SI.nMax - Max(SI.nPage - 1, 0));
+  // Old versions of JVCL called ScrollBy, but it's redundant with SetScrollInfo.
+  //ScrollBy(0, -FScrollPos.Y + NewPos);
   FScrollPos.Y := NewPos;
   SI.nPos := NewPos;
   SetScrollInfo(Handle, SB_VERT, SI, True);
@@ -1737,6 +1751,8 @@
 var
   Msg: TWMScroll;
   SI: TScrollInfo;
+  WheelScrollLines: Integer;
+  DeltaPerLine: Integer;
 begin
   Result := inherited DoMouseWheel(Shift, WheelDelta, MousePos);
   if not Result then
@@ -1747,15 +1763,40 @@
     GetScrollInfo(Handle, SB_VERT, SI);
     if SI.nMax = 0 then
       Exit;
+
+    SystemParametersInfo(SPI_GETWHEELSCROLLLINES, 0, @WheelScrollLines, 0);
+    FAccumulatedWheelDelta := FAccumulatedWheelDelta + WheelDelta;
+
     Msg.Msg := WM_VSCROLL;
-    if WheelDelta > 0 then
-      Msg.ScrollCode := SB_PAGEUP
+    if (WheelScrollLines = -1) Or IsPageMode then
+    begin
+      DeltaPerLine := 120;
+      if WheelDelta > 0 then
+        Msg.ScrollCode := SB_PAGEUP
+      else
+        Msg.ScrollCode := SB_PAGEDOWN;
+    end
     else
-      Msg.ScrollCode := SB_PAGEDOWN;
-    Msg.Pos := FScrollPos.Y;
-    Msg.Result := 0;
-    WMVScroll(Msg);
-    Refresh;
+    begin
+      DeltaPerLine := 120 div WheelScrollLines;
+      if WheelDelta > 0 then
+        Msg.ScrollCode := SB_LINEUP
+      else
+        Msg.ScrollCode := SB_LINEDOWN;
+    end;
+
+    while abs(FAccumulatedWheelDelta) >= DeltaPerLine do
+    begin
+      Msg.Pos := FScrollPos.Y;
+      Msg.Result := 0;
+      WMVScroll(Msg);
+      
+      if FAccumulatedWheelDelta > 0 then
+        FAccumulatedWheelDelta := FAccumulatedWheelDelta - DeltaPerLine
+      else
+        FAccumulatedWheelDelta := FAccumulatedWheelDelta + DeltaPerLine;
+    end;
+
     TDeactiveHintThread.Create(500, HintWindow);
     HintWindow := nil;
     Result := True;
jvprvwdocscrolling.diff (5,408 bytes)

obones

2013-01-15 15:42

administrator   ~0020360

Thanks for this, could you provide the zipped sources of a sample application showing the result of this?

2013-07-19 18:48

 

PreviewDemo.zip (82,497 bytes)

jkelley

2013-07-19 19:19

reporter   ~0020575

Sorry for taking so long to get back to you regarding this.

I've attached a demo of the preview control as we're using it. Here are some specific operations that work better with my patch:

* The size of the scroll bar's thumb button is proportional to what's visible on the screen.
* It honors Windows' settings on whether the mouse wheel should scroll N lines or page at a time. (The old version scrolls one third of a page regardless of mouse wheel setting.)
* Scrolling one line (by clicking the scroll bar's arrow buttons) was treated identically to scrolling one page (by clicking the empty space on the scroll bar).
* Scrolling is smoother. To demonstrate, zoom in by clicking the "Fit page width button" (this is 226% zoom on my monitor) and scroll by slowly dragging the thumb button up and down; in the old version, the scrolling motion is very jittery (bounces back and forth some).
* There was screen flicker when scrolling - e.g., at default settings (100%), try rapidly scrolling back and forth with mouse wheel and look for the black and gray page breaks flickering during a scroll operation. (I think that this is another example of the previous fix).
* If the zoom level is set high enough that a horizontal scroll bar would be shown (e.g., 300%), then clicking the scroll down button didn't work, and scrolling one line at a time (for example, by clicking the scroll up button) jumped a third of the printed page (which is more than is even visible on the screen). (This particular bug isn't fixed in this patch but is in my GitHub pull request.)
* The patch adds better support for wheel delta varying from its default of 120 - see http://msdn.microsoft.com/en-us/library/windows/desktop/ms645617%28v=vs.85%29.aspx.

I submitted a pull request with the current version of the bug fix: https://github.com/project-jedi/jvcl/pull/4

AHUser

2013-12-02 17:48

developer   ~0020704

Fixed in branch master.

Issue History

Date Modified Username Field Change
2012-10-01 20:54 jkelley New Issue
2012-10-01 20:54 jkelley File Added: jvprvwdocscrolling.diff
2013-01-15 15:42 obones Note Added: 0020360
2013-01-15 15:42 obones Status new => feedback
2013-07-19 18:48 jkelley File Added: PreviewDemo.zip
2013-07-19 19:19 jkelley Note Added: 0020575
2013-12-02 17:48 AHUser Note Added: 0020704
2013-12-02 17:48 AHUser Status feedback => resolved
2013-12-02 17:48 AHUser Fixed in Version => Daily / SVN
2013-12-02 17:48 AHUser Resolution open => fixed
2013-12-02 17:48 AHUser Assigned To => AHUser
2015-09-14 13:20 obones Fixed in Version Daily / GIT => 3.48