1217803 - Animating elements don't always respect border-radius mask

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Wilson Page [:wilsonpage]]

A combination of `border-radius` and `overflow: hidden` doesn't *always* mask animating child at corners.

EXAMPLE: http://jsbin.com/qofima/edit?html,css,output

Comment 1

[Chris Lord [:cwiiis]]

This reproduces 100% for me on Linux with apz and async-animations enabled.

Milan, is there anyone on gfx that can look at this?

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

With async animations, I imagine Brian could help.  For me, the "not masking" is frequent, but temporary - at least in the above example, in that I see it unmasked, but then it gets masked on the next refresh.
I'm not convinced this is a big deal, especially since it doesn't work at all in Safari, and has exactly the same behaviour in Chrome (to be fair, IE works.)

Comment 3

[Chris Lord [:cwiiis]]

By 100%, I meant while animating - it indeed settles into the right rendering once the animation finishes. Also interesting that this happens in Chrome, rather unexpected! (and kudos IE :))

Comment 4

[Benoit Girard (:BenWa)]

Total guess: I'd suggest first looking into the code that should be building the mask layer. I'd imagine we avoid building the mask layer during OMTA if the initial position doesn't require it without considering if any off the possible OMTA value require it.

If you set:
span {
  transform: scaleX(0.9);
}

making both the initial and final position require the mask then it works properly.

Comment 5

[Brian Birtles (:birtles)]

(In reply to Benoit Girard (:BenWa) from comment #4)
> Total guess: I'd suggest first looking into the code that should be building
> the mask layer. I'd imagine we avoid building the mask layer during OMTA if
> the initial position doesn't require it without considering if any off the
> possible OMTA value require it.

Looks like it:

  https://dxr.mozilla.org/mozilla-central/rev/28068d907290d1f5138a0b9e59ae2233a1c1b7a3/layout/base/FrameLayerBuilder.cpp#4133

I guess we need to do something like nsLayoutUtils::ComputeSuitableScaleForAnimation where we work out the maximum animated scale and then use that to work out the maximum content bounds before deciding if we need the mask layer or not.

Comment 6

[Brian Birtles (:birtles)]

Motozawa-san, will you have time to work on this, or is it ok to re-assign to someone else?

Comment 7

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Use max rectangle of animated element when checking whether within the rounded border range.

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Use max rectangle of animated element when checking whether within the rounded border range.

Removed unnecessary space.

Comment 9

[Matt Woodrow (:mattwoodrow)]

Couldn't we also have a similar problem with transforms other than scales?

Like if we translated the content so that it intersected with the corner when it didn't previously?

What about other async changes, like scrolling with apz?

We might need to detect if the content can at all be affected by APZ, and treat the region as infinite in that case.

Comment 10

[Markus Stange [:mstange]]

Or we could always treat the region as infinite. (And actually just replace the condition with layerClip.GetRoundedRectCount() > 0).
We enter this code for active layers, and usually they're active because their contents are in some way animated.

Comment 11

[Matt Woodrow (:mattwoodrow)]

That seems like a pretty reasonable simplification to me.

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: always-build-mask-layer

Comment 14

[Markus Stange [:mstange]]

Comment on attachment 8770791 [details] [diff] [review]
always-build-mask-layer

Review of attachment 8770791 [details] [diff] [review]:
-----------------------------------------------------------------

This will also fix bug 1286622 which I coincidentally filed today.

Please add a test.

Comment 15

[Matt Woodrow (:mattwoodrow)]

I'm not aware of an easy way to write tests for async animations compositing behaviour.

Comment 16

[Markus Stange [:mstange]]

Ok, I'm not sure how to do those offhand either, but you could turn the testcase in bug 1286622 into an async scrolling test.

Comment 17

[Matt Woodrow (:mattwoodrow)]

Attachment: Reftest

Struggling with this a bit, this test still passes.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e6d8283458c
Always build a mask layer if we have rounded corners, regardless of the visible region since this might change on the compositor. r=mstange

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2e6d8283458c