GraphicsLayer: Allow more layers to be opaque when applicable

[Noam Rosenthal]

This would include layers with a 100% solid background color.

[Simon Fraser (smfr)]

<rdar://problem/5698198>

[Noam Rosenthal]

*** Bug 40380 has been marked as a duplicate of this bug. ***

[Sam Magnuson]

Created attachment 60200 [details]
Patch

[Early Warning System Bot]

Attachment 60200 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3370100

[Sam Magnuson]

Created attachment 60201 [details]
Patch

[Noam Rosenthal]

The Qt-specific content LGTM!

[Noam Rosenthal]

question - if we use contentsRect() for the texture instead of size(), can't we set the contents to opaque if the bgcolor is opaque, regardless of the layer size? 
I'm not really sure how contentsRect is supposed to work, but if it corresponds to the element size, this should be feasible and would also save us some texture-memory.

[Noam Rosenthal]

ignore my last comment; contentsRect is only for directly composited image/media.

[Simon Fraser (smfr)]

Comment on attachment 60201 [details]
Patch

> index d531bb5d0a0e29abdfe930ec7bf2166d3b55b422..fcee1962310652d679e6977f1b587b9581242dff 100644
> +        No new tests: this is an optimization.        

You can and should add tests that dump the layer tree (layoutTestController.layerTreeAsText()). You should also add a test or two to make sure that you don't hit this optimization when you don't intend to.

>  void GraphicsLayerQtImpl::paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget)
>  {
> -    if (m_currentContent.backgroundColor.isValid())
> -        painter->fillRect(option->rect, QColor(m_currentContent.backgroundColor));
> +//     if (m_currentContent.backgroundColor.isValid())
> +//         painter->fillRect(option->rect, QColor(m_currentContent.backgroundColor));

Don't check in commented-out code.

> diff --git a/WebCore/rendering/RenderLayerBacking.cpp b/WebCore/rendering/RenderLayerBacking.cpp

> +    bool contentsOpaque = false;
> +    if (rendererHasBackground()) {
> +        const Color &bgColor = rendererBackgroundColor();

Color is sizeof(int), so no point in using a reference there.

> +        if (bgColor.alpha() == 255) {

Use !bgColor.hasAlpha().

> +            const IntRect elementBounds = m_owningLayer->localBoundingBox(),
> +                            layerBounds = compositedBounds();

We don't use multiple declarations like this. Also no need for the IntRects to be 'const'. Also elementBounds is a bad name; that's really layer bounds. So use layerBounds and compositedBounds or something.

> +            if (elementBounds.width() >= layerBounds.width() && elementBounds.height() >= layerBounds.height())
> +                contentsOpaque = true;

You should just test for rect equality. The composited bounds will never be smaller than m_owningLayer->localBoundingBox().

You should file a follow-up bug for other obvious enhancements (e.g. non-alpha background image).

r- for lack of tests.

[Noam Rosenthal]

This one is still applicable even though it's with GraphicsLayerQt, though it doesn't give us a huge performance benefit.

[Jesus Sanchez-Palencia]

Noam, Igor, is this a duplicate of https://bugs.webkit.org/show_bug.cgi?id=70634 ?

Thanks!

[Simon Fraser (smfr)]

Yep.

*** This bug has been marked as a duplicate of bug 70634 ***