From d564a3b0a5e5a99f263feb9a21d2d977a7560970 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Fri, 10 Jul 2015 19:26:15 -0700 Subject: [PATCH] Bug 1117607 - Make decoders responsible for their own frame allocations. r=tn --- image/Decoder.cpp | 200 ++++--------------------------- image/Decoder.h | 129 +++++--------------- image/RasterImage.cpp | 42 ++++--- image/RasterImage.h | 5 +- image/decoders/nsBMPDecoder.cpp | 10 +- image/decoders/nsGIFDecoder2.cpp | 58 ++++----- image/decoders/nsGIFDecoder2.h | 2 +- image/decoders/nsICODecoder.cpp | 83 +++++-------- image/decoders/nsICODecoder.h | 10 +- image/decoders/nsIconDecoder.cpp | 12 +- image/decoders/nsJPEGDecoder.cpp | 10 +- image/decoders/nsPNGDecoder.cpp | 70 ++++++----- image/decoders/nsPNGDecoder.h | 6 +- image/imgFrame.cpp | 74 ------------ image/imgFrame.h | 13 -- 15 files changed, 203 insertions(+), 521 deletions(-) diff --git a/image/Decoder.cpp b/image/Decoder.cpp index 2f35621da41d..397a87cc61c2 100644 --- a/image/Decoder.cpp +++ b/image/Decoder.cpp @@ -41,8 +41,6 @@ Decoder::Decoder(RasterImage* aImage) , mImageIsLocked(false) , mFrameCount(0) , mFailCode(NS_OK) - , mNeedsNewFrame(false) - , mNeedsToFlushData(false) , mInitialized(false) , mSizeDecode(false) , mInFrame(false) @@ -89,33 +87,6 @@ Decoder::Init() mInitialized = true; } -// Initializes a decoder whose image and observer is already being used by a -// parent decoder -void -Decoder::InitSharedDecoder(uint8_t* aImageData, uint32_t aImageDataLength, - uint32_t* aColormap, uint32_t aColormapSize, - RawAccessFrameRef&& aFrameRef) -{ - // No re-initializing - MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!"); - - mImageData = aImageData; - mImageDataLength = aImageDataLength; - mColormap = aColormap; - mColormapSize = aColormapSize; - mCurrentFrame = Move(aFrameRef); - - // We have all the frame data, so we've started the frame. - if (!IsSizeDecode()) { - mFrameCount++; - PostFrameStart(); - } - - // Implementation-specific initialization - InitInternal(); - mInitialized = true; -} - nsresult Decoder::Decode() { @@ -179,6 +150,9 @@ Decoder::Write(const char* aBuffer, uint32_t aCount) PROFILER_LABEL("ImageDecoder", "Write", js::ProfileEntry::Category::GRAPHICS); + MOZ_ASSERT(aBuffer); + MOZ_ASSERT(aCount > 0); + // We're strict about decoder errors MOZ_ASSERT(!HasDecoderError(), "Not allowed to make more decoder calls after error!"); @@ -190,12 +164,6 @@ Decoder::Write(const char* aBuffer, uint32_t aCount) // Keep track of the total number of bytes written. mBytesDecoded += aCount; - // If we're flushing data, clear the flag. - if (aBuffer == nullptr && aCount == 0) { - MOZ_ASSERT(mNeedsToFlushData, "Flushing when we don't need to"); - mNeedsToFlushData = false; - } - // If a data error occured, just ignore future data. if (HasDataError()) { return; @@ -206,28 +174,9 @@ Decoder::Write(const char* aBuffer, uint32_t aCount) return; } - MOZ_ASSERT(!NeedsNewFrame() || HasDataError(), - "Should not need a new frame before writing anything"); - MOZ_ASSERT(!NeedsToFlushData() || HasDataError(), - "Should not need to flush data before writing anything"); - // Pass the data along to the implementation. WriteInternal(aBuffer, aCount); - // If we need a new frame to proceed, let's create one and call it again. - while (NeedsNewFrame() && !HasDataError()) { - MOZ_ASSERT(!IsSizeDecode(), "Shouldn't need new frame for size decode"); - - nsresult rv = AllocateFrame(); - - if (NS_SUCCEEDED(rv)) { - // Use the data we saved when we asked for a new frame. - WriteInternal(nullptr, 0); - } - - mNeedsToFlushData = false; - } - // Finish telemetry. mDecodeTime += (TimeStamp::Now() - start); } @@ -238,6 +187,8 @@ Decoder::CompleteDecode() // Implementation-specific finalization if (!HasError()) { FinishInternal(); + } else { + FinishWithErrorInternal(); } // If the implementation left us mid-frame, finish that up. @@ -328,135 +279,51 @@ Decoder::Finish() mCurrentFrame->SetOptimizable(); } - mImage->OnDecodingComplete(); - } -} - -void -Decoder::FinishSharedDecoder() -{ - if (!HasError()) { - FinishInternal(); + mImage->OnDecodingComplete(mIsAnimated); } } nsresult -Decoder::AllocateFrame(const nsIntSize& aTargetSize /* = nsIntSize() */) +Decoder::AllocateFrame(uint32_t aFrameNum, + const nsIntSize& aTargetSize, + const nsIntRect& aFrameRect, + gfx::SurfaceFormat aFormat, + uint8_t aPaletteDepth) { - MOZ_ASSERT(mNeedsNewFrame); - - nsIntSize targetSize = aTargetSize; - if (targetSize == nsIntSize()) { - MOZ_ASSERT(HasSize()); - targetSize = mImageMetadata.GetSize(); - } - - mCurrentFrame = EnsureFrame(mNewFrameData.mFrameNum, - targetSize, - mNewFrameData.mFrameRect, - GetDecodeFlags(), - mNewFrameData.mFormat, - mNewFrameData.mPaletteDepth, - mCurrentFrame.get()); + mCurrentFrame = AllocateFrameInternal(aFrameNum, aTargetSize, aFrameRect, + GetDecodeFlags(), aFormat, + aPaletteDepth, mCurrentFrame.get()); if (mCurrentFrame) { // Gather the raw pointers the decoders will use. mCurrentFrame->GetImageData(&mImageData, &mImageDataLength); mCurrentFrame->GetPaletteData(&mColormap, &mColormapSize); - if (mNewFrameData.mFrameNum + 1 == mFrameCount) { + if (aFrameNum + 1 == mFrameCount) { PostFrameStart(); } } else { PostDataError(); } - // Mark ourselves as not needing another frame before talking to anyone else - // so they can tell us if they need yet another. - mNeedsNewFrame = false; - - // If we've received any data at all, we may have pending data that needs to - // be flushed now that we have a frame to decode into. - if (mBytesDecoded > 0) { - mNeedsToFlushData = true; - } - return mCurrentFrame ? NS_OK : NS_ERROR_FAILURE; } RawAccessFrameRef -Decoder::EnsureFrame(uint32_t aFrameNum, - const nsIntSize& aTargetSize, - const nsIntRect& aFrameRect, - uint32_t aDecodeFlags, - SurfaceFormat aFormat, - uint8_t aPaletteDepth, - imgFrame* aPreviousFrame) +Decoder::AllocateFrameInternal(uint32_t aFrameNum, + const nsIntSize& aTargetSize, + const nsIntRect& aFrameRect, + uint32_t aDecodeFlags, + SurfaceFormat aFormat, + uint8_t aPaletteDepth, + imgFrame* aPreviousFrame) { if (mDataError || NS_FAILED(mFailCode)) { return RawAccessFrameRef(); } - MOZ_ASSERT(aFrameNum <= mFrameCount, "Invalid frame index!"); - if (aFrameNum > mFrameCount) { - return RawAccessFrameRef(); - } - - // Adding a frame that doesn't already exist. This is the normal case. - if (aFrameNum == mFrameCount) { - return InternalAddFrame(aFrameNum, aTargetSize, aFrameRect, aDecodeFlags, - aFormat, aPaletteDepth, aPreviousFrame); - } - - // We're replacing a frame. It must be the first frame; there's no reason to - // ever replace any other frame, since the first frame is the only one we - // speculatively allocate without knowing what the decoder really needs. - // XXX(seth): I'm not convinced there's any reason to support this at all. We - // should figure out how to avoid triggering this and rip it out. - MOZ_ASSERT(aFrameNum == 0, "Replacing a frame other than the first?"); - MOZ_ASSERT(mFrameCount == 1, "Should have only one frame"); - MOZ_ASSERT(aPreviousFrame, "Need the previous frame to replace"); - if (aFrameNum != 0 || !aPreviousFrame || mFrameCount != 1) { - return RawAccessFrameRef(); - } - - MOZ_ASSERT(!aPreviousFrame->GetRect().IsEqualEdges(aFrameRect) || - aPreviousFrame->GetFormat() != aFormat || - aPreviousFrame->GetPaletteDepth() != aPaletteDepth, - "Replacing first frame with the same kind of frame?"); - - // Reset our state. - mInFrame = false; - RawAccessFrameRef ref = Move(mCurrentFrame); - - MOZ_ASSERT(ref, "No ref to current frame?"); - - // Reinitialize the old frame. - nsIntSize oldSize = aPreviousFrame->GetImageSize(); - bool nonPremult = - aDecodeFlags & imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA; - if (NS_FAILED(aPreviousFrame->ReinitForDecoder(oldSize, aFrameRect, aFormat, - aPaletteDepth, nonPremult))) { - NS_WARNING("imgFrame::ReinitForDecoder should succeed"); - mFrameCount = 0; - aPreviousFrame->Abort(); - return RawAccessFrameRef(); - } - - return ref; -} - -RawAccessFrameRef -Decoder::InternalAddFrame(uint32_t aFrameNum, - const nsIntSize& aTargetSize, - const nsIntRect& aFrameRect, - uint32_t aDecodeFlags, - SurfaceFormat aFormat, - uint8_t aPaletteDepth, - imgFrame* aPreviousFrame) -{ - MOZ_ASSERT(aFrameNum <= mFrameCount, "Invalid frame index!"); - if (aFrameNum > mFrameCount) { + if (aFrameNum != mFrameCount) { + MOZ_ASSERT_UNREACHABLE("Allocating frames out of order"); return RawAccessFrameRef(); } @@ -559,6 +426,7 @@ Decoder::SetSizeOnImage() void Decoder::InitInternal() { } void Decoder::WriteInternal(const char* aBuffer, uint32_t aCount) { } void Decoder::FinishInternal() { } +void Decoder::FinishWithErrorInternal() { } /* * Progress Notifications @@ -685,24 +553,6 @@ Decoder::PostDecoderError(nsresult aFailureCode) } } -void -Decoder::NeedNewFrame(uint32_t framenum, uint32_t x_offset, uint32_t y_offset, - uint32_t width, uint32_t height, - gfx::SurfaceFormat format, - uint8_t palette_depth /* = 0 */) -{ - // Decoders should never call NeedNewFrame without yielding back to Write(). - MOZ_ASSERT(!mNeedsNewFrame); - - // We don't want images going back in time or skipping frames. - MOZ_ASSERT(framenum == mFrameCount || framenum == (mFrameCount - 1)); - - mNewFrameData = NewFrameData(framenum, - nsIntRect(x_offset, y_offset, width, height), - format, palette_depth); - mNeedsNewFrame = true; -} - Telemetry::ID Decoder::SpeedHistogram() { diff --git a/image/Decoder.h b/image/Decoder.h index 3781b1ea5c80..a7892a9ac001 100644 --- a/image/Decoder.h +++ b/image/Decoder.h @@ -33,23 +33,8 @@ public: */ void Init(); - /** - * Initializes a decoder whose image and observer is already being used by a - * parent decoder. Decoders may not be re-initialized. - * - * Notifications Sent: TODO - */ - void InitSharedDecoder(uint8_t* aImageData, uint32_t aImageDataLength, - uint32_t* aColormap, uint32_t aColormapSize, - RawAccessFrameRef&& aFrameRef); - /** * Decodes, reading all data currently available in the SourceBuffer. If more - * If aBuffer is null and aCount is 0, Write() flushes any buffered data to - * the decoder. Data is buffered if the decoder wasn't able to completely - * decode it because it needed a new frame. If it's necessary to flush data, - * NeedsToFlushData() will return true. - * * data is needed, Decode() automatically ensures that it will be called again * on a DecodePool thread when the data becomes available. * @@ -69,14 +54,6 @@ public: */ bool ShouldSyncDecode(size_t aByteLimit); - /** - * Informs the shared decoder that all the data has been written. - * Should only be used if InitSharedDecoder was useed - * - * Notifications Sent: TODO - */ - void FinishSharedDecoder(); - /** * Gets the invalidation region accumulated by the decoder so far, and clears * the decoder's invalidation region. This means that each call to @@ -259,11 +236,6 @@ public: bool HasSize() const { return mImageMetadata.HasSize(); } void SetSizeOnImage(); - void SetSize(const nsIntSize& aSize, const Orientation& aOrientation) - { - PostSize(aSize.width, aSize.height, aOrientation); - } - nsIntSize GetSize() const { MOZ_ASSERT(HasSize()); @@ -279,30 +251,9 @@ public: */ RasterImage* GetImage() const { MOZ_ASSERT(mImage); return mImage.get(); } - // Tell the decoder infrastructure to allocate a frame. By default, frame 0 - // is created as an ARGB frame with no offset and with size width * height. - // If decoders need something different, they must ask for it. - // This is called by decoders when they need a new frame. These decoders - // must then save the data they have been sent but not yet processed and - // return from WriteInternal. When the new frame is created, WriteInternal - // will be called again with nullptr and 0 as arguments. - void NeedNewFrame(uint32_t frameNum, uint32_t x_offset, uint32_t y_offset, - uint32_t width, uint32_t height, - gfx::SurfaceFormat format, - uint8_t palette_depth = 0); - - virtual bool NeedsNewFrame() const { return mNeedsNewFrame; } - - // Try to allocate a frame as described in mNewFrameData and return the - // status code from that attempt. Clears mNewFrameData. - virtual nsresult AllocateFrame(const nsIntSize& aTargetSize = nsIntSize()); - - already_AddRefed GetCurrentFrame() - { - nsRefPtr frame = mCurrentFrame.get(); - return frame.forget(); - } - + // XXX(seth): This should be removed once we can optimize imgFrame objects + // off-main-thread. It only exists to support the code in Finish() for + // nsICODecoder. RawAccessFrameRef GetCurrentFrameRef() { return mCurrentFrame ? mCurrentFrame->RawAccessRef() @@ -322,6 +273,8 @@ public: protected: + friend class nsICODecoder; + virtual ~Decoder(); /* @@ -331,6 +284,7 @@ protected: virtual void InitInternal(); virtual void WriteInternal(const char* aBuffer, uint32_t aCount); virtual void FinishInternal(); + virtual void FinishWithErrorInternal(); /* * Progress notifications. @@ -396,11 +350,6 @@ protected: void PostDataError(); void PostDecoderError(nsresult aFailCode); - // Returns true if we may have stored data that we need to flush now that we - // have a new frame to decode into. Callers can use Write() to actually - // flush the data; see the documentation for that method. - bool NeedsToFlushData() const { return mNeedsToFlushData; } - /** * CompleteDecode() finishes up the decoding process after Decode() determines * that we're finished. It records final progress and does all the cleanup @@ -409,32 +358,33 @@ protected: void CompleteDecode(); /** - * Ensures that a given frame number exists with the given parameters, and - * returns a RawAccessFrameRef for that frame. - * It is not possible to create sparse frame arrays; you can only append - * frames to the current frame array, or if there is only one frame in the - * array, replace that frame. - * @aTargetSize specifies the target size we're decoding to. If we're not - * downscaling during decode, this will always be the same as the image's - * intrinsic size. + * Allocates a new frame, making it our current frame if successful. + * + * The @aFrameNum parameter only exists as a sanity check; it's illegal to + * create a new frame anywhere but immediately after the existing frames. * * If a non-paletted frame is desired, pass 0 for aPaletteDepth. */ - RawAccessFrameRef EnsureFrame(uint32_t aFrameNum, - const nsIntSize& aTargetSize, - const nsIntRect& aFrameRect, - uint32_t aDecodeFlags, - gfx::SurfaceFormat aFormat, - uint8_t aPaletteDepth, - imgFrame* aPreviousFrame); + nsresult AllocateFrame(uint32_t aFrameNum, + const nsIntSize& aTargetSize, + const nsIntRect& aFrameRect, + gfx::SurfaceFormat aFormat, + uint8_t aPaletteDepth = 0); - RawAccessFrameRef InternalAddFrame(uint32_t aFrameNum, - const nsIntSize& aTargetSize, - const nsIntRect& aFrameRect, - uint32_t aDecodeFlags, - gfx::SurfaceFormat aFormat, - uint8_t aPaletteDepth, - imgFrame* aPreviousFrame); + /// Helper method for decoders which only have 'basic' frame allocation needs. + nsresult AllocateBasicFrame() { + nsIntSize size = GetSize(); + return AllocateFrame(0, size, nsIntRect(nsIntPoint(), size), + gfx::SurfaceFormat::B8G8R8A8); + } + + RawAccessFrameRef AllocateFrameInternal(uint32_t aFrameNum, + const nsIntSize& aTargetSize, + const nsIntRect& aFrameRect, + uint32_t aDecodeFlags, + gfx::SurfaceFormat aFormat, + uint8_t aPaletteDepth, + imgFrame* aPreviousFrame); /* * Member variables. @@ -472,27 +422,6 @@ private: nsresult mFailCode; - struct NewFrameData - { - NewFrameData() { } - - NewFrameData(uint32_t aFrameNum, const nsIntRect& aFrameRect, - gfx::SurfaceFormat aFormat, uint8_t aPaletteDepth) - : mFrameNum(aFrameNum) - , mFrameRect(aFrameRect) - , mFormat(aFormat) - , mPaletteDepth(aPaletteDepth) - { } - - uint32_t mFrameNum; - nsIntRect mFrameRect; - gfx::SurfaceFormat mFormat; - uint8_t mPaletteDepth; - }; - - NewFrameData mNewFrameData; - bool mNeedsNewFrame; - bool mNeedsToFlushData; bool mInitialized; bool mSizeDecode; bool mInFrame; diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 71d624dcd161..10e743830da4 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -963,9 +963,7 @@ RasterImage::OnAddedFrame(uint32_t aNewFrameCount, return; } - MOZ_ASSERT((mFrameCount == 1 && aNewFrameCount == 1) || - mFrameCount < aNewFrameCount, - "Frame count running backwards"); + MOZ_ASSERT(aNewFrameCount <= mFrameCount + 1, "Skipped a frame?"); if (mError) { return; // We're in an error state, possibly due to OOM. Bail. @@ -1034,7 +1032,7 @@ RasterImage::SetSize(int32_t aWidth, int32_t aHeight, Orientation aOrientation) } void -RasterImage::OnDecodingComplete() +RasterImage::OnDecodingComplete(bool aIsAnimated) { MOZ_ASSERT(NS_IsMainThread()); @@ -1045,12 +1043,30 @@ RasterImage::OnDecodingComplete() // Flag that we've been decoded before. mHasBeenDecoded = true; - // Let our FrameAnimator know not to expect any more frames. - if (mAnim) { - mAnim->SetDoneDecoding(true); + if (aIsAnimated) { + if (mAnim) { + mAnim->SetDoneDecoding(true); + } else { + // The OnAddedFrame event that will create mAnim is still in the event + // queue. Wait for it. + nsCOMPtr runnable = + NS_NewRunnableMethod(this, &RasterImage::MarkAnimationDecoded); + NS_DispatchToMainThread(runnable); + } } } +void +RasterImage::MarkAnimationDecoded() +{ + MOZ_ASSERT(mAnim, "Should have an animation now"); + if (!mAnim) { + return; + } + + mAnim->SetDoneDecoding(true); +} + NS_IMETHODIMP RasterImage::SetAnimationMode(uint16_t aAnimationMode) { @@ -1452,18 +1468,6 @@ RasterImage::CreateDecoder(const Maybe& aSize, uint32_t aFlags) decoder->SetImageIsLocked(); } - if (aSize) { - // We already have the size; tell the decoder so it can preallocate a - // frame. By default, we create an ARGB frame with no offset. If decoders - // need a different type, they need to ask for it themselves. - // XXX(seth): Note that we call SetSize() and NeedNewFrame() with the - // image's intrinsic size, but AllocateFrame with our target size. - decoder->SetSize(mSize, mOrientation); - decoder->NeedNewFrame(0, 0, 0, aSize->width, aSize->height, - SurfaceFormat::B8G8R8A8); - decoder->AllocateFrame(*aSize); - } - if (aSize && decoder->HasError()) { if (gfxPrefs::ImageDecodeRetryOnAllocFailure() && mRetryCount < 10) { diff --git a/image/RasterImage.h b/image/RasterImage.h index 85ee055b695b..1fcbceea9010 100644 --- a/image/RasterImage.h +++ b/image/RasterImage.h @@ -206,7 +206,10 @@ public: void SetLoopCount(int32_t aLoopCount); /// Notification that the entire image has been decoded. - void OnDecodingComplete(); + void OnDecodingComplete(bool aIsAnimated); + + /// Helper method for OnDecodingComplete. + void MarkAnimationDecoded(); /** * Sends the provided progress notifications to ProgressTracker. diff --git a/image/decoders/nsBMPDecoder.cpp b/image/decoders/nsBMPDecoder.cpp index 03437f9dddc3..64bdc3b59902 100644 --- a/image/decoders/nsBMPDecoder.cpp +++ b/image/decoders/nsBMPDecoder.cpp @@ -437,11 +437,15 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount) return; } } - if (!mImageData) { - PostDecoderError(NS_ERROR_FAILURE); - return; + + MOZ_ASSERT(!mImageData, "Already have a buffer allocated?"); + nsresult rv = AllocateBasicFrame(); + if (NS_FAILED(rv)) { + return; } + MOZ_ASSERT(mImageData, "Should have a buffer now"); + // Prepare for transparency if ((mBIH.compression == BI_RLE8) || (mBIH.compression == BI_RLE4)) { // Clear the image, as the RLE may jump over areas diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index 633551df593e..8a533029695d 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -162,7 +162,7 @@ nsGIFDecoder2::BeginGIF() } //****************************************************************************** -void +nsresult nsGIFDecoder2::BeginImageFrame(uint16_t aDepth) { MOZ_ASSERT(HasSize()); @@ -175,35 +175,31 @@ nsGIFDecoder2::BeginImageFrame(uint16_t aDepth) format = gfx::SurfaceFormat::B8G8R8X8; } + nsIntRect frameRect(mGIFStruct.x_offset, mGIFStruct.y_offset, + mGIFStruct.width, mGIFStruct.height); + // Use correct format, RGB for first frame, PAL for following frames // and include transparency to allow for optimization of opaque images + nsresult rv = NS_OK; if (mGIFStruct.images_decoded) { - // Image data is stored with original depth and palette - NeedNewFrame(mGIFStruct.images_decoded, mGIFStruct.x_offset, - mGIFStruct.y_offset, mGIFStruct.width, mGIFStruct.height, - format, aDepth); + // Image data is stored with original depth and palette. + rv = AllocateFrame(mGIFStruct.images_decoded, GetSize(), + frameRect, format, aDepth); } else { - nsRefPtr currentFrame = GetCurrentFrame(); - - // Our first full frame is automatically created by the image decoding - // infrastructure. Just use it as long as it matches up. - if (!currentFrame->GetRect().IsEqualEdges(nsIntRect(mGIFStruct.x_offset, - mGIFStruct.y_offset, - mGIFStruct.width, - mGIFStruct.height))) { - + if (!nsIntRect(nsIntPoint(), GetSize()).IsEqualEdges(frameRect)) { // We need padding on the first frame, which means that we don't draw into // part of the image at all. Report that as transparency. PostHasTransparency(); - - // Regardless of depth of input, image is decoded into 24bit RGB - NeedNewFrame(mGIFStruct.images_decoded, mGIFStruct.x_offset, - mGIFStruct.y_offset, mGIFStruct.width, mGIFStruct.height, - format); } + + // Regardless of depth of input, the first frame is decoded into 24bit RGB. + rv = AllocateFrame(mGIFStruct.images_decoded, GetSize(), + frameRect, format); } mCurrentFrameIndex = mGIFStruct.images_decoded; + + return rv; } @@ -967,27 +963,13 @@ nsGIFDecoder2::WriteInternal(const char* aBuffer, uint32_t aCount) } // Mask to limit the color values within the colormap mColorMask = 0xFF >> (8 - realDepth); - BeginImageFrame(realDepth); - if (NeedsNewFrame()) { - // We now need a new frame from the decoder framework. We leave all our - // data in the buffer as if it wasn't consumed, copy to our hold and - // return to the decoder framework. - uint32_t size = - len + mGIFStruct.bytes_to_consume + mGIFStruct.bytes_in_hold; - if (size) { - if (SetHold(q, - mGIFStruct.bytes_to_consume + mGIFStruct.bytes_in_hold, - buf, len)) { - // Back into the decoder infrastructure so we can get called again. - GETN(9, gif_image_header_continue); - return; - } - } - break; - } else { - // FALL THROUGH + if (NS_FAILED(BeginImageFrame(realDepth))) { + mGIFStruct.state = gif_error; + return; } + + // FALL THROUGH } case gif_image_header_continue: { diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index 1d2e55301fd4..956c9de613a8 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -35,7 +35,7 @@ private: // frame size information, etc. void BeginGIF(); - void BeginImageFrame(uint16_t aDepth); + nsresult BeginImageFrame(uint16_t aDepth); void EndImageFrame(); void FlushImageData(); void FlushImageData(uint32_t fromRow, uint32_t rows); diff --git a/image/decoders/nsICODecoder.cpp b/image/decoders/nsICODecoder.cpp index e2ec0cea6a99..9dd5c3868170 100644 --- a/image/decoders/nsICODecoder.cpp +++ b/image/decoders/nsICODecoder.cpp @@ -1,4 +1,4 @@ -/* vim:set tw=80 expandtab softtabstop=4 ts=4 sw=4: */ +/* vim:set tw=80 expandtab softtabstop=2 ts=2 sw=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/. */ @@ -80,13 +80,35 @@ nsICODecoder::FinishInternal() // We shouldn't be called in error cases MOZ_ASSERT(!HasError(), "Shouldn't call FinishInternal after error!"); - // Finish the internally used decoder as well - if (mContainedDecoder) { - mContainedDecoder->FinishSharedDecoder(); - mDecodeDone = mContainedDecoder->GetDecodeDone(); - mProgress |= mContainedDecoder->TakeProgress(); - mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect()); + // Finish the internally used decoder as well. + if (mContainedDecoder && !mContainedDecoder->HasError()) { + mContainedDecoder->FinishInternal(); } + + GetFinalStateFromContainedDecoder(); +} + +void +nsICODecoder::FinishWithErrorInternal() +{ + GetFinalStateFromContainedDecoder(); +} + +void +nsICODecoder::GetFinalStateFromContainedDecoder() +{ + if (!mContainedDecoder) { + return; + } + + mDecodeDone = mContainedDecoder->GetDecodeDone(); + 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(); } // Returns a buffer filled with the bitmap file header in little endian: @@ -217,13 +239,8 @@ void nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount) { MOZ_ASSERT(!HasError(), "Shouldn't call WriteInternal after error!"); - - if (!aCount) { - if (mContainedDecoder) { - WriteToContainedDecoder(aBuffer, aCount); - } - return; - } + MOZ_ASSERT(aBuffer); + MOZ_ASSERT(aCount > 0); while (aCount && (mPos < ICONCOUNTOFFSET)) { // Skip to the # of icons. if (mPos == 2) { // if the third byte is 1: This is an icon, 2: a cursor @@ -343,9 +360,7 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount) mContainedDecoder = new nsPNGDecoder(mImage); mContainedDecoder->SetSizeDecode(IsSizeDecode()); mContainedDecoder->SetSendPartialInvalidations(mSendPartialInvalidations); - mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength, - mColormap, mColormapSize, - Move(mRefForContainedDecoder)); + mContainedDecoder->Init(); if (!WriteToContainedDecoder(mSignature, PNGSIGNATURESIZE)) { return; } @@ -422,9 +437,7 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount) bmpDecoder->SetUseAlphaData(true); mContainedDecoder->SetSizeDecode(IsSizeDecode()); mContainedDecoder->SetSendPartialInvalidations(mSendPartialInvalidations); - mContainedDecoder->InitSharedDecoder(mImageData, mImageDataLength, - mColormap, mColormapSize, - Move(mRefForContainedDecoder)); + mContainedDecoder->Init(); // The ICO format when containing a BMP does not include the 14 byte // bitmap file header. To use the code of the BMP decoder we need to @@ -627,35 +640,5 @@ nsICODecoder::ProcessDirEntry(IconDirEntry& aTarget) aTarget.mImageOffset = LittleEndian::readUint32(&aTarget.mImageOffset); } -bool -nsICODecoder::NeedsNewFrame() const -{ - if (mContainedDecoder) { - return mContainedDecoder->NeedsNewFrame(); - } - - return Decoder::NeedsNewFrame(); -} - -nsresult -nsICODecoder::AllocateFrame(const nsIntSize& aTargetSize /* = nsIntSize() */) -{ - nsresult rv; - - if (mContainedDecoder) { - rv = mContainedDecoder->AllocateFrame(aTargetSize); - mCurrentFrame = mContainedDecoder->GetCurrentFrameRef(); - mProgress |= mContainedDecoder->TakeProgress(); - mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect()); - return rv; - } - - // Grab a strong ref that we'll later hand over to the contained decoder. This - // lets us avoid creating a RawAccessFrameRef off-main-thread. - rv = Decoder::AllocateFrame(aTargetSize); - mRefForContainedDecoder = GetCurrentFrameRef(); - return rv; -} - } // namespace image } // namespace mozilla diff --git a/image/decoders/nsICODecoder.h b/image/decoders/nsICODecoder.h index 787d699025d5..8bc1532120f9 100644 --- a/image/decoders/nsICODecoder.h +++ b/image/decoders/nsICODecoder.h @@ -40,17 +40,16 @@ public: virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override; virtual void FinishInternal() override; - virtual nsresult AllocateFrame(const nsIntSize& aTargetSize - /* = nsIntSize() */) override; - -protected: - virtual bool NeedsNewFrame() const override; + virtual void FinishWithErrorInternal() override; private: // Writes to the contained decoder and sets the appropriate errors // Returns true if there are no errors. bool WriteToContainedDecoder(const char* aBuffer, uint32_t aCount); + // Gets decoder state from the contained decoder so it's visible externally. + void GetFinalStateFromContainedDecoder(); + // Processes a single dir entry of the icon resource void ProcessDirEntry(IconDirEntry& aTarget); // Sets the hotspot property of if we have a cursor @@ -84,7 +83,6 @@ private: uint32_t mRowBytes; // How many bytes of the row were already received int32_t mOldLine; // Previous index of the line nsRefPtr mContainedDecoder; // Contains either a BMP or PNG resource - RawAccessFrameRef mRefForContainedDecoder; // Avoid locking off-main-thread char mDirEntryArray[ICODIRENTRYSIZE]; // Holds the current dir entry buffer IconDirEntry mDirEntry; // Holds a decoded dir entry diff --git a/image/decoders/nsIconDecoder.cpp b/image/decoders/nsIconDecoder.cpp index 63bc7d8f35b6..661183259ca5 100644 --- a/image/decoders/nsIconDecoder.cpp +++ b/image/decoders/nsIconDecoder.cpp @@ -73,11 +73,17 @@ nsIconDecoder::WriteInternal(const char* aBuffer, uint32_t aCount) break; } - if (!mImageData) { - PostDecoderError(NS_ERROR_OUT_OF_MEMORY); - return; + { + MOZ_ASSERT(!mImageData, "Already have a buffer allocated?"); + nsresult rv = AllocateBasicFrame(); + if (NS_FAILED(rv)) { + mState = iconStateFinished; + return; + } } + MOZ_ASSERT(mImageData, "Should have a buffer now"); + // Book Keeping aBuffer++; aCount--; diff --git a/image/decoders/nsJPEGDecoder.cpp b/image/decoders/nsJPEGDecoder.cpp index 63e95a95b834..3bdeca666467 100644 --- a/image/decoders/nsJPEGDecoder.cpp +++ b/image/decoders/nsJPEGDecoder.cpp @@ -396,14 +396,20 @@ nsJPEGDecoder::WriteInternal(const char* aBuffer, uint32_t aCount) mInfo.buffered_image = mDecodeStyle == PROGRESSIVE && jpeg_has_multiple_scans(&mInfo); - if (!mImageData) { + MOZ_ASSERT(!mImageData, "Already have a buffer allocated?"); + nsIntSize targetSize = mDownscaler ? mDownscaler->TargetSize() : GetSize(); + nsresult rv = AllocateFrame(0, targetSize, + nsIntRect(nsIntPoint(), targetSize), + gfx::SurfaceFormat::B8G8R8A8); + if (NS_FAILED(rv)) { mState = JPEG_ERROR; - PostDecoderError(NS_ERROR_OUT_OF_MEMORY); MOZ_LOG(GetJPEGDecoderAccountingLog(), LogLevel::Debug, ("} (could not initialize image frame)")); return; } + MOZ_ASSERT(mImageData, "Should have a buffer now"); + if (mDownscaler) { nsresult rv = mDownscaler->BeginFrame(GetSize(), mImageData, diff --git a/image/decoders/nsPNGDecoder.cpp b/image/decoders/nsPNGDecoder.cpp index bc5286a11330..4124c04acc7b 100644 --- a/image/decoders/nsPNGDecoder.cpp +++ b/image/decoders/nsPNGDecoder.cpp @@ -141,38 +141,43 @@ nsPNGDecoder::~nsPNGDecoder() } // CreateFrame() is used for both simple and animated images -void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset, - int32_t width, int32_t height, - gfx::SurfaceFormat format) +nsresult +nsPNGDecoder::CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset, + int32_t aWidth, int32_t aHeight, + gfx::SurfaceFormat aFormat) { MOZ_ASSERT(HasSize()); - if (format == gfx::SurfaceFormat::B8G8R8A8) { + if (aFormat == gfx::SurfaceFormat::B8G8R8A8) { PostHasTransparency(); } - // Our first full frame is automatically created by the image decoding - // infrastructure. Just use it as long as it matches up. - nsIntRect neededRect(x_offset, y_offset, width, height); - nsRefPtr currentFrame = GetCurrentFrame(); - if (!currentFrame->GetRect().IsEqualEdges(neededRect)) { - if (mNumFrames == 0) { - // We need padding on the first frame, which means that we don't draw into - // part of the image at all. Report that as transparency. - PostHasTransparency(); - } - - NeedNewFrame(mNumFrames, x_offset, y_offset, width, height, format); - } else if (mNumFrames != 0) { - NeedNewFrame(mNumFrames, x_offset, y_offset, width, height, format); + nsIntRect frameRect(aXOffset, aYOffset, aWidth, aHeight); + if (mNumFrames == 0 && + !nsIntRect(nsIntPoint(), GetSize()).IsEqualEdges(frameRect)) { + // We need padding on the first frame, which means that we don't draw into + // part of the image at all. Report that as transparency. + PostHasTransparency(); } - mFrameRect = neededRect; + // XXX(seth): Some tests depend on the first frame of PNGs being B8G8R8A8. + // This is something we should fix. + gfx::SurfaceFormat format = aFormat; + if (mNumFrames == 0) { + format = gfx::SurfaceFormat::B8G8R8A8; + } + + nsresult rv = AllocateFrame(mNumFrames, GetSize(), frameRect, format); + if (NS_FAILED(rv)) { + return rv; + } + + mFrameRect = frameRect; MOZ_LOG(GetPNGDecoderAccountingLog(), LogLevel::Debug, ("PNGDecoderAccounting: nsPNGDecoder::CreateFrame -- created " "image frame with %dx%d pixels in container %p", - width, height, + aWidth, aHeight, &mImage)); #ifdef PNG_APNG_SUPPORTED @@ -186,6 +191,8 @@ void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset, } } #endif + + return NS_OK; } // set timeout and frame disposal method for the current frame @@ -651,7 +658,11 @@ nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) decoder->mFrameIsHidden = true; } else { #endif - decoder->CreateFrame(0, 0, width, height, decoder->format); + nsresult rv = decoder->CreateFrame(0, 0, width, height, decoder->format); + if (NS_FAILED(rv)) { + png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY + } + MOZ_ASSERT(decoder->mImageData, "Should have a buffer now"); #ifdef PNG_APNG_SUPPORTED } #endif @@ -674,12 +685,6 @@ nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY } } - - if (decoder->NeedsNewFrame()) { - // We know that we need a new frame, so pause input so the decoder - // infrastructure can give it to us. - png_process_data_pause(png_ptr, /* save = */ 1); - } } void @@ -831,13 +836,12 @@ nsPNGDecoder::frame_info_callback(png_structp png_ptr, png_uint_32 frame_num) width = png_get_next_frame_width(png_ptr, decoder->mInfo); height = png_get_next_frame_height(png_ptr, decoder->mInfo); - decoder->CreateFrame(x_offset, y_offset, width, height, decoder->format); - - if (decoder->NeedsNewFrame()) { - // We know that we need a new frame, so pause input so the decoder - // infrastructure can give it to us. - png_process_data_pause(png_ptr, /* save = */ 1); + nsresult rv = + decoder->CreateFrame(x_offset, y_offset, width, height, decoder->format); + if (NS_FAILED(rv)) { + png_longjmp(decoder->mPNG, 5); // NS_ERROR_OUT_OF_MEMORY } + MOZ_ASSERT(decoder->mImageData, "Should have a buffer now"); } #endif diff --git a/image/decoders/nsPNGDecoder.h b/image/decoders/nsPNGDecoder.h index 2c976dac9d84..ac2bf3cae0f5 100644 --- a/image/decoders/nsPNGDecoder.h +++ b/image/decoders/nsPNGDecoder.h @@ -31,9 +31,9 @@ public: virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override; virtual Telemetry::ID SpeedHistogram() override; - void CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset, - int32_t width, int32_t height, - gfx::SurfaceFormat format); + nsresult CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset, + int32_t aWidth, int32_t aHeight, + gfx::SurfaceFormat aFormat); void EndImageFrame(); // Check if PNG is valid ICO (32bpp RGBA) diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp index 1e0546f17f39..c699bbc95e5e 100644 --- a/image/imgFrame.cpp +++ b/image/imgFrame.cpp @@ -167,80 +167,6 @@ imgFrame::~imgFrame() mPalettedImageData = nullptr; } -nsresult -imgFrame::ReinitForDecoder(const nsIntSize& aImageSize, - const nsIntRect& aRect, - SurfaceFormat aFormat, - uint8_t aPaletteDepth /* = 0 */, - bool aNonPremult /* = false */) -{ - MonitorAutoLock lock(mMonitor); - - if (mDecoded.x != 0 || mDecoded.y != 0 || - mDecoded.width != 0 || mDecoded.height != 0) { - MOZ_ASSERT_UNREACHABLE("Shouldn't reinit after write"); - return NS_ERROR_FAILURE; - } - if (mAborted) { - MOZ_ASSERT_UNREACHABLE("Shouldn't reinit if aborted"); - return NS_ERROR_FAILURE; - } - if (mLockCount < 1) { - MOZ_ASSERT_UNREACHABLE("Shouldn't reinit unless locked"); - return NS_ERROR_FAILURE; - } - - // Restore everything (except mLockCount, which we need to keep) to how it was - // when we were first created. - // XXX(seth): This is probably a little excessive, but I want to be *really* - // sure that nothing got missed. - mDecoded = nsIntRect(0, 0, 0, 0); - mTimeout = 100; - mDisposalMethod = DisposalMethod::NOT_SPECIFIED; - mBlendMethod = BlendMethod::OVER; - mHasNoAlpha = false; - mAborted = false; - mPaletteDepth = 0; - mNonPremult = false; - mSinglePixel = false; - mCompositingFailed = false; - mOptimizable = false; - mImageSize = IntSize(); - mSize = IntSize(); - mOffset = nsIntPoint(); - mSinglePixelColor = Color(); - - // Release all surfaces. - mImageSurface = nullptr; - mOptSurface = nullptr; - mVBuf = nullptr; - mVBufPtr = nullptr; - free(mPalettedImageData); - mPalettedImageData = nullptr; - - // Reinitialize. - nsresult rv = InitForDecoder(aImageSize, aRect, aFormat, - aPaletteDepth, aNonPremult); - if (NS_FAILED(rv)) { - return rv; - } - - // We were locked before; perform the same actions we would've performed when - // we originally got locked. - if (mImageSurface) { - mVBufPtr = mVBuf; - return NS_OK; - } - - if (!mPalettedImageData) { - MOZ_ASSERT_UNREACHABLE("We got optimized somehow during reinit"); - return NS_ERROR_FAILURE; - } - - // Paletted images don't have surfaces, so there's nothing to do. - return NS_OK; -} - nsresult imgFrame::InitForDecoder(const nsIntSize& aImageSize, const nsIntRect& aRect, diff --git a/image/imgFrame.h b/image/imgFrame.h index 460ff837b812..9c9a0262128f 100644 --- a/image/imgFrame.h +++ b/image/imgFrame.h @@ -157,19 +157,6 @@ public: GraphicsFilter aFilter, uint32_t aImageFlags); - /** - * Reinitializes an existing imgFrame with new parameters. You must be holding - * a RawAccessFrameRef to the imgFrame, and it must never have been written - * to, marked finished, or aborted. - * - * XXX(seth): We will remove this in bug 1117607. - */ - nsresult ReinitForDecoder(const nsIntSize& aImageSize, - const nsIntRect& aRect, - SurfaceFormat aFormat, - uint8_t aPaletteDepth = 0, - bool aNonPremult = false); - DrawableFrameRef DrawableRef(); RawAccessFrameRef RawAccessRef();