Bug 1457249 - Use actually calculated value for the assertion that checks animation value is unchanged when we decided to skip calculation for the animation on debug build. r?kats draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Sat, 28 Apr 2018 18:09:27 +0900
changeset 789344 e622c8dd136d651a720d35a7cbae02ca2aa6f09e
parent 789343 cf1ccb0a01739d610b225d94c4239db3b1556851
push id108255
push userhikezoe@mozilla.com
push dateSat, 28 Apr 2018 09:12:02 +0000
reviewerskats
bugs1457249
milestone61.0a1
Bug 1457249 - Use actually calculated value for the assertion that checks animation value is unchanged when we decided to skip calculation for the animation on debug build. r?kats Before this change, the value which was set by SetShadowBaseTransform() has been used for the assertion, but it is possible that the value is changed by APZ. And it's hard to tell whether the value has been changed by APZ or not and it's hard to *reverse-calculate* the differences in the past APZ at the moment we want to do the assert. So after this patch, on debug build we don't actually skip the calculation for unchanged animations and use the newly calculated value for the assertion. MozReview-Commit-ID: 8fCcvvbUMHe
gfx/layers/AnimationHelper.cpp
gfx/layers/composite/AsyncCompositionManager.cpp
--- a/gfx/layers/AnimationHelper.cpp
+++ b/gfx/layers/AnimationHelper.cpp
@@ -151,16 +151,24 @@ AnimationHelper::SampleAnimationForEachN
   TimeStamp aTime,
   AnimationArray& aAnimations,
   InfallibleTArray<AnimData>& aAnimationData,
   RefPtr<RawServoAnimationValue>& aAnimationValue)
 {
   MOZ_ASSERT(!aAnimations.IsEmpty(), "Should be called with animations");
 
   bool hasInEffectAnimations = false;
+#ifdef DEBUG
+  // In cases where this function returns a SampleResult::Skipped, we actually
+  // do populate aAnimationValue in debug mode, so that we can MOZ_ASSERT at the
+  // call site that the value that would have been computed matches the stored
+  // value that we end up using. This flag is used to ensure we populate
+  // aAnimationValue in this scenario.
+  bool shouldBeSkipped = false;
+#endif
   // Process in order, since later aAnimations override earlier ones.
   for (size_t i = 0, iEnd = aAnimations.Length(); i < iEnd; ++i) {
     Animation& animation = aAnimations[i];
     AnimData& animData = aAnimationData[i];
 
     MOZ_ASSERT((!animation.originTime().IsNull() &&
                 animation.startTime().type() ==
                   MaybeTimeDuration::TTimeDuration) ||
@@ -199,17 +207,21 @@ AnimationHelper::SampleAnimationForEachN
     // FIXME Bug 1455476: We should do this optimizations for the case where
     // the layer has multiple animations.
     if (iEnd == 1 &&
         !dom::KeyframeEffectReadOnly::HasComputedTimingChanged(
           computedTiming,
           iterCompositeOperation,
           animData.mProgressOnLastCompose,
           animData.mCurrentIterationOnLastCompose)) {
+#ifdef DEBUG
+      shouldBeSkipped = true;
+#else
       return SampleResult::Skipped;
+#endif
     }
 
     uint32_t segmentIndex = 0;
     size_t segmentSize = animation.segments().Length();
     AnimationSegment* segment = animation.segments().Elements();
     while (segment->endPortion() < computedTiming.mProgress.Value() &&
            segmentIndex < segmentSize - 1) {
       ++segment;
@@ -231,17 +243,21 @@ AnimationHelper::SampleAnimationForEachN
     // timing functions (e.g. the throbber animation on tab or frame based
     // animations).
     // FIXME Bug 1455476: Like the above optimization, we should apply this
     // optimizations for multiple animation cases as well.
     if (iEnd == 1 &&
         animData.mSegmentIndexOnLastCompose == segmentIndex &&
         !animData.mPortionInSegmentOnLastCompose.IsNull() &&
         animData.mPortionInSegmentOnLastCompose.Value() == portion) {
+#ifdef DEBUG
+      shouldBeSkipped = true;
+#else
       return SampleResult::Skipped;
+#endif
     }
 
     AnimationPropertySegment animSegment;
     animSegment.mFromKey = 0.0;
     animSegment.mToKey = 1.0;
     animSegment.mFromValue =
       AnimationValue(animData.mStartValues[segmentIndex]);
     animSegment.mToValue =
@@ -255,16 +271,23 @@ AnimationHelper::SampleAnimationForEachN
     aAnimationValue =
       Servo_ComposeAnimationSegment(
         &animSegment,
         aAnimationValue,
         animData.mEndValues.LastElement(),
         iterCompositeOperation,
         portion,
         computedTiming.mCurrentIteration).Consume();
+
+#ifdef DEBUG
+    if (shouldBeSkipped) {
+      return SampleResult::Skipped;
+    }
+#endif
+
     hasInEffectAnimations = true;
     animData.mProgressOnLastCompose = computedTiming.mProgress;
     animData.mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
     animData.mSegmentIndexOnLastCompose = segmentIndex;
     animData.mPortionInSegmentOnLastCompose.SetValue(portion);
   }
 
 #ifdef DEBUG
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -708,27 +708,37 @@ SampleAnimations(Layer* aLayer,
             // 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());
                 MOZ_ASSERT(FuzzyEqualsMultiplicative(
-                  layer->AsHostLayer()->GetShadowOpacity(),
+                  Servo_AnimationValue_GetOpacity(animationValue),
                   *(aStorage->GetAnimationOpacity(layer->GetCompositorAnimationsId()))));
                 break;
               case eCSSProperty_transform: {
                 MOZ_ASSERT(
                   layer->AsHostLayer()->GetShadowTransformSetByAnimation());
+
                 AnimatedValue* transform =
                   aStorage->GetAnimatedValue(layer->GetCompositorAnimationsId());
+
+                const TransformData& transformData =
+                  animations[0].data().get_TransformData();
+                Matrix4x4 frameTransform =
+                  ServoAnimationValueToMatrix4x4(animationValue, transformData);
+                Matrix4x4 transformInDevice =
+                  FrameTransformToTransformInDevice(frameTransform,
+                                                    layer,
+                                                    transformData);
                 MOZ_ASSERT(
                   transform->mTransform.mTransformInDevSpace.FuzzyEqualsMultiplicative(
-                    (layer->AsHostLayer()->GetShadowBaseTransform())));
+                  transformInDevice));
                 break;
               }
               default:
                 MOZ_ASSERT_UNREACHABLE("Unsupported properties");
                 break;
             }
 #endif
             break;