Fix smooth mouse wheel zooming

[Al-4SW]

Fixes #4362

This PR addresses issues with smooth mouse scroll zooming.

Issue: Stutter/'Lag' on zooms with multiple scroll wheel events

Fix: The condition for applying easing to 'wheel' events has been reverted back to v4.5.0.

This fixes the issue where zoom events with multiple wheel events sometimes bypass the easing block in renderFrame() on the first renderFrame of subsequent wheelEvents (wheelEventTimeDiff===0), causing the stutter effect.

Issue: Jitter and Lag for fast 'free scrolling' wheel events (Particularly Chromium based browsers when DevTools is closed)

Fix: Wheel events with 'wheelEventTimeDiff' values of 0 are now used to calculate easing, with a new minimum 'minWheelEventTimeDiff' value applied.

This fixes the issue with small zoom level change caused by fast free scrolling wheels. By normalising the 'wheelEventTimeDiff' value used to calculate easing, we achieve smooth and responsive fast zooms in Chrome, emulating the behaviour when DevTools is open (see below).

DevTools open / closed in Chrome exhibits different fast scroll performance

Test results below indicate the cause for this behaviour. The number of very fast (<1ms) 'lastWheelEventTimeDiff ' values goes from around 80% of all events when DevTools are closed, to 7-8% when open. When open, around 60% of all values are at least 5ms, and none are 0.

4.5.0 Smooth Zoom Tests (For Benchmark Comparison)

5.0.0pre8 Smooth Zoom Tests

Fix Smooth Zoom Tests

Before - Multiple Stepped Wheel Scrolls
https://github.com/user-attachments/assets/87303797-cde0-4494-87fb-ce1b4e25ad5c

After - Multiple Stepped Wheel Scrolls
https://github.com/user-attachments/assets/ade3ffcf-5b0d-4e9e-afea-5b79d3dfc29b

Before - Free Scrolling Wheel
https://github.com/user-attachments/assets/8ae39794-814c-456f-91f5-fc26b478b91d

After - Free Scrolling Wheel
https://github.com/user-attachments/assets/a8cc186c-269e-447b-a068-9f2fae432c67

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.84%. Comparing base (9a85e82) to head (a077d3d).
Report is 15 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5154      +/-   ##
==========================================
+ Coverage   91.82%   91.84%   +0.01%     
==========================================
  Files         278      278              
  Lines       38347    38336      -11     
  Branches     6705     6714       +9     
==========================================
- Hits        35213    35209       -4     
+ Misses       3001     2997       -4     
+ Partials      133      130       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[HarelM]

In what unit is this? Milliseconds? Why is added? Shouldn't it be used with min/max instead?

[Al-4SW]

Yes, it's a time adjustment in milliseconds to make sure we don't get low values for multiple fast wheel events.

'lastWheelEventTimeDiff' in milliseconds is always increasing in value from the last wheelEvent, until targetZoom is reached or another wheelEvent interrupts the process. We can't cap it to a max value. If the easing function gets the same 't' time value, it will pass the same 'k' easing value to the interpolation function and we get back the same eased mid-zoom level.

The renderFrame() easing block is expecting increasing 'lastWheelEventTimeDiff' values in ms which are used to interpolate new intermediate zoom levels between startZoom and targetZoom on each renderFrame().

For example, here is output of logging renderFrame() calls for a single tick wheel event in release 5.0.0pre.8 (rounded to 3dp):

diff=41.000|t=0.205|k = 0.307|zoom=1.382|startZ=1.280| targetZ=1.614
diff=49.000|t=0.245|k = 0.397|zoom=1.412|startZ=1.280| targetZ=1.614
diff=56.000|t=0.28|k = 0.473|zoom=1.438|startZ=1.280| targetZ=1.614
diff=68.000|t=0.34|k = 0.588|zoom=1.476|startZ=1.280| targetZ=1.614
diff=81.000|t=0.405|k = 0.690|zoom=1.510|startZ=1.280| targetZ=1.614
diff=95.000|t=0.475|k = 0.776|zoom=1.539|startZ=1.280| targetZ=1.614
diff=107.000|t=0.535|k = 0.835|zoom=1.559|startZ=1.280| targetZ=1.614
diff=121.000|t=0.605|k = 0.889|zoom=1.576|startZ=1.280| targetZ=1.614
diff=134.000|t=0.67|k = 0.927|zoom=1.589|startZ=1.280| targetZ=1.614
diff=146.000|t=0.73|k = 0.953|zoom=1.598|startZ=1.280| targetZ=1.614
diff=161.000|t=0.805|k = 0.977|zoom=1.606|startZ=1.280| targetZ=1.614
diff=174.000|t=0.87|k = 0.990|zoom=1.610|startZ=1.280| targetZ=1.614
diff=187.000|t=0.935|k = 0.998|zoom=1.613|startZ=1.280| targetZ=1.614
diff=200.000|t=1|k = 1.000|zoom=1.614|startZ=1.280| targetZ=1.614

Same again but for this fix PR:
diff+minDiff=47.000|t=0.235|k = 0.375|zoom=1.405|startZ=1.280| targetZ=1.614
diff+minDiff=59.000|t=0.295|k = 0.503|zoom=1.448|startZ=1.280| targetZ=1.614
diff+minDiff=63.000|t=0.315|k = 0.542|zoom=1.461|startZ=1.280| targetZ=1.614
diff+minDiff=74.000|t=0.37|k = 0.637|zoom=1.493|startZ=1.280| targetZ=1.614
diff+minDiff=86.000|t=0.43|k = 0.723|zoom=1.521|startZ=1.280| targetZ=1.614
diff+minDiff=100.000|t=0.5|k = 0.802|zoom=1.548|startZ=1.280| targetZ=1.614
diff+minDiff=113.000|t=0.565|k = 0.860|zoom=1.567|startZ=1.280| targetZ=1.614
diff+minDiff=126.000|t=0.63|k = 0.904|zoom=1.582|startZ=1.280| targetZ=1.614
diff+minDiff=140.000|t=0.7|k = 0.941|zoom=1.594|startZ=1.280| targetZ=1.614
diff+minDiff=153.000|t=0.765|k = 0.965|zoom=1.602|startZ=1.280| targetZ=1.614
diff+minDiff=167.000|t=0.835|k = 0.984|zoom=1.608|startZ=1.280| targetZ=1.614
diff+minDiff=179.000|t=0.895|k = 0.994|zoom=1.612|startZ=1.280| targetZ=1.614
diff+minDiff=193.000|t=0.965|k = 0.999|zoom=1.613|startZ=1.280| targetZ=1.614
diff+minDiff=207.000|t=1|k = 1.000|zoom=1.614|startZ=1.280| targetZ=1.614

For a test, If I fix lastWheelEventTimeDiff = 1 and input that into the 't' calculation, this is the output which fires may times per second...no zooming because there's no increase in timeDiff:

diff=1.000|t=0.005|k = 0.002|zoom=0.442|startZ=0.441| targetZ=0.775

[HarelM]

I'm not sure I follow all this console printing... The question was about using min/max operation vs addition, which I'm not sure I got a straight answer...

[Al-4SW]

Ok, fair enough... The short answer then is yes, it needs to be an addition.

You did also ask 'why'? ... which is what I'm trying to show with the logs

[HarelM]

Yes, but I'm not sure I can understand the meaning of the logs easily, thus I wanted a higher level answer, if possible, of why addition is needed to a variable with the name "minWheel...".
Sorry for nagging...

[Al-4SW]

not an issue at all... I understand you need to be happy that what I have submitted is correct and makes sense.

I understand that you are asking why not use:

lastWheelEventTimeDiff = Math.max(lastWheelEventTimeDiff,minWheelEventTimeDiff);

This fails the 'Zooms for multiple mouse wheel ticks with easing for smooth zooming' test, because we get multiple renderFrames where lastWheelEventTimeDiff < minWheelEventTimeDiff, which means that for these renderFrames, no new interpolated zoom level is provided, because we keep providing it the same lastWheelEventTimeDiff value of 5 (minWheelEventTimeDiff).

In actual testing, its not really noticeable. But maybe there's an edge case I'm missing. There's probably a 'just right' scroll wheel speed where this may be noticeable and perceived as lag/judder.

With my proposed method, it is impossible to get the same interpolated zoom level twice, because we're always +5 the actual calculated time difference value. I think this is the more robust and failproof approach.

[Al-4SW]

....and I called it minWheelEventTimeDiff because we can and do often get a lastWheelEventTimeDiff of 0, which with this fix is increased to 5, so 5 is always the lowest value ever provided to calculate 't' for the easing function.

[HarelM]

Thanks for the detailed explanation!
I would consider changing the name to wheelEventTimeDiffAdjustment, this is optinal.

[Al-4SW]

That sounds better, I'll change it and push it with the test changes later. Thanks!

[HarelM]

This is a great research and explanation, THANKS!

[HarelM]

Thanks!

[Al-4SW]

Great! Thanks Harel