Bug 1464568 - Set the shadow base transform for animation explicitly even if the transform value hasn't changed. r?kats draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 05 Jun 2018 12:50:36 +0900
changeset 803970 eba3e2c3fc0458b54d545b2ca80de8f63b1c52ab
parent 803969 416594be43034f92de5b2d36ace148ef85541bdb
child 803971 5f10edbd6784b167178841a79d8c86309b349757
push id112247
push userhikezoe@mozilla.com
push dateTue, 05 Jun 2018 05:13:07 +0000
reviewerskats
bugs1464568
milestone62.0a1
Bug 1464568 - Set the shadow base transform for animation explicitly even if the transform value hasn't changed. r?kats 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
@@ -704,54 +704,65 @@ 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());
-
                 MOZ_ASSERT(previousValue);
-
+#ifdef DEBUG
                 const TransformData& transformData =
                   animations[0].data().get_TransformData();
                 Matrix4x4 frameTransform =
                   ServoAnimationValueToMatrix4x4(animationValue, transformData);
                 Matrix4x4 transformInDevice =
                   FrameTransformToTransformInDevice(frameTransform,
                                                     layer,
                                                     transformData);
                 MOZ_ASSERT(
                   previousValue->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.
+                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;