Bug 1257101. imgFrame::IsImageComplete says whether we've had pixels decoded to the whole image rect, but it's used to check if the frame is finished decoding. These are different things when the image has more than one progress pass. r=seth
authorTimothy Nikkel <tnikkel@gmail.com>
Wed, 23 Mar 2016 19:31:42 -0500
changeset 290186 c75b2b195f28a5ed556a7dfc12e9b45bc56c971a
parent 290185 3e142ba20ecad6ee3aab402740a7f8f903089a5b
child 290187 531508a2e75575e007d3eb578c77fd3ecf5fee1e
push id18353
push usercbook@mozilla.com
push dateThu, 24 Mar 2016 15:20:25 +0000
treeherderfx-team@40ae8489939e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1257101
milestone48.0a1
Bug 1257101. imgFrame::IsImageComplete says whether we've had pixels decoded to the whole image rect, but it's used to check if the frame is finished decoding. These are different things when the image has more than one progress pass. r=seth This means that in RasterImage::LookupFrame when we are asked to do a sync decode (if needed) we use WaitUntilComplete to wait until the frame is finished decoding. But we would actually return after the next progressive pass notified the monitor to wake up. Thus, we would draw a not-fully-decoded image even though the sync decode flag was passed. The change in FrameAnimator means that we won't draw the next frame in an animated image until all progressive passes of that image are complete. This seems like what we want anyways. There is one real use of IsImageComplete left, in imgFrame::Draw, where we need to know if the decoded image data covers the whole image frame. (There are a couple of uses of IsImageComplete in asserts.)
image/FrameAnimator.cpp
image/RasterImage.cpp
image/SurfaceCache.cpp
image/decoders/nsICODecoder.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -138,17 +138,17 @@ FrameAnimator::AdvanceFrame(TimeStamp aT
   // above should obey this, the MOZ_ASSERT records this invariant.
   MOZ_ASSERT(nextFrameIndex < mImage->GetNumFrames());
   RawAccessFrameRef nextFrame = GetRawFrame(nextFrameIndex);
 
   // 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 canDisplay = mDoneDecoding ||
-                    (nextFrame && nextFrame->IsImageComplete());
+                    (nextFrame && nextFrame->IsFinished());
 
   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;
   }
 
   // Bad data
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -360,17 +360,17 @@ RasterImage::LookupFrame(uint32_t aFrame
 
   MOZ_ASSERT(!result.DrawableRef()->GetIsPaletted(),
              "Should not have a paletted frame");
 
   // Sync decoding guarantees that we got the frame, but if it's owned by an
   // async decoder that's currently running, the contents of the frame may not
   // be available yet. Make sure we get everything.
   if (mHasSourceData && (aFlags & FLAG_SYNC_DECODE)) {
-    result.DrawableRef()->WaitUntilComplete();
+    result.DrawableRef()->WaitUntilFinished();
   }
 
   return Move(result.DrawableRef());
 }
 
 uint32_t
 RasterImage::GetCurrentFrameIndex() const
 {
@@ -607,17 +607,17 @@ RasterImage::GetFrameInternal(const IntS
   // The image doesn't have a usable surface because it's been optimized away or
   // because it's a partial update frame from an animation. Create one. (In this
   // case we fall back to returning a surface at our intrinsic size, even if a
   // different size was originally specified.)
   if (!frameSurf) {
     frameSurf = CopyFrame(aWhichFrame, aFlags);
   }
 
-  if (!frameRef->IsImageComplete()) {
+  if (!frameRef->IsFinished()) {
     return MakePair(DrawResult::INCOMPLETE, Move(frameSurf));
   }
 
   return MakePair(DrawResult::SUCCESS, Move(frameSurf));
 }
 
 Pair<DrawResult, RefPtr<layers::Image>>
 RasterImage::GetCurrentImage(ImageContainer* aContainer, uint32_t aFlags)
@@ -1445,17 +1445,17 @@ RasterImage::DrawInternal(DrawableFrameR
                           gfxContext* aContext,
                           const IntSize& aSize,
                           const ImageRegion& aRegion,
                           Filter aFilter,
                           uint32_t aFlags)
 {
   gfxContextMatrixAutoSaveRestore saveMatrix(aContext);
   ImageRegion region(aRegion);
-  bool frameIsComplete = aFrameRef->IsImageComplete();
+  bool frameIsFinished = aFrameRef->IsFinished();
 
   // By now we may have a frame with the requested size. If not, we need to
   // adjust the drawing parameters accordingly.
   IntSize finalSize = aFrameRef->GetImageSize();
   bool couldRedecodeForBetterFrame = false;
   if (finalSize != aSize) {
     gfx::Size scale(double(aSize.width) / finalSize.width,
                     double(aSize.height) / finalSize.height);
@@ -1464,17 +1464,17 @@ RasterImage::DrawInternal(DrawableFrameR
 
     couldRedecodeForBetterFrame = CanDownscaleDuringDecode(aSize, aFlags);
   }
 
   if (!aFrameRef->Draw(aContext, region, aFilter, aFlags)) {
     RecoverFromInvalidFrames(aSize, aFlags);
     return DrawResult::TEMPORARY_ERROR;
   }
-  if (!frameIsComplete) {
+  if (!frameIsFinished) {
     return DrawResult::INCOMPLETE;
   }
   if (couldRedecodeForBetterFrame) {
     return DrawResult::WRONG_SIZE;
   }
   return DrawResult::SUCCESS;
 }
 
@@ -1523,17 +1523,17 @@ RasterImage::Draw(gfxContext* aContext,
     // Getting the frame (above) touches the image and kicks off decoding.
     if (mDrawStartTime.IsNull()) {
       mDrawStartTime = TimeStamp::Now();
     }
     return DrawResult::NOT_READY;
   }
 
   bool shouldRecordTelemetry = !mDrawStartTime.IsNull() &&
-                               ref->IsImageComplete();
+                               ref->IsFinished();
 
   auto result = DrawInternal(Move(ref), aContext, aSize,
                              aRegion, aFilter, flags);
 
   if (shouldRecordTelemetry) {
       TimeDuration drawLatency = TimeStamp::Now() - mDrawStartTime;
       Telemetry::Accumulate(Telemetry::IMAGE_DECODE_ON_DRAW_LATENCY,
                             int32_t(drawLatency.ToMicroseconds()));
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -176,17 +176,17 @@ public:
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
   CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
 
   bool IsDecoded() const
   {
-    return !IsPlaceholder() && mSurface->IsImageComplete();
+    return !IsPlaceholder() && mSurface->IsFinished();
   }
 
   // A helper type used by SurfaceCacheImpl::CollectSizeOfSurfaces.
   struct MOZ_STACK_CLASS SurfaceMemoryReport
   {
     SurfaceMemoryReport(nsTArray<SurfaceMemoryCounter>& aCounters,
                         MallocSizeOf                    aMallocSizeOf)
       : mCounters(aCounters)
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -94,17 +94,17 @@ nsICODecoder::GetFinalStateFromContained
   mDataError = mDataError || mContainedDecoder->HasDataError();
   mFailCode = NS_SUCCEEDED(mFailCode) ? mContainedDecoder->GetDecoderError()
                                       : mFailCode;
   mDecodeAborted = mContainedDecoder->WasAborted();
   mProgress |= mContainedDecoder->TakeProgress();
   mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
   mCurrentFrame = mContainedDecoder->GetCurrentFrameRef();
 
-  MOZ_ASSERT(HasError() || !mCurrentFrame || mCurrentFrame->IsImageComplete());
+  MOZ_ASSERT(HasError() || !mCurrentFrame || mCurrentFrame->IsFinished());
 }
 
 // A BMP inside of an ICO has *2 height because of the AND mask
 // that follows the actual bitmap.  The BMP shouldn't know about
 // this difference though.
 bool
 nsICODecoder::FixBitmapHeight(int8_t* bih)
 {
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -135,16 +135,17 @@ imgFrame::imgFrame()
   : mMonitor("imgFrame")
   , mDecoded(0, 0, 0, 0)
   , mLockCount(0)
   , mTimeout(100)
   , mDisposalMethod(DisposalMethod::NOT_SPECIFIED)
   , mBlendMethod(BlendMethod::OVER)
   , mHasNoAlpha(false)
   , mAborted(false)
+  , mFinished(false)
   , mOptimizable(false)
   , mPalettedImageData(nullptr)
   , mPaletteDepth(0)
   , mNonPremult(false)
   , mSinglePixel(false)
   , mCompositingFailed(false)
 {
   static bool hasCheckedOptimize = false;
@@ -155,17 +156,18 @@ imgFrame::imgFrame()
     hasCheckedOptimize = true;
   }
 }
 
 imgFrame::~imgFrame()
 {
 #ifdef DEBUG
   MonitorAutoLock lock(mMonitor);
-  MOZ_ASSERT(mAborted || IsImageCompleteInternal());
+  MOZ_ASSERT(mAborted || AreAllPixelsWritten());
+  MOZ_ASSERT(mAborted || mFinished);
 #endif
 
   free(mPalettedImageData);
   mPalettedImageData = nullptr;
 }
 
 nsresult
 imgFrame::InitForDecoder(const nsIntSize& aImageSize,
@@ -316,17 +318,22 @@ imgFrame::InitWithDrawable(gfxDrawable* 
   if (!canUseDataSurface) {
     // We used an offscreen surface, which is an "optimized" surface from
     // imgFrame's perspective.
     mOptSurface = target->Snapshot();
   }
 
   // If we reach this point, we should regard ourselves as complete.
   mDecoded = GetRect();
-  MOZ_ASSERT(IsImageComplete());
+  mFinished = true;
+
+#ifdef DEBUG
+  MonitorAutoLock lock(mMonitor);
+  MOZ_ASSERT(AreAllPixelsWritten());
+#endif
 
   return NS_OK;
 }
 
 nsresult
 imgFrame::Optimize()
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -560,17 +567,17 @@ bool imgFrame::Draw(gfxContext* aContext
   MonitorAutoLock lock(mMonitor);
 
   nsIntMargin padding(mOffset.y,
                       mImageSize.width - (mOffset.x + mSize.width),
                       mImageSize.height - (mOffset.y + mSize.height),
                       mOffset.x);
 
   bool doPadding = padding != nsIntMargin(0,0,0,0);
-  bool doPartialDecode = !IsImageCompleteInternal();
+  bool doPartialDecode = !AreAllPixelsWritten();
 
   if (mSinglePixel && !doPadding && !doPartialDecode) {
     if (mSinglePixelColor.a == 0.0) {
       return true;
     }
     RefPtr<DrawTarget> dt = aContext->GetDrawTarget();
     dt->FillRect(ToRect(aRegion.Rect()),
                  ColorPattern(mSinglePixelColor),
@@ -621,21 +628,16 @@ imgFrame::ImageUpdatedInternal(const nsI
 
   mDecoded.UnionRect(mDecoded, aUpdateRect);
 
   // clamp to bounds, in case someone sends a bogus updateRect (I'm looking at
   // you, gif decoder)
   nsIntRect boundsRect(mOffset, mSize);
   mDecoded.IntersectRect(mDecoded, boundsRect);
 
-  // If the image is now complete, wake up anyone who's waiting.
-  if (IsImageCompleteInternal()) {
-    mMonitor.NotifyAll();
-  }
-
   return NS_OK;
 }
 
 void
 imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
                  DisposalMethod aDisposalMethod /* = DisposalMethod::KEEP */,
                  int32_t aRawTimeout /* = 0 */,
                  BlendMethod aBlendMethod /* = BlendMethod::OVER */)
@@ -646,16 +648,20 @@ imgFrame::Finish(Opacity aFrameOpacity /
   if (aFrameOpacity == Opacity::FULLY_OPAQUE) {
     mHasNoAlpha = true;
   }
 
   mDisposalMethod = aDisposalMethod;
   mTimeout = aRawTimeout;
   mBlendMethod = aBlendMethod;
   ImageUpdatedInternal(GetRect());
+  mFinished = true;
+
+  // The image is now complete, wake up anyone who's waiting.
+  mMonitor.NotifyAll();
 }
 
 nsIntRect
 imgFrame::GetRect() const
 {
   return gfx::IntRect(mOffset, mSize);
 }
 
@@ -829,18 +835,18 @@ imgFrame::UnlockImageData()
 {
   MonitorAutoLock lock(mMonitor);
 
   MOZ_ASSERT(mLockCount > 0, "Unlocking an unlocked image!");
   if (mLockCount <= 0) {
     return NS_ERROR_FAILURE;
   }
 
-  MOZ_ASSERT(mLockCount > 1 || IsImageCompleteInternal() || mAborted,
-             "Should have marked complete or aborted before unlocking");
+  MOZ_ASSERT(mLockCount > 1 || mFinished || mAborted,
+             "Should have Finish()'d or aborted before unlocking");
 
   // If we're about to become unlocked, we don't need to hold on to our data
   // surface anymore. (But we don't need to do anything for paletted images,
   // which don't have surfaces.)
   if (mLockCount == 1 && !mPalettedImageData) {
     // We can't safely optimize off-main-thread, so create a runnable to do it.
     if (!NS_IsMainThread()) {
       nsCOMPtr<nsIRunnable> runnable = new UnlockImageDataRunnable(this);
@@ -987,40 +993,40 @@ imgFrame::Abort()
 
   mAborted = true;
 
   // Wake up anyone who's waiting.
   mMonitor.NotifyAll();
 }
 
 bool
-imgFrame::IsImageComplete() const
+imgFrame::IsFinished() const
 {
   MonitorAutoLock lock(mMonitor);
-  return IsImageCompleteInternal();
+  return mFinished;
 }
 
 void
-imgFrame::WaitUntilComplete() const
+imgFrame::WaitUntilFinished() const
 {
   MonitorAutoLock lock(mMonitor);
 
   while (true) {
     // Return if we're aborted or complete.
-    if (mAborted || IsImageCompleteInternal()) {
+    if (mAborted || mFinished) {
       return;
     }
 
     // Not complete yet, so we'll have to wait.
     mMonitor.Wait();
   }
 }
 
 bool
-imgFrame::IsImageCompleteInternal() const
+imgFrame::AreAllPixelsWritten() const
 {
   mMonitor.AssertCurrentThreadOwns();
   return mDecoded.IsEqualInterior(nsIntRect(mOffset.x, mOffset.y,
                                             mSize.width, mSize.height));
 }
 
 bool imgFrame::GetCompositingFailed() const
 {
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -207,27 +207,27 @@ public:
    * You must always call either Finish() or Abort() before releasing the last
    * RawAccessFrameRef pointing to an imgFrame.
    */
   void Abort();
 
   /**
    * Returns true if this imgFrame is completely decoded.
    */
-  bool IsImageComplete() const;
+  bool IsFinished() const;
 
   /**
    * Blocks until this imgFrame is either completely decoded, or is marked as
    * aborted.
    *
    * Note that calling this on the main thread _blocks the main thread_. Be very
    * careful in your use of this method to avoid excessive main thread jank or
    * deadlock.
    */
-  void WaitUntilComplete() const;
+  void WaitUntilFinished() const;
 
   /**
    * Returns the number of bytes per pixel this imgFrame requires.  This is a
    * worst-case value that does not take into account the effects of format
    * changes caused by Optimize(), since an imgFrame is not optimized throughout
    * its lifetime.
    */
   uint32_t GetBytesPerPixel() const { return GetIsPaletted() ? 1 : 4; }
@@ -273,17 +273,17 @@ private: // methods
   ~imgFrame();
 
   nsresult LockImageData();
   nsresult UnlockImageData();
   nsresult Optimize();
 
   void AssertImageDataLocked() const;
 
-  bool IsImageCompleteInternal() const;
+  bool AreAllPixelsWritten() const;
   nsresult ImageUpdatedInternal(const nsIntRect& aUpdateRect);
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
   uint32_t GetImageBytesPerRow() const;
   uint32_t GetImageDataLength() const;
   int32_t GetStride() const;
   already_AddRefed<SourceSurface> GetSurfaceInternal();
 
   uint32_t PaletteDataLength() const
@@ -337,16 +337,17 @@ private: // data
   int32_t mTimeout; // -1 means display forever.
 
   DisposalMethod mDisposalMethod;
   BlendMethod    mBlendMethod;
   SurfaceFormat  mFormat;
 
   bool mHasNoAlpha;
   bool mAborted;
+  bool mFinished;
   bool mOptimizable;
 
 
   //////////////////////////////////////////////////////////////////////////////
   // Effectively const data, only mutated in the Init methods.
   //////////////////////////////////////////////////////////////////////////////
 
   IntSize      mImageSize;