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 <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

2 files changed, 63 insertions, 35 deletions

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<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t transparentPixel) const
+sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkStreamBuffer* streamBuffer, SkColorType colorType,
+                                              size_t transparentPixel) const
 {
     if (!m_isDefined)
         return nullptr;
@@ -323,8 +324,14 @@ sk_sp<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t tran
     }
     m_packColorProc = proc;
 
+    const size_t bytes = m_colors * SK_BYTES_PER_COLORMAP_ENTRY;
+    sk_sp<SkData> 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<SkColorTable> SkGIFColorMap::buildTable(SkColorType colorType, size_t tran
     return m_table;
 }
 
-sk_sp<SkColorTable> SkGifImageReader::getColorTable(SkColorType colorType, size_t index) const {
+sk_sp<SkColorTable> 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<SkColorTable> 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<const unsigned char*>(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<SkData> data(streamBuffer->getDataAtPosition(block.blockPosition, len));
+        if (!data) {
+            return false;
+        }
+        if (!m_lzwContext->doLZW(reinterpret_cast<const unsigned char*>(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<const unsigned char*>(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<SkColorTable> buildTable(SkColorType dstColorType, size_t transparentPixel) const;
+    sk_sp<SkColorTable> buildTable(SkStreamBuffer*, SkColorType dstColorType,
+                                   size_t transparentPixel) const;
 
 private:
     bool m_isDefined;
+    size_t m_position;
     size_t m_colors;
-    sk_sp<SkData> m_rawData;
     mutable PackColorProc m_packColorProc;
     mutable sk_sp<SkColorTable> 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<SkGIFLZWContext> m_lzwContext;
-    std::vector<sk_sp<SkData>> m_lzwBlocks; // LZW blocks for this frame.
+    // LZW blocks for this frame.
+    std::vector<SkGIFLZWBlock> 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<SkColorTable> getColorTable(SkColorType dstColorType, size_t index) const;
+    sk_sp<SkColorTable> getColorTable(SkColorType dstColorType, size_t index);
 
     bool firstFrameHasAlpha() const { return m_firstFrameHasAlpha; }