Bug 233272 - Momentum animator: Short scrolls are too far, medium scrolls aren't far enough
Summary: Momentum animator: Short scrolls are too far, medium scrolls aren't far enough
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-17 12:30 PST by Tim Horton
Modified: 2021-11-17 16:24 PST (History)
2 users (show)

See Also:


Attachments
Patch (3.18 KB, patch)
2021-11-17 12:32 PST, Tim Horton
no flags Details | Formatted Diff | Diff
patch (3.18 KB, patch)
2021-11-17 14:23 PST, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2021-11-17 12:30:32 PST
Momentum animator: Short scrolls are too far, medium scrolls aren't far enough
Comment 1 Tim Horton 2021-11-17 12:32:18 PST
Created attachment 444550 [details]
Patch
Comment 2 Tim Horton 2021-11-17 14:23:45 PST
Created attachment 444577 [details]
patch
Comment 3 Tim Horton 2021-11-17 14:24:11 PST
<rdar://problem/85472653>
Comment 4 EWS 2021-11-17 15:55:54 PST
Committed r285964 (244364@main): <https://commits.webkit.org/244364@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444577 [details].
Comment 5 Darin Adler 2021-11-17 16:01:38 PST
Comment on attachment 444577 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=444577&action=review

> Source/WebCore/platform/mac/ScrollingEffectsController.mm:139
> +        float value = fabs(originalValue);
> +        float powerLow = 6.7 * pow(value, -.166);
> +        float powerHigh = 36.3 * pow(value, -.392);
> +        const float transitionVelocity = 2000;

This currently mixes float and double, as a cleanup (not at ll urgent) we should use std::abs instead of fabs, which will keep it float instead of converting float to double and then back to float. And 6.7f, std::pow, -.166f, so that is done as float instead of as double then converted to float. Then same for powerHigh.

Also nice to use constexpr instead of const.

> Source/WebCore/platform/mac/ScrollingEffectsController.mm:146
> +        return copysign(value * multiplier, originalValue);

This converts to double and then back to float. Should use std::copysign.
Comment 6 Tim Horton 2021-11-17 16:24:39 PST
... all good points