Bug 195378 - Layer with no backing store should still hit-test over a scroller
Summary: Layer with no backing store should still hit-test over a scroller
Status: RESOLVED FIXED
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: 195926
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-06 14:14 PST by Simon Fraser (smfr)
Modified: 2019-03-19 04:07 PDT (History)
6 users (show)

See Also:


Attachments
Testcase (898 bytes, text/html)
2019-03-06 14:15 PST, Simon Fraser (smfr)
no flags Details
patch (35.46 KB, patch)
2019-03-18 10:26 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (35.05 KB, patch)
2019-03-18 10:39 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (35.08 KB, patch)
2019-03-19 01:46 PDT, Antti Koivisto
no flags 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) 2019-03-06 14:14:43 PST
Layers can have no backing store but still either have background-color, or even be totally empty, and they should still hit-test over a scroller.
Comment 1 Simon Fraser (smfr) 2019-03-06 14:15:01 PST
Created attachment 363791 [details]
Testcase
Comment 2 Radar WebKit Bug Importer 2019-03-06 14:15:42 PST
<rdar://problem/48652078>
Comment 3 Antti Koivisto 2019-03-18 10:26:55 PDT
Created attachment 365026 [details]
patch
Comment 4 Antti Koivisto 2019-03-18 10:39:26 PDT
Created attachment 365028 [details]
patch
Comment 5 Simon Fraser (smfr) 2019-03-18 11:13:53 PDT
Comment on attachment 365028 [details]
patch

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

r- to fix the pointer-events toggle issue (if it happens to work, please add a test). Don't forget you can use immediateScrollElementAtContentPointToOffset now).

> Source/WebCore/rendering/RenderLayerBacking.cpp:768
> +        updateEventRegion();

Here you're assuming that a layer configuration update will always happen when something changes that affects where renderers are relative to their enclosing compositing layer. This assumption relies on the fact that RenderLayerBacking::setContentsNeedDisplay() calls setNeedsCompositingConfigurationUpdate(), but if future optimizations allow us to update layer configuration in fewer cases, that might break.

In fact, this might break now with something like:

  <div class="composited"><div id="child"></div>

and dynamic toggle of "pointer-events" style on child. That won't issue a repaint (hopefully) and won't update event regions.

You should also add a testcase that just moves elements around and tests event regions, so we can detect regressions caused by future compositing optimizations.
Comment 6 Antti Koivisto 2019-03-18 12:15:06 PDT
https://bugs.webkit.org/show_bug.cgi?id=195902 for pointe-events issue
Comment 7 WebKit Commit Bot 2019-03-18 12:43:04 PDT
Comment on attachment 365028 [details]
patch

Clearing flags on attachment: 365028

Committed r243092: <https://trac.webkit.org/changeset/243092>
Comment 8 WebKit Commit Bot 2019-03-18 12:43:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Ryan Haddad 2019-03-18 13:13:06 PDT
This change broke the Windows build:

c:\cygwin\home\buildbot\worker\win10-release\build\source\webcore\rendering\renderlayerbacking.cpp(1437): error C2668: 'WebCore::GraphicsContext::GraphicsContext': ambiguous call to overloaded function (compiling source file C:\cygwin\home\buildbot\worker\win10-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource-043dd90b-8.cpp) [C:\cygwin\home\buildbot\worker\win10-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  c:\cygwin\home\buildbot\worker\win10-release\build\source\webcore\platform\graphics\graphicscontext.h(266): note: could be 'WebCore::GraphicsContext::GraphicsContext(PlatformGraphicsContext *)' (compiling source file C:\cygwin\home\buildbot\worker\win10-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource-043dd90b-8.cpp)
  c:\cygwin\home\buildbot\worker\win10-release\build\source\webcore\platform\graphics\graphicscontext.h(522): note: or       'WebCore::GraphicsContext::GraphicsContext(HDC,bool)' (compiling source file C:\cygwin\home\buildbot\worker\win10-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource-043dd90b-8.cpp)
  c:\cygwin\home\buildbot\worker\win10-release\build\source\webcore\rendering\renderlayerbacking.cpp(1437): note: while trying to match the argument list '(nullptr)' (compiling source file C:\cygwin\home\buildbot\worker\win10-release\build\WebKitBuild\Release\DerivedSources\WebCore\unified-sources\UnifiedSource-043dd90b-8.cpp)

https://build.webkit.org/builders/Apple%20Win%2010%20Release%20%28Build%29/builds/2965
Comment 10 Antti Koivisto 2019-03-18 13:29:30 PDT
Windows build fix https://trac.webkit.org/changeset/243096/webkit
Comment 11 WebKit Commit Bot 2019-03-18 18:53:40 PDT
Re-opened since this is blocked by bug 195926
Comment 12 Simon Fraser (smfr) 2019-03-18 18:56:19 PDT
This cause assertions like:

 ASSERTION FAILED: clipRectsContext.rootLayer == m_clipRectsCache->m_clipRectsRoot[clipRectsType] 

0   com.apple.JavaScriptCore      	0x000000070f1be55e WTFCrash + 14 (Assertions.cpp:305)
1   com.apple.WebCore             	0x00000006f84d14eb WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x00000006fbba5629 WebCore::RenderLayer::updateClipRects(WebCore::RenderLayer::ClipRectsContext const&) + 345 (RenderLayer.cpp:5349)
3   com.apple.WebCore             	0x00000006fbba6acf WebCore::RenderLayer::parentClipRects(WebCore::RenderLayer::ClipRectsContext const&) const + 255
4   com.apple.WebCore             	0x00000006fbba6bbf WebCore::RenderLayer::backgroundClipRect(WebCore::RenderLayer::ClipRectsContext const&) const::$_1::operator()() const + 223 (RenderLayer.cpp:5499)
5   com.apple.WebCore             	0x00000006fbb9a77e WebCore::RenderLayer::backgroundClipRect(WebCore::RenderLayer::ClipRectsContext const&) const + 158 (RenderLayer.cpp:5507)
6   com.apple.WebCore             	0x00000006fbba10b0 WebCore::RenderLayer::calculateRects(WebCore::RenderLayer::ClipRectsContext const&, WebCore::LayoutRect const&, WebCore::LayoutRect&, WebCore::ClipRect&, WebCore::ClipRect&, WebCore::LayoutSize const&) const + 112 (RenderLayer.cpp:5519)
7   com.apple.WebCore             	0x00000006fbb9ec0f WebCore::RenderLayer::collectFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul>&, WebCore::RenderLayer const*, WebCore::LayoutRect const&, WebCore::RenderLayer::PaginationInclusionMode, WebCore::ClipRectsType, WebCore::OverlayScrollbarSizeRelevancy, WebCore::ShouldRespectOverflowClip, WebCore::LayoutSize const&, WebCore::LayoutRect const*, WebCore::ShouldApplyRootOffsetToFragments) + 303 (RenderLayer.cpp:4516)
8   com.apple.WebCore             	0x00000006fbb9ba05 WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>) + 2885 (RenderLayer.cpp:4339)

because the paint triggered by RenderLayerBacking::updateEventRegion() caches the wrong clip rect roots.

Also, why are we paying the cost for this event region computation on macOS?
Comment 13 Simon Fraser (smfr) 2019-03-18 19:03:48 PDT
compositing/repaint/requires-backing-repaint.html is a test that hit this assertion, on macOS.
Comment 14 Antti Koivisto 2019-03-19 01:46:16 PDT
Created attachment 365138 [details]
patch

Don't collect the region for layers that have paintsIntoCompositedAncestor() set.
Comment 15 WebKit Commit Bot 2019-03-19 04:07:48 PDT
Comment on attachment 365138 [details]
patch

Clearing flags on attachment: 365138

Committed r243134: <https://trac.webkit.org/changeset/243134>
Comment 16 WebKit Commit Bot 2019-03-19 04:07:50 PDT
All reviewed patches have been landed.  Closing bug.