Bug 23308 – layoutDeltas need to be applied before/after transforms

• Component: Layout and Rendering
• Status: NEW

[Simon Fraser (smfr)]

The repaint codepath applies the total layoutDelta before converting repaint rects to absolute coords. This is wrong; the individual deltas for each RenderObject need to be applied in the right places as computeAbsoluteRepaintRects() walks up the container tree, because of transforms.

I haven't managed yet to track down a bug that this causes.

[Simon Fraser (smfr)]

Created attachment 26692 [details]
WIP patch, not ready for prime-time

[Rashmi Shyamasundar]

Created attachment 184114 [details]
canvas-earth.html present in the attached zip, shows a rotating earth.

In webkit-efl port, the earth-image stops rotating and vanishes after a few seconds. This is using canvas pattern, transform and fill along with requestAnimaitonFrame() to achieve the animation.

Please check if this issue could be because of the bug described here.

[Simon Fraser (smfr)]

No, it's unrelated. This bug is about an internal implementation detail of webkit.

[Mark Wang]

This rendering issue happens in my project, which happens only if accelerated-compositing is on.

I found the change below can fix this issue in my project temporarily.   

In RenderObject.cpp: RenderObject::repaintRectangle()

Change
    // FIXME: layoutDelta needs to be applied in parts before/after transforms and
    // repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308
    dirtyRect.move(view->layoutDelta());

    RenderLayerModelObject* repaintContainer = containerForRepaint();

into 
    // FIXME: layoutDelta needs to be applied in parts before/after transforms and
    // repaint containers. https://bugs.webkit.org/show_bug.cgi?id=23308

    RenderLayerModelObject* repaintContainer = containerForRepaint();
#if USE(ACCELERATED_COMPOSITING)
    if (repaintContainer != this || !(view->usesCompositing() && repaintContainer->hasLayer() && repaintContainer->layer()->isComposited()))
#endif
        dirtyRect.move(view->layoutDelta());

[Simon Fraser (smfr)]

Mark, can you please attach a testcase that shows this?

[Mark Wang]

Created attachment 254620 [details]
The case of reproduing the rendering issue.

The attached simple.html is to reproduce the rendering issue when accelerated-compositing is ON. It can be reproduced on Safari (version: 5.1.7(7534.57.2)) and epiphany (version: Web 3.10.3). My environment is webkitgtk2.4.8 (webkit1) on broadcom STB which also can reproduce this rendering issue. 

The reproducing steps:
1/ Loading simple.html.
2/ Click 'Scroll' button two times.
3/ Click 'Scroll' button. The green focused bar will be moved from the bottom to the top. At the moment, you will see the middle 'li' elment is not updated.

[Mark Wang]

Created attachment 254621 [details]
The result on Safari.

The attached test case is rendered wrong on Safari, but it is right on Chrome. Please compare them.

[Mark Wang]

Created attachment 254622 [details]
The test result on Chrome.

The attached test case is rendered right on Chrome.

[Mark Wang]

The call stack when this issue happened.
(gdb) p rect
$1 = (const WebCore::FloatRect &) @0x7fed7f08: {m_location = {m_x = 0, m_y = -119}, m_size = {m_width = 200, m_height = 37}}
(gdb) bt
#0  WebCore::GraphicsLayerTextureMapper::setNeedsDisplayInRect (this=0x6b212d80, rect=...) at Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:118
#1  0x768a48f8 in WebCore::RenderLayerBacking::setContentsNeedDisplayInRect (this=0x686a60b0, r=...) at Source/WebCore/rendering/RenderLayerBacking.cpp:1759
#2  0x7687d080 in WebCore::RenderLayer::setBackingNeedsRepaintInRect (this=0x68c16c40, r=...) at Source/WebCore/rendering/RenderLayer.cpp:5333
#3  0x7690c21c in WebCore::RenderObject::repaintUsingContainer (this=0x68c16bd0, repaintContainer=0x68c16bd0, r=..., immediate=false) at Source/WebCore/rendering/RenderObject.cpp:1348
#4  0x7690c560 in WebCore::RenderObject::repaintRectangle (this=0x68c16bd0, r=..., immediate=false) at Source/WebCore/rendering/RenderObject.cpp:1388
#5  0x766c514c in WebCore::RenderBlock::layoutBlock (this=0x68c16bd0, relayoutChildren=false, pageLogicalHeight=...) at Source/WebCore/rendering/RenderBlock.cpp:1678
#6  0x766c32f0 in WebCore::RenderBlock::layout (this=0x68c16bd0) at Source/WebCore/rendering/RenderBlock.cpp:1413
#7  0x766cbce8 in WebCore::RenderBlock::layoutBlockChild (this=0x68c164b0, child=0x68c16bd0, marginInfo=..., previousFloatLogicalBottom=..., maxFloatLogicalBottom=...)
    at Source/WebCore/rendering/RenderBlock.cpp:2550
#8  0x766cb7a4 in WebCore::RenderBlock::layoutBlockChildren (this=0x68c164b0, relayoutChildren=false, maxFloatLogicalBottom=...) at Source/WebCore/rendering/RenderBlock.cpp:2493
#9  0x766c44b4 in WebCore::RenderBlock::layoutBlock (this=0x68c164b0, relayoutChildren=false, pageLogicalHeight=...) at Source/WebCore/rendering/RenderBlock.cpp:1592
#10 0x766c32f0 in WebCore::RenderBlock::layout (this=0x68c164b0) at Source/WebCore/rendering/RenderBlock.cpp:1413
#11 0x766772e4 in WebCore::RenderObject::layoutIfNeeded() () from sdk_tbc_premium_phase1/target/usr/lib/libwebkitgtk-1.0.so.0
#12 0x766cdb14 in WebCore::RenderBlock::layoutPositionedObjects (this=0x68c16370, relayoutChildren=false, fixedPositionObjectsOnly=false) at Source/WebCore/rendering/RenderBlock.cpp:2795
#13 0x766cd004 in WebCore::RenderBlock::simplifiedLayout (this=0x68c16370) at Source/WebCore/rendering/RenderBlock.cpp:2688
#14 0x766c3f08 in WebCore::RenderBlock::layoutBlock (this=0x68c16370, relayoutChildren=false, pageLogicalHeight=...) at Source/WebCore/rendering/RenderBlock.cpp:1535
#15 0x766c32f0 in WebCore::RenderBlock::layout (this=0x68c16370) at Source/WebCore/rendering/RenderBlock.cpp:1413
#16 0x76577b10 in WebCore::FrameView::layout (this=0x68c8cc80, allowSubtree=true) at Source/WebCore/page/FrameView.cpp:1233
#17 0x7658601c in WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive (this=0x68c8cc80) at Source/WebCore/page/FrameView.cpp:3505
#18 0x765860cc in WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive (this=0x6db76c80) at Source/WebCore/page/FrameView.cpp:3515
#19 0x75be7608 in WebKit::AcceleratedCompositingContext::flushAndRenderLayers (this=0x48a018) at Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextDFB.cpp:385
#20 0x75be7bfc in WebKit::AcceleratedCompositingContext::layerFlushTimerFired (this=0x48a018) at Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextDFB.cpp:481
#21 0x75be7288 in WebKit::AcceleratedCompositingContext::layerFlushTimerFiredCallback (context=0x48a018) at Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextDFB.cpp:342
#22 0x75be8870 in WebKit::AcceleratedCompositingContext::displayRefreshFired (this=0x48a018, monotonicTimeNow=0) at Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextDFB.cpp:567
#23 0x76ed8630 in WebCore::DisplayRefreshMonitorClient::fireDisplayRefreshIfNeeded (this=0x48a01c, timestamp=0) at Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:51
#24 0x76ed8a84 in WebCore::DisplayRefreshMonitor::displayDidRefresh (this=0x6db55948) at Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:118
#25 0x771bf650 in WebCore::fireArtificialVsyncTimer (user_data=0x6db55948) at Source/WebCore/platform/graphics/gtk/DisplayRefreshMonitorGtk.cpp:52
#26 0x73b5a60c in g_timeout_dispatch () from sdk_tbc_premium_phase1/target/usr/lib/libglib-2.0.so.0
#27 0x73b595a4 in g_main_context_dispatch () from sdk_tbc_premium_phase1/target/usr/lib/libglib-2.0.so.0
#28 0x73b599d4 in g_main_context_iterate.clone.5 () from sdk_tbc_premium_phase1/target/usr/lib/libglib-2.0.so.0
#29 0x73b5a0ac in g_main_loop_run () from sdk_tbc_premium_phase1/target/usr/lib/libglib-2.0.so.0
#30 0x751ca8e0 in gtk_main () from sdk_tbc_premium_phase1/target/usr/lib/libgtk-directfb-2.0.so.0
#31 0x00408bcc in ?? ()

GraphicsLayerTextureMapper::setNeedsDisplayInRect() reports a dirty rectangle(x:0,y:-119,width:200,height:37). In fact, this dirty rect will cause nothing is drawn when GraphicsLayerTextureMapper::updateBackingStoreIfNeeded() is called, because this dirty rect is clipped in GraphicsLayerTextureMapper::updateBackingStoreIfNeeded().

This invalidate dirty rectangle is caused by the call of "dirtyRect.move(view->layoutDelta())" in RenderObject::repaintRectangle(), which seems to be called unnecessarily.

[Simon Fraser (smfr)]

Still reproduces in recent Safari.

[Ahmad Saleem]

I was able to reproduce this in Safari 15.5 on macOS 12.4 using attached test. All other browsers Chrome Canary 104 and Firefox Nightly 103 has intended behaviour similar to attached Chrome screenshot. Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/94260301>

[Ethan Wong]

Created attachment 464954 [details]
A case that reproduces this issue.

I got a rendering bug in our production page recently, which could be simplified into this html page that reproduces the layer repainting issue.

I believe the issue I encounter is exactly the same as this bug.

[Ethan Wong]

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