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 288546 a811164e762da26f4d1cdcc7221b9a2e914f0609
parent 288545 5b2891e4650c9b2a1bd5dc83703b19d4a4f681f5
child 288547 acbe5c09bbb702586bd8c4b19c24d150e45f118a
push id30084
push userkwierso@gmail.com
push dateTue, 15 Mar 2016 00:39:07 +0000
treeherdermozilla-central@422077f61bcb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1218620
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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