Bug 194289 - Glitch-free async scrolling
Summary: Glitch-free async scrolling
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 194433
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-05 08:52 PST by Antti Koivisto
Modified: 2024-03-19 08:35 PDT (History)
7 users (show)

See Also:


Attachments
wip (34.91 KB, patch)
2019-02-05 08:55 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
test case (5.98 KB, text/html)
2019-02-05 09:24 PST, Antti Koivisto
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2019-02-05 08:52:46 PST
Glitch free fixed positioning in iframes.
Comment 1 Antti Koivisto 2019-02-05 08:55:47 PST
Created attachment 361194 [details]
wip
Comment 2 Antti Koivisto 2019-02-05 09:01:13 PST
The existing code computes fixed position rect based on the last layout scroll position in web process and then tries to make it match the actual layer position by applying offsets in the UI process.

This patch gets rid of all that and simply computes the correct fixed position rect entirely in the UI (scrolling tree) side.
Comment 3 Antti Koivisto 2019-02-05 09:24:56 PST
Created attachment 361197 [details]
test case
Comment 4 Simon Fraser (smfr) 2019-02-05 12:26:28 PST
Design statement: all data which is input to the scrolling tree should be computed independent from any scroll positions or viewport rects which the scrolling tree itself manipulates.

That implies that the scrolling tree should compute layer positions simply using the data in the scrolling tree + the current state of scrolling-tree-owned scroll positions (for the root, frames and overflow) and viewport rects.

We'll need to be careful that the "viewport constraints" used for fixed and sticky are "pure" in the sense the they don't encode the current layout and viewport rects. I'm not sure what should happen when a programmatic scroll (when zoomed) affects visual and viewport rects; they need to be update synchronously in the web process for various APIs to work correctly, then I guess the "requested scroll position" is sent to the scrolling tree, and the scrolling thread/UI process basically replicate the same logic and hopefully end up with the same rects?
Comment 5 Simon Fraser (smfr) 2019-02-05 14:21:34 PST
Things to test while you're working on this (all should test with busy content that is triggering layouts):

* fixed in the main frame
* fixed in subframes
* fixed in overflow scroll (can test the shipping code path with -webkit-overflow-scrolling: touch)
* sticky relative to the main frame
* sticky relative to overflow scroll (-webkit-overflow-scrolling: touch)
* behavior of fixed when zoomed on iOS (layout vs. visual viewports)
* behavior of fixed while zooming on iOS (layout/visual viewports)

Test everything on macOS too, including with zooming.
Comment 6 Radar WebKit Bug Importer 2019-02-05 16:33:27 PST
<rdar://problem/47836678>
Comment 7 Simon Fraser (smfr) 2019-02-07 09:49:26 PST
Pulling scroll positions out of layer geometry seems do-able, with a bit of refactoring of RenderLayer::updateLayerPosition(). We'd also need to have RenderLayer::offsetFromAncestor() be able to ignore scroll positions, and plumb some state through all the callers (e.g. clip rect computation) that need scroll-position-independent values, avoiding clip rect caching at the same time.

Scroll position also proposes through layout (see offsetFromContainer()), and it seems harder to extract that. It's also very unclear where that feeds into data that can affect compositing and scrolling tree state.
Comment 8 Simon Fraser (smfr) 2019-02-07 20:44:52 PST
> Scroll position also proposes through layout (see offsetFromContainer()),
^propagates
Comment 9 Antti Koivisto 2019-02-08 05:21:34 PST
Moved the patch to https://bugs.webkit.org/show_bug.cgi?id=194433 since this has turned into architecture master bug.
Comment 10 Simon Fraser (smfr) 2019-02-13 22:27:41 PST
There's another tricky piece to having the scrolling tree inputs be scroll position-independent for layers inside overflow scroll: for a positioned layer whose position is affected by overflow scrolling, that position is a factor of both container scroll position and other CSS positioning. If we were to do as this bug proposes, updating geometry on such layers in a normal compositing update would have to be followed by running the scrolling tree layer positioning logic. So every compositing update would have to be followed by a scrolling tree pass. Maybe not terrible, but some additional complexity.

Right now, avoiding jank on such layers is done via the ScrollingLayerPositionAction::Sync stuff, which is also error-prone.
Comment 11 Simon Fraser (smfr) 2019-02-13 22:29:57 PST
(In reply to Simon Fraser (smfr) from comment #10)

> were to do as this bug proposes, updating geometry on such layers in a
> normal compositing update would have to be followed by running the scrolling
> tree layer positioning logic. So every compositing update would have to be
> followed by a scrolling tree pass.

Actually you only need to do the scrolling tree pass on each compositing flush (and you have to do that before computing tiling areas which rely on these positions).
Comment 12 Simon Fraser (smfr) 2019-02-13 22:32:37 PST
(In reply to Simon Fraser (smfr) from comment #11)

> Actually you only need to do the scrolling tree pass on each compositing
> flush (and you have to do that before computing tiling areas which rely on
> these positions).

And with UI-side compositing we effectively do this already in RemoteLayerTreeDrawingAreaProxy::commitLayerTree(), by calling m_webPageProxy.scrollingCoordinatorProxy()->viewportChangedViaDelegatedScrolling().
Comment 13 Antti Koivisto 2019-02-13 23:02:43 PST
Right. This is more like a recognition (and sharpening) of how things already work rather than a new architectural concept.
Comment 14 Simon Fraser (smfr) 2019-03-07 08:45:59 PST
Some more detail:

The geometry of fixed layers is based on the geometry of layers in the z-order ancestor chain (because the layer tree is unavoidably in z-order, because that's the only way to have CA do the right back-to-front ordering. That means that a fixed layer can be deeply nested in the z-order tree; even though its containing block is trivially the RenderView, we must compute the position for that fixed layer relative to its parent in z-order, which can be some arbitrary layer, even inside of overflow:scroll.

That, in turn, means that layout information is baked into FixedPositionViewportConstraints::m_layerPositionAtLastLayout, and we need pass m_viewportRectAtLastLayout in order to compute deltas in the scrolling tree.

This means that it's impossible to unbake scroll positions out of inputs to the scrolling tree in anything other than the most trivial cases.
Comment 15 Simon Fraser (smfr) 2019-03-07 09:34:24 PST
With fixed, there's one thing you could do: tell the scrolling tree the viewport-relative offsets, then, on the scrolling thread/UI process side, do two -[layer convertPoint:toLayer:] to the root, and to the parent layer, and then set the fixed layer positions as the difference. However, that means you're using the CALayer tree as a source of data, so you risk racing with layer flushing (on macOS).

Also, that wouldn't work for sticky, where part of the constraints are containing block bounds, which aren't represented in the layer or scrolling trees at present. It would also not work for my up-coming positioned layers, whose containing blocks aren't represented in either tree either.