Bug 230450 - Have ScrollAnimation work in terms of offsets, not positions
Summary: Have ScrollAnimation work in terms of offsets, not positions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks: 230465
  Show dependency treegraph
 
Reported: 2021-09-18 16:16 PDT by Simon Fraser (smfr)
Modified: 2021-09-19 18:17 PDT (History)
13 users (show)

See Also:


Attachments
Patch (31.57 KB, patch)
2021-09-18 16:19 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (32.44 KB, patch)
2021-09-18 21:57 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (32.49 KB, patch)
2021-09-19 10:10 PDT, Simon Fraser (smfr)
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2021-09-18 16:16:39 PDT
Have ScrollAnimation work in terms of offsets, not positions
Comment 1 Simon Fraser (smfr) 2021-09-18 16:19:59 PDT
Created attachment 438569 [details]
Patch
Comment 2 Simon Fraser (smfr) 2021-09-18 21:57:36 PDT
Created attachment 438577 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-09-19 10:10:43 PDT
Created attachment 438598 [details]
Patch
Comment 4 Myles C. Maxfield 2021-09-19 11:52:18 PDT
Comment on attachment 438598 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Scroll positions can have negagive values in RTL content. It's simpler for ScrollAnimation

negagive
Comment 5 Antti Koivisto 2021-09-19 12:19:42 PDT
Comment on attachment 438598 [details]
Patch

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

> Source/WebCore/platform/ScrollAnimationSmooth.cpp:76
> -bool ScrollAnimationSmooth::startAnimatedScrollToDestination(const FloatPoint& fromPosition, const FloatPoint& destinationPosition)
> +bool ScrollAnimationSmooth::startAnimatedScrollToDestination(const FloatPoint& fromOffset, const FloatPoint& destinationOffset)

I think the difference between "position" and "offset" here is too subtle and just changing names doesn't really make things clear. It would be nice to capture it to a type.
Comment 6 Simon Fraser (smfr) 2021-09-19 18:16:06 PDT
https://trac.webkit.org/changeset/282741/webkit
Comment 7 Radar WebKit Bug Importer 2021-09-19 18:17:15 PDT
<rdar://problem/83289902>