From ad5c60edad248a7bc66c0aa9c4068547469d9b76 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Wed, 26 Nov 2014 01:37:56 -0800 Subject: [PATCH] Bug 1057904 (Part 1) - Use RawAccessRef in FrameBlender and related classes and clean up. r=tn --- image/src/FrameBlender.cpp | 203 ++++++++++++++++------------------- image/src/FrameBlender.h | 36 ++++--- image/src/FrameSequence.cpp | 103 ------------------ image/src/FrameSequence.h | 206 ------------------------------------ image/src/RasterImage.cpp | 53 ++-------- image/src/imgFrame.cpp | 35 ++++++ image/src/imgFrame.h | 1 + image/src/moz.build | 1 - 8 files changed, 158 insertions(+), 480 deletions(-) delete mode 100644 image/src/FrameSequence.cpp delete mode 100644 image/src/FrameSequence.h diff --git a/image/src/FrameBlender.cpp b/image/src/FrameBlender.cpp index 06e406671c1..9f47601816e 100644 --- a/image/src/FrameBlender.cpp +++ b/image/src/FrameBlender.cpp @@ -6,6 +6,7 @@ #include "FrameBlender.h" #include "mozilla/MemoryReporting.h" +#include "mozilla/Move.h" #include "MainThreadUtils.h" #include "pixman.h" @@ -16,14 +17,10 @@ using namespace gfx; namespace image { -FrameBlender::FrameBlender(FrameSequence* aSequenceToUse /* = nullptr */) - : mFrames(aSequenceToUse) - , mAnim(nullptr) +FrameBlender::FrameBlender() + : mAnim(nullptr) , mLoopCount(-1) { - if (!mFrames) { - mFrames = new FrameSequence(); - } } FrameBlender::~FrameBlender() @@ -31,47 +28,57 @@ FrameBlender::~FrameBlender() delete mAnim; } -already_AddRefed -FrameBlender::GetFrameSequence() +already_AddRefed +FrameBlender::GetFrame(uint32_t aFrameNum) { - nsRefPtr seq(mFrames); - return seq.forget(); + if (mAnim && mAnim->lastCompositedFrameIndex == int32_t(aFrameNum)) { + nsRefPtr frame = mAnim->compositingFrame.get(); + return frame.forget(); + } + return RawGetFrame(aFrameNum); } already_AddRefed -FrameBlender::GetFrame(uint32_t framenum) const +FrameBlender::RawGetFrame(uint32_t aFrameNum) { if (!mAnim) { - NS_ASSERTION(framenum == 0, "Don't ask for a frame > 0 if we're not animated!"); - return mFrames->GetFrame(0).GetFrame(); + NS_ASSERTION(aFrameNum == 0, + "Don't ask for a frame > 0 if we're not animated!"); + aFrameNum = 0; } - if (mAnim->lastCompositedFrameIndex == int32_t(framenum)) { - return mAnim->compositingFrame.GetFrame(); + if (aFrameNum >= mFrames.Length()) { + return nullptr; } - return mFrames->GetFrame(framenum).GetFrame(); + nsRefPtr frame = mFrames[aFrameNum].get(); + return frame.forget(); } -already_AddRefed -FrameBlender::RawGetFrame(uint32_t framenum) const +int32_t +FrameBlender::GetFrameDisposalMethod(uint32_t aFrameNum) const { if (!mAnim) { - NS_ASSERTION(framenum == 0, "Don't ask for a frame > 0 if we're not animated!"); - return mFrames->GetFrame(0).GetFrame(); + NS_ASSERTION(aFrameNum == 0, + "Don't ask for a frame > 0 if we're not animated!"); + aFrameNum = 0; } - return mFrames->GetFrame(framenum).GetFrame(); + if (aFrameNum >= mFrames.Length()) { + return FrameBlender::kDisposeNotSpecified; + } + return mFrames[aFrameNum]->GetFrameDisposalMethod(); } uint32_t FrameBlender::GetNumFrames() const { - return mFrames->GetNumFrames(); + return mFrames.Length(); } int32_t -FrameBlender::GetTimeoutForFrame(uint32_t framenum) const +FrameBlender::GetTimeoutForFrame(uint32_t aFrameNum) { - nsRefPtr frame = RawGetFrame(framenum); + nsRefPtr frame = RawGetFrame(aFrameNum); const int32_t timeout = frame->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. @@ -85,8 +92,10 @@ FrameBlender::GetTimeoutForFrame(uint32_t framenum) const // 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) + if (timeout >= 0 && timeout <= 10 && mLoopCount != 0) { return 100; + } + return timeout; } @@ -103,60 +112,33 @@ FrameBlender::GetLoopCount() const } void -FrameBlender::RemoveFrame(uint32_t framenum) +FrameBlender::RemoveFrame(uint32_t aFrameNum) { - NS_ABORT_IF_FALSE(framenum < GetNumFrames(), "Deleting invalid frame!"); - - mFrames->RemoveFrame(framenum); + MOZ_ASSERT(aFrameNum < GetNumFrames(), "Deleting invalid frame!"); + mFrames.RemoveElementAt(aFrameNum); } void FrameBlender::ClearFrames() { - // Forget our old frame sequence, letting whoever else has it deal with it. - mFrames = new FrameSequence(); + mFrames.Clear(); + mFrames.Compact(); } void -FrameBlender::InsertFrame(uint32_t framenum, imgFrame* aFrame) +FrameBlender::InsertFrame(uint32_t aFrameNum, RawAccessFrameRef&& aRef) { - NS_ABORT_IF_FALSE(framenum <= GetNumFrames(), "Inserting invalid frame!"); - mFrames->InsertFrame(framenum, aFrame); - if (GetNumFrames() > 1) { - EnsureAnimExists(); - } -} + MOZ_ASSERT(aRef, "Need a reference to a frame"); + MOZ_ASSERT(aFrameNum <= GetNumFrames(), "Inserting invalid frame"); -already_AddRefed -FrameBlender::SwapFrame(uint32_t framenum, imgFrame* aFrame) -{ - NS_ABORT_IF_FALSE(framenum < GetNumFrames(), "Swapping invalid frame!"); - - nsRefPtr ret; - - // Steal the imgFrame from wherever it's currently stored - if (mAnim && mAnim->lastCompositedFrameIndex == int32_t(framenum)) { - ret = mAnim->compositingFrame.Forget(); - mAnim->lastCompositedFrameIndex = -1; - nsRefPtr toDelete(mFrames->SwapFrame(framenum, aFrame)); - } else { - ret = mFrames->SwapFrame(framenum, aFrame); - } - - return ret.forget(); -} - -void -FrameBlender::EnsureAnimExists() -{ - if (!mAnim) { - // Create the animation context + mFrames.InsertElementAt(aFrameNum, Move(aRef)); + if (GetNumFrames() == 2) { + MOZ_ASSERT(!mAnim, "Shouldn't have an animation context yet"); mAnim = new Anim(); - - // We should only get into this code path directly after we've created our - // second frame (hence we know we're animated). - MOZ_ASSERT(GetNumFrames() == 2); } + + MOZ_ASSERT(GetNumFrames() < 2 || mAnim, + "If we're animated we should have an animation context now"); } //****************************************************************************** @@ -167,20 +149,16 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, uint32_t aPrevFrameIndex, uint32_t aNextFrameIndex) { - if (!aDirtyRect) { - return false; - } + nsRefPtr prevFrame = GetFrame(aPrevFrameIndex); + nsRefPtr nextFrame = GetFrame(aNextFrameIndex); - const FrameDataPair& prevFrame = mFrames->GetFrame(aPrevFrameIndex); - const FrameDataPair& nextFrame = mFrames->GetFrame(aNextFrameIndex); - if (!prevFrame.HasFrameData() || !nextFrame.HasFrameData()) { - return false; - } + MOZ_ASSERT(prevFrame && nextFrame, "Should have frames here"); - int32_t prevFrameDisposalMethod = prevFrame->GetFrameDisposalMethod(); + int32_t prevFrameDisposalMethod = GetFrameDisposalMethod(aPrevFrameIndex); if (prevFrameDisposalMethod == FrameBlender::kDisposeRestorePrevious && - !mAnim->compositingPrevFrame) + !mAnim->compositingPrevFrame) { prevFrameDisposalMethod = FrameBlender::kDisposeClear; + } nsIntRect prevFrameRect = prevFrame->GetRect(); bool isFullPrevFrame = (prevFrameRect.x == 0 && prevFrameRect.y == 0 && @@ -190,10 +168,11 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, // Optimization: DisposeClearAll if the previous frame is the same size as // container and it's clearing itself if (isFullPrevFrame && - (prevFrameDisposalMethod == FrameBlender::kDisposeClear)) + (prevFrameDisposalMethod == FrameBlender::kDisposeClear)) { prevFrameDisposalMethod = FrameBlender::kDisposeClearAll; + } - int32_t nextFrameDisposalMethod = nextFrame->GetFrameDisposalMethod(); + int32_t nextFrameDisposalMethod = GetFrameDisposalMethod(aNextFrameIndex); nsIntRect nextFrameRect = nextFrame->GetRect(); bool isFullNextFrame = (nextFrameRect.x == 0 && nextFrameRect.y == 0 && nextFrameRect.width == mSize.width && @@ -260,14 +239,13 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, // Create the Compositing Frame if (!mAnim->compositingFrame) { - mAnim->compositingFrame.SetFrame(new imgFrame()); - nsresult rv = - mAnim->compositingFrame->InitForDecoder(mSize, SurfaceFormat::B8G8R8A8); + nsRefPtr newFrame = new imgFrame; + nsresult rv = newFrame->InitForDecoder(mSize, SurfaceFormat::B8G8R8A8); if (NS_FAILED(rv)) { - mAnim->compositingFrame.SetFrame(nullptr); + mAnim->compositingFrame.reset(); return false; } - mAnim->compositingFrame.LockAndGetData(); + mAnim->compositingFrame = newFrame->RawAccessRef(); needToBlankComposite = true; } else if (int32_t(aNextFrameIndex) != mAnim->lastCompositedFrameIndex+1) { @@ -310,18 +288,18 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, if (needToBlankComposite) { // If we just created the composite, it could have anything in its // buffer. Clear whole frame - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); } else { // Only blank out previous frame area (both color & Mask/Alpha) - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect(), prevFrameRect); } break; case FrameBlender::kDisposeClearAll: - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); break; @@ -329,16 +307,16 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, // It would be better to copy only the area changed back to // compositingFrame. if (mAnim->compositingPrevFrame) { - CopyFrameImage(mAnim->compositingPrevFrame.GetFrameData(), + CopyFrameImage(mAnim->compositingPrevFrame->GetRawData(), mAnim->compositingPrevFrame->GetRect(), - mAnim->compositingFrame.GetFrameData(), + mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); // destroy only if we don't need it for this frame's disposal if (nextFrameDisposalMethod != FrameBlender::kDisposeRestorePrevious) - mAnim->compositingPrevFrame.SetFrame(nullptr); + mAnim->compositingPrevFrame.reset(); } else { - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); } break; @@ -353,22 +331,22 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, if (mAnim->lastCompositedFrameIndex != int32_t(aNextFrameIndex - 1)) { if (isFullPrevFrame && !prevFrame->GetIsPaletted()) { // Just copy the bits - CopyFrameImage(prevFrame.GetFrameData(), + CopyFrameImage(prevFrame->GetRawData(), prevFrame->GetRect(), - mAnim->compositingFrame.GetFrameData(), + mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); } else { if (needToBlankComposite) { // Only blank composite when prev is transparent or not full. if (prevFrame->GetHasAlpha() || !isFullPrevFrame) { - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); } } - DrawFrameTo(prevFrame.GetFrameData(), prevFrameRect, + DrawFrameTo(prevFrame->GetRawData(), prevFrameRect, prevFrame->PaletteDataLength(), prevFrame->GetHasAlpha(), - mAnim->compositingFrame.GetFrameData(), + mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect(), FrameBlendMethod(prevFrame->GetBlendMethod())); } @@ -377,7 +355,7 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, } else if (needToBlankComposite) { // If we just created the composite, it could have anything in it's // buffers. Clear them - ClearFrame(mAnim->compositingFrame.GetFrameData(), + ClearFrame(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect()); } @@ -390,29 +368,27 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect, // It would be better if we just stored the area that nextFrame is going to // overwrite. if (!mAnim->compositingPrevFrame) { - mAnim->compositingPrevFrame.SetFrame(new imgFrame()); - nsresult rv = - mAnim->compositingPrevFrame->InitForDecoder(mSize, - SurfaceFormat::B8G8R8A8); + nsRefPtr newFrame = new imgFrame; + nsresult rv = newFrame->InitForDecoder(mSize, SurfaceFormat::B8G8R8A8); if (NS_FAILED(rv)) { - mAnim->compositingPrevFrame.SetFrame(nullptr); + mAnim->compositingPrevFrame.reset(); return false; } - mAnim->compositingPrevFrame.LockAndGetData(); + mAnim->compositingPrevFrame = newFrame->RawAccessRef(); } - CopyFrameImage(mAnim->compositingFrame.GetFrameData(), + CopyFrameImage(mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect(), - mAnim->compositingPrevFrame.GetFrameData(), + mAnim->compositingPrevFrame->GetRawData(), mAnim->compositingPrevFrame->GetRect()); } // blit next frame into it's correct spot - DrawFrameTo(nextFrame.GetFrameData(), nextFrameRect, + DrawFrameTo(nextFrame->GetRawData(), nextFrameRect, nextFrame->PaletteDataLength(), nextFrame->GetHasAlpha(), - mAnim->compositingFrame.GetFrameData(), + mAnim->compositingFrame->GetRawData(), mAnim->compositingFrame->GetRect(), FrameBlendMethod(nextFrame->GetBlendMethod())); @@ -590,14 +566,23 @@ size_t FrameBlender::SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation, MallocSizeOf aMallocSizeOf) const { - size_t n = mFrames->SizeOfDecodedWithComputedFallbackIfHeap(aLocation, aMallocSizeOf); + size_t n = 0; + + for (uint32_t i = 0; i < mFrames.Length(); ++i) { + n += mFrames[i]->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, + aMallocSizeOf); + } if (mAnim) { if (mAnim->compositingFrame) { - n += mAnim->compositingFrame->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, aMallocSizeOf); + n += mAnim->compositingFrame + ->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, + aMallocSizeOf); } if (mAnim->compositingPrevFrame) { - n += mAnim->compositingPrevFrame->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, aMallocSizeOf); + n += mAnim->compositingPrevFrame + ->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, + aMallocSizeOf); } } diff --git a/image/src/FrameBlender.h b/image/src/FrameBlender.h index a7fc62f8469..f70774420c1 100644 --- a/image/src/FrameBlender.h +++ b/image/src/FrameBlender.h @@ -9,14 +9,12 @@ #include "mozilla/MemoryReporting.h" #include "gfxTypes.h" -#include "FrameSequence.h" +#include "imgFrame.h" #include "nsCOMPtr.h" namespace mozilla { namespace image { -class imgFrame; - /** * FrameBlender stores and gives access to imgFrames. It also knows how to * blend frames from previous to next, looping if necessary. @@ -32,28 +30,25 @@ public: * * If aSequenceToUse is not specified, it will be allocated automatically. */ - explicit FrameBlender(FrameSequence* aSequenceToUse = nullptr); + FrameBlender(); ~FrameBlender(); bool DoBlend(nsIntRect* aDirtyRect, uint32_t aPrevFrameIndex, uint32_t aNextFrameIndex); - already_AddRefed GetFrameSequence(); - /** * Get the @aIndex-th frame, including (if applicable) any results of * blending. */ - already_AddRefed GetFrame(uint32_t aIndex) const; + already_AddRefed GetFrame(uint32_t aIndex); /** * Get the @aIndex-th frame in the frame index, ignoring results of blending. */ - already_AddRefed RawGetFrame(uint32_t aIndex) const; + already_AddRefed RawGetFrame(uint32_t aIndex); - void InsertFrame(uint32_t framenum, imgFrame* aFrame); - void RemoveFrame(uint32_t framenum); - already_AddRefed SwapFrame(uint32_t framenum, imgFrame* aFrame); + void InsertFrame(uint32_t aFrameNum, RawAccessFrameRef&& aRef); + void RemoveFrame(uint32_t aFrameNum); void ClearFrames(); /* The total number of frames in this image. */ @@ -64,7 +59,7 @@ public: * 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; + int32_t GetTimeoutForFrame(uint32_t aFrameNum); /* * Set number of times to loop the image. @@ -112,6 +107,15 @@ public: private: + /** + * Get the disposal method of the @aIndex-th frame. + * + * Note that it's not safe to use GetFrame(aIndex)->GetFrameDisposalMethod() + * instead, because the frame GetFrame() returns may be a blended frame which + * does not have the correct disposal method set. + */ + int32_t GetFrameDisposalMethod(uint32_t aIndex) const; + struct Anim { //! Track the last composited frame for Optimizations (See DoComposite code) @@ -125,7 +129,7 @@ private: * lastCompositedFrameIndex to -1. Code assume that if * lastCompositedFrameIndex >= 0 then compositingFrame exists. */ - FrameDataPair compositingFrame; + RawAccessFrameRef compositingFrame; /** the previous composited frame, for DISPOSE_RESTORE_PREVIOUS * @@ -133,15 +137,13 @@ private: * stored in cases where the image specifies it wants the last frame back * when it's done with the current frame. */ - FrameDataPair compositingPrevFrame; + RawAccessFrameRef compositingPrevFrame; Anim() : lastCompositedFrameIndex(-1) {} }; - void EnsureAnimExists(); - /** Clears an area of with transparent black. * * @param aFrameData Target Frame data @@ -180,7 +182,7 @@ private: private: // data //! All the frames of the image - nsRefPtr mFrames; + nsTArray mFrames; nsIntSize mSize; Anim* mAnim; int32_t mLoopCount; diff --git a/image/src/FrameSequence.cpp b/image/src/FrameSequence.cpp deleted file mode 100644 index 5131f164a1c..00000000000 --- a/image/src/FrameSequence.cpp +++ /dev/null @@ -1,103 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "FrameSequence.h" - -namespace mozilla { -namespace image { - -FrameSequence::~FrameSequence() -{ - ClearFrames(); -} - -const FrameDataPair& -FrameSequence::GetFrame(uint32_t framenum) const -{ - if (framenum >= mFrames.Length()) { - static FrameDataPair empty; - return empty; - } - - return mFrames[framenum]; -} - -uint32_t -FrameSequence::GetNumFrames() const -{ - return mFrames.Length(); -} - -void -FrameSequence::RemoveFrame(uint32_t framenum) -{ - NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Deleting invalid frame!"); - - mFrames.RemoveElementAt(framenum); -} - -void -FrameSequence::ClearFrames() -{ - // Since FrameDataPair holds an nsAutoPtr to its frame, clearing the mFrames - // array also deletes all the frames. - mFrames.Clear(); -} - -void -FrameSequence::InsertFrame(uint32_t framenum, imgFrame* aFrame) -{ - NS_ABORT_IF_FALSE(framenum <= mFrames.Length(), "Inserting invalid frame!"); - mFrames.InsertElementAt(framenum, aFrame); - if (GetNumFrames() > 1) { - // If we're creating our second element, we now know we're animated. - // Therefore, we need to lock the first frame too. - if (GetNumFrames() == 2) { - mFrames[0].LockAndGetData(); - } - - // Whenever we have more than one frame, we always lock *all* our frames - // so we have all the image data pointers. - mFrames[framenum].LockAndGetData(); - } -} - -already_AddRefed -FrameSequence::SwapFrame(uint32_t framenum, imgFrame* aFrame) -{ - NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Swapping invalid frame!"); - - FrameDataPair ret; - - // Steal the imgFrame. - if (framenum < mFrames.Length()) { - ret = mFrames[framenum]; - } - - if (aFrame) { - mFrames.ReplaceElementAt(framenum, aFrame); - } else { - mFrames.RemoveElementAt(framenum); - } - - return ret.GetFrame(); -} - -size_t -FrameSequence::SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation, - MallocSizeOf aMallocSizeOf) const -{ - size_t n = 0; - for (uint32_t i = 0; i < mFrames.Length(); ++i) { - FrameDataPair fdp = mFrames.SafeElementAt(i, FrameDataPair()); - NS_ABORT_IF_FALSE(fdp, "Null frame in frame array!"); - n += fdp->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, aMallocSizeOf); - } - - return n; -} - -} // namespace image -} // namespace mozilla diff --git a/image/src/FrameSequence.h b/image/src/FrameSequence.h deleted file mode 100644 index f011243c7ca..00000000000 --- a/image/src/FrameSequence.h +++ /dev/null @@ -1,206 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_imagelib_FrameSequence_h_ -#define mozilla_imagelib_FrameSequence_h_ - -#include "nsTArray.h" -#include "mozilla/MemoryReporting.h" -#include "mozilla/Move.h" -#include "gfxTypes.h" -#include "imgFrame.h" - -namespace mozilla { -namespace image { - -/** - * FrameDataPair is a slightly-smart tuple of (frame, raw frame data) where the - * raw frame data is allowed to be (and is, initially) null. - * - * If you call LockAndGetData, you will be able to call GetFrameData() on that - * instance, and when the FrameDataPair is destructed, the imgFrame lock will - * be unlocked. - */ -class FrameDataPair -{ -public: - explicit FrameDataPair(imgFrame* frame) - : mFrame(frame) - , mFrameData(nullptr) - {} - - FrameDataPair() - : mFrameData(nullptr) - {} - - FrameDataPair(const FrameDataPair& aOther) - : mFrame(aOther.mFrame) - , mFrameData(nullptr) - {} - - FrameDataPair(FrameDataPair&& aOther) - : mFrame(Move(aOther.mFrame)) - , mFrameData(aOther.mFrameData) - { - aOther.mFrameData = nullptr; - } - - ~FrameDataPair() - { - if (mFrameData) { - mFrame->UnlockImageData(); - } - } - - FrameDataPair& operator=(const FrameDataPair& aOther) - { - if (&aOther != this) { - mFrame = aOther.mFrame; - mFrameData = nullptr; - } - return *this; - } - - FrameDataPair& operator=(FrameDataPair&& aOther) - { - MOZ_ASSERT(&aOther != this, "Moving to self"); - mFrame = Move(aOther.mFrame); - mFrameData = aOther.mFrameData; - aOther.mFrameData = nullptr; - return *this; - } - - // Lock the frame and store its mFrameData. The frame will be unlocked (and - // deleted) when this FrameDataPair is deleted. - void LockAndGetData() - { - if (mFrame) { - if (NS_SUCCEEDED(mFrame->LockImageData())) { - if (mFrame->GetIsPaletted()) { - mFrameData = reinterpret_cast(mFrame->GetPaletteData()); - } else { - mFrameData = mFrame->GetImageData(); - } - } - } - } - - // Null out this FrameDataPair and return its frame. You must ensure the - // frame will be deleted separately. - already_AddRefed Forget() - { - if (mFrameData) { - mFrame->UnlockImageData(); - } - - mFrameData = nullptr; - return mFrame.forget(); - } - - bool HasFrameData() const - { - if (mFrameData) { - MOZ_ASSERT(!!mFrame); - } - return !!mFrameData; - } - - uint8_t* GetFrameData() const - { - return mFrameData; - } - - already_AddRefed GetFrame() const - { - nsRefPtr frame = mFrame; - return frame.forget(); - } - - // Resets this FrameDataPair to work with a different frame. Takes ownership - // of the frame, deleting the old frame (if any). - void SetFrame(imgFrame* frame) - { - if (mFrameData) { - mFrame->UnlockImageData(); - } - - mFrame = frame; - mFrameData = nullptr; - } - - imgFrame* operator->() const - { - return mFrame.get(); - } - - bool operator==(imgFrame* other) const - { - return mFrame == other; - } - - operator bool() const - { - return mFrame != nullptr; - } - -private: - nsRefPtr mFrame; - uint8_t* mFrameData; -}; - -/** - * FrameSequence stores image frames (and their associated raw data pointers). - * It is little more than a smart array. - */ -class FrameSequence -{ - ~FrameSequence(); - -public: - - NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FrameSequence) - - /** - * Get the read-only (frame, data) pair at index aIndex. - */ - const FrameDataPair& GetFrame(uint32_t aIndex) const; - - /** - * Insert a frame into the array. FrameSequence takes ownership of the frame. - */ - void InsertFrame(uint32_t framenum, imgFrame* aFrame); - - /** - * Remove (and delete) the frame at index framenum. - */ - void RemoveFrame(uint32_t framenum); - - /** - * Swap aFrame with the frame at sequence framenum, and return that frame. - * You take ownership over the frame returned. - */ - already_AddRefed SwapFrame(uint32_t framenum, imgFrame* aFrame); - - /** - * Remove (and delete) all frames. - */ - void ClearFrames(); - - /* The total number of frames in this image. */ - uint32_t GetNumFrames() const; - - size_t SizeOfDecodedWithComputedFallbackIfHeap(gfxMemoryLocation aLocation, - MallocSizeOf aMallocSizeOf) const; - -private: // data - //! All the frames of the image - nsTArray mFrames; -}; - -} // namespace image -} // namespace mozilla - -#endif /* mozilla_imagelib_FrameSequence_h_ */ diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 72f894c0d95..8c331c3866d 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -361,15 +361,6 @@ RasterImage::~RasterImage() ReentrantMonitorAutoEnter lock(mDecodingMonitor); DecodePool::StopDecoding(this); mDecoder = nullptr; - - // Unlock the last frame (if we have any). Our invariant is that, while we - // have a decoder open, the last frame is always locked. - // This would be done in ShutdownDecoder, but since mDecoder is non-null, - // we didn't call ShutdownDecoder and we need to do it manually. - if (GetNumFrames() > 0) { - nsRefPtr curframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1); - curframe->UnlockImageData(); - } } // Release any HQ scaled frames from the surface cache. @@ -1023,18 +1014,20 @@ RasterImage::InternalAddFrameHelper(uint32_t framenum, imgFrame *aFrame, if (framenum > GetNumFrames()) return NS_ERROR_INVALID_ARG; - nsRefPtr frame(aFrame); - - // We are in the middle of decoding. This will be unlocked when we finish - // decoding or switch to another frame. - frame->LockImageData(); + nsRefPtr frame = aFrame; + RawAccessFrameRef ref = frame->RawAccessRef(); + if (!ref) { + // Probably the OS discarded the frame. Exceedingly unlikely since we just + // created it, but it could happen. + return NS_ERROR_FAILURE; + } if (paletteData && paletteLength) frame->GetPaletteData(paletteData, paletteLength); frame->GetImageData(imageData, imageLength); - mFrameBlender.InsertFrame(framenum, frame); + mFrameBlender.InsertFrame(framenum, Move(ref)); frame.forget(aRetFrame); return NS_OK; @@ -1071,13 +1064,6 @@ RasterImage::InternalAddFrame(uint32_t framenum, NS_WARNING("imgFrame::Init should succeed"); NS_ENSURE_SUCCESS(rv, rv); - // We know we are in a decoder. Therefore, we must unlock the previous frame - // when we move on to decoding into the next frame. - if (GetNumFrames() > 0) { - nsRefPtr prevframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1); - prevframe->UnlockImageData(); - } - if (GetNumFrames() == 0) { return InternalAddFrameHelper(framenum, frame, imageData, imageLength, paletteData, paletteLength, aRetFrame); @@ -1241,11 +1227,6 @@ RasterImage::EnsureFrame(uint32_t aFrameNum, int32_t aX, int32_t aY, } // Not reusable, so replace the frame directly. - - // We know this frame is already locked, because it's the one we're currently - // writing to. - frame->UnlockImageData(); - mFrameBlender.RemoveFrame(aFrameNum); nsRefPtr newFrame(new imgFrame()); nsIntRect frameRect(aX, aY, aWidth, aHeight); @@ -1329,8 +1310,7 @@ RasterImage::DecodingComplete() // into the first frame again immediately and this produces severe tearing. if (mMultipart) { if (GetNumFrames() == 1) { - mMultipartDecodedFrame = mFrameBlender.SwapFrame(GetCurrentFrameIndex(), - mMultipartDecodedFrame); + mMultipartDecodedFrame = mFrameBlender.GetFrame(GetCurrentFrameIndex()); } 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 @@ -1931,14 +1911,6 @@ RasterImage::InitDecoder(bool aDoSizeDecode) NS_ABORT_IF_FALSE(0, "Shouldn't get here!"); } - // If we already have frames, we're probably in the multipart/x-mixed-replace - // case. Regardless, we need to lock the last frame. Our invariant is that, - // while we have a decoder open, the last frame is always locked. - if (GetNumFrames() > 0) { - nsRefPtr curframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1); - curframe->LockImageData(); - } - // Initialize the decoder mDecoder->SetSizeDecode(aDoSizeDecode); mDecoder->SetDecodeFlags(mFrameDecodeFlags); @@ -2001,13 +1973,6 @@ RasterImage::ShutdownDecoder(ShutdownReason aReason) decoder->Finish(aReason); - // Unlock the last frame (if we have any). Our invariant is that, while we - // have a decoder open, the last frame is always locked. - if (GetNumFrames() > 0) { - nsRefPtr curframe = mFrameBlender.RawGetFrame(GetNumFrames() - 1); - curframe->UnlockImageData(); - } - // Kill off our decode request, if it's pending. (If not, this call is // harmless.) DecodePool::StopDecoding(this); diff --git a/image/src/imgFrame.cpp b/image/src/imgFrame.cpp index dd9d461fe32..fcc0015b805 100644 --- a/image/src/imgFrame.cpp +++ b/image/src/imgFrame.cpp @@ -579,9 +579,34 @@ imgFrame::GetStride() const return VolatileSurfaceStride(mSize, mFormat); } +<<<<<<< found SurfaceFormat imgFrame::GetFormat() const { +||||||| expected +bool imgFrame::GetNeedsBackground() const +{ + // We need a background painted if we have alpha or we're incomplete. +======= +bool imgFrame::GetNeedsBackground() const +{ + // We need a background painted if we're incomplete. + if (!ImageComplete()) { + return true; + } + + // We need a background painted if we might not be opaque. +>>>>>>> replacement +<<<<<<< found return mFormat; +||||||| expected + return (mFormat == SurfaceFormat::B8G8R8A8 || !ImageComplete()); +======= + if (mFormat == SurfaceFormat::B8G8R8A8 && !mHasNoAlpha) { + return true; + } + + return false; +>>>>>>> replacement } uint32_t imgFrame::GetImageBytesPerRow() const @@ -653,6 +678,16 @@ uint32_t* imgFrame::GetPaletteData() const return data; } +uint8_t* +imgFrame::GetRawData() const +{ + MOZ_ASSERT(mLockCount, "Should be locked to call GetRawData()"); + if (mPalettedImageData) { + return mPalettedImageData; + } + return GetImageData(); +} + nsresult imgFrame::LockImageData() { MOZ_ASSERT(NS_IsMainThread()); diff --git a/image/src/imgFrame.h b/image/src/imgFrame.h index 55052e88097..ca9fd0a940c 100644 --- a/image/src/imgFrame.h +++ b/image/src/imgFrame.h @@ -96,6 +96,7 @@ public: uint8_t* GetImageData() const; void GetPaletteData(uint32_t **aPalette, uint32_t *length) const; uint32_t* GetPaletteData() const; + uint8_t* GetRawData() const; int32_t GetRawTimeout() const; void SetRawTimeout(int32_t aTimeout); diff --git a/image/src/moz.build b/image/src/moz.build index c4ebae1ba68..1d022ea87cb 100644 --- a/image/src/moz.build +++ b/image/src/moz.build @@ -22,7 +22,6 @@ UNIFIED_SOURCES += [ 'DynamicImage.cpp', 'FrameAnimator.cpp', 'FrameBlender.cpp', - 'FrameSequence.cpp', 'FrozenImage.cpp', 'Image.cpp', 'ImageFactory.cpp',