Bug 226399 - background-attachment:fixed inside position:fixed shouldn't cause slow scrolling
Summary: background-attachment:fixed inside position:fixed shouldn't cause slow scrolling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
Depends on: 226453
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-28 15:09 PDT by Cameron McCormack (:heycam)
Modified: 2021-06-03 14:26 PDT (History)
13 users (show)

See Also:


Attachments
Squashed patch with dependencies for EWS (49.28 KB, patch)
2021-05-30 22:36 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Squashed patch with dependencies for EWS (52.32 KB, patch)
2021-05-31 23:08 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (50.02 KB, patch)
2021-06-01 15:21 PDT, Cameron McCormack (:heycam)
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (54.30 KB, patch)
2021-06-01 17:30 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (54.30 KB, patch)
2021-06-02 19:56 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron McCormack (:heycam) 2021-05-28 15:09:12 PDT
Background images that are background-attachment:fixed don't need to cause slow scrolling when inside position:fixed elements, since they won't move.
Comment 1 Cameron McCormack (:heycam) 2021-05-28 15:09:47 PDT
<rdar://72830073>
Comment 2 Cameron McCormack (:heycam) 2021-05-30 22:36:54 PDT
Created attachment 430175 [details]
Squashed patch with dependencies for EWS
Comment 3 Cameron McCormack (:heycam) 2021-05-31 23:08:47 PDT
Created attachment 430232 [details]
Squashed patch with dependencies for EWS
Comment 4 Cameron McCormack (:heycam) 2021-06-01 15:21:03 PDT
Created attachment 430297 [details]
Patch
Comment 5 Cameron McCormack (:heycam) 2021-06-01 16:05:10 PDT
Comment on attachment 430297 [details]
Patch

Let's fix up this build bustage from dropping the bug 226453 patch.
Comment 6 Cameron McCormack (:heycam) 2021-06-01 17:30:58 PDT
Created attachment 430309 [details]
Patch
Comment 7 Simon Fraser (smfr) 2021-06-01 21:01:13 PDT
Comment on attachment 430309 [details]
Patch

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

Seems OK, but it's worrisome that some layout tests that don't have fixed backgrounds have different repaint behavior.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:4920
> +            // scrolling tree propagates DescendantScrollersHaveSynchronousScrolling bits up the
> +            // tree, but shouldUpdateScrollLayerPositionSynchronously looks at the scrolling state
> +            // tree instead.)

Yeah that sucks.
Comment 8 Cameron McCormack (:heycam) 2021-06-01 22:43:36 PDT
(In reply to Simon Fraser (smfr) from comment #7)
> Seems OK, but it's worrisome that some layout tests that don't have fixed
> backgrounds have different repaint behavior.

I'll investigate that before landing.
Comment 9 Cameron McCormack (:heycam) 2021-06-02 18:52:13 PDT
>    bool hasSynchronousScrollingReasons(ScrollingNodeID nodeID) const { return !synchronousScrollingReasons(nodeID); }

It was a missing ! here. 😐
Comment 10 Cameron McCormack (:heycam) 2021-06-02 19:56:01 PDT
Created attachment 430438 [details]
Patch
Comment 11 EWS 2021-06-03 14:26:35 PDT
Committed r278419 (238443@main): <https://commits.webkit.org/238443@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430438 [details].