Bug 788409: When the compositor runs past the end of an animation, just fill forwards until the main thread catches up. r=dbaron a=blocking-basecamp
authorDavid Zbarsky <dzbarsky@gmail.com>, David Baron <dbaron@dbaron.org>, Chris Jones <jones.chris.g@gmail.com>
Thu, 22 Nov 2012 15:49:06 -0800
changeset 114049 a2858a64fe77536590e4b5b943b226ff5fc5e7db
parent 114048 45162db9a860c4c6d7a6b4a78815ecbaf1417f72
child 114050 4d2479573ecdeb3d622a2f5c324a8e245aef6264
push id18518
push usercjones@mozilla.com
push dateThu, 22 Nov 2012 23:49:11 +0000
treeherdermozilla-inbound@a2858a64fe77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, blocking-basecamp
bugs788409
milestone20.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 788409: When the compositor runs past the end of an animation, just fill forwards until the main thread catches up. r=dbaron a=blocking-basecamp
gfx/layers/ipc/CompositorParent.cpp
layout/style/nsAnimationManager.cpp
layout/style/nsAnimationManager.h
--- a/gfx/layers/ipc/CompositorParent.cpp
+++ b/gfx/layers/ipc/CompositorParent.cpp
@@ -715,22 +715,16 @@ SampleAnimations(Layer* aLayer, TimeStam
       animation.numIterations() : NS_IEEEPositiveInfinity();
     double positionInIteration =
       ElementAnimations::GetPositionInIteration(animation.startTime(),
                                                 aPoint,
                                                 animation.duration(),
                                                 numIterations,
                                                 animation.direction());
 
-    if (positionInIteration == -1) {
-      animations.RemoveElementAt(i);
-      animationData.RemoveElementAt(i);
-      continue;
-    }
-
     NS_ABORT_IF_FALSE(0.0 <= positionInIteration &&
                       positionInIteration <= 1.0,
                       "position should be in [0-1]");
 
     int segmentIndex = 0;
     AnimationSegment* segment = animation.segments().Elements();
     while (segment->endPortion() < positionInIteration) {
       ++segment;
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -49,39 +49,42 @@ ElementAnimations::GetPositionInIteratio
 {
   // Set |currentIterationCount| to the (fractional) number of
   // iterations we've completed up to the current position.
   TimeDuration currentTimeDuration = aCurrentTime - aStartTime;
   double currentIterationCount =
     currentTimeDuration / aDuration;
   bool dispatchStartOrIteration = false;
   if (currentIterationCount >= aIterationCount) {
-    if (!aAnimation) {
-      // We are on the compositor, so send a signal that the animation is over.
-      // The main thread will fire the animationend event.
-      return -1;
+    if (aAnimation) {
+      // Dispatch 'animationend' when needed.
+      if (aIsForElement &&
+          aAnimation->mLastNotification !=
+            ElementAnimation::LAST_NOTIFICATION_END) {
+        aAnimation->mLastNotification = ElementAnimation::LAST_NOTIFICATION_END;
+        // XXXdz: if this animation was done on the compositor, we should
+        // invalidate the frame and update style once we start throttling style
+        // updates.
+        AnimationEventInfo ei(aEa->mElement, aAnimation->mName, NS_ANIMATION_END,
+                              currentTimeDuration);
+        aEventsToDispatch->AppendElement(ei);
+      }
+
+      if (!aAnimation->FillsForwards()) {
+        // No animation data.
+        return -1;
+      }
+    } else {
+      // If aAnimation is null, that means we're on the compositor
+      // thread.  We want to just keep filling forwards until the main
+      // thread gets around to updating the compositor thread (which
+      // might take a little while).  So just assume we fill fowards and
+      // move on.
     }
-    // Dispatch 'animationend' when needed.
-    if (aIsForElement &&
-        aAnimation->mLastNotification !=
-          ElementAnimation::LAST_NOTIFICATION_END) {
-      aAnimation->mLastNotification = ElementAnimation::LAST_NOTIFICATION_END;
-      // XXXdz: if this animation was done on the compositor, we should
-      // invalidate the frame and update style once we start throttling style
-      // updates.
-      AnimationEventInfo ei(aEa->mElement, aAnimation->mName, NS_ANIMATION_END,
-                            currentTimeDuration);
-      aEventsToDispatch->AppendElement(ei);
-    }
-
-    if (!aAnimation->FillsForwards()) {
-      // No animation data.
-      return -1;
-    }
-    currentIterationCount = double(aAnimation->mIterationCount);
+    currentIterationCount = aIterationCount;
   } else {
     if (aAnimation && !aAnimation->IsPaused()) {
       aEa->mNeedsRefreshes = true;
     }
     if (currentIterationCount < 0.0) {
       NS_ASSERTION(aAnimation, "Should not run animation that hasn't started yet on the compositor");
       if (!aAnimation->FillsBackwards()) {
         // No animation data.
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -123,17 +123,19 @@ struct ElementAnimations : public mozill
   // animation and returns the position in the current iteration.  Note that
   // this only works when we know that the animation is currently running.
   // This way of calling the function can be used from the compositor.  Note
   // that if the animation has not started yet, has already ended, or is paused,
   // it should not be run from the compositor.  When this function is called 
   // from the main thread, we need the actual ElementAnimation* in order to 
   // get correct animation-fill behavior and to fire animation events.
   // This function returns -1 for the position if the animation should not be
-  // run (because it is not currently active and has no fill behavior.)
+  // run (because it is not currently active and has no fill behavior), but
+  // only does so if aAnimation is non-null; with a null aAnimation it is an
+  // error to give aCurrentTime < aStartTime, and fill-forwards is assumed.
   static double GetPositionInIteration(TimeStamp aStartTime,
                                        TimeStamp aCurrentTime,
                                        TimeDuration aDuration,
                                        double aIterationCount,
                                        uint32_t aDirection,
                                        bool IsForElement = true,
                                        ElementAnimation* aAnimation = nullptr,
                                        ElementAnimations* aEa = nullptr,