1312307 - Graphical glitch on Netflix preview animation

Status: RESOLVED DUPLICATE of bug 1311196

(Core :: DOM: Animation, defect, P1)

Description

[Brian Birtles (:birtles)]

Attachment: Screencast demonstrating problem

STR:
1. Load Netflix preview page: https://www.netflix.com/browse
   (Needs an account.)
2. Hover over one of the preview images
3. Hover over a preview image on a *different* row

Expected results:
Smooth animations

Actual results:
Sometimes the zoom animation flickers back from the enlarged size to a smaller size.

See the attached video. In particular observe the *second* time the U.N.C.L.E. video is hovered and, even more clearly, when "The Motivation" video is hovered.

Looking at the animations panel, it seems like there is an initial CSS animation which is cancelled? Then there are a bunch of CSS transitions that run on the compositor.

The attached video is from a release build with the patches from bug 1311196 applied. Unfortunately they don't appear to fix the bug. The behavior without the patches applied is similar, perhaps slightly worse, I'm not sure.

Reverting to changeset 0d101ebfd95c (https://hg.mozilla.org/mozilla-central/rev/0d101ebfd95c)--parent changeset of bug 1223658--however does appear to fix the problem so I'm marking this as blocking bug 1223658 for now.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

I am convinced this is also caused by https://hg.mozilla.org/mozilla-central/rev/00799db70975 .

Unlikely transition's case (bug 1311196), I guess this will be hard to solve.  In transition case, the final value of the transitions is the same as (new) underlying value, so even if the transition with fill:forwards keeps running on the compositor,  additive or accumulte animation will be composed correctly. 

We might be able to check that if final animated value equals to the new underlying value, then try to keep the animations on the compositor until the animations are pulled back.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Brian, can you please try this build and see if what happens on Netflix once the build is completed?  Unfortunately I have no account on Netflix.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=80db1eac8f798909ecdaa16d492c4cd590a43935

This build based on patches for missing keyframes (bug 1305325) with a quick fix what I commented in comment 1.
This build works fine on the transition case.

Comment 3

[Brian Birtles (:birtles)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Brian, can you please try this build and see if what happens on Netflix once
> the build is completed?  Unfortunately I have no account on Netflix.
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=80db1eac8f798909ecdaa16d492c4cd590a43935
> 
> This build based on patches for missing keyframes (bug 1305325) with a quick
> fix what I commented in comment 1.
> This build works fine on the transition case.

With that build the transitions don't run at all on the Netflix site. Only the CSS animations. There's some NS_ERROR_FAILURE in the Web console but it seems to be triggered from deep in Netflix's JS bundle so I'm not sure if it's related (it could be stopping the code that would have set up the transitions).

Comment 4

[Hiroyuki Ikezoe (:hiro)]

A quick update:

I can reproduce the flicker on Linux.  As far as I can tell, there are two kind of flickers. One is on transform transition caused by https://hg.mozilla.org/mozilla-central/rev/00799db70975 . The other is on switching the preview images, I am guessing this is caused by https://hg.mozilla.org/mozilla-central/rev/7ee13a1fd665 (sending delay animations to the compositor).  The former flicker seems to be fixed by the fix I mentioned in comment 1.  Unlike Win64 build that Brian commented in comment 3, I can see all transitions there.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Hmm, debug build might not be suitable to track this bug down.  I can also see the flicker on switching preview images on a debug build before landing bug 1223658. (The revision is 0d101ebfd95c which is a prior revision to https://hg.mozilla.org/mozilla-central/rev/b66b75c2d042)

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Additional quick update:

Attachment #8803731 [details] (a patch for treeherder case in bug 1311196) seems to fix the flicker on the transform transition if the patch  is applied on the revision which landed bug 1223658 <https://hg.mozilla.org/mozilla-central/rev/7ee13a1fd665>.  But interestingly. as Brian noted somewhere, the patch does not fix if the patch is applied to today's tip.  There might be some other cause.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> Additional quick update:
> 
> Attachment #8803731 [details] (a patch for treeherder case in bug 1311196)
> seems to fix the flicker on the transform transition if the patch  is
> applied on the revision which landed bug 1223658
> <https://hg.mozilla.org/mozilla-central/rev/7ee13a1fd665>.  But
> interestingly. as Brian noted somewhere, the patch does not fix if the patch
> is applied to today's tip.  There might be some other cause.

Oops, attachment #8803732 [details] is the correct number I meant.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Bisecting has been done.  There is nothing.  And I realized that I forgot to drop the original line 'animation->fillMode() = static_cast<uint8_t>(timing.mFill);' in the patch (attachment #8803732 [details]).  Whereas while I was bisecting, I did change local tree by hand, the handy change dropped the line correctly.. I am so sorry!

Comment 9

[Brian Birtles (:birtles)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)
> Bisecting has been done.  There is nothing.  And I realized that I forgot to
> drop the original line 'animation->fillMode() =
> static_cast<uint8_t>(timing.mFill);' in the patch (attachment #8803732 [details]
> [details]).  Whereas while I was bisecting, I did change local tree by hand,
> the handy change dropped the line correctly.. I am so sorry!

Does that mean that bug 1311196 will fix this (with the correct patch)? If so, that's great news!

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Brian Birtles (:birtles, high review load) from comment #9)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #8)
> > Bisecting has been done.  There is nothing.  And I realized that I forgot to
> > drop the original line 'animation->fillMode() =
> > static_cast<uint8_t>(timing.mFill);' in the patch (attachment #8803732 [details]
> > [details]).  Whereas while I was bisecting, I did change local tree by hand,
> > the handy change dropped the line correctly.. I am so sorry!
> 
> Does that mean that bug 1311196 will fix this (with the correct patch)? If
> so, that's great news!

Yes, great, but I am exhausted!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Brian Birtles (:birtles, high review load, away most of 27 Oct-4 Nov) from comment #9)
> Does that mean that bug 1311196 will fix this (with the correct patch)? If
> so, that's great news!

Can we verify that this is now the case?

Comment 12

[Brian Birtles (:birtles)]

Yes, this is now fixed by bug 1311196.

Comment 13

[Julien Cristau [:jcristau]]

updating 52 status per comment 12