Bug 62465 – Overlap test needs to consider children of composited layers

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adrienne Walker]

Overlap test needs to consider children of composited elements

[Adrienne Walker]

Created attachment 96759 [details]
Patch

[Simon Fraser (smfr)]

Can you explain more how this is different from bug 62181?

[Adrienne Walker]

(In reply to comment #2)
> Can you explain more how this is different from bug 62181?

Sure! Consider this layer tree:

A (empty layer bounds)
-B (layer that should be underneath D)
-C
D (layer that should be on top of B)

If it's not clear, B and C are children of A.  All layers can be composited.

In bug 62181, A has a webkit-transform (requiring compositing if any descendants are composited) and C is an accelerated canvas.

In this bug, A is composited itself because it has an animation.  C is irrelevant.

In both cases, layer B does not get added to the overlap map, but there are different reasons why this happens.  In both cases, B does not need to be composited.

computeCompositingRequirements visits the parent layer before the children.  The difference between the two cases is whether we know that we are compositing when we process layer B.  In bug 62181, A is not known to be composited until we have processed all of its children.  In this bug, A is already known to be composited when we process B.

In bug 62181, we belatedly find out that A is composited after we have already processed B, which is why we have to go back and rewalk the tree to add them to the overlap map.  In this bug, we know that A is composited already, so we can just add B to the overlap map during the initial traversal.

(This bug could also be fixed by having needsCompositing walk up the parent tree to see if any parent layer needed to be composited.)

[Adrienne Walker]

On a side note, I also think that both of these patches are the correct way to handle what the "extend empty layer bounds to 1x1" hack is trying to do in the RenderLayerCompositor::addToOverlapMap function.  The more I stare at that line of code, the more I think it is unneeded and incorrect.

[Simon Fraser (smfr)]

What real-world site is affected by this?

[James Robinson]

In Chromium, this causes portions of the maps.google.com interface to vanish when zooming in or out of the map, which triggers an animation of the -webkit-transform: scale() property.

This doesn't seem to affect Safari, possibly because of differences in how CoreAnimation handles this transition vs handling the intermediate frames from within WebKit.

[Simon Fraser (smfr)]

Comment on attachment 96759 [details]
Patch

I guess this isn't an issue in Safari because RenderLayerCompositor::didStartAcceleratedAnimation() does a setCompositingConsultsOverlap(false)

Can you make a testcase that will work for safari too? Try using <video> to cause compositing.

[Adrienne Walker]

(In reply to comment #7)
> (From update of attachment 96759 [details])
> I guess this isn't an issue in Safari because RenderLayerCompositor::didStartAcceleratedAnimation() does a setCompositingConsultsOverlap(false)
> 
> Can you make a testcase that will work for safari too? Try using <video> to cause compositing.

I tried to make a test case triggering compositing not using animation, but couldn't do it.  This has to be triggered by a composited parent whose bounds don't contain a non-composited child.

I could be wrong, but when I added child elements to <video> or <canvas> they get ignored.  Transformed layers ignore overlap, so that's not an option.  All the other compositing triggers (frames, plugins, clipping) all clip to their bounds, so that doesn't work either.  Animation seems like the only case.

[Simon Fraser (smfr)]

If I fix bug 49857 that will change.

[Simon Fraser (smfr)]

Comment on attachment 96759 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Any child layer with a compositing ancestor will be put into a
> +        composited layer even though they themselves don't need compositing.

Are you describing the symptoms of bug 50192 here?

[Adrienne Walker]

(In reply to comment #10)
> (From update of attachment 96759 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96759&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Any child layer with a compositing ancestor will be put into a
> > +        composited layer even though they themselves don't need compositing.
> 
> Are you describing the symptoms of bug 50192 here?

No.  I should have been more precise in my language: s/put/drawn/.  (Is there better language to describe this?)

This bug and bug 62181 are about layers not being made composited when they should be, because the overlap map does not contain enough information about non-composited layers with a composited parent.

Bug 50192 is about layers that don't need their own composited layer, but get one anyway.

[Simon Fraser (smfr)]

I have committed the fix for 49857, so you can have a testcase that works in Safari as well now.

[Adrienne Walker]

(In reply to comment #12)
> I have committed the fix for 49857, so you can have a testcase that works in Safari as well now.

Thanks! Is an updated test case the only thing blocking an R+ from you at this point?

[Simon Fraser (smfr)]

On this one, yes. I'll think more about the other one.

[Adrienne Walker]

Created attachment 96981 [details]
Patch

[Adrienne Walker]

Committed r88698: <http://trac.webkit.org/changeset/88698>