Bug 890743 - Display 0-delay, single loop GIFs instantly. r=seth
authorAli Akhtarzada <ali@comoyo.com>
Fri, 06 Dec 2013 10:45:24 +0100
changeset 159628 263980931d1be65ecf6fe06a48bf3412c179327a
parent 159627 7c17d67dac5b6124dc2d14fae03923e35bbf1c44
child 159629 76ddaf7db5bf467c4ae6772342a1315193e7a482
push id25808
push usercbook@mozilla.com
push dateTue, 10 Dec 2013 12:03:31 +0000
treeherdermozilla-central@7fb91a422c5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs890743
milestone29.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 890743 - Display 0-delay, single loop GIFs instantly. r=seth Do not artificially create a frame delay for GIF files that do not loop and have an inter-frame delay of 0. Move timeout calculation to FrameBlender and only access raw timeout of imgFrame from within FrameBlender.
image/src/Decoder.cpp
image/src/FrameAnimator.cpp
image/src/FrameAnimator.h
image/src/FrameBlender.cpp
image/src/FrameBlender.h
image/src/RasterImage.cpp
image/src/imgFrame.cpp
image/src/imgFrame.h
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -344,17 +344,17 @@ Decoder::PostFrameStop(FrameBlender::Fra
   // Update our state
   mInFrame = false;
 
   if (aFrameAlpha == FrameBlender::kFrameOpaque) {
     mCurrentFrame->SetHasNoAlpha();
   }
 
   mCurrentFrame->SetFrameDisposalMethod(aDisposalMethod);
-  mCurrentFrame->SetTimeout(aTimeout);
+  mCurrentFrame->SetRawTimeout(aTimeout);
   mCurrentFrame->SetBlendMethod(aBlendMethod);
   mCurrentFrame->ImageUpdated(mCurrentFrame->GetRect());
 
   // Flush any invalidations before we finish the frame
   FlushInvalidations();
 
   // Fire notifications
   if (mObserver) {
--- a/image/src/FrameAnimator.cpp
+++ b/image/src/FrameAnimator.cpp
@@ -8,58 +8,57 @@
 
 #include "imgIContainer.h"
 
 using namespace mozilla::image;
 using namespace mozilla;
 
 FrameAnimator::FrameAnimator(FrameBlender& aFrameBlender)
   : mCurrentAnimationFrameIndex(0)
-  , mLoopCount(-1)
+  , mLoopCounter(-1)
   , mFrameBlender(aFrameBlender)
   , mAnimationMode(imgIContainer::kNormalAnimMode)
   , mDoneDecoding(false)
 {
 }
 
-uint32_t
+int32_t
 FrameAnimator::GetSingleLoopTime() const
 {
   // If we aren't done decoding, we don't know the image's full play time.
   if (!mDoneDecoding) {
-    return 0;
+    return -1;
   }
 
   // If we're not looping, a single loop time has no meaning
   if (mAnimationMode != imgIContainer::kNormalAnimMode) {
-    return 0;
+    return -1;
   }
 
   uint32_t looptime = 0;
   for (uint32_t i = 0; i < mFrameBlender.GetNumFrames(); ++i) {
-    int32_t timeout = mFrameBlender.RawGetFrame(i)->GetTimeout();
-    if (timeout > 0) {
+    int32_t timeout = mFrameBlender.GetTimeoutForFrame(i);
+    if (timeout >= 0) {
       looptime += static_cast<uint32_t>(timeout);
     } else {
       // If we have a frame that never times out, we're probably in an error
       // case, but let's handle it more gracefully.
       NS_WARNING("Negative frame timeout - how did this happen?");
-      return 0;
+      return -1;
     }
   }
 
   return looptime;
 }
 
 TimeStamp
 FrameAnimator::GetCurrentImgFrameEndTime() const
 {
-  imgFrame* currentFrame = mFrameBlender.RawGetFrame(mCurrentAnimationFrameIndex);
   TimeStamp currentFrameTime = mCurrentAnimationFrameTime;
-  int64_t timeout = currentFrame->GetTimeout();
+  int32_t timeout = mFrameBlender.GetTimeoutForFrame(mCurrentAnimationFrameIndex);
 
   if (timeout < 0) {
     // We need to return a sentinel value in this case, because our logic
     // doesn't work correctly if we have a negative timeout value. We use
     // one year in the future as the sentinel because it works with the loop
     // in RequestRefresh() below.
     // XXX(seth): It'd be preferable to make our logic work correctly with
     // negative timeouts.
@@ -77,17 +76,17 @@ FrameAnimator::GetCurrentImgFrameEndTime
 FrameAnimator::RefreshResult
 FrameAnimator::AdvanceFrame(TimeStamp aTime)
 {
   NS_ASSERTION(aTime <= TimeStamp::Now(),
                "Given time appears to be in the future");
 
   uint32_t currentFrameIndex = mCurrentAnimationFrameIndex;
   uint32_t nextFrameIndex = currentFrameIndex + 1;
-  uint32_t timeout = 0;
+  int32_t timeout = 0;
 
   RefreshResult ret;
 
   // If we're done decoding, we know we've got everything we're going to get.
   // If we aren't, we only display fully-downloaded frames; everything else
   // gets delayed.
   bool needToWait = !mDoneDecoding &&
                     mFrameBlender.RawGetFrame(nextFrameIndex) &&
@@ -96,40 +95,45 @@ FrameAnimator::AdvanceFrame(TimeStamp aT
   if (needToWait) {
     // 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;
   } else {
     // If we're done decoding the next frame, go ahead and display it now and
     // reinit with the next frame's delay time.
     if (mFrameBlender.GetNumFrames() == nextFrameIndex) {
-      // End of Animation, unless we are looping forever
+      // End of an animation loop...
 
-      // If animation mode is "loop once", it's time to stop animating
-      if (mAnimationMode == imgIContainer::kLoopOnceAnimMode || mLoopCount == 0) {
+      // If we are not looping forever, initialize the loop counter
+      if (mLoopCounter < 0 && mFrameBlender.GetLoopCount() >= 0) {
+        mLoopCounter = mFrameBlender.GetLoopCount();
+      }
+
+      // If animation mode is "loop once", or we're at end of loop counter, it's time to stop animating
+      if (mAnimationMode == imgIContainer::kLoopOnceAnimMode || mLoopCounter == 0) {
         ret.animationFinished = true;
       }
 
       nextFrameIndex = 0;
 
-      if (mLoopCount > 0) {
-        mLoopCount--;
+      if (mLoopCounter > 0) {
+        mLoopCounter--;
       }
 
       // If we're done, exit early.
       if (ret.animationFinished) {
         return ret;
       }
     }
 
-    timeout = mFrameBlender.GetFrame(nextFrameIndex)->GetTimeout();
+    timeout = mFrameBlender.GetTimeoutForFrame(nextFrameIndex);
   }
 
   // Bad data
-  if (!(timeout > 0)) {
+  if (timeout < 0) {
     ret.animationFinished = true;
     ret.error = true;
   }
 
   if (nextFrameIndex == 0) {
     ret.dirtyRect = mFirstFrameRefreshArea;
   } else {
     // Change frame
@@ -241,22 +245,16 @@ FrameAnimator::SetFirstFrameRefreshArea(
 }
 
 void
 FrameAnimator::UnionFirstFrameRefreshArea(const nsIntRect& aRect)
 {
   mFirstFrameRefreshArea.UnionRect(mFirstFrameRefreshArea, aRect);
 }
 
-void
-FrameAnimator::SetLoopCount(int loopcount)
-{
-  mLoopCount = loopcount;
-}
-
 uint32_t
 FrameAnimator::GetCurrentAnimationFrameIndex() const
 {
   return mCurrentAnimationFrameIndex;
 }
 
 nsIntRect
 FrameAnimator::GetFirstFrameRefreshArea() const
--- a/image/src/FrameAnimator.h
+++ b/image/src/FrameAnimator.h
@@ -71,22 +71,16 @@ public:
 
   /**
    * Call when you need to re-start animating. Ensures we start from the first
    * frame.
    */
   void ResetAnimation();
 
   /**
-   * Number of times to loop the image.
-   * @note -1 means forever.
-   */
-  void SetLoopCount(int32_t aLoopCount);
-
-  /**
    * The animation mode of the image.
    *
    * Constants defined in imgIContainer.idl.
    */
   void SetAnimationMode(uint16_t aAnimationMode);
 
   /**
    * Set the area to refresh when we loop around to the first frame.
@@ -120,19 +114,20 @@ public:
    */
   nsIntRect GetFirstFrameRefreshArea() const;
 
 private: // methods
   /**
    * Gets the length of a single loop of this image, in milliseconds.
    *
    * If this image is not finished decoding, is not animated, or it is animated
-   * but does not loop, returns 0.
+   * but does not loop, returns -1. Can return 0 in the case of an animated image
+   * that has a 0ms delay between its frames and does not loop.
    */
-  uint32_t GetSingleLoopTime() const;
+  int32_t GetSingleLoopTime() const;
 
   /**
    * Advances the animation. Typically, this will advance a single frame, but it
    * may advance multiple frames. This may happen if we have infrequently
    * "ticking" refresh drivers (e.g. in background tabs), or extremely short-
    * lived animation frames.
    *
    * @param aTime the time that the animation should advance to. This will
@@ -156,17 +151,17 @@ private: // data
 
   //! the time that the animation advanced to the current frame
   TimeStamp mCurrentAnimationFrameTime;
 
   //! The current frame index we're on. 0 to (numFrames - 1).
   uint32_t mCurrentAnimationFrameIndex;
 
   //! number of loops remaining before animation stops (-1 no stop)
-  int32_t mLoopCount;
+  int32_t mLoopCounter;
 
   //! All the frames of the image, shared with our owner
   FrameBlender& mFrameBlender;
 
   //! The animation mode of this image. Constants defined in imgIContainer.
   uint16_t mAnimationMode;
 
   //! Whether this image is done being decoded.
--- a/image/src/FrameBlender.cpp
+++ b/image/src/FrameBlender.cpp
@@ -14,16 +14,17 @@ using namespace mozilla;
 using namespace mozilla::image;
 
 namespace mozilla {
 namespace image {
 
 FrameBlender::FrameBlender(FrameSequence* aSequenceToUse /* = nullptr */)
  : mFrames(aSequenceToUse)
  , mAnim(nullptr)
+ , mLoopCount(-1)
 {
   if (!mFrames) {
     mFrames = new FrameSequence();
   }
 }
 
 FrameBlender::~FrameBlender()
 {
@@ -61,16 +62,50 @@ FrameBlender::RawGetFrame(uint32_t frame
 }
 
 uint32_t
 FrameBlender::GetNumFrames() const
 {
   return mFrames->GetNumFrames();
 }
 
+int32_t
+FrameBlender::GetTimeoutForFrame(uint32_t framenum) const
+{
+  const int32_t timeout = RawGetFrame(framenum)->GetRawTimeout();
+  // Ensure a minimal time between updates so we don't throttle the UI thread.
+  // consider 0 == unspecified and make it fast but not too fast.  Unless we have
+  // a single loop GIF. See bug 890743, bug 125137, bug 139677, and bug 207059.
+  // The behavior of recent IE and Opera versions seems to be:
+  // IE 6/Win:
+  //   10 - 50ms go 100ms
+  //   >50ms go correct speed
+  // Opera 7 final/Win:
+  //   10ms goes 100ms
+  //   >10ms go correct speed
+  // It seems that there are broken tools out there that set a 0ms or 10ms
+  // timeout when they really want a "default" one.  So munge values in that
+  // range.
+  if (timeout >= 0 && timeout <= 10 && mLoopCount != 0)
+    return 100;
+  return timeout;
+}
+
+void
+FrameBlender::SetLoopCount(int32_t aLoopCount)
+{
+  mLoopCount = aLoopCount;
+}
+
+int32_t
+FrameBlender::GetLoopCount() const
+{
+  return mLoopCount;
+}
+
 void
 FrameBlender::RemoveFrame(uint32_t framenum)
 {
   NS_ABORT_IF_FALSE(framenum < GetNumFrames(), "Deleting invalid frame!");
 
   mFrames->RemoveFrame(framenum);
 }
 
@@ -376,18 +411,18 @@ FrameBlender::DoBlend(nsIntRect* aDirtyR
               nextFrame.GetFrame()->PaletteDataLength(),
               nextFrame.GetFrame()->GetHasAlpha(),
               mAnim->compositingFrame.GetFrameData(),
               mAnim->compositingFrame.GetFrame()->GetRect(),
               FrameBlendMethod(nextFrame.GetFrame()->GetBlendMethod()));
 
   // Set timeout of CompositeFrame to timeout of frame we just composed
   // Bug 177948
-  int32_t timeout = nextFrame->GetTimeout();
-  mAnim->compositingFrame->SetTimeout(timeout);
+  int32_t timeout = nextFrame->GetRawTimeout();
+  mAnim->compositingFrame->SetRawTimeout(timeout);
 
   // Tell the image that it is fully 'downloaded'.
   nsresult rv = mAnim->compositingFrame->ImageUpdated(mAnim->compositingFrame->GetRect());
   if (NS_FAILED(rv)) {
     return false;
   }
 
   mAnim->lastCompositedFrameIndex = int32_t(aNextFrameIndex);
--- a/image/src/FrameBlender.h
+++ b/image/src/FrameBlender.h
@@ -54,16 +54,30 @@ public:
   void InsertFrame(uint32_t framenum, imgFrame* aFrame);
   void RemoveFrame(uint32_t framenum);
   imgFrame* SwapFrame(uint32_t framenum, imgFrame* aFrame);
   void ClearFrames();
 
   /* The total number of frames in this image. */
   uint32_t GetNumFrames() const;
 
+  /*
+   * Returns the frame's adjusted timeout. If the animation loops and the timeout
+   * falls in between a certain range then the timeout is adjusted so that
+   * it's never 0. If the animation does not loop then no adjustments are made.
+   */
+  int32_t GetTimeoutForFrame(uint32_t framenum) const;
+
+  /*
+   * Set number of times to loop the image.
+   * @note -1 means loop forever.
+   */
+  void SetLoopCount(int32_t aLoopCount);
+  int32_t GetLoopCount() const;
+
   void Discard();
 
   void SetSize(nsIntSize aSize) { mSize = aSize; }
 
   size_t SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation,
                                                  mozilla::MallocSizeOf aMallocSizeOf) const;
 
   void ResetAnimation();
@@ -164,14 +178,15 @@ private:
                               uint8_t *aDstPixels, const nsIntRect& aDstRect,
                               FrameBlendMethod aBlendMethod);
 
 private: // data
   //! All the frames of the image
   nsRefPtr<FrameSequence> mFrames;
   nsIntSize mSize;
   Anim* mAnim;
+  int32_t mLoopCount;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif /* mozilla_imagelib_FrameBlender_h_ */
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -809,17 +809,17 @@ RasterImage::GetFirstFrameDelay()
 {
   if (mError)
     return -1;
 
   bool animated = false;
   if (NS_FAILED(GetAnimated(&animated)) || !animated)
     return -1;
 
-  return mFrameBlender.GetFrame(0)->GetTimeout();
+  return mFrameBlender.GetTimeoutForFrame(0);
 }
 
 nsresult
 RasterImage::CopyFrame(uint32_t aWhichFrame,
                        uint32_t aFlags,
                        gfxImageSurface **_retval)
 {
   if (aWhichFrame > FRAME_MAX_VALUE)
@@ -1434,17 +1434,17 @@ RasterImage::StartAnimation()
     return NS_ERROR_FAILURE;
 
   NS_ABORT_IF_FALSE(ShouldAnimate(), "Should not animate!");
 
   EnsureAnimExists();
 
   imgFrame* currentFrame = GetCurrentImgFrame();
   // A timeout of -1 means we should display this frame forever.
-  if (currentFrame && currentFrame->GetTimeout() < 0) {
+  if (currentFrame && mFrameBlender.GetTimeoutForFrame(GetCurrentImgFrameIndex()) < 0) {
     mAnimationFinished = true;
     return NS_ERROR_ABORT;
   }
 
   if (mAnim) {
     // We need to set the time that this initial frame was first displayed, as
     // this is used in AdvanceFrame().
     mAnim->InitAnimationFrameTimeIfNecessary();
@@ -1531,17 +1531,18 @@ RasterImage::GetFrameIndex(uint32_t aWhi
 
 void
 RasterImage::SetLoopCount(int32_t aLoopCount)
 {
   if (mError)
     return;
 
   if (mAnim) {
-    mAnim->SetLoopCount(aLoopCount);
+    // No need to set this if we're not an animation
+    mFrameBlender.SetLoopCount(aLoopCount);
   }
 }
 
 nsresult
 RasterImage::AddSourceData(const char *aBuffer, uint32_t aCount)
 {
   MutexAutoLock lock(mDecodingMutex);
 
--- a/image/src/imgFrame.cpp
+++ b/image/src/imgFrame.cpp
@@ -740,38 +740,22 @@ void imgFrame::ApplyDirtToSurfaces()
     if (mQuartzSurface)
       mQuartzSurface->Flush();
 #endif
 
     mDirty = false;
   }
 }
 
-int32_t imgFrame::GetTimeout() const
+int32_t imgFrame::GetRawTimeout() const
 {
-  // Ensure a minimal time between updates so we don't throttle the UI thread.
-  // consider 0 == unspecified and make it fast but not too fast.  See bug
-  // 125137, bug 139677, and bug 207059.  The behavior of recent IE and Opera
-  // versions seems to be:
-  // IE 6/Win:
-  //   10 - 50ms go 100ms
-  //   >50ms go correct speed
-  // Opera 7 final/Win:
-  //   10ms goes 100ms
-  //   >10ms go correct speed
-  // It seems that there are broken tools out there that set a 0ms or 10ms
-  // timeout when they really want a "default" one.  So munge values in that
-  // range.
-  if (mTimeout >= 0 && mTimeout <= 10)
-    return 100;
-  else
-    return mTimeout;
+  return mTimeout;
 }
 
-void imgFrame::SetTimeout(int32_t aTimeout)
+void imgFrame::SetRawTimeout(int32_t aTimeout)
 {
   mTimeout = aTimeout;
 }
 
 int32_t imgFrame::GetFrameDisposalMethod() const
 {
   return mDisposalMethod;
 }
--- a/image/src/imgFrame.h
+++ b/image/src/imgFrame.h
@@ -48,18 +48,18 @@ public:
   uint32_t GetImageDataLength() const;
   bool GetIsPaletted() const;
   bool GetHasAlpha() const;
   void GetImageData(uint8_t **aData, uint32_t *length) const;
   uint8_t* GetImageData() const;
   void GetPaletteData(uint32_t **aPalette, uint32_t *length) const;
   uint32_t* GetPaletteData() const;
 
-  int32_t GetTimeout() const;
-  void SetTimeout(int32_t aTimeout);
+  int32_t GetRawTimeout() const;
+  void SetRawTimeout(int32_t aTimeout);
 
   int32_t GetFrameDisposalMethod() const;
   void SetFrameDisposalMethod(int32_t aFrameDisposalMethod);
   int32_t GetBlendMethod() const;
   void SetBlendMethod(int32_t aBlendMethod);
   bool ImageComplete() const;
 
   void SetHasNoAlpha();