Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wheel scrolling inconsistent, fix suggestion #5367

Open
Zaczero opened this issue Jan 16, 2025 · 20 comments
Open

Wheel scrolling inconsistent, fix suggestion #5367

Zaczero opened this issue Jan 16, 2025 · 20 comments
Labels
bug Something isn't working enhancement New feature or request PR is more than welcomed Extra attention is needed

Comments

@Zaczero
Copy link

Zaczero commented Jan 16, 2025

maplibre-gl-js version: 5.0.1

browser: Chromium

Steps to Trigger Behavior

this._type = (Math.abs(timeDelta * value) < 200) ? 'trackpad' : 'wheel';

This line assumes that if two initial scrolls happen in quick succession, they indicate a trackpad. This leads to frequent false positives, especially when using a free-spinning mouse wheel (like the MX Master 3 series). I also encounter this issue on a regular mouse if I scroll quickly enough.

I believe a good solution would be to maintain a short history of deltaY values and check if they are all equal. The idea is similar to using const wheelZoomDelta = 4.000244140625;, but the problem is that it depends on the system's sensitivity settings.

Image

Perhaps assume "wheel" by default and, when deltaY changes in a way that isn’t simply multiplication (for example, 109.09090672642736 and -218.18181345285473 are effectively equal, notice small floating-point errors), switch to “trackpad” until a new scroll action begins. This would require storing just 1 variable (minDeltaY = min(minDeltaY, abs(e.deltaY) / 2)) instead of working with arrays.

Expected Behavior

Wheel scrolling consistent

Actual Behavior

Wheel scrolling inconsistent

@HarelM
Copy link
Collaborator

HarelM commented Jan 16, 2025

I think this is a continue of the following issue:

I think we should continue the discussion there with all the relevant people involved.
Let me know if I should reopen this ticket or not based on what you read there.

@HarelM HarelM added the need more info Further information is requested label Jan 16, 2025
@Zaczero
Copy link
Author

Zaczero commented Jan 16, 2025

I saw that issue earlier and I don't think I have choppiness/lagging issues. The issue is about wrong scroll type detection causing scroll to sometimes be "normal" and sometimes to be super quick due to trackpad scaling being used incorrectly.

@HarelM
Copy link
Collaborator

HarelM commented Jan 17, 2025

In that case you'll need to dig into this. This seems like a hard nut to crack due to many hardware devices acting differently...

@HarelM HarelM added bug Something isn't working enhancement New feature or request PR is more than welcomed Extra attention is needed and removed need more info Further information is requested labels Jan 17, 2025
@Zaczero
Copy link
Author

Zaczero commented Jan 17, 2025

How about the already proposed fix:

Perhaps assume "wheel" by default and, when deltaY changes in a way that isn’t simply multiplication (for example, 109.09090672642736 and -218.18181345285473 are effectively equal, notice small floating-point errors), switch to “trackpad” until a new scroll action begins. This would require storing just 1 variable (minDeltaY = min(minDeltaY, abs(e.deltaY) / 2)) instead of working with arrays.

This should adapt better than some dev's defined constant of 4.000244140625 and change is few lines max.

@HarelM
Copy link
Collaborator

HarelM commented Jan 17, 2025

I'm not super familiar with this code, this is why I suggested the other thread, where people tried out some stuff (and failed a few times too probably). I belive the array is needed for inertia, but that a very shallow guess...

@Zaczero
Copy link
Author

Zaczero commented Jan 18, 2025

If others are experiencing issues and the code for distinguishing between trackpads and mice is visibly buggy, would it be reasonable to remove it and always assume a mouse until a proper implementation is in place? I am migrating the OpenStreetMap frontend from Leaflet, and this issue is the biggest complaint I have—otherwise, MapLibre is perfect. Let's wait for a solid implementation and test it well when it's ready.

I temporarily worked around this issue by setting trackpad/mouse zoom speeds to the same value with setZoomRate/setWheelZoomRate.

@HarelM
Copy link
Collaborator

HarelM commented Jan 18, 2025

If you have a workaround you can use I think it's a better approach than removing some code.
As I said, I'm not an expert in this area and other people did find a good working solution to their issues according to the relevant PRs etc, so we can't assume it doesn't work well for them.
In any case, any contribution to this repo would greatly appreciated.

@Al-4SW
Copy link
Contributor

Al-4SW commented Jan 19, 2025

I have some experience with scroll_zoom.ts and also have a Logitech 2S free scrolling wheel which I previously did some quite extensive scroll zoom testing with for #5154.

In my testing, I never had the free scrolling wheel or a stepped wheel detected as trackpad, they were always correctly detected as wheel in the code block quoted on line 202 above in the first post. (The wheelZoomDelta test on line 183 checking for specific deltaY 4.000244... never returned true in any of my testing . I think this value is specific to some MacOS setups).

Out of curiosity I did some tests earlier today on v5.0.1 to try and reproduce the issue raised here. Using a Logitech 2S mouse in stepped and free mode, and also a Corsair M55 stepped wheel, I cannot reproduce this issue in either Chrome of Firefox. Wheel is always correctly detected. I also tested a Thinkpad trackpad in Chrome and Firefox, this gets detected correctly every time as a trackpad, on line 202 in Firefox and line 187 in Chrome.

I am wondering if the issue you experiencing ("scroll to sometimes be 'normal' and sometimes to be super quick") is caused by the MX 3S models 'smartshift' feature? I understand this automatically switches the mouse into free scroll mode, and could perhaps then be triggering a change in zoom rate in renderFrame() due to this line:

const zoomRate = (this._type === 'wheel' && Math.abs(this._delta) > wheelZoomDelta) ? this._wheelZoomRate : this._defaultZoomRate;

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

For extra context. When testing on a classical mouse the issue also happens. In both situations it's rare but it happens, and when it does it's noticeable and annoying. The bug goes away after finished scrolling and a short delay (probably the 400ms timeout). I think MX is more affected because it is triggered by two false conditions as opposed to just one in classical mouse. Basically when scrolling there is a high chance it will fullfil the Math.abs(value) < 4 condition and switch to trackpad until rest. I assume MacOS magic mouse would also be as buggy as MX.

@Al-4SW
Copy link
Contributor

Al-4SW commented Jan 19, 2025

What are the values of 'value' you are getting? In Chrome with the 2S in free scroll mode my min abs value is 120, so I'm never anywhere near triggering the <4 condition

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

Okay I talked nonsense. Smallest value I got was 13.(3). Perhaps it's the same condition as described originally (timeDelta * value) but because the MX is more precise, there are smaller timeDeltas between events, and smaller values making the Math.abs(timeDelta * value) < 200 very frequent.

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

For reference

Trackpad deltas
VM861:2 30.547395128115213
VM861:2 35.07293243913124
VM861:2 38.46708780657896
VM861:2 33.941547316648666
VM861:2 36.20431756161381
VM861:2 30.547395128115213
VM861:2 18.102158780806906
VM861:2 13.576619880333746
VM861:2 6.788309940166873
VM861:2 5.6569248176843
VM861:2 4.525539695201727
4VM861:2 1.1313849238004317
VM861:2 -2.2627698476008633
VM861:2 -1.1313849238004317
VM861:2 -2.2627698476008633
VM861:2 -4.525539695201727
VM861:2 -2.2627698476008633
VM861:2 -4.525539695201727
VM861:2 -6.788309940166873
VM861:2 -9.051079390403453
VM861:2 -11.3138496353686
VM861:2 -27.153239760667493
VM861:2 -24.890469515702346
VM861:2 -20.364929025772053
VM861:2 -18.102158780806906
VM861:2 -14.70800500281632
MX deltas
VM861:2 53.33333121405716
VM861:2 19.999999205271436
VM861:2 13.33333280351429
VM861:2 19.999999205271436
VM861:2 13.33333280351429
VM861:2 26.66666560702858
VM861:2 13.33333280351429
VM861:2 19.999999205271436
VM861:2 13.33333280351429
VM861:2 -79.99999682108574
VM861:2 -39.99999841054287
2VM861:2 -33.333332008785725
VM861:2 -39.99999841054287
VM861:2 -13.33333280351429
VM861:2 -26.66666560702858
2VM861:2 -19.999999205271436

@Al-4SW
Copy link
Contributor

Al-4SW commented Jan 19, 2025

hmm ok interesting - looks like from the deltaY values for 3S behaves more like a trackpad. On the 2S, i get pretty consistent values 120 120 240 120 etc

Do you have any smooth scroll extensions in Chrome which could be affecting the values?

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

Talking about the broader picture, user experience.

After applying constant zoomRate to trackpad/mouse like:

    map.scrollZoom.setWheelZoomRate(zoomRate)
    map.scrollZoom.setZoomRate(zoomRate)

everything works nice and feels great. Both on trackpad and the mouse. When using original behavior, the mouse is buggy and the trackpad zoom unnaturally quick -- very different feel from typical website scroll. Is there even a good reason for having trackpad/mouse differentiation in the first place? Logically, the user will configure trackpad scroll speed to feel nice for them, why do we need to accelerate it further.

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

Do you have any smooth scroll extensions in Chrome which could be affecting the values?

No, only default browser smooth scroll.


I recorded a video explaining how weird default behavior feels on a trackpad. Idk if that makes sense. To me it really seems like a lot of code logic for worse experience and buggy detection. Of course that's just my personal opinion and your experience may differ.

1000022179.mp4

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

This feature was added in bc50719 12 years ago. States:
fixes #206
refs #212

But none of the linked issues/prs really explain why one would want such behavior. They seem quite unrelated actually. Maybe some number typo?

I understand the goal of this feature is to make scrolling with touchpad faster than normal using hacky heuristics. But I don't understand why - it feels unnatural and different from other browsing experiences. Systems nowadays have options to configure different speeds for trackpads and mouse, maybe it wasn't as simple 12 years ago. But today, even assuming the algorithm is bug-free, I think this feature makes the user experience worse.

@Al-4SW
Copy link
Contributor

Al-4SW commented Jan 19, 2025

I also experience fast trackpad scroll in Chrome like you show. Try it in Firefox though, its slower and much more like how you show with your adjusted zoomrates. Strangely enough, wheel scroll is much slower in Chrome than in Firefox!

A rationalised single zoom rate would be great I think if it worked well for all device types / browser combinations...but I guess that won't be the case and that's why the wheel / trackpad types with different zoom rates were introduced?

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

About Firefox, try switching to xinput2: https://wiki.archlinux.org/title/Firefox#Touchscreen_gestures_and_pixel-perfect_trackpad_scrolling By default Firefox uses some old implementation that doesn't support some modern mice, and assuming trackpad, features like high precision scroll. Maybe that will explain why you see differences in Firefox/Chrome.

About second, Google Maps doesn't seem to differentiate that (and they have much a bigger userbase). The original commit didn't explain why such feature was added so we can only speculate. Speculate about theoretical examples in which it can help: like "fixing" bad client system sensitivity configuration. Or state real issues with this code: worse experience for users with properly configured system sensitivity, and buggy zoom behavior in rare cases on normal mice.

@Al-4SW
Copy link
Contributor

Al-4SW commented Jan 19, 2025

About Firefox, try switching to xinput2

I wasn't aware of this, but I'm not sure if this is the issue - It is the same on Windows for me too, Firefox has faster default scroll wheel speeds.

About second, Google Maps doesn't seem to differentiate that

I think perhaps (speculation I know!) that Google Maps may implement some form of normalisation then? Something like this: https://github.com/facebookarchive/fixed-data-table/blob/master/src/vendor_upstream/dom/normalizeWheel.js

Have you been able to test and verify exactly which code block in scroll_zoom.ts is causing the issue with the MX3 sometimes fast sometimes normal speeds? i.e is it because the wheel scroll is identified as a trackpad for one zoom event, and then another time for a different zoom is identified as a wheel... or is it getting consistently identified as the same device type, but changing mid-zoom due to the line on 291 I quoted earlier?

@Zaczero
Copy link
Author

Zaczero commented Jan 19, 2025

I've read that normalizeWheel code and it seems unlikely to help in this situation. It helps to work around standardizing scroll event data seen in old browsers. It doesn't differentiate between trackpads and mouse any more than a standard "wheel" event does. The wheel event can operate in lines (typically classic mouse) and pixels (high precision, trackpads) modes, which MapLibre already utilizes and acts accordingly.

Have you been able to test and verify

Nope, I haven't been working on that project (yet). I don't see it as worthwhile to put debugging time into this algorithm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants