Bug 104396 – [chromium] Support GraphicsLayer::setContentsToSolidColor

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Noam Rosenthal]

After https://bugs.webkit.org/show_bug.cgi?id=103786, background colors are directly composited in WebKit.
Chromium compositor still doesn't have support for that. Adding background colors to chromium compositor can help with memory footprint and texture-uploads for certain content.

[James Robinson]

This should be an easy thing to add for whoever's interested - just have to call setBackgroundColor(), make sure the draws content flags are all set up correctly, and test it.

No'am - where are the layout tests for this feature?

[Noam Rosenthal]

(In reply to comment #1)
> This should be an easy thing to add for whoever's interested - just have to call setBackgroundColor(), make sure the draws content flags are all set up correctly, and test it.
> 
> No'am - where are the layout tests for this feature?

Ref tests, compositing/background-color.

[James Robinson]

Created attachment 181396 [details]
wip, does not work fully

[James Robinson]

Created attachment 190837 [details]
Patch

[James Robinson]

This patch works fine except:

1.) Edge AA isn't enabled for these layers, so the solid color layers are jaggier than their painted equivalents.  Will be fixed soon by https://code.google.com/p/chromium/issues/detail?id=166570

2.) 170-odd tests will need text rebaselines due to layers not having (drawsContent 1) any more

[WebKit Review Bot]

Comment on attachment 190837 [details]
Patch

Attachment 190837 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16819512

New failing tests:
compositing/geometry/clip.html
compositing/contents-opaque/contents-opaque-background-color.html
compositing/geometry/limit-layer-bounds-positioned.html
compositing/geometry/bounds-ignores-hidden-composited-descendant.html
http/tests/cache/subresource-failover-to-network.html
compositing/contents-opaque/contents-opaque-layer-transform.html
compositing/geometry/layer-due-to-layer-children-deep.html
compositing/geometry/fixed-position-composited-switch.html
compositing/geometry/limit-layer-bounds-transformed-overflow.html
compositing/iframes/become-overlapped-iframe.html
compositing/filters/sw-shadow-overlaps-hw-layer.html
compositing/geometry/preserve-3d-switching.html
compositing/geometry/limit-layer-bounds-overflow-root.html
compositing/filters/sw-layer-overlaps-hw-shadow.html
compositing/geometry/limit-layer-bounds-fixed-positioned.html
compositing/geometry/layer-due-to-layer-children-deep-switch.html
compositing/geometry/ancestor-overflow-change.html
compositing/geometry/limit-layer-bounds-positioned-transition.html
compositing/geometry/clip-inside.html
compositing/geometry/bounds-ignores-hidden-dynamic-negzindex.html
compositing/columns/composited-in-paginated.html
compositing/iframes/become-composited-nested-iframes.html
compositing/contents-opaque/contents-opaque-layer-opacity.html
compositing/geometry/limit-layer-bounds-transformed.html
compositing/iframes/composited-parent-iframe.html
compositing/geometry/flipped-writing-mode.html
compositing/overflow-trumps-transform-style.html
compositing/backing/no-backing-for-clip.html
compositing/backing/no-backing-for-clip-overlap.html
compositing/backing/no-backing-for-perspective.html

[James Robinson]

Support for AA for these sorts of layers had landed: https://code.google.com/p/chromium/issues/detail?id=166570.  I can sort out how to deal with the text rebaselines if someone reviews this - Enne?

[Adrienne Walker]

Comment on attachment 190837 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:528
> +    if (color.isValid()) {

When can this happen? Is removing the layer entirely the logical equivalent of what happens in the normal path if you have a layer that is nothing but a background-color with an invalid color?

[James Robinson]

(In reply to comment #8)
> (From update of attachment 190837 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190837&action=review
> 
> > Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:528
> > +    if (color.isValid()) {
> 
> When can this happen? Is removing the layer entirely the logical equivalent of what happens in the normal path if you have a layer that is nothing but a background-color with an invalid color?

RenderLayerBacking.cpp has this:

    Color backgroundColor;
    if (isSimpleContainer)
        backgroundColor = rendererBackgroundColor();

    // An unset (invalid) color will remove the solid color.
    m_graphicsLayer->setContentsToSolidColor(backgroundColor);

so basically passing a non-valid color is a way to remove the solid color layer and go back to the normal path.  If isSimpleContainer is true, we'll always get a valid Color here.

[Adrienne Walker]

Comment on attachment 190837 [details]
Patch

R=me.  Thanks for the explanation.

[David Reveman]

Fyi, we probably want to hold off on this until crbug.com/218873 has been solved.

[James Robinson]

(In reply to comment #11)
> Fyi, we probably want to hold off on this until crbug.com/218873 has been solved.

Will do.

[James Robinson]

Committed r146826: <http://trac.webkit.org/changeset/146826>