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

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Dana Jansens <danakj at chromium.org>  2011-10-25 10:17:23 PST ---
Updated patch to address feedback, except for the extra tree walk.  I hope I understood your critiques correctly, comments below.

(In reply to comment #9)
> (From update of attachment 112233 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112233&action=review
> updateGraphicsLayerConfiguration() is about changes to the layer tree structure, not just about setting flags on GraphicsLayers.

I believe you meant to not set the layerConfigChanged flag, since the hierarchy didn't change.  If you meant to update the opaque flag in a different location entirely, can you suggest where?

> 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.

Renamed to findOpaqueRenderLayer. This will go away once I find another tree walk to merge it into.

> 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.

>From a comment enne@ made, and bug 49135, I split this logic into three pieces:
1. contents foreground (image data etc)
2. contents background (background color)
3. contents exterior (borders, padding, shadow, etc.)

Currently:
- RenderBoxModelObject overrides background and exterior.
- RenderBlock extends exterior for column rule.
- RenderImage overrides foreground.

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

See RenderBoxModelObject::opaqueContentsExterior, many improvements here.

> 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.

Will work on this piece next, but would appreciate feedback on the rest of the code as it is.

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

Renamed to updateContentsOpaque(), since it sets the "contentsOpaque" flag on the graphics layer.  If you're thinking of something else, can you suggest what you're looking for?

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