Bug 169586 – Subpixel rendering causes repainting compositing layers moved by CSS animation

• Status: REOPENED
• Component: Layout and Rendering

[Fujii Hironori]

Created attachment 304349 [details]
test case of moving compositing layers by CSS animation

Subpixel rendering causes repainting compositing layers moved by CSS animation.
This seems the intended effect of subpixel rendering.
But, this degrades CSS animation performance.
I think WebKit needs a solution, for example disabling subpixel rendering.

[Fujii Hironori]

Created attachment 304350 [details]
screenshot of GTK+ port MiniBrowser

[Fujii Hironori]

Here are a simple patch to disable subpixel rendering:

> diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
> index 46c9ecd..4c72640 100644
> --- a/Source/WebCore/rendering/RenderLayerBacking.cpp
> +++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
> @@ -889,8 +889,7 @@ private:
>  LayoutRect RenderLayerBacking::computePrimaryGraphicsLayerRect(const LayoutRect& parentGraphicsLayerRect) const
>  {
>      ComputedOffsets compositedBoundsOffset(m_owningLayer, compositedBounds(), parentGraphicsLayerRect, LayoutRect());
> -    return LayoutRect(encloseRectToDevicePixels(LayoutRect(toLayoutPoint(compositedBoundsOffset.fromParentGraphicsLayer()), compositedBounds().size()),
> -        deviceScaleFactor()));
> +    return LayoutRect(toLayoutPoint(compositedBoundsOffset.fromParentGraphicsLayer()), compositedBounds().size());
>  }
>  
>  LayoutRect RenderLayerBacking::computeParentGraphicsLayerRect(RenderLayer* compositedAncestor, LayoutSize& ancestorClippingLayerOffset) const

[Simon Fraser (smfr)]

Pretty sure we'd repaint even without the subpixel rendering.

[Fujii Hironori]

What do you mean?
The repainting counter won't be raised if I disable subpixel rendering with the patch (comment 2).
And, CSS animation performance was good before subpixel rendering has been landed.

[Fujii Hironori]

Created attachment 304364 [details]
screenshot of Mac port

Mac port also has this issue.

[Fujii Hironori]

One idea is coming into my mind. Disabling subpixel rendering only for moving layers.

[zalan]

(In reply to comment #3)
> Pretty sure we'd repaint even without the subpixel rendering.
Simon, do you know why we are calling RenderLayerBacking::updateGeometry on each frame?

computePrimaryGraphicsLayerRect
RenderLayerBacking::updateGeometry
1   0x104bf8e54 WebCore::RenderLayerCompositor::layerStyleChanged(WebCore::StyleDifference, WebCore::RenderLayer&, WebCore::RenderStyle const*)
2   0x104be5a6c WebCore::RenderLayer::styleChanged(WebCore::StyleDifference, WebCore::RenderStyle const*)
3   0x104c020ae WebCore::RenderLayerModelObject::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
4   0x104b585d4 WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
5   0x104b240eb WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
6   0x104b4411c WebCore::RenderBlockFlow::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*)
7   0x104b8cd9e WebCore::RenderElement::setStyle(WebCore::RenderStyle&&, WebCore::StyleDifference)
8   0x104cc68f9 WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&)
9   0x104cc5ca0 WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&)
10  0x104cc59eb WebCore::RenderTreeUpdater::commit(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >)
11  0x10410b049 WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)
12  0x103ff5b96 WebCore::CSSAnimationControllerPrivate::updateAnimations(WebCore::SetChanged)
13  0x103ff5cde WebCore::CSSAnimationControllerPrivate::updateAnimationTimer(WebCore::SetChanged)
14  0x103ff4e7c WebCore::CSSAnimationControllerPrivate::animationTimerFired()
15  0x104f8d6b0 WebCore::ThreadTimers::sharedTimerFiredInternal()
16  0x1049b912f WebCore::timerFired(__CFRunLoopTimer*, void*)

[Simon Fraser (smfr)]

This is a non-hardware animation, so we just update style and layout every frame.

[Fujii Hironori]

So, you mean web developers should use 'transform' property instread of 'top' property for animation.
I got the idea. Thank you. Closed as INVALID.

[zalan]

Reopening. We should find a way to address this set of extra repaintings.