Bug 1430884 - Throttle transform animations without %0 or 100% keyframe. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Mon, 25 Jun 2018 18:29:28 +0900
changeset 810092 b0e8b2afc65d8a3d9cd34bdfc856696c99dda5ea
parent 810091 3f1e9aa7038b7654bff26421f102bc8f7a518f1b
child 810093 2e209f77396800abb39d16abada9f1deca946dda
push id113892
push userhikezoe@mozilla.com
push dateMon, 25 Jun 2018 09:32:09 +0000
reviewersbirtles
bugs1430884
milestone62.0a1
Bug 1430884 - Throttle transform animations without %0 or 100% keyframe. r?birtles MozReview-Commit-ID: 3vLAlSkLz97
dom/animation/KeyframeEffect.cpp
dom/animation/test/mozilla/file_restyles.html
layout/style/nsStyleStruct.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -1612,19 +1612,35 @@ KeyframeEffect::CalculateCumulativeChang
     }
 
     for (const AnimationPropertySegment& segment : property.mSegments) {
       // In case composite operation is not 'replace' or value is null,
       // we can't throttle animations which will not cause any layout changes
       // on invisible elements because we can't calculate the change hint for
       // such properties until we compose it.
       if (!segment.HasReplaceableValues()) {
-        mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
-        return;
+        if (property.mProperty != eCSSProperty_transform) {
+          mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
+          return;
+        }
+        // We try a little harder to optimize transform animations simply
+        // because they are so common (the second-most commonly animated
+        // property at the time of writing).  So if we encounter a transform
+        // segment that needs composing with the underlying value, we just add
+        // all the change hints a transform animation is known to be able to
+        // generate.
+        mCumulativeChangeHint |= nsChangeHint_AddOrRemoveTransform |
+                                 nsChangeHint_RepaintFrame |
+                                 nsChangeHint_UpdateContainingBlock |
+                                 nsChangeHint_UpdateOverflow |
+                                 nsChangeHint_UpdatePostTransformOverflow |
+                                 nsChangeHint_UpdateTransformLayer;
+        continue;
       }
+
       RefPtr<ComputedStyle> fromContext =
         CreateComputedStyleForAnimationValue(property.mProperty,
                                              segment.mFromValue,
                                              presContext,
                                              aComputedStyle);
       if (!fromContext) {
         mCumulativeChangeHint = ~nsChangeHint_Hints_CanIgnoreIfNotVisible;
         return;
--- a/dom/animation/test/mozilla/file_restyles.html
+++ b/dom/animation/test/mozilla/file_restyles.html
@@ -1773,12 +1773,35 @@ waitForAllPaints(() => {
 
     const markers = await observeStyling(5);
 
     is(markers.length, 0,
        'Transform animations on visibility hidden element should be throttled');
     await ensureElementRemoval(div);
   });
 
+  add_task(async function restyling_transform_aimations_on_invisible_element() {
+    if (isAndroid) {
+      // FIXME: Bug 1470798: Enable this test on Android.
+      return;
+    }
+    // 'opacity: 0' prevents transform animations to be sent to the compositor.
+    const div = addDiv(null, { style: 'visibility: hidden; opacity: 0' });
+
+    const animation =
+      div.animate([ { transform: 'rotate(360deg)' } ],
+                  { duration: 100 * MS_PER_SEC,
+                    iterations: Infinity });
+
+    await waitForAnimationReadyToRestyle(animation);
+
+    const markers = await observeStyling(5);
+
+    is(markers.length, 0,
+       'Transform animations without 100% keyframe on visibility hidden ' +
+       'element should be throttled');
+    await ensureElementRemoval(div);
+  });
+
 });
 
 </script>
 </body>
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3689,16 +3689,19 @@ nsStyleDisplay::FinishStyle(
   GenerateCombinedTransform();
 }
 
 static inline nsChangeHint
 CompareTransformValues(const RefPtr<nsCSSValueSharedList>& aList,
                        const RefPtr<nsCSSValueSharedList>& aNewList)
 {
   nsChangeHint result = nsChangeHint(0);
+
+  // Note: If we add a new change hint for transform changes here, we have to
+  // modify KeyframeEffect::CalculateCumulativeChangeHint too!
   if (!aList != !aNewList || (aList && *aList != *aNewList)) {
     result |= nsChangeHint_UpdateTransformLayer;
     if (aList && aNewList) {
       result |= nsChangeHint_UpdatePostTransformOverflow;
     } else {
       result |= nsChangeHint_UpdateOverflow;
     }
   }
@@ -3813,16 +3816,20 @@ nsStyleDisplay::CalcDifference(const nsS
 
   /* If we've added or removed the transform property, we need to reconstruct the frame to add
    * or remove the view object, and also to handle abs-pos and fixed-pos containers.
    */
   if (HasTransformStyle() != aNewData.HasTransformStyle()) {
     // We do not need to apply nsChangeHint_UpdateTransformLayer since
     // nsChangeHint_RepaintFrame will forcibly invalidate the frame area and
     // ensure layers are rebuilt (or removed).
+    //
+    // Note: If we add a new change hint for transform changes here or in
+    // CompareTransformValues(), we have to modify
+    // KeyframeEffect::CalculateCumulativeChangeHint too!
     hint |= nsChangeHint_UpdateContainingBlock |
             nsChangeHint_AddOrRemoveTransform |
             nsChangeHint_UpdateOverflow |
             nsChangeHint_RepaintFrame;
   } else {
     /* Otherwise, if we've kept the property lying around and we already had a
      * transform, we need to see whether or not we've changed the transform.
      * If so, we need to recompute its overflow rect (which probably changed