| Summary: | Momentum animator: Short scrolls are too far, medium scrolls aren't far enough | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, simon.fraser | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2021-11-17 12:30:32 PST
Created attachment 444550 [details]
Patch
Created attachment 444577 [details]
patch
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 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. ... all good points |