Bug 194002 - Add some basic geometry information to the scrolling tree
Summary: Add some basic geometry information to the scrolling tree
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
: 193880 (view as bug list)
Depends on:
Blocks: 172917
  Show dependency treegraph
 
Reported: 2019-01-29 20:28 PST by Simon Fraser (smfr)
Modified: 2019-02-01 15:51 PST (History)
5 users (show)

See Also:


Attachments
Patch (98.90 KB, patch)
2019-01-29 20:41 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (99.45 KB, patch)
2019-01-29 20:56 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (100.20 KB, patch)
2019-01-29 21:46 PST, Simon Fraser (smfr)
koivisto: review+
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.58 MB, application/zip)
2019-01-29 23:45 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2019-01-29 20:28:23 PST
State nodes need geometry so we can hit-test in the scrolling tree.
Comment 1 Radar WebKit Bug Importer 2019-01-29 20:31:37 PST
<rdar://problem/47656294>
Comment 2 Simon Fraser (smfr) 2019-01-29 20:41:28 PST Comment hidden (obsolete)
Comment 3 Simon Fraser (smfr) 2019-01-29 20:49:25 PST
*** Bug 193880 has been marked as a duplicate of this bug. ***
Comment 4 Simon Fraser (smfr) 2019-01-29 20:56:31 PST Comment hidden (obsolete)
Comment 5 Simon Fraser (smfr) 2019-01-29 21:46:37 PST
Created attachment 360552 [details]
Patch
Comment 6 Antti Koivisto 2019-01-29 23:35:26 PST
Comment on attachment 360552 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3928
> +    if (m_renderView.frame().isMainFrame())
> +        scrollingGeometry.parentRelativeScrollableRect = frameView.frameRect();
> +    else
> +        scrollingGeometry.parentRelativeScrollableRect = LayoutRect({ }, LayoutSize(frameView.size()));

Why are these cases different?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:3980
> +    if (const Vector<LayoutUnit>* offsets = layer.horizontalSnapOffsets())
> +        scrollingGeometry.horizontalSnapOffsets = *offsets;
> +    if (const Vector<LayoutUnit>* offsets = layer.verticalSnapOffsets())
> +        scrollingGeometry.verticalSnapOffsets = *offsets;
> +    if (const Vector<ScrollOffsetRange<LayoutUnit>>* ranges = layer.horizontalSnapOffsetRanges())
> +        scrollingGeometry.horizontalSnapOffsetRanges = *ranges;
> +    if (const Vector<ScrollOffsetRange<LayoutUnit>>* ranges = layer.verticalSnapOffsetRanges())
> +        scrollingGeometry.verticalSnapOffsetRanges = *ranges;

auto*
Comment 7 Build Bot 2019-01-29 23:44:58 PST
Comment on attachment 360552 [details]
Patch

Attachment 360552 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10948296

New failing tests:
compositing/overflow/overflow-overlay-with-touch.html
compositing/scrolling/overflow-scrolling-layers-are-self-painting.html
platform/ios/media/video-play-glyph-composited-outside-overflow-scrolling-touch-container.html
compositing/overflow/overflow-auto-with-touch.html
compositing/overflow/textarea-scroll-touch.html
compositing/overflow/overflow-auto-with-touch-toggle.html
compositing/overflow/scrolling-content-clip-to-viewport.html
compositing/rtl/rtl-scrolling-with-transformed-descendants.html
compositing/overflow/scrolling-without-painting.html
compositing/overflow/updating-scrolling-content.html
Comment 8 Build Bot 2019-01-29 23:45:00 PST
Created attachment 360559 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 9 Simon Fraser (smfr) 2019-01-30 08:22:06 PST
(In reply to Antti Koivisto from comment #6)
> Comment on attachment 360552 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360552&action=review
> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:3928
> > +    if (m_renderView.frame().isMainFrame())
> > +        scrollingGeometry.parentRelativeScrollableRect = frameView.frameRect();
> > +    else
> > +        scrollingGeometry.parentRelativeScrollableRect = LayoutRect({ }, LayoutSize(frameView.size()));
> 
> Why are these cases different?

For subframes, we don't want the FrameView's location (which is the offset from its parent Widget). I guess the root FrameView always has a location of 0,0? I'd have to check with top insets, headers etc.

> 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:3980
> > +    if (const Vector<LayoutUnit>* offsets = layer.horizontalSnapOffsets())
> > +        scrollingGeometry.horizontalSnapOffsets = *offsets;
> > +    if (const Vector<LayoutUnit>* offsets = layer.verticalSnapOffsets())
> > +        scrollingGeometry.verticalSnapOffsets = *offsets;
> > +    if (const Vector<ScrollOffsetRange<LayoutUnit>>* ranges = layer.horizontalSnapOffsetRanges())
> > +        scrollingGeometry.horizontalSnapOffsetRanges = *ranges;
> > +    if (const Vector<ScrollOffsetRange<LayoutUnit>>* ranges = layer.verticalSnapOffsetRanges())
> > +        scrollingGeometry.verticalSnapOffsetRanges = *ranges;
> 
> auto*

Right.
Comment 10 Simon Fraser (smfr) 2019-01-30 08:28:14 PST
Also this is just the start. We'll also need geometry info for the viewport-contained layers, so I expect this to get more complicated. I also want to build a way to test this without having to generate wheel events.
Comment 11 Simon Fraser (smfr) 2019-01-30 08:34:06 PST
https://trac.webkit.org/changeset/240713/webkit