From cb1d37e391537bbbcefd130f9acb06c16b1dabef Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Fri, 25 May 2018 06:52:03 -0400 Subject: [PATCH] Bug 1382683 - Part 2. Switch nsGIFDecoder2 to write pixels in blocks instead of individually. r=tnikkel nsGIFDecoder2::YieldPixel is sufficiently complex that the optimizer does not appear to inline it with the rest of the templated methods. As such there is a high cost to calling it. This patch modifies it to yield a requested number of pixels before exiting, allowing us to amortize the cost of calling across a row instead of a pixel. Based on profiling, this will significantly reduce the time require to decode a frame. --- image/decoders/nsGIFDecoder2.cpp | 209 +++++++++++++++++-------------- image/decoders/nsGIFDecoder2.h | 8 +- 2 files changed, 119 insertions(+), 98 deletions(-) diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index 013c85e58894..f649de48fa20 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -292,10 +292,12 @@ nsGIFDecoder2::ColormapIndexToPixel(uint8_t aIndex) } template -NextPixel -nsGIFDecoder2::YieldPixel(const uint8_t* aData, - size_t aLength, - size_t* aBytesReadOut) +Tuple> +nsGIFDecoder2::YieldPixels(const uint8_t* aData, + size_t aLength, + size_t* aBytesReadOut, + PixelSize* aPixelBlock, + int32_t aBlockSize) { MOZ_ASSERT(aData); MOZ_ASSERT(aBytesReadOut); @@ -304,108 +306,119 @@ nsGIFDecoder2::YieldPixel(const uint8_t* aData, // Advance to the next byte we should read. const uint8_t* data = aData + *aBytesReadOut; - // If we don't have any decoded data to yield, try to read some input and - // produce some. - if (mGIFStruct.stackp == mGIFStruct.stack) { - while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) { - // Feed the next byte into the decoder's 32-bit input buffer. - mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits; - mGIFStruct.bits += 8; - data += 1; - *aBytesReadOut += 1; - } - - if (mGIFStruct.bits < mGIFStruct.codesize) { - return AsVariant(WriteState::NEED_MORE_DATA); - } - - // Get the leading variable-length symbol from the data stream. - int code = mGIFStruct.datum & mGIFStruct.codemask; - mGIFStruct.datum >>= mGIFStruct.codesize; - mGIFStruct.bits -= mGIFStruct.codesize; - - const int clearCode = ClearCode(); - - // Reset the dictionary to its original state, if requested - if (code == clearCode) { - mGIFStruct.codesize = mGIFStruct.datasize + 1; - mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1; - mGIFStruct.avail = clearCode + 2; - mGIFStruct.oldcode = -1; - return AsVariant(WriteState::NEED_MORE_DATA); - } - - // Check for explicit end-of-stream code. It should only appear after all - // image data, but if that was the case we wouldn't be in this function, so - // this is always an error condition. - if (code == (clearCode + 1)) { - return AsVariant(WriteState::FAILURE); - } - - if (mGIFStruct.oldcode == -1) { - if (code >= MAX_BITS) { - return AsVariant(WriteState::FAILURE); // The code's too big; something's wrong. + int32_t written = 0; + while (aBlockSize > written) { + // If we don't have any decoded data to yield, try to read some input and + // produce some. + if (mGIFStruct.stackp == mGIFStruct.stack) { + while (mGIFStruct.bits < mGIFStruct.codesize && *aBytesReadOut < aLength) { + // Feed the next byte into the decoder's 32-bit input buffer. + mGIFStruct.datum += int32_t(*data) << mGIFStruct.bits; + mGIFStruct.bits += 8; + data += 1; + *aBytesReadOut += 1; } - mGIFStruct.firstchar = mGIFStruct.oldcode = code; - - // Yield a pixel at the appropriate index in the colormap. - mGIFStruct.pixels_remaining--; - return AsVariant(ColormapIndexToPixel(mGIFStruct.suffix[code])); - } - - int incode = code; - if (code >= mGIFStruct.avail) { - *mGIFStruct.stackp++ = mGIFStruct.firstchar; - code = mGIFStruct.oldcode; - - if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { - return AsVariant(WriteState::FAILURE); // Stack overflow; something's wrong. - } - } - - while (code >= clearCode) { - if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) { - return AsVariant(WriteState::FAILURE); + if (mGIFStruct.bits < mGIFStruct.codesize) { + return MakeTuple(written, Some(WriteState::NEED_MORE_DATA)); } - *mGIFStruct.stackp++ = mGIFStruct.suffix[code]; - code = mGIFStruct.prefix[code]; + // Get the leading variable-length symbol from the data stream. + int code = mGIFStruct.datum & mGIFStruct.codemask; + mGIFStruct.datum >>= mGIFStruct.codesize; + mGIFStruct.bits -= mGIFStruct.codesize; - if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { - return AsVariant(WriteState::FAILURE); // Stack overflow; something's wrong. + const int clearCode = ClearCode(); + + // Reset the dictionary to its original state, if requested + if (code == clearCode) { + mGIFStruct.codesize = mGIFStruct.datasize + 1; + mGIFStruct.codemask = (1 << mGIFStruct.codesize) - 1; + mGIFStruct.avail = clearCode + 2; + mGIFStruct.oldcode = -1; + return MakeTuple(written, Some(WriteState::NEED_MORE_DATA)); } + + // Check for explicit end-of-stream code. It should only appear after all + // image data, but if that was the case we wouldn't be in this function, so + // this is always an error condition. + if (code == (clearCode + 1)) { + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + if (mGIFStruct.oldcode == -1) { + if (code >= MAX_BITS) { + // The code's too big; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + mGIFStruct.firstchar = mGIFStruct.oldcode = code; + + // Yield a pixel at the appropriate index in the colormap. + mGIFStruct.pixels_remaining--; + aPixelBlock[written++] = + ColormapIndexToPixel(mGIFStruct.suffix[code]); + continue; + } + + int incode = code; + if (code >= mGIFStruct.avail) { + *mGIFStruct.stackp++ = mGIFStruct.firstchar; + code = mGIFStruct.oldcode; + + if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { + // Stack overflow; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + } + + while (code >= clearCode) { + if ((code >= MAX_BITS) || (code == mGIFStruct.prefix[code])) { + return MakeTuple(written, Some(WriteState::FAILURE)); + } + + *mGIFStruct.stackp++ = mGIFStruct.suffix[code]; + code = mGIFStruct.prefix[code]; + + if (mGIFStruct.stackp >= mGIFStruct.stack + MAX_BITS) { + // Stack overflow; something's wrong. + return MakeTuple(written, Some(WriteState::FAILURE)); + } + } + + *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code]; + + // Define a new codeword in the dictionary. + if (mGIFStruct.avail < 4096) { + mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode; + mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar; + mGIFStruct.avail++; + + // If we've used up all the codewords of a given length increase the + // length of codewords by one bit, but don't exceed the specified maximum + // codeword size of 12 bits. + if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) && + (mGIFStruct.avail < 4096)) { + mGIFStruct.codesize++; + mGIFStruct.codemask += mGIFStruct.avail; + } + } + + mGIFStruct.oldcode = incode; } - *mGIFStruct.stackp++ = mGIFStruct.firstchar = mGIFStruct.suffix[code]; - - // Define a new codeword in the dictionary. - if (mGIFStruct.avail < 4096) { - mGIFStruct.prefix[mGIFStruct.avail] = mGIFStruct.oldcode; - mGIFStruct.suffix[mGIFStruct.avail] = mGIFStruct.firstchar; - mGIFStruct.avail++; - - // If we've used up all the codewords of a given length increase the - // length of codewords by one bit, but don't exceed the specified maximum - // codeword size of 12 bits. - if (((mGIFStruct.avail & mGIFStruct.codemask) == 0) && - (mGIFStruct.avail < 4096)) { - mGIFStruct.codesize++; - mGIFStruct.codemask += mGIFStruct.avail; - } + if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) { + MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?"); + return MakeTuple(written, Some(WriteState::FAILURE)); } - mGIFStruct.oldcode = incode; + // Yield a pixel at the appropriate index in the colormap. + mGIFStruct.pixels_remaining--; + aPixelBlock[written++] + = ColormapIndexToPixel(*--mGIFStruct.stackp); } - if (MOZ_UNLIKELY(mGIFStruct.stackp <= mGIFStruct.stack)) { - MOZ_ASSERT_UNREACHABLE("No decoded data but we didn't return early?"); - return AsVariant(WriteState::FAILURE); - } - - // Yield a pixel at the appropriate index in the colormap. - mGIFStruct.pixels_remaining--; - return AsVariant(ColormapIndexToPixel(*--mGIFStruct.stackp)); + return MakeTuple(written, Maybe()); } /// Expand the colormap from RGB to Packed ARGB as needed by Cairo. @@ -1032,8 +1045,12 @@ nsGIFDecoder2::ReadLZWData(const char* aData, size_t aLength) size_t bytesRead = 0; auto result = mGIFStruct.images_decoded == 0 - ? mPipe.WritePixels([&]{ return YieldPixel(data, length, &bytesRead); }) - : mPipe.WritePixels([&]{ return YieldPixel(data, length, &bytesRead); }); + ? mPipe.WritePixelBlocks([&](uint32_t* aPixelBlock, int32_t aBlockSize) { + return YieldPixels(data, length, &bytesRead, aPixelBlock, aBlockSize); + }) + : mPipe.WritePixelBlocks([&](uint8_t* aPixelBlock, int32_t aBlockSize) { + return YieldPixels(data, length, &bytesRead, aPixelBlock, aBlockSize); + }); if (MOZ_UNLIKELY(bytesRead > length)) { MOZ_ASSERT_UNREACHABLE("Overread?"); diff --git a/image/decoders/nsGIFDecoder2.h b/image/decoders/nsGIFDecoder2.h index c95b94a32b54..03aa7ba7d54c 100644 --- a/image/decoders/nsGIFDecoder2.h +++ b/image/decoders/nsGIFDecoder2.h @@ -65,8 +65,12 @@ private: ColormapIndexToPixel(uint8_t aIndex); /// A generator function that performs LZW decompression and yields pixels. - template NextPixel - YieldPixel(const uint8_t* aData, size_t aLength, size_t* aBytesReadOut); + template Tuple> + YieldPixels(const uint8_t* aData, + size_t aLength, + size_t* aBytesReadOut, + PixelSize* aPixelBlock, + int32_t aBlockSize); /// Checks if we have transparency, either because the header indicates that /// there's alpha, or because the frame rect doesn't cover the entire image.