Bug 1218620 - Allow opacity animation running on compositor even if the frame has any restricted transforms. r=birtles
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 14 Mar 2016 09:07:48 +0900
changeset 288549 a811164e762da26f4d1cdcc7221b9a2e914f0609
parent 288548 5b2891e4650c9b2a1bd5dc83703b19d4a4f681f5
child 288550 acbe5c09bbb702586bd8c4b19c24d150e45f118a
push id18167
push userkwierso@gmail.com
push dateTue, 15 Mar 2016 00:40:50 +0000
treeherderfx-team@992db1cffc5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1218620
milestone48.0a1
Bug 1218620 - Allow opacity animation running on compositor even if the frame has any restricted transforms. r=birtles The type name has been changed and re-ordered. MozReview-Commit-ID: 78jrJ6a9Pro
dom/animation/AnimationPerformanceWarning.cpp
dom/animation/AnimationPerformanceWarning.h
dom/animation/EffectCompositor.cpp
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
dom/animation/test/chrome/test_animation_property_state.html
dom/locales/en-US/chrome/layout/layout_errors.properties
--- a/dom/animation/AnimationPerformanceWarning.cpp
+++ b/dom/animation/AnimationPerformanceWarning.cpp
@@ -47,25 +47,25 @@ AnimationPerformanceWarning::ToLocalized
       key = "AnimationWarningTransformBackfaceVisibilityHidden";
       break;
     case Type::TransformPreserve3D:
       key = "AnimationWarningTransformPreserve3D";
       break;
     case Type::TransformSVG:
       key = "AnimationWarningTransformSVG";
       break;
+    case Type::TransformWithGeometricProperties:
+      key = "AnimationWarningTransformWithGeometricProperties";
+      break;
     case Type::TransformFrameInactive:
       key = "AnimationWarningTransformFrameInactive";
       break;
     case Type::OpacityFrameInactive:
       key = "AnimationWarningOpacityFrameInactive";
       break;
-    case Type::WithGeometricProperties:
-      key = "AnimationWarningWithGeometricProperties";
-      break;
   }
 
   nsresult rv =
     nsContentUtils::GetLocalizedString(nsContentUtils::eLAYOUT_PROPERTIES,
                                        key, aLocalizedString);
   return NS_SUCCEEDED(rv);
 }
 
--- a/dom/animation/AnimationPerformanceWarning.h
+++ b/dom/animation/AnimationPerformanceWarning.h
@@ -16,19 +16,19 @@ namespace mozilla {
 // Represents the reason why we can't run the CSS property on the compositor.
 struct AnimationPerformanceWarning
 {
   enum class Type : uint8_t {
     ContentTooLarge,
     TransformBackfaceVisibilityHidden,
     TransformPreserve3D,
     TransformSVG,
+    TransformWithGeometricProperties,
     TransformFrameInactive,
     OpacityFrameInactive,
-    WithGeometricProperties
   };
 
   explicit AnimationPerformanceWarning(Type aType)
     : mType(aType) { }
 
   AnimationPerformanceWarning(Type aType,
                               std::initializer_list<int32_t> aParams)
     : mType(aType)
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -106,23 +106,25 @@ FindAnimationsForCompositor(const nsIFra
     MOZ_ASSERT(effect && effect->GetAnimation());
     Animation* animation = effect->GetAnimation();
 
     if (!animation->IsPlaying()) {
       continue;
     }
 
     AnimationPerformanceWarning::Type warningType;
-    if (effect->ShouldBlockCompositorAnimations(aFrame,
-                                                warningType)) {
+    if (aProperty == eCSSProperty_transform &&
+        effect->ShouldBlockAsyncTransformAnimations(aFrame,
+                                                    warningType)) {
       if (aMatches) {
         aMatches->Clear();
       }
-      effect->SetPerformanceWarning(
-        aProperty, AnimationPerformanceWarning(warningType));
+      EffectCompositor::SetPerformanceWarning(
+        aFrame, aProperty,
+        AnimationPerformanceWarning(warningType));
       return false;
     }
 
     if (!effect->HasAnimationOfProperty(aProperty)) {
       continue;
     }
 
     if (aMatches) {
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -2136,17 +2136,17 @@ KeyframeEffectReadOnly::CanAnimateTransf
     aPerformanceWarning = AnimationPerformanceWarning::Type::TransformSVG;
     return false;
   }
 
   return true;
 }
 
 bool
-KeyframeEffectReadOnly::ShouldBlockCompositorAnimations(
+KeyframeEffectReadOnly::ShouldBlockAsyncTransformAnimations(
   const nsIFrame* aFrame,
   AnimationPerformanceWarning::Type& aPerformanceWarning) const
 {
   // We currently only expect this method to be called when this effect
   // is attached to a playing Animation. If that ever changes we'll need
   // to update this to only return true when that is the case since paused,
   // filling, cancelled Animations etc. shouldn't stop other Animations from
   // running on the compositor.
@@ -2156,17 +2156,17 @@ KeyframeEffectReadOnly::ShouldBlockCompo
     // If a property is overridden in the CSS cascade, it should not block other
     // animations from running on the compositor.
     if (!property.mWinsInCascade) {
       continue;
     }
     // Check for geometric properties
     if (IsGeometricProperty(property.mProperty)) {
       aPerformanceWarning =
-        AnimationPerformanceWarning::Type::WithGeometricProperties;
+        AnimationPerformanceWarning::Type::TransformWithGeometricProperties;
       return true;
     }
 
     // Check for unsupported transform animations
     if (property.mProperty == eCSSProperty_transform) {
       if (!CanAnimateTransformOnCompositor(aFrame,
                                            aPerformanceWarning)) {
         return true;
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -301,34 +301,26 @@ public:
   void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                     nsCSSPropertySet& aSetProperties);
   // Returns true if at least one property is being animated on compositor.
   bool IsRunningOnCompositor() const;
   void SetIsRunningOnCompositor(nsCSSProperty aProperty, bool aIsRunning);
 
   void GetPropertyState(nsTArray<AnimationPropertyState>& aStates) const;
 
-  // Returns true if this effect, applied to |aFrame|, contains
-  // properties that mean we shouldn't run *any* compositor animations on this
-  // element.
+  // Returns true if this effect, applied to |aFrame|, contains properties
+  // that mean we shouldn't run transform compositor animations on this element.
   //
   // For example, if we have an animation of geometric properties like 'left'
-  // and 'top' on an element, we force all 'transform' and 'opacity' animations
-  // running at the same time on the same element to run on the main thread.
-  //
-  // Similarly, some transform animations cannot be run on the compositor and
-  // when that is the case we simply disable all compositor animations
-  // on the same element.
+  // and 'top' on an element, we force all 'transform' animations running at
+  // the same time on the same element to run on the main thread.
   //
-  // Bug 1218620 - It seems like we don't need to be this restrictive. Wouldn't
-  // it be ok to do 'opacity' animations on the compositor in either case?
-  //
-  // When returning true, |aOutPerformanceWarning| stores the reason why
-  // we shouldn't run the compositor animations.
-  bool ShouldBlockCompositorAnimations(
+  // When returning true, |aPerformanceWarning| stores the reason why
+  // we shouldn't run the transform animations.
+  bool ShouldBlockAsyncTransformAnimations(
     const nsIFrame* aFrame,
     AnimationPerformanceWarning::Type& aPerformanceWarning) const;
 
   nsIDocument* GetRenderedDocument() const;
   nsPresContext* GetPresContext() const;
 
   // Associates a warning with the animated property on the specified frame
   // indicating why, for example, the property could not be animated on the
--- a/dom/animation/test/chrome/test_animation_property_state.html
+++ b/dom/animation/test/chrome/test_animation_property_state.html
@@ -74,17 +74,18 @@ function assert_property_state_on_compos
   var sortedActual = actual.sort(compare_property_state);
   var sortedExpected = expected.sort(compare_property_state);
 
   for (var i = 0; i < sortedActual.length; i++) {
     assert_equals(sortedActual[i].property,
                   sortedExpected[i].property,
                   'CSS property name should match');
     assert_true(sortedActual[i].runningOnCompositor,
-                'runningOnCompositor property should be true');
+                'runningOnCompositor property should be true on ' +
+                sortedActual[i].property);
     assert_not_exists(sortedActual[i], 'warning',
                       'warning property should not be set');
   }
 }
 
 var gAnimationsTests = [
   {
     desc: 'animations on compositor',
@@ -140,33 +141,74 @@ var gAnimationsTests = [
       },
       {
         property: 'transform',
         runningOnCompositor: true
       }
     ]
   },
   {
+    desc: 'opacity on compositor with animation of geometric properties',
+    frames: {
+      width: ['100px', '200px'],
+      opacity: [0, 1]
+    },
+    expected: [
+      {
+        property: 'width',
+        runningOnCompositor: false
+      },
+      {
+        property: 'opacity',
+        runningOnCompositor: true
+      }
+    ]
+  },
+  {
     // FIXME: Once we have KeyframeEffect.setFrames, we should rewrite
     // this test case to check that runningOnCompositor is restored to true
     // after 'width' keyframe is removed from the keyframes.
-    desc: 'animation on compositor with animation of geometric properties',
+    desc: 'transform on compositor with animation of geometric properties',
     frames: {
       width: ['100px', '200px'],
       transform: ['translate(0px)', 'translate(100px)']
     },
     expected: [
       {
         property: 'width',
         runningOnCompositor: false
       },
       {
         property: 'transform',
         runningOnCompositor: false,
-        warning: 'AnimationWarningWithGeometricProperties'
+        warning: 'AnimationWarningTransformWithGeometricProperties'
+      }
+    ]
+  },
+  {
+    desc: 'opacity and transform on compositor with animation of geometric ' +
+          'properties',
+    frames: {
+      width: ['100px', '200px'],
+      opacity: [0, 1],
+      transform: ['translate(0px)', 'translate(100px)']
+    },
+    expected: [
+      {
+        property: 'width',
+        runningOnCompositor: false
+      },
+      {
+        property: 'opacity',
+        runningOnCompositor: true
+      },
+      {
+        property: 'transform',
+        runningOnCompositor: false,
+        warning: 'AnimationWarningTransformWithGeometricProperties'
       }
     ]
   },
 ];
 
 gAnimationsTests.forEach(function(subtest) {
   promise_test(function(t) {
     var div = addDiv(t, { class: 'compositable' });
@@ -203,16 +245,253 @@ var gPerformanceWarningTests = [
     expected: [
       {
         property: 'transform',
         runningOnCompositor: false,
         warning: 'AnimationWarningTransformBackfaceVisibilityHidden'
       }
     ]
   },
+  {
+    desc: 'opacity and transform with preserve-3d',
+    frames: {
+      opacity: [0, 1],
+      transform: ['translate(0px)', 'translate(100px)']
+    },
+    style: 'transform-style: preserve-3d',
+    expected: [
+      {
+        property: 'opacity',
+        runningOnCompositor: true
+      },
+      {
+        property: 'transform',
+        runningOnCompositor: false,
+        warning: 'AnimationWarningTransformPreserve3D'
+      }
+    ]
+  },
+  {
+    desc: 'opacity and transform with backface-visibility:hidden',
+    frames: {
+      opacity: [0, 1],
+      transform: ['translate(0px)', 'translate(100px)']
+    },
+    style: 'backface-visibility: hidden;',
+    expected: [
+      {
+        property: 'opacity',
+        runningOnCompositor: true
+      },
+      {
+        property: 'transform',
+        runningOnCompositor: false,
+        warning: 'AnimationWarningTransformBackfaceVisibilityHidden'
+      }
+    ]
+  },
+];
+
+var gMultipleAsyncAnimationsTests = [
+  {
+    desc: 'opacity and transform with preserve-3d',
+    style: 'transform-style: preserve-3d',
+    animations: [
+      {
+        frames: {
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformPreserve3D'
+          }
+        ]
+      },
+      {
+        frames: {
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'opacity',
+            runningOnCompositor: true,
+          }
+        ]
+      }
+    ],
+  },
+  {
+    desc: 'opacity and transform with backface-visibility:hidden',
+    style: 'backface-visibility: hidden;',
+    animations: [
+      {
+        frames: {
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformBackfaceVisibilityHidden'
+          }
+        ]
+      },
+      {
+        frames: {
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'opacity',
+            runningOnCompositor: true,
+          }
+        ]
+      }
+    ],
+  },
+];
+
+// FIXME: Once we have KeyframeEffect.setFrames, we should rewrite
+// these test cases to check that runningOnCompositor is restored to true
+// after 'width' keyframe is removed from the keyframes.
+var gMultipleAsyncAnimationsWithGeometricKeyframeTests = [
+  {
+    desc: 'transform and opacity with animation of geometric properties',
+    animations: [
+      {
+        frames: {
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformWithGeometricProperties'
+          }
+        ]
+      },
+      {
+        frames: {
+          width: ['100px', '200px'],
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'width',
+            runningOnCompositor: false,
+          },
+          {
+            property: 'opacity',
+            runningOnCompositor: true,
+          }
+        ]
+      }
+    ],
+  },
+  {
+    desc: 'opacity and transform with animation of geometric properties',
+    animations: [
+      {
+        frames: {
+          width: ['100px', '200px'],
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'width',
+            runningOnCompositor: false,
+          },
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformWithGeometricProperties'
+          }
+        ]
+      },
+      {
+        frames: {
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'opacity',
+            runningOnCompositor: true,
+          }
+        ]
+      }
+    ],
+  },
+];
+
+// Test cases that check results of adding/removing 'width' animation on the
+// same element which has async animations.
+var gMultipleAsyncAnimationsWithGeometricAnimationTests = [
+  {
+    desc: 'transform',
+    animations: [
+      {
+        frames: {
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformWithGeometricProperties'
+          }
+        ]
+      },
+    ]
+  },
+  {
+    desc: 'opacity',
+    animations: [
+      {
+        frames: {
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'opacity',
+            runningOnCompositor: true
+          }
+        ]
+      },
+    ]
+  },
+  {
+    desc: 'opacity and transform',
+    animations: [
+      {
+        frames: {
+          transform: ['translate(0px)', 'translate(100px)']
+        },
+        expected: [
+          {
+            property: 'transform',
+            runningOnCompositor: false,
+            warning: 'AnimationWarningTransformWithGeometricProperties'
+          }
+        ]
+      },
+      {
+        frames: {
+          opacity: [0, 1]
+        },
+        expected: [
+          {
+            property: 'opacity',
+            runningOnCompositor: true,
+          }
+        ]
+      }
+    ],
+  },
 ];
 
 function start() {
   var bundleService = SpecialPowers.Cc['@mozilla.org/intl/stringbundle;1']
     .getService(SpecialPowers.Ci.nsIStringBundleService);
   gStringBundle = bundleService
     .createBundle("chrome://global/locale/layout_errors.properties");
 
@@ -247,16 +526,117 @@ function start() {
       })).then(t.step_func(function() {
         assert_property_state_on_compositor(
           animation.effect.getPropertyState(),
           subtest.expected);
       }));
     }, subtest.desc);
   });
 
+  gMultipleAsyncAnimationsTests.forEach(function(subtest) {
+    promise_test(function(t) {
+      var div = addDiv(t, { class: 'compositable' });
+      var animations = subtest.animations.map(function(anim) {
+        var animation = div.animate(anim.frames, 100000);
+
+        // Bind expected values to animation object.
+        animation.expected = anim.expected;
+        return animation;
+      });
+      return waitForAllAnimations(animations).then(t.step_func(function() {
+        animations.forEach(t.step_func(function(anim) {
+          assert_property_state_on_compositor(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+        div.style = subtest.style;
+        return waitForFrame();
+      })).then(t.step_func(function() {
+        animations.forEach(t.step_func(function(anim) {
+          assert_animation_property_state_equals(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+        div.style = '';
+        return waitForFrame();
+      })).then(t.step_func(function() {
+        animations.forEach(t.step_func(function(anim) {
+          assert_property_state_on_compositor(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+      }));
+    }, 'Multiple animations: ' + subtest.desc);
+  });
+
+  gMultipleAsyncAnimationsWithGeometricKeyframeTests.forEach(function(subtest) {
+    promise_test(function(t) {
+      var div = addDiv(t, { class: 'compositable' });
+      var animations = subtest.animations.map(function(anim) {
+        var animation = div.animate(anim.frames, 100000);
+
+        // Bind expected values to animation object.
+        animation.expected = anim.expected;
+        return animation;
+      });
+      return waitForAllAnimations(animations).then(t.step_func(function() {
+        animations.forEach(t.step_func(function(anim) {
+          assert_animation_property_state_equals(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+      }));
+    }, 'Multiple animations with geometric property: ' + subtest.desc);
+  });
+
+  gMultipleAsyncAnimationsWithGeometricAnimationTests.forEach(function(subtest) {
+    promise_test(function(t) {
+      var div = addDiv(t, { class: 'compositable' });
+      var animations = subtest.animations.map(function(anim) {
+        var animation = div.animate(anim.frames, 100000);
+
+        // Bind expected values to animation object.
+        animation.expected = anim.expected;
+        return animation;
+      });
+
+      var widthAnimation;
+
+      return waitForAllAnimations(animations).then(t.step_func(function() {
+        animations.forEach(t.step_func(function(anim) {
+          assert_property_state_on_compositor(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+      })).then(t.step_func(function() {
+        // Append 'width' animation on the same element.
+        widthAnimation = div.animate({ width: ['100px', '200px'] }, 100000);
+        return waitForFrame();
+      })).then(t.step_func(function() {
+        // Now transform animations are not running on compositor because of
+        // the 'width' animation.
+        animations.forEach(t.step_func(function(anim) {
+          assert_animation_property_state_equals(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+        // Remove the 'width' animation.
+        widthAnimation.cancel();
+        return waitForFrame();
+      })).then(t.step_func(function() {
+        // Now all animations are running on compositor.
+        animations.forEach(t.step_func(function(anim) {
+          assert_property_state_on_compositor(
+            anim.effect.getPropertyState(),
+            anim.expected);
+        }));
+      }));
+    }, 'Multiple async animations and geometric animation: ' + subtest.desc);
+  });
+
   promise_test(function(t) {
     var div = addDiv(t, { class: 'compositable' });
     var animation = div.animate(
       { transform: ['translate(0px)', 'translate(100px)'] }, 100000);
     return animation.ready.then(t.step_func(function() {
       assert_animation_property_state_equals(
         animation.effect.getPropertyState(),
         [ { property: 'transform', runningOnCompositor: true } ]);
--- a/dom/locales/en-US/chrome/layout/layout_errors.properties
+++ b/dom/locales/en-US/chrome/layout/layout_errors.properties
@@ -19,16 +19,16 @@ ScrollLinkedEffectFound2=This site appea
 AnimationWarningContentTooLarge=Async animation disabled because frame size (%1$S, %2$S) is bigger than the viewport (%3$S, %4$S) or the visual rectangle (%5$S, %6$S) is larger than the max allowed value (%7$S)
 ## LOCALIZATION NOTE(AnimationWarningTransformBackfaceVisibilityHidde):
 ## 'backface-visibility: hidden' is a CSS property, don't translate it.
 AnimationWarningTransformBackfaceVisibilityHidden=Async animation of 'backface-visibility: hidden' transforms is not supported
 ## LOCALIZATION NOTE(AnimationWarningTransformPreserve3D):
 ## 'transform-style: preserve-3d' is a CSS property, don't translate it.
 AnimationWarningTransformPreserve3D=Async animation of 'transform-style: preserve-3d' transforms is not supported
 ## LOCALIZATION NOTE(AnimationWarningTransformSVG,
+##                   AnimationWarningTransformWithGeometricProperties,
 ##                   AnimationWarningTransformFrameInactive,
-##                   AnimationWarningOpacityFrameInactive,
-##                   AnimationWarningWithGeometricProperties):
+##                   AnimationWarningOpacityFrameInactive):
 ## 'transform' and 'opacity' mean CSS property names, don't translate it.
 AnimationWarningTransformSVG=Async 'transform' animations of aFrames with SVG transforms is not supported
+AnimationWarningTransformWithGeometricProperties=Async animation of 'transform' not possible due to animation of geometric properties on the same element
 AnimationWarningTransformFrameInactive=Async animation disabled because frame was not marked active for 'transform' animation
 AnimationWarningOpacityFrameInactive=Async animation disabled because frame was not marked active for 'opacity' animation
-AnimationWarningWithGeometricProperties=Async animation of 'transform' or 'opacity' not possible due to animation of geometric properties on the same element