Bug 195378 – Layer with no backing store should still hit-test over a scroller

• Component: Scrolling
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

Created attachment 363791 [details]
Testcase

[Radar WebKit Bug Importer]

<rdar://problem/48652078>

[Antti Koivisto]

Created attachment 365026 [details]
patch

[Antti Koivisto]

Created attachment 365028 [details]
patch

[Simon Fraser (smfr)]

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.

[Antti Koivisto]

https://bugs.webkit.org/show_bug.cgi?id=195902 for pointe-events issue

[WebKit Commit Bot]

Comment on attachment 365028 [details]
patch

Clearing flags on attachment: 365028

Committed r243092: <https://trac.webkit.org/changeset/243092>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

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

[Antti Koivisto]

Windows build fix https://trac.webkit.org/changeset/243096/webkit

[WebKit Commit Bot]

Re-opened since this is blocked by bug 195926

[Simon Fraser (smfr)]

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?

[Simon Fraser (smfr)]

compositing/repaint/requires-backing-repaint.html is a test that hit this assertion, on macOS.

[Antti Koivisto]

Created attachment 365138 [details]
patch

Don't collect the region for layers that have paintsIntoCompositedAncestor() set.

[WebKit Commit Bot]

Comment on attachment 365138 [details]
patch

Clearing flags on attachment: 365138

Committed r243134: <https://trac.webkit.org/changeset/243134>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.