From 932efed7c89c69616e283fdfef65e86b9d9da381 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Fri, 16 Dec 2016 11:39:51 -0500 Subject: GIF: Avoid copying/storing data when possible If the input SkStream has a length and position, do not copy and store LZW blocks or ColorMaps. Instead, mark the position and size, and read from the stream when necessary. This will save memory in Chromium's use case, which has already buffered all of its data. In the case where we *do* need to copy, store it on the SkStreamBuffer. This allows SkGifImageReader to have simpler code. Add tests. Change-Id: Ic65fa766328ae2e5974b2084bc2099e19aced731 Reviewed-on: https://skia-review.googlesource.com/6157 Reviewed-by: Matt Sarett Commit-Queue: Leon Scroggins --- third_party/gif/SkGifImageReader.cpp | 56 ++++++++++++++++++++++-------------- third_party/gif/SkGifImageReader.h | 42 ++++++++++++++++++--------- 2 files changed, 63 insertions(+), 35 deletions(-) (limited to 'third_party/gif') diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp index 9a14018734..ae5663f651 100644 --- a/third_party/gif/SkGifImageReader.cpp +++ b/third_party/gif/SkGifImageReader.cpp @@ -305,7 +305,8 @@ bool SkGIFLZWContext::doLZW(const unsigned char* block, size_t bytesInBlock) return true; } -sk_sp SkGIFColorMap::buildTable(SkColorType colorType, size_t transparentPixel) const +sk_sp SkGIFColorMap::buildTable(SkStreamBuffer* streamBuffer, SkColorType colorType, + size_t transparentPixel) const { if (!m_isDefined) return nullptr; @@ -323,8 +324,14 @@ sk_sp SkGIFColorMap::buildTable(SkColorType colorType, size_t tran } m_packColorProc = proc; + const size_t bytes = m_colors * SK_BYTES_PER_COLORMAP_ENTRY; + sk_sp rawData(streamBuffer->getDataAtPosition(m_position, bytes)); + if (!rawData) { + return nullptr; + } + SkASSERT(m_colors <= SK_MAX_COLORS); - const uint8_t* srcColormap = m_rawData->bytes(); + const uint8_t* srcColormap = rawData->bytes(); SkPMColor colorStorage[SK_MAX_COLORS]; for (size_t i = 0; i < m_colors; i++) { if (i == transparentPixel) { @@ -341,18 +348,19 @@ sk_sp SkGIFColorMap::buildTable(SkColorType colorType, size_t tran return m_table; } -sk_sp SkGifImageReader::getColorTable(SkColorType colorType, size_t index) const { +sk_sp SkGifImageReader::getColorTable(SkColorType colorType, size_t index) { if (index >= m_frames.size()) { return nullptr; } const SkGIFFrameContext* frameContext = m_frames[index].get(); const SkGIFColorMap& localColorMap = frameContext->localColorMap(); + const size_t transPix = frameContext->transparentPixel(); if (localColorMap.isDefined()) { - return localColorMap.buildTable(colorType, frameContext->transparentPixel()); + return localColorMap.buildTable(&m_streamBuffer, colorType, transPix); } if (m_globalColorMap.isDefined()) { - return m_globalColorMap.buildTable(colorType, frameContext->transparentPixel()); + return m_globalColorMap.buildTable(&m_streamBuffer, colorType, transPix); } return nullptr; } @@ -360,7 +368,8 @@ sk_sp SkGifImageReader::getColorTable(SkColorType colorType, size_ // Perform decoding for this frame. frameComplete will be true if the entire frame is decoded. // Returns false if a decoding error occurred. This is a fatal error and causes the SkGifImageReader to set the "decode failed" flag. // Otherwise, either not enough data is available to decode further than before, or the new data has been decoded successfully; returns true in this case. -bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete) +bool SkGIFFrameContext::decode(SkStreamBuffer* streamBuffer, SkGifCodec* client, + bool* frameComplete) { *frameComplete = false; if (!m_lzwContext) { @@ -379,8 +388,14 @@ bool SkGIFFrameContext::decode(SkGifCodec* client, bool* frameComplete) // Some bad GIFs have extra blocks beyond the last row, which we don't want to decode. while (m_currentLzwBlock < m_lzwBlocks.size() && m_lzwContext->hasRemainingRows()) { - if (!m_lzwContext->doLZW(reinterpret_cast(m_lzwBlocks[m_currentLzwBlock]->data()), - m_lzwBlocks[m_currentLzwBlock]->size())) { + const auto& block = m_lzwBlocks[m_currentLzwBlock]; + const size_t len = block.blockSize; + + sk_sp data(streamBuffer->getDataAtPosition(block.blockPosition, len)); + if (!data) { + return false; + } + if (!m_lzwContext->doLZW(reinterpret_cast(data->data()), len)) { return false; } ++m_currentLzwBlock; @@ -402,7 +417,7 @@ bool SkGifImageReader::decode(size_t frameIndex, bool* frameComplete) { SkGIFFrameContext* currentFrame = m_frames[frameIndex].get(); - return currentFrame->decode(m_client, frameComplete); + return currentFrame->decode(&m_streamBuffer, m_client, frameComplete); } // Parse incoming GIF data stream into internal data structures. @@ -428,22 +443,19 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) } while (true) { - const size_t bytesBuffered = m_streamBuffer.buffer(m_bytesToConsume); - if (bytesBuffered < m_bytesToConsume) { - // The stream does not yet have enough data. Mark that we need less next time around, - // and return. - m_bytesToConsume -= bytesBuffered; + if (!m_streamBuffer.buffer(m_bytesToConsume)) { + // The stream does not yet have enough data. return true; } switch (m_state) { - case SkGIFLZW: + case SkGIFLZW: { SkASSERT(!m_frames.empty()); - // FIXME: All this copying might be wasteful for e.g. SkMemoryStream - m_frames.back()->addLzwBlock(m_streamBuffer.get(), m_streamBuffer.bytesBuffered()); + auto* frame = m_frames.back().get(); + frame->addLzwBlock(m_streamBuffer.markPosition(), m_bytesToConsume); GETN(1, SkGIFSubBlock); break; - + } case SkGIFLZWStart: { SkASSERT(!m_frames.empty()); m_frames.back()->setDataSize(this->getOneByte()); @@ -494,7 +506,7 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) } case SkGIFGlobalColormap: { - m_globalColorMap.setRawData(m_streamBuffer.get(), m_streamBuffer.bytesBuffered()); + m_globalColorMap.setTablePosition(m_streamBuffer.markPosition()); GETN(1, SkGIFImageStart); break; } @@ -631,7 +643,7 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) case SkGIFApplicationExtension: { // Check for netscape application extension. - if (m_streamBuffer.bytesBuffered() == 11) { + if (m_bytesToConsume == 11) { const unsigned char* currentComponent = reinterpret_cast(m_streamBuffer.get()); @@ -789,7 +801,6 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) // The decoder needs to stop, so we return here, before // flushing the buffer. Next time through, we'll be in the same // state, requiring the same amount in the buffer. - m_bytesToConsume = 0; return true; } @@ -820,7 +831,8 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) case SkGIFImageColormap: { SkASSERT(!m_frames.empty()); - m_frames.back()->localColorMap().setRawData(m_streamBuffer.get(), m_streamBuffer.bytesBuffered()); + auto& cmap = m_frames.back()->localColorMap(); + cmap.setTablePosition(m_streamBuffer.markPosition()); GETN(1, SkGIFLZWStart); break; } diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h index 2843a3538f..0481ba2908 100644 --- a/third_party/gif/SkGifImageReader.h +++ b/third_party/gif/SkGifImageReader.h @@ -137,37 +137,51 @@ private: const SkGIFFrameContext* m_frameContext; }; +struct SkGIFLZWBlock { + public: + SkGIFLZWBlock(size_t position, size_t size) + : blockPosition(position), blockSize(size) {} + + const size_t blockPosition; + const size_t blockSize; +}; + class SkGIFColorMap final { public: SkGIFColorMap() : m_isDefined(false) + , m_position(0) , m_colors(0) , m_packColorProc(nullptr) { } void setNumColors(size_t colors) { + SkASSERT(!m_colors); + SkASSERT(!m_position); + m_colors = colors; } - size_t numColors() const { return m_colors; } + void setTablePosition(size_t position) { + SkASSERT(!m_isDefined); - void setRawData(const char* data, size_t size) - { - // FIXME: Can we avoid this copy? - m_rawData = SkData::MakeWithCopy(data, size); - SkASSERT(m_colors * SK_BYTES_PER_COLORMAP_ENTRY == size); + m_position = position; m_isDefined = true; } + + size_t numColors() const { return m_colors; } + bool isDefined() const { return m_isDefined; } // Build RGBA table using the data stream. - sk_sp buildTable(SkColorType dstColorType, size_t transparentPixel) const; + sk_sp buildTable(SkStreamBuffer*, SkColorType dstColorType, + size_t transparentPixel) const; private: bool m_isDefined; + size_t m_position; size_t m_colors; - sk_sp m_rawData; mutable PackColorProc m_packColorProc; mutable sk_sp m_table; }; @@ -201,12 +215,12 @@ public: { } - void addLzwBlock(const void* data, size_t size) + void addLzwBlock(size_t position, size_t size) { - m_lzwBlocks.push_back(SkData::MakeWithCopy(data, size)); + m_lzwBlocks.push_back(SkGIFLZWBlock(position, size)); } - bool decode(SkGifCodec* client, bool* frameDecoded); + bool decode(SkStreamBuffer*, SkGifCodec* client, bool* frameDecoded); int frameId() const { return m_frameId; } void setRect(unsigned x, unsigned y, unsigned width, unsigned height) @@ -266,7 +280,9 @@ private: unsigned m_delayTime; // Display time, in milliseconds, for this image in a multi-image GIF. std::unique_ptr m_lzwContext; - std::vector> m_lzwBlocks; // LZW blocks for this frame. + // LZW blocks for this frame. + std::vector m_lzwBlocks; + SkGIFColorMap m_localColorMap; size_t m_currentLzwBlock; @@ -359,7 +375,7 @@ public: } // Return the color table for frame index (which may be the global color table). - sk_sp getColorTable(SkColorType dstColorType, size_t index) const; + sk_sp getColorTable(SkColorType dstColorType, size_t index); bool firstFrameHasAlpha() const { return m_firstFrameHasAlpha; } -- cgit v1.2.3