Bug 856486 (Part 2) - Buffer the last fully-decoded frame for multipart images. r=jrmuizel
authorSeth Fowler <seth@mozilla.com>
Wed, 03 Apr 2013 19:19:38 -0700
changeset 133405 4293821af365a44a59540257e8c3b706d25ff2c1
parent 133404 35461c20c883e9ebd50922348f8f5f8ce788aae5
child 133406 195d5b99252c964bf0af61854888d70452f474e8
push idunknown
push userunknown
push dateunknown
reviewersjrmuizel
bugs856486
milestone22.0a2
Bug 856486 (Part 2) - Buffer the last fully-decoded frame for multipart images. r=jrmuizel
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -380,16 +380,17 @@ NS_IMPL_THREADSAFE_ISUPPORTS3(RasterImag
 #endif
 
 //******************************************************************************
 RasterImage::RasterImage(imgStatusTracker* aStatusTracker,
                          nsIURI* aURI /* = nullptr */) :
   ImageResource(aStatusTracker, aURI), // invoke superclass's constructor
   mSize(0,0),
   mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
+  mMultipartDecodedFrame(nullptr),
   mAnim(nullptr),
   mLoopCount(-1),
   mLockCount(0),
   mDecodeCount(0),
 #ifdef DEBUG
   mFramesNotified(0),
 #endif
   mDecodingMutex("RasterImage"),
@@ -453,16 +454,18 @@ RasterImage::~RasterImage()
     }
   }
 
   delete mAnim;
 
   for (unsigned int i = 0; i < mFrames.Length(); ++i)
     delete mFrames[i];
 
+  delete mMultipartDecodedFrame;
+
   // Total statistics
   num_containers--;
   total_source_bytes -= mSourceData.Length();
 
   if (NS_IsMainThread()) {
     DiscardTracker::Remove(&mDiscardTrackerNode);
   }
 }
@@ -1582,16 +1585,35 @@ RasterImage::DecodingComplete()
   //
   // We don't optimize the frame for multipart images because we reuse
   // the frame.
   if ((mFrames.Length() == 1) && !mMultipart) {
     rv = mFrames[0]->Optimize();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
+  // Double-buffer our frame in the multipart case, since we'll start decoding
+  // into mFrames again immediately and this produces severe tearing.
+  if (mMultipart) {
+    if (mFrames.Length() == 1) {
+      imgFrame* swapFrame = mMultipartDecodedFrame;
+      mMultipartDecodedFrame = GetImgFrameNoDecode(GetCurrentFrameIndex());
+      mFrames.Clear();
+      if (swapFrame) {
+        mFrames.AppendElement(swapFrame);
+      }
+    } else {
+      // Don't double buffer for animated multipart images. It entails more
+      // complexity and it's not really needed since we already are smart about
+      // not displaying the still-decoding frame of an animated image. We may
+      // have already stored an extra frame, though, so we'll release it here.
+      delete mMultipartDecodedFrame;
+    }
+  }
+
   return NS_OK;
 }
 
 //******************************************************************************
 /* void StartAnimation () */
 nsresult
 RasterImage::StartAnimation()
 {
@@ -2484,16 +2506,20 @@ RasterImage::Discard(bool force)
   for (int i = 0; i < old_frame_count; ++i)
     delete mFrames[i];
   mFrames.Clear();
 
   // Clear our downscaled frame.
   mScaleResult.status = SCALE_INVALID;
   mScaleResult.frame = nullptr;
 
+  // Clear the last decoded multipart frame.
+  delete mMultipartDecodedFrame;
+  mMultipartDecodedFrame = nullptr;
+
   // Flag that we no longer have decoded frames for this image
   mDecoded = false;
 
   // Notify that we discarded
   if (mStatusTracker)
     mStatusTracker->OnDiscard();
 
   mDecodeRequest = nullptr;
@@ -3204,19 +3230,29 @@ RasterImage::Draw(gfxContext *aContext,
   }
 
   // If a synchronous draw is requested, flush anything that might be sitting around
   if (aFlags & FLAG_SYNC_DECODE) {
     nsresult rv = SyncDecode();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  uint32_t frameIndex = aWhichFrame == FRAME_FIRST ? 0
-                                                   : GetCurrentImgFrameIndex();
-  imgFrame *frame = GetDrawableImgFrame(frameIndex);
+  imgFrame* frame = nullptr;
+
+  if (mMultipart) {
+    // In the multipart case we prefer to use mMultipartDecodedFrame, which is
+    // the most recent one we completely decoded, rather than display the real
+    // current frame and risk severe tearing.
+    frame = mMultipartDecodedFrame;
+  }
+  if (!frame) {
+    uint32_t frameIndex = aWhichFrame == FRAME_FIRST ? 0
+                                                     : GetCurrentImgFrameIndex();
+    frame = GetDrawableImgFrame(frameIndex);
+  }
   if (!frame) {
     return NS_OK; // Getting the frame (above) touches the image and kicks off decoding
   }
 
   DrawWithPreDownscaleIfNeeded(frame, aContext, aFilter, aUserSpaceToImageSpace, aFill, aSubimage);
 
   if (mDecoded && !mDrawStartTime.IsNull()) {
       TimeDuration drawLatency = TimeStamp::Now() - mDrawStartTime;
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -726,17 +726,20 @@ private: // data
   // Valid flag bits are imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA
   // and imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION.
   uint32_t                   mFrameDecodeFlags;
 
   //! All the frames of the image
   // IMPORTANT: if you use mFrames in a method, call EnsureImageIsDecoded() first
   // to ensure that the frames actually exist (they may have been discarded to save
   // memory, or we may be decoding on draw).
-  nsTArray<imgFrame *>       mFrames;
+  nsTArray<imgFrame*>        mFrames;
+
+  // The last frame we decoded for multipart images.
+  imgFrame*                  mMultipartDecodedFrame;
 
   nsCOMPtr<nsIProperties>    mProperties;
 
   // IMPORTANT: if you use mAnim in a method, call EnsureImageIsDecoded() first to ensure
   // that the frames actually exist (they may have been discarded to save memory, or
   // we maybe decoding on draw).
   RasterImage::Anim*        mAnim;