Bug 1555548 - Send other transform-like properties as non-animating animation into compositor. r=hiro

Not only animating transform-like properties, but also non-animating ones have
to be passed into the compositor, so the final transform matrix could
take them into account (on the compositor).

Differential Revision: https://phabricator.services.mozilla.com/D33580

--HG--
extra : moz-landing-system : lando

gfx/layers/AnimationHelper.cpp

@@ -319,6 +319,7 @@ AnimationHelper::SampleResult AnimationHelper::SampleAnimationForEachNode(
   MOZ_ASSERT(aAnimationValues.IsEmpty(),
              "Should be called with empty aAnimationValues");
 
+  nsTArray<RefPtr<RawServoAnimationValue>> nonAnimatingValues;
   for (PropertyAnimationGroup& group : aPropertyAnimationGroups) {
     // Initialize animation value with base style.
     RefPtr<RawServoAnimationValue> currValue = group.mBaseStyle;
@@ -327,6 +328,23 @@ AnimationHelper::SampleResult AnimationHelper::SampleAnimationForEachNode(
                                             group.mAnimations.Length() == 1
                                         ? CanSkipCompose::IfPossible
                                         : CanSkipCompose::No;
+
+    MOZ_ASSERT(
+        !group.mAnimations.IsEmpty() ||
+            nsCSSPropertyIDSet::TransformLikeProperties().HasProperty(
+                group.mProperty),
+        "Only transform-like properties can have empty PropertyAnimation list");
+
+    // For properties which are not animating (i.e. their values are always the
+    // same), we store them in a different array, and then merge them into the
+    // final result (a.k.a. aAnimationValues) because we shouldn't take them
+    // into account for SampleResult. (In other words, these properties
+    // shouldn't affect the optimization.)
+    if (group.mAnimations.IsEmpty()) {
+      nonAnimatingValues.AppendElement(std::move(currValue));
+      continue;
+    }
+
     SampleResult result = SampleAnimationForProperty(
         aPreviousFrameTime, aCurrentFrameTime, aPreviousValue, canSkipCompose,
         group.mAnimations, currValue);
@@ -374,8 +392,12 @@ AnimationHelper::SampleResult AnimationHelper::SampleAnimationForEachNode(
   }
 #endif
 
-  return aAnimationValues.IsEmpty() ? SampleResult::None
-                                    : SampleResult::Sampled;
+  SampleResult rv =
+      aAnimationValues.IsEmpty() ? SampleResult::None : SampleResult::Sampled;
+  if (rv == SampleResult::Sampled) {
+    aAnimationValues.AppendElements(nonAnimatingValues);
+  }
+  return rv;
 }
 
 static dom::FillMode GetAdjustedFillMode(const Animation& aAnimation) {
@@ -441,6 +463,17 @@ nsTArray<PropertyAnimationGroup> AnimationHelper::ExtractAnimations(
       currBaseStyle = &animation.baseStyle();
     }
 
+    // If this layers::Animation sets isNotAnimating to true, it only has
+    // base style and doesn't have any animation information, so we can skip
+    // the rest steps. (And so its PropertyAnimationGroup::mAnimation will be
+    // an empty array.)
+    if (animation.isNotAnimating()) {
+      MOZ_ASSERT(nsCSSPropertyIDSet::TransformLikeProperties().HasProperty(
+                     animation.property()),
+                 "Only transform-like properties could set this true");
+      continue;
+    }
+
     PropertyAnimation* propertyAnimation =
         currData->mAnimations.AppendElement();
 

gfx/layers/ipc/LayersMessages.ipdlh

@@ -249,6 +249,11 @@ struct Animation {
   // True if the animation has a fixed current time (e.g. paused and
   // forward-filling animations).
   bool isNotPlaying;
+  // True if this is not an animating property. For some transform-like
+  // properties, we just send their baseStyles for merging with other animating
+  // properties. In this case, we don't have animation information on this
+  // property, and so don't need to do interpolation.
+  bool isNotAnimating;
   // The base style that animations should composite with. This is only set for
   // animations with a composite mode of additive or accumulate, and only for
   // the first animation in the set (i.e. the animation that is lowest in the

layout/painting/nsDisplayList.cpp

@@ -281,9 +281,9 @@ static Translation GetTranslate(const StyleTranslate& aValue,
   return result;
 }
 
-static void AddTransformFunctions(
-    const StyleTransform& aTransform, TransformReferenceBox& aRefBox,
-    InfallibleTArray<TransformFunction>& aFunctions) {
+static void AddTransformFunctions(const StyleTransform& aTransform,
+                                  TransformReferenceBox& aRefBox,
+                                  nsTArray<TransformFunction>& aFunctions) {
   for (const StyleTransformOperation& op : aTransform.Operations()) {
     switch (op.tag) {
       case StyleTransformOperation::Tag::RotateX: {
@@ -593,6 +593,7 @@ static void AddAnimationForProperty(nsIFrame* aFrame,
   animation->iterationComposite() = static_cast<uint8_t>(
       aAnimation->GetEffect()->AsKeyframeEffect()->IterationComposite());
   animation->isNotPlaying() = !aAnimation->IsPlaying();
+  animation->isNotAnimating() = false;
 
   TransformReferenceBox refBox(aFrame);
 
@@ -683,11 +684,12 @@ GroupAnimationsByProperty(const nsTArray<RefPtr<dom::Animation>>& aAnimations,
   return groupedAnims;
 }
 
-static void AddAnimationsForProperty(
+static bool AddAnimationsForProperty(
     nsIFrame* aFrame, const EffectSet* aEffects,
     const nsTArray<RefPtr<dom::Animation>>& aCompositorAnimations,
     const Maybe<TransformData>& aData, nsCSSPropertyID aProperty,
     Send aSendFlag, AnimationInfo& aAnimationInfo) {
+  bool addedAny = false;
   // Add from first to last (since last overrides)
   for (dom::Animation* anim : aCompositorAnimations) {
     if (!anim->IsRelevant()) {
@@ -732,7 +734,9 @@ static void AddAnimationsForProperty(
     AddAnimationForProperty(aFrame, *property, anim, aData, aSendFlag,
                             aAnimationInfo);
     keyframeEffect->SetIsRunningOnCompositor(aProperty, true);
+    addedAny = true;
   }
+  return addedAny;
 }
 
 static Maybe<TransformData> CreateAnimationData(
@@ -782,6 +786,58 @@ static Maybe<TransformData> CreateAnimationData(
                             hasPerspectiveParent));
 }
 
+static void AddNonAnimatingTransformLikePropertiesStyles(
+    const nsCSSPropertyIDSet& aNonAnimatingProperties, nsIFrame* aFrame,
+    const Maybe<TransformData>& aData, Send aSendFlag,
+    AnimationInfo& aAnimationInfo) {
+  auto appendFakeAnimation = [&aAnimationInfo, &aData, aSendFlag](
+                                 nsCSSPropertyID aProperty,
+                                 Animatable&& aBaseStyle) {
+    layers::Animation* animation =
+        (aSendFlag == Send::NextTransaction)
+            ? aAnimationInfo.AddAnimationForNextTransaction()
+            : aAnimationInfo.AddAnimation();
+    animation->property() = aProperty;
+    animation->baseStyle() = std::move(aBaseStyle);
+    animation->data() = aData;
+    animation->easingFunction() = null_t();
+    animation->isNotAnimating() = true;
+  };
+
+  for (nsCSSPropertyID id : aNonAnimatingProperties) {
+    switch (id) {
+      case eCSSProperty_transform:
+        if (!aFrame->StyleDisplay()->mTransform.IsNone()) {
+          TransformReferenceBox refBox(aFrame);
+          nsTArray<TransformFunction> transformFunctions;
+          AddTransformFunctions(aFrame->StyleDisplay()->mTransform, refBox,
+                                transformFunctions);
+          appendFakeAnimation(id, Animatable(std::move(transformFunctions)));
+        }
+        break;
+      case eCSSProperty_translate:
+        if (!aFrame->StyleDisplay()->mTranslate.IsNone()) {
+          TransformReferenceBox refBox(aFrame);
+          appendFakeAnimation(
+              id, GetTranslate(aFrame->StyleDisplay()->mTranslate, refBox));
+        }
+        break;
+      case eCSSProperty_rotate:
+        if (!aFrame->StyleDisplay()->mRotate.IsNone()) {
+          appendFakeAnimation(id, GetRotate(aFrame->StyleDisplay()->mRotate));
+        }
+        break;
+      case eCSSProperty_scale:
+        if (!aFrame->StyleDisplay()->mScale.IsNone()) {
+          appendFakeAnimation(id, GetScale(aFrame->StyleDisplay()->mScale));
+        }
+        break;
+      default:
+        MOZ_ASSERT_UNREACHABLE("Unsupported transform-like properties");
+    }
+  }
+}
+
 static void AddAnimationsForDisplayItem(nsIFrame* aFrame,
                                         nsDisplayListBuilder* aBuilder,
                                         nsDisplayItem* aItem,
@@ -829,9 +885,38 @@ static void AddAnimationsForDisplayItem(nsIFrame* aFrame,
   const HashMap<nsCSSPropertyID, nsTArray<RefPtr<dom::Animation>>>
       compositorAnimations =
           GroupAnimationsByProperty(matchedAnimations, propertySet);
+  // Bug 1424900: Drop this pref check after shipping individual transforms.
+  const bool hasMultipleTransformLikeProperties =
+      StaticPrefs::IndividualTransform() &&
+      aType == DisplayItemType::TYPE_TRANSFORM;
+  nsCSSPropertyIDSet nonAnimatingProperties =
+      nsCSSPropertyIDSet::TransformLikeProperties();
   for (auto iter = compositorAnimations.iter(); !iter.done(); iter.next()) {
+    bool added =
         AddAnimationsForProperty(aFrame, effects, iter.get().value(), data,
                                  iter.get().key(), aSendFlag, aAnimationInfo);
+    if (hasMultipleTransformLikeProperties && added) {
+      nonAnimatingProperties.RemoveProperty(iter.get().key());
+    }
+  }
+
+  // If some transform-like properties have animations, but others not, and
+  // those non-animating transform-like properties have non-none
+  // transform/translate/rotate/scale styles, we also pass their styles into
+  // the compositor, so the final transform matrix (on the compositor) could
+  // take them into account.
+  if (hasMultipleTransformLikeProperties &&
+      // For these cases we don't need to send the property style values to
+      // the compositor:
+      // 1. No property has running animations on the compositor. (i.e. All
+      //    properties should be handled by main thread)
+      // 2. All properties have running animations on the compositor.
+      //    (i.e. Those running animations should override the styles.)
+      !nonAnimatingProperties.Equals(
+          nsCSSPropertyIDSet::TransformLikeProperties()) &&
+      !nonAnimatingProperties.IsEmpty()) {
+    AddNonAnimatingTransformLikePropertiesStyles(
+        nonAnimatingProperties, aFrame, data, aSendFlag, aAnimationInfo);
   }
 }
 

layout/style/test/test_animations_omta.html

@@ -79,6 +79,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=964646
       75%  { transform: translate(120px); animation-timing-function: linear }
       100% { transform: translate(20px); animation-timing-function: ease-out }
     }
+    @keyframes kf_scale {
+      to { scale: 2.25 2.25; }
+    }
 
     @keyframes always_fifty {
       from, to { transform: translate(50px) }
@@ -2614,5 +2617,104 @@ addAsyncAnimTest(async function() {
   done_div();
 });
 
+// Multiple transform-like properties animation. The non-animating properties
+// shouldn't be overridden by animating ones.
+addAsyncAnimTest(async function() {
+  new_div("translate: 100px; " +
+          "scale: 1 1; " +
+          "transform: translate(200px); " +
+          "transition: all 10s linear");
+  await waitForPaintsFlushed();
+
+  // No transition on transform property.
+  gDiv.style.translate = "200px";
+  gDiv.style.scale = "2 2";
+  await waitForPaintsFlushed();
+
+  omta_is("transform", { compositorValue: { tx: 300 },
+                         usesMultipleProperties: true },
+          RunningOn.Compositor,
+          "transform-like properties transition runs on compositor thread");
+
+  advance_clock(5000);
+
+  if (isGeckoView()) {
+    todo(false,
+         "Bug 1536392: GeckoView with e10s: cannot get correct OMTA style");
+  } else {
+    omta_is("transform",
+            // The order is: translate, scale, transform.
+            // So the translate() in transform should be multiplied by 1.5.
+            { compositorValue: [ 1.5, 0, 0, 1.5, (150 + 200*1.5), 0 ],
+              usesMultipleProperties: true },
+            RunningOn.Compositor,
+            "transform-like properties on compositor at 5s");
+  }
+
+  done_div();
+});
+
+// Multiple transform-like properties animation with delay. The delayed
+// animating properties shouldn't be overridden.
+//
+// Note:
+// In delay phase, the SampleResult should be None, even though there is
+// an non-animating property which is also sent to the compositor.
+// If the SampleResult is Sampled in this case, we may get an incorrect result
+// ("scale" would be "1" because the final matrix is calculated by a default
+// scale value which overrides the scale css style).
+// That's why we shouldn't take non-animating properties into account on
+// SampleResult.
+addAsyncAnimTest(async function() {
+  new_div("translate: 100px; " +
+          "scale: 1.25 1.25; " +
+          "animation: kf_scale 10s 2.5s linear");
+  await waitForPaintsFlushed();
+
+  // All transform-like properties use DisplayItemType::TYPE_TRANSFORM,
+  // so using "transform" is enough.
+  var compositorStr =
+    SpecialPowers.DOMWindowUtils.getOMTAStyle(gDiv, "transform");
+  ok(compositorStr === "",
+     "transform-like properties should not run on the compositor at 0s " +
+     "(in delay phase)");
+  var computedStr = window.getComputedStyle(gDiv).translate;
+  ok(computedStr === "100px",
+     "The computed value of translate property should be equal to 100px " +
+     "in delay phase, got " + computedStr);
+  computedStr = window.getComputedStyle(gDiv).scale;
+  ok(computedStr === "1.25",
+     "The computed value of scale property should be equal to 1.25 " +
+     "in delay phase, got " + computedStr);
+
+  advance_clock(2500);
+
+  if (isGeckoView()) {
+    todo(false,
+         "Bug 1536392: GeckoView with e10s: cannot get correct OMTA style");
+  } else {
+    omta_is("transform", { compositorValue: [ 1.25, 0, 0, 1.25, 100, 0 ],
+                           usesMultipleProperties: true },
+            RunningOn.Compositor,
+            "transform-like properties on compositor at 2.5s");
+  }
+
+  advance_clock(5000);
+
+  if (isGeckoView()) {
+    todo(false,
+         "Bug 1536392: GeckoView with e10s: cannot get correct OMTA style");
+  } else {
+    omta_is("transform",
+            { compositorValue: [ 1.75, 0, 0, 1.75, 100, 0 ],
+              usesMultipleProperties: true },
+            RunningOn.Compositor,
+            "transform-like properties on compositor at 7.5s");
+  }
+
+  done_div();
+});
+
+
 </script>
 </html>