Bug 1288040 (Part 11) - Clean up RefreshResult. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 19 Jul 2016 18:20:13 -0700
changeset 331029 4dee7c0653260c0913348e7dd4ac2c284b553fc5
parent 331028 7a652ffa8bfb8fc9a9d71f78573f7a639a129b00
child 331030 6bb7f6c316bb8c0c5a898439250288a7bd306827
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1288040
milestone50.0a1
Bug 1288040 (Part 11) - Clean up RefreshResult. r=edwin
image/FrameAnimator.cpp
image/FrameAnimator.h
image/RasterImage.cpp
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -110,17 +110,17 @@ FrameAnimator::GetCurrentImgFrameEndTime
 
   TimeDuration durationOfTimeout =
     TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds()));
   TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
 
   return currentFrameEndTime;
 }
 
-FrameAnimator::RefreshResult
+RefreshResult
 FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
 {
   NS_ASSERTION(aTime <= TimeStamp::Now(),
                "Given time appears to be in the future");
   PROFILER_LABEL_FUNC(js::ProfileEntry::Category::GRAPHICS);
 
   RefreshResult ret;
 
@@ -152,27 +152,27 @@ FrameAnimator::AdvanceFrame(AnimationSta
     if (aState.mLoopRemainingCount < 0 && aState.LoopCount() >= 0) {
       aState.mLoopRemainingCount = aState.LoopCount();
     }
 
     // If animation mode is "loop once", or we're at end of loop counter,
     // it's time to stop animating.
     if (aState.mAnimationMode == imgIContainer::kLoopOnceAnimMode ||
         aState.mLoopRemainingCount == 0) {
-      ret.animationFinished = true;
+      ret.mAnimationFinished = true;
     }
 
     nextFrameIndex = 0;
 
     if (aState.mLoopRemainingCount > 0) {
       aState.mLoopRemainingCount--;
     }
 
     // If we're done, exit early.
-    if (ret.animationFinished) {
+    if (ret.mAnimationFinished) {
       return ret;
     }
   }
 
   // There can be frames in the surface cache with index >= mImage->GetNumFrames()
   // that GetRawFrame can access because the decoding thread has decoded them, but
   // RasterImage hasn't acknowledged those frames yet. We don't want to go past
   // what RasterImage knows about so that we stay in sync with RasterImage. The code
@@ -188,26 +188,26 @@ FrameAnimator::AdvanceFrame(AnimationSta
 
   if (!canDisplay) {
     // Uh oh, the frame we want to show is currently being decoded (partial)
     // Wait until the next refresh driver tick and try again
     return ret;
   }
 
   if (GetTimeoutForFrame(nextFrameIndex) == FrameTimeout::Forever()) {
-    ret.animationFinished = true;
+    ret.mAnimationFinished = true;
   }
 
   if (nextFrameIndex == 0) {
-    ret.dirtyRect = aState.FirstFrameRefreshArea();
+    ret.mDirtyRect = aState.FirstFrameRefreshArea();
   } else {
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
 
     // Change frame
-    if (!DoBlend(&ret.dirtyRect, currentFrameIndex, nextFrameIndex)) {
+    if (!DoBlend(&ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) {
       // something went wrong, move on to next
       NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed");
       nextFrame->SetCompositingFailed(true);
       aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState);
       aState.mCurrentAnimationFrameIndex = nextFrameIndex;
 
       return ret;
     }
@@ -233,22 +233,22 @@ FrameAnimator::AdvanceFrame(AnimationSta
         TimeDuration::FromMilliseconds(loops * loopTime.AsMilliseconds());
     }
   }
 
   // Set currentAnimationFrameIndex at the last possible moment
   aState.mCurrentAnimationFrameIndex = nextFrameIndex;
 
   // If we're here, we successfully advanced the frame.
-  ret.frameAdvanced = true;
+  ret.mFrameAdvanced = true;
 
   return ret;
 }
 
-FrameAnimator::RefreshResult
+RefreshResult
 FrameAnimator::RequestRefresh(AnimationState& aState, const TimeStamp& aTime)
 {
   // only advance the frame if the current time is greater than or
   // equal to the current frame's end time.
   TimeStamp currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
 
   // By default, an empty RefreshResult.
   RefreshResult ret;
@@ -258,20 +258,20 @@ FrameAnimator::RequestRefresh(AnimationS
 
     RefreshResult frameRes = AdvanceFrame(aState, aTime);
 
     // Accumulate our result for returning to callers.
     ret.Accumulate(frameRes);
 
     currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
 
-    // if we didn't advance a frame, and our frame end time didn't change,
+    // If we didn't advance a frame, and our frame end time didn't change,
     // then we need to break out of this loop & wait for the frame(s)
-    // to finish downloading
-    if (!frameRes.frameAdvanced && (currentFrameEndTime == oldFrameEndTime)) {
+    // to finish downloading.
+    if (!frameRes.mFrameAdvanced && (currentFrameEndTime == oldFrameEndTime)) {
       break;
     }
   }
 
   return ret;
 }
 
 LookupResult
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -126,16 +126,46 @@ private:
 
   //! The animation mode of this image. Constants defined in imgIContainer.
   uint16_t mAnimationMode;
 
   //! Whether this image is done being decoded.
   bool mDoneDecoding;
 };
 
+/**
+ * RefreshResult is used to let callers know how the state of the animation
+ * changed during a call to FrameAnimator::RequestRefresh().
+ */
+struct RefreshResult
+{
+  RefreshResult()
+    : mFrameAdvanced(false)
+    , mAnimationFinished(false)
+  { }
+
+  /// Merges another RefreshResult's changes into this RefreshResult.
+  void Accumulate(const RefreshResult& aOther)
+  {
+    mFrameAdvanced = mFrameAdvanced || aOther.mFrameAdvanced;
+    mAnimationFinished = mAnimationFinished || aOther.mAnimationFinished;
+    mDirtyRect = mDirtyRect.Union(aOther.mDirtyRect);
+  }
+
+  // The region of the image that has changed.
+  gfx::IntRect mDirtyRect;
+
+  // If true, we changed frames at least once. Note that, due to looping, we
+  // could still have ended up on the same frame!
+  bool mFrameAdvanced : 1;
+
+  // Whether the animation has finished playing.
+  bool mAnimationFinished : 1;
+};
+
 class FrameAnimator
 {
 public:
   FrameAnimator(RasterImage* aImage, gfx::IntSize aSize)
     : mImage(aImage)
     , mSize(aSize)
     , mLastCompositedFrameIndex(-1)
   {
@@ -143,44 +173,16 @@ public:
   }
 
   ~FrameAnimator()
   {
     MOZ_COUNT_DTOR(FrameAnimator);
   }
 
   /**
-   * Return value from RequestRefresh. Tells callers what happened in that call
-   * to RequestRefresh.
-   */
-  struct RefreshResult
-  {
-    // The dirty rectangle to be re-drawn after this RequestRefresh().
-    nsIntRect dirtyRect;
-
-    // Whether any frame changed, and hence the dirty rect was set.
-    bool frameAdvanced : 1;
-
-    // Whether the animation has finished playing.
-    bool animationFinished : 1;
-
-    RefreshResult()
-      : frameAdvanced(false)
-      , animationFinished(false)
-    { }
-
-    void Accumulate(const RefreshResult& other)
-    {
-      frameAdvanced = frameAdvanced || other.frameAdvanced;
-      animationFinished = animationFinished || other.animationFinished;
-      dirtyRect = dirtyRect.Union(other.dirtyRect);
-    }
-  };
-
-  /**
    * Re-evaluate what frame we're supposed to be on, and do whatever blending
    * is necessary to get us to that frame.
    *
    * Returns the result of that blending, including whether the current frame
    * changed and what the resulting dirty rectangle is.
    */
   RefreshResult RequestRefresh(AnimationState& aState, const TimeStamp& aTime);
 
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -171,34 +171,34 @@ RasterImage::RequestRefresh(const TimeSt
   }
 
   EvaluateAnimation();
 
   if (!mAnimating) {
     return;
   }
 
-  FrameAnimator::RefreshResult res;
+  RefreshResult res;
   if (mAnimationState) {
     MOZ_ASSERT(mFrameAnimator);
     res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime);
   }
 
-  if (res.frameAdvanced) {
+  if (res.mFrameAdvanced) {
     // Notify listeners that our frame has actually changed, but do this only
     // once for all frames that we've now passed (if AdvanceFrame() was called
     // more than once).
     #ifdef DEBUG
       mFramesNotified++;
     #endif
 
-    NotifyProgress(NoProgress, res.dirtyRect);
+    NotifyProgress(NoProgress, res.mDirtyRect);
   }
 
-  if (res.animationFinished) {
+  if (res.mAnimationFinished) {
     mAnimationFinished = true;
     EvaluateAnimation();
   }
 }
 
 //******************************************************************************
 NS_IMETHODIMP
 RasterImage::GetWidth(int32_t* aWidth)