[Webkit-unassigned] [Bug 70634] Mark GraphicsLayers as opaque when possible

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=70634


Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #112233|                            |review-
               Flag|                            |




--- Comment #9 from Simon Fraser (smfr) <simon.fraser at apple.com>  2011-10-24 13:31:20 PST ---
(From update of attachment 112233)
View in context: https://bugs.webkit.org/attachment.cgi?id=112233&action=review

> Source/WebCore/rendering/RenderLayerBacking.cpp:285
> +    if (updateOpaque())
> +        layerConfigChanged = true;

updateGraphicsLayerConfiguration() is about changes to the layer tree structure, not just about setting flags on GraphicsLayers.

> Source/WebCore/rendering/RenderLayerBacking.cpp:614
> +bool RenderLayerBacking::updateOpaqueRenderLayer(RenderLayer* layer, RenderLayer* ancestor, const IntPoint& ancestorDelta)

The method name needs improvement. It's not updating the render layer to be opaque, it's computing whether the layer should be considered opaque.

> Source/WebCore/rendering/RenderLayerBacking.cpp:619
> +    bool seeThru = (style->visitedDependentColor(CSSPropertyBackgroundColor).hasAlpha()

I don't like seeThru.

> Source/WebCore/rendering/RenderLayerBacking.cpp:620
> +                    && (!renderObject->isImage() || !toRenderImage(renderObject)->backgroundIsObscured()))

It would be clearer to separate 'contents are opaque' from 'backgrounds and borders are opaque". It would also make sense to push questions about RenderObject opaqueness down to RenderObject and subclasses. This could share a lot of logic with the code that would be added for bug 49135.

> Source/WebCore/rendering/RenderLayerBacking.cpp:626
> +                   || style->borderLeftIsTransparent()
> +                   || style->borderRightIsTransparent()
> +                   || style->borderTopIsTransparent()
> +                   || style->borderBottomIsTransparent();

You should check to see if there is a border at all.

> Source/WebCore/rendering/RenderLayerBacking.cpp:647
> +    RenderLayer* child = layer->firstChild();
> +    while (child) {
> +        if (updateOpaqueRenderLayer(child, layer, delta))
> +            return true;
> +        child = child->nextSibling();
> +    }

Sucks to do another layer tree walk here (and probably expensive too). If needed, we could compute this same info during one of the existing tree walks.

> Source/WebCore/rendering/RenderLayerBacking.cpp:651
> +bool RenderLayerBacking::updateOpaque()

Needs better name.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.