Summary: | RenderLayerCompositor doesn't properly clip graphics layer sizes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrienne Walker <enne> | ||||||||
Component: | WebCore Misc. | Assignee: | Adrienne Walker <enne> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | enne, jamesr, simon.fraser | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Adrienne Walker
2012-03-05 20:20:49 PST
Created attachment 130895 [details]
Patch
(In reply to comment #1) > Created an attachment (id=130895) [details] > Patch I'm not super happy with this patch because of the extra work walking up layer trees, but it's a first cut at getting the job done. The other approach I considered was to switch calculateCompositedBounds to iterate through RenderLayer children directly rather than through the z-order and normal flow lists. There's some hairiness with which layers get included in those lists and which don't, but it might end up being cleaner. smfr: What do you think? I wonder if we can make use of cached clip rects somehow, or, if we need to, cache a new clip rect for use here. I don't like how clippingLayerClosestToAncestor() uses recursion when it could iterate. Created attachment 130911 [details]
Remove recursion
(In reply to comment #3) > I wonder if we can make use of cached clip rects somehow, or, if we need to, cache a new clip rect for use here. I may just be misunderstanding clip rects, but the cached clip rects root didn't seem to necessarily be the composited layer passed originally to calculateCompositedBounds. > I don't like how clippingLayerClosestToAncestor() uses recursion when it could iterate. Fixed. (In reply to comment #5) > (In reply to comment #3) > > I wonder if we can make use of cached clip rects somehow, or, if we need to, cache a new clip rect for use here. > > I may just be misunderstanding clip rects, but the cached clip rects root didn't seem to necessarily be the composited layer passed originally to calculateCompositedBounds. smfr: If you want to use cached clip rects, do you have any suggestions on how best to go about that? Is it a possibility to collapse clip rects down the layer tree hierarchy and store directly on child render layers when we build the z-order and normal flow lists? Or, do those lists not get dirtied at the right time? I think the " the cached clip rects root" should be the ancestor compositing layer, but I'd have to reload all the clip rects logic into my brain to know for sure. (In reply to comment #7) > I think the " the cached clip rects root" should be the ancestor compositing layer, but I'd have to reload all the clip rects logic into my brain to know for sure. As far as I can tell, sometimes layer->clippingRoot() can be an ancestor of that layer's ancestor compositing layer. So, I can use the cached clip rects, but they are not in the right space. Transforming them from a container layer to a local layer would require using localToAbsolute and absoluteToLocal (to respect transforms), and therefore a double walk of the render object hierarchy. That seems more expensive than my original solution of walking up fewer render layers. (In reply to comment #8) > (In reply to comment #7) > > I think the " the cached clip rects root" should be the ancestor compositing layer, but I'd have to reload all the clip rects logic into my brain to know for sure. > > As far as I can tell, sometimes layer->clippingRoot() can be an ancestor of that layer's ancestor compositing layer. Er, ignore this. I misread some code. Let me take another look and see if I can get clip rects to work. Created attachment 132168 [details]
Use cached clip rects
(In reply to comment #10) > Created an attachment (id=132168) [details] > Use cached clip rects smfr: Do you have any time to look at this? Comment on attachment 132168 [details] Use cached clip rects View in context: https://bugs.webkit.org/attachment.cgi?id=132168&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:511 > - > // The bounds of the GraphicsLayer created for a compositing layer is the union of the bounds of all the descendant I want that blank line :) (In reply to comment #12) > (From update of attachment 132168 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132168&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:511 > > - > > // The bounds of the GraphicsLayer created for a compositing layer is the union of the bounds of all the descendant > > I want that blank line :) No problem, you can have it. I've got plenty of extras. Committed r111456: <http://trac.webkit.org/changeset/111456> |