Assertion failure: transform->mTransform.mTransformInDevSpace.FuzzyEqualsMultiplicative( (layer->AsHostLayer()->GetShadowBaseTransform())), at /builds/worker/workspace/build/src/gfx/layers/composite/AsyncCompositionManager.cpp:696 STR: Using a linux64 debug build, open http://giphy.com/. Wait for images to load/animate. Scroll down. Should not assert. I reproduced off recent mozilla-central debug builds (local and off treeherder, ccfd7b716a91). Looks like the assert in question was added by bug 1454324.

I could reproduce the assertion locally and am debugging it now, but it's weird. The transform value in question that was used to SetShadowBaseTransform()[1] in ApplyAnimatedValue actually equals to the transform value in the animation storage. I did also confirmed GetShadowTransformSetByAnimation value is true. So, I suspect there is another place that we call SetShadowBaseTransform() without calling SetShadowTransformSetByAnimation() and I guess we should call SetShadowTransformSetByAnimation(false) in the place. But if there is really such place, it means we can't use the shadow transform value in the assertion. sigh. [1] https://hg.mozilla.org/mozilla-central/file/63a0e2f626fe/gfx/layers/composite/AsyncCompositionManager.cpp#l632

Oh yeah, maybe it's ApplyAsyncTransformToScrollbarForContent. ApplyAsyncTransformToScrollbarForContent calls SetShadowTransform and SetShadowTransform calls SetShadowBaseTransform without calling SetShadowTransformSetByAnimation(). And it seems to me that it's correct. So we have to figure out another way for the assertion, I might give up adding the assertion there. https://hg.mozilla.org/mozilla-central/file/63a0e2f626fe/gfx/layers/composite/AsyncCompositionManager.cpp#l1134

Oh well, ApplyAsyncTransformToScrollbarForContent is only for scroll bar. Other call sites of SetShadowTransform change the transform value anyway.

It seems to be hard to reverse-calculate the transform changes accumulated by APZ. So I've decided to not skip the calculation on debug build and use the assertion. This is the most reliable way to check the animation value is unchanged. Note that even if we don't skip the calculation, we don't update animation storage with the calculated values, so the optimization should work as it is. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c7348def93077d28b00791d49afc2b541f08296

There was a garbage line that I should have removed; :/ (the value was used for the tolerance for FuzzyEquals?) https://hg.mozilla.org/try/rev/2d2f4741d3fc0ef6d138c3e49769ae4cf1fd7904#l2.13 A new try; https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3caac9b2238b621688bbe7f39ab4d05b2fc396f

Comment on attachment 8971469 [details] Bug 1457249 - Assert that there is an animation value set by animations when we skip calculation for newly animation value. https://reviewboard.mozilla.org/r/240220/#review246102

Comment on attachment 8971470 [details] Bug 1457249 - Factor out the function that converts servo's animation value to matrix. https://reviewboard.mozilla.org/r/240222/#review246104

Comment on attachment 8971471 [details] Bug 1457249 - Factor out the function that converts transform into device space. https://reviewboard.mozilla.org/r/240224/#review246106

Comment on attachment 8971472 [details] Bug 1457249 - Use actually calculated value for the assertion that checks animation value is unchanged when we decided to skip calculation for the animation on debug build. https://reviewboard.mozilla.org/r/240226/#review246110 ::: gfx/layers/AnimationHelper.cpp:160 (Diff revision 1) > { > MOZ_ASSERT(!aAnimations.IsEmpty(), "Should be called with animations"); > > bool hasInEffectAnimations = false; > +#ifdef DEBUG > + bool shouldBeSkipped = false; It would be worth adding a code comment here explaining a little bit what's going on. Something like this: In cases where this function returns a SampleResult::Skipped, we actually do populate aAnimationValue in debug mode, so that we can MOZ_ASSERT at the call site that the value that would have been computed matches the stored value that we end up using. This flag is used to ensure we populate aAnimationValue in this scenario.

Thank you, kats! I will add the comment there.

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1794c5b65d03 Assert that there is an animation value set by animations when we skip calculation for newly animation value. r=kats https://hg.mozilla.org/integration/autoland/rev/ac3a8c1f0776 Factor out the function that converts servo's animation value to matrix. r=kats https://hg.mozilla.org/integration/autoland/rev/053966aefdb3 Factor out the function that converts transform into device space. r=kats https://hg.mozilla.org/integration/autoland/rev/ccef67dba025 Use actually calculated value for the assertion that checks animation value is unchanged when we decided to skip calculation for the animation on debug build. r=kats

https://hg.mozilla.org/mozilla-central/rev/1794c5b65d03 https://hg.mozilla.org/mozilla-central/rev/ac3a8c1f0776 https://hg.mozilla.org/mozilla-central/rev/053966aefdb3 https://hg.mozilla.org/mozilla-central/rev/ccef67dba025