Bug 258351 – [LBSE] Errorous/unnecessary repainting when viewBox is used on <svg> elements

• Component: SVG
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

Created attachment 466775 [details]
Slow SVG

This used to work in the downstream PoC - but since the implementations are way different nowadays, we need to re-investigate.
Currently when viewBox is enabled, the accelerated transform animation performance is artifically lower, since there are unnecessary repaints triggered, but not when viewBoxis omitted -- one sees only compositing steps, no painting (no increase of layer repaint counters, etc)

Testcase (fast w/o viewBox, try enabling viewBox).
<?xml version="1.0" encoding="utf-8"?>
<svg xmlns="http://www.w3.org/2000/svg" fooviewBox="0 0 800 600" width="100%" height="100%">
<style type="text/css">
path#demoScene1 {
    transform-origin: 50% 50%;
    animation: rotateAroundCenterAnimationFrames 2s linear 0s infinite;
}

@keyframes rotateAroundCenterAnimationFrames {
    0% { transform: rotate(0deg); }
   50% { transform: rotate(180deg); }
  100% { transform: rotate(359deg); }
}
</style>
<path id="demoScene1" d="m233.58,47.875c0,7.476-2.158,8.743-8.992,8.743-6.972,0-9.123-1.268-9.123-8.743,0-6.714,2.151-7.723,9.123-7.723,6.834,0.001,8.992,1.01,8.992,7.723zm-1.27,65.47c0,9.976,4.809,15.219,4.809,15.219s-6.383,5.037-11.303,5.037c-6.464,0-9.215-11.614-9.215-16.417v-50.182h15.709v46.343z"/>
</svg>

[Radar WebKit Bug Importer]

<rdar://problem/111445734>

[Nikolas Zimmermann]

When showing the compositing borders in WebInspector, a spurious layer is visible, that gets frequently repainted according to the increasing repaint count.

Breaking on GraphicLayerCA::setNeedsDisplay reveals:

* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 7.2
  * frame #0: 0x0000000284c8aa8c WebCore`WebCore::GraphicsLayerCA::setNeedsDisplay(this=0x00000001310fc560) at GraphicsLayerCA.cpp:902:10
    frame #1: 0x0000000284b8a0f0 WebCore`WebCore::GraphicsLayer::setSize(this=0x00000001310fc560, size=0x000000016bccd2bc) at GraphicsLayer.cpp:550:9
    frame #2: 0x0000000284c89810 WebCore`WebCore::GraphicsLayerCA::setSize(this=0x00000001310fc560, size=0x000000016bccd2bc) at GraphicsLayerCA.cpp:618:20
    frame #3: 0x00000002852bc378 WebCore`WebCore::RenderLayerBacking::updateGeometry(this=0x00000001310b5ff0, compositedAncestor=0x0000000144003eb0) at RenderLayerBacking.cpp:1435:22
    frame #4: 0x00000002852cdd28 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x00000001311079c0, layer=0x00000001440018c0, childLayersOfEnclosingLayer=0x000000016bccd808, traversalState=0x000000016bccd8a0, scrollingTreeState=0x000000016bccd8c0, updateLevel=(m_storage = 2)) at RenderLayerCompositor.cpp:1425:27
    frame #5: 0x00000002852ce1c0 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x00000001311079c0, layer=0x0000000144003eb0, childLayersOfEnclosingLayer=0x000000016bccda68, traversalState=0x000000016bccdb00, scrollingTreeState=0x000000016bccdb20, updateLevel=(m_storage = 0)) at RenderLayerCompositor.cpp:1490:13
    frame #6: 0x00000002852ce1c0 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x00000001311079c0, layer=0x0000000144002940, childLayersOfEnclosingLayer=0x000000016bccdd38, traversalState=0x000000016bccdd48, scrollingTreeState=0x000000016bccdd90, updateLevel=(m_storage = 0)) at RenderLayerCompositor.cpp:1490:13
    frame #7: 0x00000002852cb458 WebCore`WebCore::RenderLayerCompositor::updateCompositingLayers(this=0x00000001311079c0, updateType=AfterStyleChange, updateRoot=0x0000000144002940) at RenderLayerCompositor.cpp:971:9
    frame #8: 0x00000002852ca7fc WebCore`WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout(this=0x00000001311079c0) at RenderLayerCompositor.cpp:607:12
    frame #9: 0x00000002846f59e8 WebCore`WebCore::LocalFrameView::updateCompositingLayersAfterStyleChange(this=0x0000000143140600) at LocalFrameView.cpp:811:39
    frame #10: 0x00000002837e4450 WebCore`WebCore::Document::resolveStyle(this=0x0000000143142400, type=Normal) at Document.cpp:2205:46
    frame #11: 0x00000002837e5034 WebCore`WebCore::Document::updateStyleIfNeeded(this=0x0000000143142400) at Document.cpp:2303:5
    frame #12: 0x000000028470b110 WebCore`WebCore::LocalFrameView::updateLayoutAndStyleIfNeededRecursive(this=0x0000000143140600) at LocalFrameView.cpp:4970:44
    frame #13: 0x000000028476d238 WebCore`WebCore::Page::layoutIfNeeded(this=0x000000013104b700) at Page.cpp:1678:15
    frame #14: 0x000000028476df70 WebCore`WebCore::Page::updateRendering(this=0x000000013104b700) at Page.cpp:1830:5
    frame #15: 0x000000011d0d20e4 WebKit`WebKit::WebPage::updateRendering(this=0x000000014c010c08) at WebPage.cpp:4736:13
    frame #16: 0x000000011c258010 WebKit`WebKit::TiledCoreAnimationDrawingArea::updateRendering(this=0x0000000131094b00, flushType=Normal) at TiledCoreAnimationDrawingArea.mm:361:18
    frame #17: 0x000000011c25c804 WebKit`WebKit::TiledCoreAnimationDrawingArea::renderingUpdateRunLoopCallback(this=0x0000000131094b00) at TiledCoreAnimationDrawingArea.mm:871:5
    frame #18: 0x000000011c25e55c WebKit`WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(this=0x00000001310071b8)::$_0::operator()() const at TiledCoreAnimationDrawingArea.mm:90:15
    frame #19: 0x000000011c25e508 WebKit`WTF::Detail::CallableWrapper<WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(WebKit::WebPage&, WebKit::WebPageCreationParameters const&)::$_0, void>::call(this=0x00000001310071b0) at Function.h:53:39
    frame #20: 0x0000000282ac5314 WebCore`WTF::Function<void ()>::operator(this=0x00000001310f1340)() const at Function.h:82:35
    frame #21: 0x0000000284986ca4 WebCore`WebCore::RunLoopObserver::runLoopObserverFired(this=0x00000001310f1340) at RunLoopObserver.cpp:41:5

Summary:
Page::updateRendering() updates the document style w/o performing relayout, leading to a LocalFrameView::updateCompositingLayersAfterStyleChange() call. That in turn updates compositing layers (in our case an accelerated transform animation is running which has to be applied). The geometry of the backing store of a certain layer is updated, which leads to a GraphicsLayer::setNeedsDisplay() call.

I checked frame #3 0x00000002852bc378 WebCore`WebCore::RenderLayerBacking::updateGeometry(this=0x00000001310b5ff0, compositedAncestor=0x0000000144003eb0) at RenderLayerBacking.cpp:1435:22
the &m_owningLayer=0x1440018c0. Layer tree:

layer 0x144002940 scrollableArea 0x13104f480 at (0,0) size 1013x576 (composited [root], bounds=at (0,0) size 1013x576, drawsContent=1, paints into ancestor=0)
  RenderView 0x144002510 at (0,0) size 1013x576
 positive z-order list (1)
  layer 0x144003eb0 scrollableArea 0x13104fb40 at (0,0) size 1013x576 (composited [transform with composited descendants], bounds=at (0,0) size 1013x576, drawsContent=1, paints into ancestor=0)
    RenderSVGRoot 0x144003d30 {svg} at (0,0) size 1013x576
   positive z-order list (1)
*** layer 0x1440018c0 at (0,0) size 1013x576 (composited [transform with composited descendants], bounds=at (716.42,192.27) size 90.34x55.89, drawsContent=1, paints into ancestor=0)
      RenderSVGViewportContainer 0x1440041e0 at (0,0) size 1013x576
     positive z-order list (1)
      layer 0x1440009a0 at (215.45,40.14) size 23x94 (composited [animation], bounds=at (0,0) size 21.67x93.47, drawsContent=1, paints into ancestor=0)
        RenderSVGPath 0x1440040b0 {path} at (215.45,40.14) size 21.67x93.47 [fill={[type=SOLID] [color=#000000]}] [data="M 233.58 47.875 C 233.58 55.351 231.422 56.618 224.588 56.618 C 217.616 56.618 215.465 55.35 215.465 47.875 C 215.465 41.161 217.616 40.152 224.588 40.152 C 231.422 40.153 233.58 41.162 233.58 47.875 Z M 232.31 113.345 C 232.31 123.321 237.119 128.564 237.119 128.564 C 237.119 128.564 230.736 133.601 225.816 133.601 C 219.352 133.601 216.601 121.987 216.601 117.184 L 216.601 67.002 L 232.31 67.002 L 232.31 113.345 Z"] id="demoScene1"

The layer whose backing store is updated is the one associated with the anonymous RenderSVGViewportContainer (that encloses the whole SVG render tree, and is the only direct child of RenderSVGRoot).

[Nikolas Zimmermann]

GraphicsLayerCA::setNeedsDisplay() is responsible for the repaints:

void GraphicsLayerCA::setNeedsDisplay()
{
    if (!drawsContent())
        return;

    m_needsFullRepaint = true;
    m_dirtyRects.clear();
    noteLayerPropertyChanged(DirtyRectsChanged);
    addRepaintRect(FloatRect(FloatPoint(), m_size));
}

But only if drawsContent() = true.
So how does a GraphicsLayerCA obtain the 'drawsContent()' flag? 

<snippet>
void RenderLayerBacking::updateDrawsContent(PaintedContentsInfo& contentsInfo)
{
...
    bool hasPaintedContent = containsPaintedContent(contentsInfo);

    // FIXME: we could refine this to only allocate backing for one of these layers if possible.
    m_graphicsLayer->setDrawsContent(hasPaintedContent);
    if (m_foregroundLayer)
        m_foregroundLayer->setDrawsContent(hasPaintedContent);
...
}
</snippet>

RenderLayerBacking::containsPaintedContent() is relevant:


<snippet>
bool RenderLayerBacking::containsPaintedContent(PaintedContentsInfo& contentsInfo) const
{
    if (contentsInfo.isSimpleContainer() || paintsIntoWindow() || paintsIntoCompositedAncestor() || m_artificiallyInflatedBounds || m_owningLayer.isReflection())
        return false;
..
</snippet>

So a "simple container" should not even trigger repaints. Does RenderSVGViewportContainer qualify as "simple container" ?

<snippet>
bool RenderLayerBacking::isSimpleContainerCompositingLayer(PaintedContentsInfo& contentsInfo) const
{
    if (m_owningLayer.isRenderViewLayer())
        return false;
        
    if (hasBackingSharingLayers())
        return false;
        
    if (renderer().isRenderReplaced() && !isCompositedPlugin(renderer()))
        return false;
            
    if (renderer().isTextControl())
        return false;
    
    if (contentsInfo.paintsBoxDecorations() || contentsInfo.paintsContent())
        return false;
...
</snippet>


"contentsInfo.paintsContent()" returns true in our testcase, causing the observed problem. To shorten the trace a bit: RenderLayerBacking::paintsContent() contains the logic that determines if "contentsInfo.paintsContent()" returns true or false.

And here's the issue:

<snippet>
bool RenderLayerBacking::paintsContent(RenderLayer::PaintedContentRequest& request) const
{
    m_owningLayer.updateDescendantDependentFlags();
        
    bool paintsContent = false;
    
    if (m_owningLayer.hasVisibleContent() && m_owningLayer.hasNonEmptyChildRenderers(request))
        paintsContent = true;
    
    if (request.isSatisfied())
        return paintsContent;

    if (isPaintDestinationForDescendantLayers(request))
        paintsContent = true;
        
    if (request.isSatisfied())
        return paintsContent;
        
#if ENABLE(LAYER_BASED_SVG_ENGINE)
    if (is<RenderSVGModelObject>(m_owningLayer.renderer())) {
        // FIXME: [LBSE] Eventually cache if we're part of a RenderSVGHiddenContainer subtree to avoid tree walks.
        // FIXME: [LBSE] Eventually refine the logic to end up with a narrower set of conditions (webkit.org/b/243417).
        paintsContent = m_owningLayer.hasVisibleContent() && !lineageOfType<RenderSVGHiddenContainer>(m_owningLayer.renderer()).first();
        request.setHasPaintedContent();
    }
    
    if (request.isSatisfied())
        return paintsContent;
#endif
</snippet>

m_owningLayer.hasVisibleContent() returns true for the layer associated with the RenderSVGViewportContainer.

The repaint problem observed here, is a regression of "[LBSE] Enable compositing support for SVG elements" (https://commits.webkit.org/253054@main), which added the logic that sets 'request.setHasPaintedContent()' and 'paintsContent = true'.

The problem is understood now, checking the landscape of solutions for a proper choice :-)

[Nikolas Zimmermann]

Pull request: https://github.com/WebKit/WebKit/pull/18136

[EWS]

Committed 269034@main (af30a9bb6eef): <https://commits.webkit.org/269034@main>

Reviewed commits have been landed. Closing PR #18136 and removing active labels.