Bug 1464568 - Set the shadow base transform for animation explicitly even if the transform value hasn't changed. r=kats, a=RyanVM
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 05 Jun 2018 12:50:36 +0900
changeset 473644 6c95fd0a106cc788151588f3f478b8fa634ce117
parent 473643 8d255e70fccd1b820a2d86b5d080064f0339b580
child 473645 622e8a464432ae5a24c92f78387a0b3a615fd51c
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats, RyanVM
bugs1464568
milestone61.0
Bug 1464568 - Set the shadow base transform for animation explicitly even if the transform value hasn't changed. r=kats, a=RyanVM That's because the shadow base transform value might have been changed by APZC. The test case in this patch fail without this fix on non-WebRender and the test case is skipped on WebRender since the bug should never happen on WebRender because WebRender manages animation transform value and APZ transform value separately. MozReview-Commit-ID: Itgh0QL1su6
gfx/layers/apz/test/mochitest/helper_bug1464568_transform.html
gfx/layers/apz/test/mochitest/mochitest.ini
gfx/layers/apz/test/mochitest/test_bug1464568.html
gfx/layers/composite/AsyncCompositionManager.cpp
new file mode 100644
--- /dev/null
+++ b/gfx/layers/apz/test/mochitest/helper_bug1464568_transform.html
@@ -0,0 +1,60 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test that transform animation is correctly placed during asynchronous scrolling</title>
+  <script src="apz_test_utils.js"></script>
+  <script src="/tests/SimpleTest/paint_listener.js"></script>
+  <meta name="viewport" content="width=device-width"/>
+  <style>
+    #anim {
+      background: green;
+      width: 100px;
+      height: 100px;
+      animation: anim 100s step-start;
+    }
+    @keyframes anim {
+      from { transform: translateX(100px); }
+      to { transform: translateX(200px); }
+    }
+  </style>
+</head>
+<body>
+ <!--
+  This height should be smaller than window height, otherwise the animation
+  followed by this element will be out of view, thus the animation doesn't run
+  on the compositor.
+  -->
+ <div style="height: 500px"></div>
+ <div id="anim"></div>
+</body>
+<script>
+'use strict';
+
+const utils = SpecialPowers.getDOMWindowUtils(window);
+
+async function test_transform() {
+  utils.setDisplayPortForElement(0, 0, 300, 1000, document.documentElement, 1);
+  await promiseAllPaintsDone();
+
+  let transform = utils.getOMTCTransform(anim);
+  is(transform, "matrix(1, 0, 0, 1, 200, 0)",
+     "The element shouldn't be moved before scrolling");
+
+  utils.setAsyncScrollOffset(document.documentElement, 0, 300);
+
+  await new Promise(resolve => waitForApzFlushedRepaints(resolve));
+
+  transform = utils.getOMTCTransform(anim);
+  is(transform, "matrix(1, 0, 0, 1, 200, -300)",
+     "Element should have been moved by the offset");
+}
+
+if (utils.layerManagerType == 'WebRender') {
+  ok(true, "This test doesn't need to run on WebRender");
+  subtestDone();
+} else {
+  waitUntilApzStable().then(test_transform).then(subtestDone);
+}
+
+</script>
+</html>
--- a/gfx/layers/apz/test/mochitest/mochitest.ini
+++ b/gfx/layers/apz/test/mochitest/mochitest.ini
@@ -9,16 +9,17 @@
     helper_bug1151663.html
     helper_bug1162771.html
     helper_bug1271432.html
     helper_bug1280013.html
     helper_bug1285070.html
     helper_bug1299195.html
     helper_bug1346632.html
     helper_bug1414336.html
+    helper_bug1464568_transform.html
     helper_click.html
     helper_div_pan.html
     helper_drag_click.html
     helper_drag_scroll.html
     helper_iframe_pan.html
     helper_iframe1.html
     helper_iframe2.html
     helper_hittest_backface_hidden.html
@@ -54,16 +55,18 @@
 [test_bug1151667.html]
   skip-if = (os == 'android') || webrender # wheel events not supported on mobile, bug 1424752 for webrender
 [test_bug1253683.html]
   skip-if = (os == 'android') # wheel events not supported on mobile
 [test_bug1277814.html]
   skip-if = (os == 'android') # wheel events not supported on mobile
 [test_bug1304689.html]
 [test_bug1304689-2.html]
+[test_bug1464568.html]
+  skip-if = (toolkit == 'android') # setAsyncScrollOffset doesn't work on mobile
 [test_frame_reconstruction.html]
   skip-if = webrender # bug 1424752
 [test_group_mouseevents.html]
   skip-if = (toolkit == 'android') # mouse events not supported on mobile
 [test_group_pointerevents.html]
   skip-if = os == 'win' && os_version == '10.0' # Bug 1404836
 [test_group_touchevents.html]
   skip-if = webrender # bug 1424752
new file mode 100644
--- /dev/null
+++ b/gfx/layers/apz/test/mochitest/test_bug1464568.html
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="apz_test_utils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+    if (isApzEnabled()) {
+      SimpleTest.waitForExplicitFinish();
+
+      const subtests = [
+        { file: 'helper_bug1464568_transform.html',
+          prefs: [["apz.test.logging_enabled", true]] },
+      ];
+      // Run the actual test in its own window, because it requires that the
+      // root APZC be scrollable. Mochitest pages themselves often run
+      // inside an iframe which means we have no control over the root APZC.
+      window.onload = () => {
+        runSubtestsSeriallyInFreshWindows(subtests)
+        .then(SimpleTest.finish, SimpleTest.finish);
+      };
+    }
+  </script>
+</head>
+<body>
+</body>
+</html>
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -699,55 +699,70 @@ SampleAnimations(Layer* aLayer,
             ApplyAnimatedValue(layer,
                                aStorage,
                                animation.property(),
                                animation.data(),
                                animationValue);
             break;
           }
           case AnimationHelper::SampleResult::Skipped:
-            // We don't need to update animation values for this layer since
-            // the values haven't changed.
-#ifdef DEBUG
-            // Sanity check that the animation value is surely unchanged.
             switch (animations[0].property()) {
               case eCSSProperty_opacity:
                 MOZ_ASSERT(
                   layer->AsHostLayer()->GetShadowOpacitySetByAnimation());
+#ifdef DEBUG
                 // Disable this assertion until the root cause is fixed in bug
                 // 1459775.
                 // MOZ_ASSERT(FuzzyEqualsMultiplicative(
                 //   Servo_AnimationValue_GetOpacity(animationValue),
                 //   *(aStorage->GetAnimationOpacity(layer->GetCompositorAnimationsId()))));
+#endif
                 break;
               case eCSSProperty_transform: {
                 MOZ_ASSERT(
                   layer->AsHostLayer()->GetShadowTransformSetByAnimation());
-
+#ifdef DEBUG
                 AnimatedValue* transform =
                   aStorage->GetAnimatedValue(layer->GetCompositorAnimationsId());
 
                 const TransformData& transformData =
                   animations[0].data().get_TransformData();
                 Matrix4x4 frameTransform =
                   ServoAnimationValueToMatrix4x4(animationValue, transformData);
                 Matrix4x4 transformInDevice =
                   FrameTransformToTransformInDevice(frameTransform,
                                                     layer,
                                                     transformData);
                 MOZ_ASSERT(
                   transform->mTransform.mTransformInDevSpace.FuzzyEqualsMultiplicative(
                   transformInDevice));
+#endif
+                // In the case of transform we have to set the unchanged
+                // transform value again becasue APZC might have modified the
+                // previous shadow base transform value.
+                AnimatedValue* previousValue =
+                  aStorage->GetAnimatedValue(layer->GetCompositorAnimationsId());
+                MOZ_ASSERT(previousValue);
+                HostLayer* layerCompositor = layer->AsHostLayer();
+                layerCompositor->SetShadowBaseTransform(
+                  // FIXME: Bug 1459775: It seems possible that we somehow try
+                  // to sample animations and skip it even if the previous value
+                  // has been discarded from the animation storage when we enable
+                  // layer tree cache. So for the safety, in the case where we
+                  // have no previous animation value, we set non-animating value
+                  // instead.
+                  previousValue
+                    ? previousValue->mTransform.mTransformInDevSpace
+                    : layer->GetBaseTransform());
                 break;
               }
               default:
                 MOZ_ASSERT_UNREACHABLE("Unsupported properties");
                 break;
             }
-#endif
             break;
           case AnimationHelper::SampleResult::None: {
             HostLayer* layerCompositor = layer->AsHostLayer();
             layerCompositor->SetShadowBaseTransform(layer->GetBaseTransform());
             layerCompositor->SetShadowTransformSetByAnimation(false);
             layerCompositor->SetShadowOpacity(layer->GetOpacity());
             layerCompositor->SetShadowOpacitySetByAnimation(false);
             break;