1072819 - Lock screen fade out animation "stutters"

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[Bruce Huang [:bhuang] (inactive)]

When unlocking the device, the fade out animation does not smoothly transition off the screen.  The fade out animation pauses briefly at the end before just disappearing off the screen.  Slow motion video here: https://www.youtube.com/watch?v=AeV0HeBeOYc
Adding a longer timer on the screen just causes the animation to pause before completing the transition.  Modified video here: https://www.youtube.com/watch?v=jzeW-7Tdl5c

The animation should smoothly fade out or it looks like the unlock animation "blinks" before finishing.

Comment 1

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

RunningOn.Compositor can somehow be used to see if we're using compositor animation (OMTA), right Brian?

Comment 2

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Milan Sreckovic [:milan] from comment #1)
> RunningOn.Compositor can somehow be used to see if we're using compositor
> animation (OMTA), right Brian?

For mochitests, right, we have some utilities in layout/style/test/animation_utils.js that basically just call methods on DOMWindowUtils to inspect the values on the compositor and check if the animation is running there.

(For script with chrome privileges we can also just query that by calling elem.getAnimationPlayers()[0].isRunningOnCompositor.)

I'm not sure what the sudden jump is at the end. If it were a jump at the start, perhaps bug 927349 might help.

As a temporary workaround, adjusting the timing function to have a slight ease-out might mean we get more frames towards the end of the transition.

Comment 3

[Greg Weng [:snowmantw][:gweng][:λ]]

While the video may now show the bug well, here is what we have tried and discovered:

1. Adjust the fade-out animation from current 0.6s to 5s, 2.5s and 1s to see the effect
2. Perform each animation according to the same, standard unlocking way (slide to rightmost, and release the finger).

We discovered:

No matter how long the seconds it occurs, the animation, as you can see in the video, would get a slight delay *during* the animation is performing. This delay doesn't occur at the start of the animation, but always occurs when the fading out image become as large as a specific size. In the slow-motion video it becomes obvious at near 0:04. In another video you can see my finger put at a specific height of the screen, where indicates that when the Clock on LockScreen reach it, the animation get stalled a while.

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

sorry, typo: now show the bug well -> *not* show the bug well

Comment 5

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Matt, is it possible we're re-rasterizing the layer once we reach a certain scale factor here? I thought we didn't do that for CSS Animations/Transitions?

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Greg, are you using CSS Animations/Transitions for the animation?

Comment 7

[Matt Woodrow (:mattwoodrow)]

We shouldn't be re-rasterizing because of scale changes during an animation [1].

It's possible that isn't working, or something else is triggering invalidation. Enabling paint flashing would show if we re-rasterize.



[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#3782

Comment 8

[Greg Weng [:snowmantw][:gweng][:λ]]

Yes, the animation is here:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/window.css#L109

and it perform the animation here:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/window.css#L130

If Gaia need to do something to avoid this issue please let me know.

Comment 9

[Olof Wickström]

Please fix for 2.1.
This bug impacts all users. 
It gives an negative impact to first time experience.

The user gets the impression that SW quality is low and/or that "CPU performance" is not good enough to run the animation.

Comment 10

[Greg Weng [:snowmantw][:gweng][:λ]]

If this actually is a graphic bug then I think it should be moved into Core::Graphic component.

Comment 11

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Greg, please try turning on paint flashing to see if we are re-rasterizing as per Matt's comment 7.

Also, try removing that middle keyframe in 'fadeInApp'. I don't *think* switching keyframe intervals should trigger re-rasterizing but it's not needed anyway.

Comment 12

[Greg Weng [:snowmantw][:gweng][:λ]]

After remove the 50% keyframe, the animation goes well without any issue. I would ask Alive to see if it's necessary, while I don't know whether this is a Gecko bug (add middle keyframe delay the animation) or not. I would also post the videos here.

Comment 13

[Greg Weng [:snowmantw][:gweng][:λ]]

With 50% keyframe and it shutter (animation: 2.5 secs)

https://www.youtube.com/watch?v=xiLLGl0w0oY

Without 50% keyframe and it doesn't shutter (animation: 2.5 secs)

https://www.youtube.com/watch?v=xcRAQZBCuuQ

Comment 14

[Greg Weng [:snowmantw][:gweng][:λ]]

Discussed with Bruce:

1. If keyframe is an error from Gaia's CSS, and there is no graphic issue, we can remove it directly for master and v2.1

2. If there are some graphic bug, while removing the keyframe still works, we can solve the issue with Gaia patch first, and fire another bug for Gecko to solve the problem completely.

Comment 15

[Greg Weng [:snowmantw][:gweng][:λ]]

With repainting result:

No shuttering:

http://youtu.be/xDv0ljyAyHw

Shuttering:

http://youtu.be/ZEr1Ra5fmbw

Comment 16

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

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #14)
> Discussed with Bruce:
> 
> 1. If keyframe is an error from Gaia's CSS, and there is no graphic issue,
> we can remove it directly for master and v2.1
> 
> 2. If there are some graphic bug, while removing the keyframe still works,
> we can solve the issue with Gaia patch first, and fire another bug for Gecko
> to solve the problem completely.

You could open a bug for "optimize the case where we have a frame in the middle" scenario.  It may or may not be easy to optimize that case, and depending on the effort it may or may not be worth it, but it sounds like you have a solution for 2.1 just removing the extra frame.

I would share this with the rest of the Gaia team, I imagine it is possible other places are doing this kind of an animation, not realizing they're slowing things down.

Comment 17

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Hi Greg, I think the issue you have been seeing is to do with the surprising way animation easing functions (animation-timing-function) work in CSS Animations. You have:

  animation: fadeInApp 0.6s forwards cubic-bezier(0.7, 0.0, 1.0, 1.0);

That sets the timing-function to 'cubic-bezier(0.7, 0.0, 1.0, 1.0)'

CSS Animations says that that timing function applies between *each* pair of keyframes, *not* over the whole animation. That's why it seems to slow down in the middle.

When you get rid of the middle keyframe the easing effect applies across the whole duration of the animation removing the slow-down.

Comment 18

[howie [:howie]]

Triage: blocking.

Comment 19

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

(In reply to howie [:howie] from comment #18)
> Triage: blocking.

This is not really documenting the reason for the decision, just the group of people that were there when the decision was made.  If we're going to insist on the extra documentation for blockers, let's actually put the extra information in here as well.

Comment 20

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch

Comment 21

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.1

Comment 22

[Greg Weng [:snowmantw][:gweng][:λ]]

From Gaia side I want to resolve the possible issue first.

Today Jerry suggest me to try with some linear effect with keyframes (my original idea is to add 100 keyframe, 1% per frame). If it still shutters then we may have a graphic bug, since if we set the stop point at the correct point of the linear it should perform well as we only set two points. But I don't know whether this is a reasonable test case.

Comment 23

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Please see comment 17. This is a bug in the CSS markup.

Comment 24

[Greg Weng [:snowmantw][:gweng][:λ]]

master: https://github.com/mozilla-b2g/gaia/pull/24766

Comment 25

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8499730 [details] [review]
Patch v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): The old LockScreen animation implementation is wrong.
[User impact] if declined: Animation still shutter
[Testing completed]: Gaia-Try only failed at those irrelevant tests. Since this patch is only for CSS animation progress, this prove those tests are intermittent.
[Risk to taking this patch] (and alternatives if risky): No
[String changes made]: No

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/46fc0163f38d2fa04f7d41d7e8ca697b6849899c

Comment 27

[John Thomas [:Johnt]]

This bug does NOT occur on the Flame 2.1 (319mb) and the Flame 2.2 (319mb)

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2
BuildID: 20141010040202
Gaia: 1036b544b7e102592bd9fab95cd9317329ac1293
Gecko: 50b689feab5f
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1 KK (319mb) (Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141010000201
Gaia: bc8eb493311c58f1f311a56b8b645b52bfbd2f71
Gecko: 72c13d8631ff
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Result: Lock screen fades animation runs smoothly.