sumatrapdfreader / sumatrapdf

SumatraPDF reader
http://www.sumatrapdfreader.org
GNU General Public License v3.0
13.62k stars 1.73k forks source link

Incorrect handling of WM_MOUSEWHEEL messages causes fine-scrolling to be 120x faster #3032

Closed expikr closed 5 months ago

expikr commented 2 years ago

From Microsoft's WM_MOUSEWHEEL documentation

The wheel rotation will be a multiple of WHEEL_DELTA, which is set at 120. This is the threshold for action to be taken, and one such action (for example, scrolling one increment) should occur for each delta.

The delta was set to 120 to allow Microsoft or other vendors to build finer-resolution wheels (a freely-rotating wheel with no notches) to send more messages per rotation, but with a smaller value in each message. To use this feature, you can either add the incoming delta values until WHEEL_DELTA is reached (so for a delta-rotation you get the same response), or scroll partial lines in response to the more frequent messages. You can also choose your scroll granularity and accumulate deltas until it is reached.

Current code treats any wheel delta to be treated as if the user has scrolled a full 120 unit:

https://github.com/sumatrapdfreader/sumatrapdf/blob/c161582ece62c873f2f0c0e531913644df7c01ea/src/previewer/PdfPreview.cpp#L305

This means that devices capable of sending single-count scrolling inputs end up having their inputs multiplied by 120.

Mozilla Firefox correctly handles single-count scrolling inputs, by comparison. In fact, it seems that it is the only browser available on Windows that does, as all Chromium based apps (tested: Edge, Chrome, VSCode, Discord, various Electron apps) also fail to correctly handle fine scrolling, though with the opposite failure mode of of dropping counts (i.e. "negative acceleration") when fine-scrolling are sent rapidly.

expikr commented 2 years ago

How Firefox handles wheel deltas:

https://github.com/mozilla/gecko-dev/blob/657feb8e6d7a18057cf68cd68c700ae10e574329/widget/windows/WinMouseScrollHandler.cpp

It seems that the majority of the logic was implemented by @masayuki-nakano.

GitHubRulesOK commented 2 years ago

for the entry you quote I do not know but think that part is redundant as never seen it recently scroll a line only pages, but it could be my settings win 7 10 and 11 all scroll one page at a time as I can't find a means to switch that to continuous scroll

@kjk did previewer previously work as continuous scrolling pages ? apparently not

Preview handler only does full page. https://forum.sumatrapdfreader.org/t/outlook-folders-previewer-change-set-zoom-width/2506

expikr commented 2 years ago

for the entry you quote I do not know but think that part is redundant as never seen it recently scroll a line only pages, but it could be my settings win 7 10 andd 11 all scroll one page at a time as I cant find a means to switch that to continuous

@kjk did previewer previously work as continuous scrolling pages ?

Regular mice wheels only send +/-120 deltas. Many OEM devices such as touchpads/digitizers are capable of sending fine-grained single count deltas. You can emulate those devices as well using the SendInput API, which is in fact what Windows Precision Touchpad driver does to convert digitizer inputs into mouse motion.

kjk commented 5 months ago

@expikr Have you observed this in practice or was it just based on reading the code?

This particular line of code is from preview handler. Sumatra has much more complicated WM_MOUSEWHEEL handling.

expikr commented 5 months ago

It started with observations of erratic behaviour in practice. Then I tried to look into the code to find the possible culprit, but this may not be the source of it. However, the ground truth is the erratic scrolling as described.

GitHubRulesOK commented 5 months ago

@expikr

There have been many recent changes over the last few days so perhaps re-test with todays 3.6.16081 https://www.sumatrapdfreader.org/prerelease

kjk commented 5 months ago

I'm going to close that because it doesn't accurately describe WM_MOUSEWHEEL in SumatraPDF.

If you want some improvement to scrolling behavior, I need enough information to reproduce the issue myself.

masayuki-nakano commented 5 months ago

Although I'm not the user of SumatraPDF, according to the code showed in this list, the message handlers do not work with high-precision aware mice (including touchpads) like enabling smooth scroll of Logitech mice (not for Gaming). Starting from Vista, the delta value of WM_MOUSEWHEEL and WM_MOUSEHWHEEL messages may be less than 120. E.g., if the registries are set to default (3 lines and 3 chars to scroll per 120), the scroll should occur per 40 delta (= 120 / 3). Then, such mice may send the messages with 40 delta values 3-times for wheel operation for 120 delta (e.g., 1 notch of non-smooth wheel). The reason why the mice want to do that is, their developers expect 3 smaller scroll rather than 1 normal scroll makes users feel "smooth".

Therefore, I made Firefox/Gecko accumulates the raw delta values of the messages (from a request from Logitech developers). Then, when the delta values becomes equal or larger than 1 line delta values, Firefox/Gecko decreases the accumulated raw delta value and handles 1-line scroll. However, SumatraPDF look like that it just checks whether the delta is positive or negative. Therefore, a lot of wheel messages cause too fast scroll (up to x120 faster).

Note that the code does not support per-page scroll too. I believe that number of users who like per-page scroll is not majority. However, it's better to support for UX.

kjk commented 5 months ago

@masayuki-nakano thanks for sharing your experience. While I don't claim our handling of WM_MOUSEWHEEL is perfect, to clarify: the code you high-lighted is just dispatch.

The actual handling is https://github.com/sumatrapdfreader/sumatrapdf/blob/22a40f8dcb1ff71e31bd0974bd38ca8e72253afb/src/Canvas.cpp#L1153, the scroll logic starts at https://github.com/sumatrapdfreader/sumatrapdf/blob/22a40f8dcb1ff71e31bd0974bd38ca8e72253afb/src/Canvas.cpp#L1232 (before that is zoom logic if Ctrl is pressed).

We do account for delta != 120 and we do the accumulation logic, see win->wheelAccumDelta

We scroll by line if accumulated value goes over gDeltaPerLine.

In some cases we flip the whole page on scroll.

We scroll faster if mouse of scrollbar.

We scroll by half page instead of line if you press Alt (faster)

tactilis commented 5 months ago

@kjk

We scroll by half page instead of line if you press Alt (faster)

That's great, and I see that it is in the Manual, so I should have known this.

The only downside is that when you release the Alt key at the end of scrolling, most of the time but not always, it activates SumatraPDF's menu (a normal Windows action), which inhibits normal mousewheel scrolling, so you have to press Alt again (or press Esc or left click the mouse) to cancel the menu.

Can anything be done to prevent the release of Alt after mousewheel scrolling from activating the menu?

kjk commented 5 months ago

@tactilis I believe I just significantly improved Alt suppression while scrolling with https://github.com/sumatrapdfreader/sumatrapdf/commit/7e12ebbbcec449756507e96f5a7bc12d6b286a92

So it should be present in next (pre-release](https://www.sumatrapdfreader.org/prerelease) in about 24hrs

I also documented various ways of scrolling / zooming with this commit

Will be on the website soon and in pre-release in manual embedded in Sumatra (F1).

expikr commented 5 months ago

You can use this to test smooth scrolling behaviour in SumatraPDF.

GitHubRulesOK commented 5 months ago

@expikr

Firstly why does everybody expect lightweight Windows users have 64bit OS I had to find 32bit exe and edit VBS to run that app,

and whilst it works in Edge fairly well OOB (replacing its jerky scan lines scroll) it makes SumatraPDF and Autoscroll opposite to my normal (jerkier than without it) Perhaps I dont know which setting to alter ! but I dont have to set anything for click on off scrolling with the $5 mouse and it run smooth and or fast.

expikr commented 5 months ago

We're talking about sub-120 scrolling behaviour here.