Bug 233272 – Momentum animator: Short scrolls are too far, medium scrolls aren't far enough

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Momentum animator: Short scrolls are too far, medium scrolls aren't far enough

[Tim Horton]

Created attachment 444550 [details]
Patch

[Tim Horton]

Created attachment 444577 [details]
patch

[Tim Horton]

<rdar://problem/85472653>

[EWS]

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].

[Darin Adler]

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.

[Tim Horton]

... all good points