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, 132 insertions, 18 deletions

diff --git a/src/codec/SkStreamBuffer.cpp b/src/codec/SkStreamBuffer.cpp
index 1e6063706e..754065d97a 100644
--- a/src/codec/SkStreamBuffer.cpp
+++ b/src/codec/SkStreamBuffer.cpp
@@ -9,15 +9,79 @@
 
 SkStreamBuffer::SkStreamBuffer(SkStream* stream)
     : fStream(stream)
+    , fPosition(0)
     , fBytesBuffered(0)
+    , fHasLengthAndPosition(stream->hasLength() && stream->hasPosition())
+    , fTrulyBuffered(0)
 {}
 
-size_t SkStreamBuffer::buffer(size_t bytesToBuffer) {
+SkStreamBuffer::~SkStreamBuffer() {
+    fMarkedData.foreach([](size_t, SkData** data) { (*data)->unref(); });
+}
+
+const char* SkStreamBuffer::get() const {
+    SkASSERT(fBytesBuffered >= 1);
+    if (fHasLengthAndPosition && fTrulyBuffered < fBytesBuffered) {
+        const size_t bytesToBuffer = fBytesBuffered - fTrulyBuffered;
+        char* dst = SkTAddOffset<char>(const_cast<char*>(fBuffer), fTrulyBuffered);
+        SkDEBUGCODE(const size_t bytesRead =)
+        // This stream is rewindable, so it should be safe to call the non-const
+        // read()
+        const_cast<SkStream*>(fStream.get())->read(dst, bytesToBuffer);
+        SkASSERT(bytesRead == bytesToBuffer);
+        fTrulyBuffered = fBytesBuffered;
+    }
+    return fBuffer;
+}
+
+bool SkStreamBuffer::buffer(size_t totalBytesToBuffer) {
     // FIXME (scroggo): What should we do if the client tries to read too much?
     // Should not be a problem in GIF.
-    SkASSERT(fBytesBuffered + bytesToBuffer <= kMaxSize);
+    SkASSERT(totalBytesToBuffer <= kMaxSize);
+
+    if (totalBytesToBuffer <= fBytesBuffered) {
+        return true;
+    }
+
+    if (fHasLengthAndPosition) {
+        const size_t remaining = fStream->getLength() - fStream->getPosition() + fTrulyBuffered;
+        fBytesBuffered = SkTMin(remaining, totalBytesToBuffer);
+    } else {
+        const size_t extraBytes = totalBytesToBuffer - fBytesBuffered;
+        const size_t bytesBuffered = fStream->read(fBuffer + fBytesBuffered, extraBytes);
+        fBytesBuffered += bytesBuffered;
+    }
+    return fBytesBuffered == totalBytesToBuffer;
+}
+
+size_t SkStreamBuffer::markPosition() {
+    SkASSERT(fBytesBuffered >= 1);
+    if (!fHasLengthAndPosition) {
+        sk_sp<SkData> data(SkData::MakeWithCopy(fBuffer, fBytesBuffered));
+        SkASSERT(nullptr == fMarkedData.find(fPosition));
+        fMarkedData.set(fPosition, data.release());
+    }
+    return fPosition;
+}
+
+sk_sp<SkData> SkStreamBuffer::getDataAtPosition(size_t position, size_t length) {
+    if (!fHasLengthAndPosition) {
+        SkData** data = fMarkedData.find(position);
+        SkASSERT(data);
+        SkASSERT((*data)->size() == length);
+        return sk_ref_sp<SkData>(*data);
+    }
+
+    SkASSERT(position + length <= fStream->getLength());
+
+    const size_t oldPosition = fStream->getPosition();
+    if (!fStream->seek(position)) {
+        return nullptr;
+    }
 
-    const size_t bytesBuffered = fStream->read(fBuffer + fBytesBuffered, bytesToBuffer);
-    fBytesBuffered += bytesBuffered;
-    return bytesBuffered;
+    sk_sp<SkData> data(SkData::MakeUninitialized(length));
+    void* dst = data->writable_data();
+    const bool success = fStream->read(dst, length) == length;
+    fStream->seek(oldPosition);
+    return success ? data : nullptr;
 }
diff --git a/src/codec/SkStreamBuffer.h b/src/codec/SkStreamBuffer.h
index f04ddcbfd8..2b60775dd2 100644
--- a/src/codec/SkStreamBuffer.h
+++ b/src/codec/SkStreamBuffer.h
@@ -8,8 +8,10 @@
 #ifndef SkStreamBuffer_DEFINED
 #define SkStreamBuffer_DEFINED
 
+#include "SkData.h"
 #include "SkStream.h"
 #include "SkTypes.h"
+#include "../private/SkTHash.h"
 
 /**
  *  Helper class for reading from a stream that may not have all its data
@@ -25,28 +27,29 @@ public:
     // Takes ownership of the SkStream.
     SkStreamBuffer(SkStream*);
 
-    /**
-     *  Return a pointer the buffered data.
-     *
-     *  The number of bytes buffered is the sum of values returned by calls to
-     *  buffer() since the last call to flush().
-     */
-    const char* get() const { SkASSERT(fBytesBuffered >= 1); return fBuffer; }
+    ~SkStreamBuffer();
 
     /**
-     *  Bytes in the buffer.
+     *  Return a pointer the buffered data.
      *
-     *  Sum of the values returned by calls to buffer() since the last call to
-     *  flush().
+     *  The number of bytes buffered is the number passed to buffer()
+     *  after the last call to flush().
      */
-    size_t bytesBuffered() const { return fBytesBuffered; }
+    const char* get() const;
 
     /**
      *  Buffer from the stream into our buffer.
      *
-     *  Returns the number of bytes successfully buffered.
+     *  If this call returns true, get() can be used to access |bytes| bytes
+     *  from the stream. In addition, markPosition() can be called to mark this
+     *  position and enable calling getAtPosition() later to retrieve |bytes|
+     *  bytes.
+     *
+     *  @param bytes Total number of bytes desired.
+     *
+     *  @return Whether all bytes were successfully buffered.
      */
-    size_t buffer(size_t bytes);
+    bool buffer(size_t bytes);
 
     /**
      *  Flush the buffer.
@@ -54,15 +57,62 @@ public:
      *  After this call, no bytes are buffered.
      */
     void flush() {
+        if (fHasLengthAndPosition) {
+            if (fTrulyBuffered < fBytesBuffered) {
+                fStream->move(fBytesBuffered - fTrulyBuffered);
+            }
+            fTrulyBuffered = 0;
+        }
+        fPosition += fBytesBuffered;
         fBytesBuffered = 0;
     }
 
+    /**
+     *  Mark the current position in the stream to return to it later.
+     *
+     *  This is the position of the start of the buffer. After this call, a
+     *  a client can call getDataAtPosition to retrieve all the bytes currently
+     *  buffered.
+     *
+     *  @return size_t Position which can be passed to getDataAtPosition later
+     *      to retrieve the data currently buffered.
+     */
+    size_t markPosition();
+
+    /**
+     *  Retrieve data at position, as previously marked by markPosition().
+     *
+     *  @param position Position to retrieve data, as marked by markPosition().
+     *  @param length   Amount of data required at position.
+     *  @return SkData The data at position.
+     */
+    sk_sp<SkData> getDataAtPosition(size_t position, size_t length);
+
 private:
     static constexpr size_t kMaxSize = 256 * 3;
 
     std::unique_ptr<SkStream>   fStream;
+    size_t                      fPosition;
     char                        fBuffer[kMaxSize];
     size_t                      fBytesBuffered;
+    // If the stream has a length and position, we can make two optimizations:
+    // - We can skip buffering
+    // - During parsing, we can store the position and size of data that is
+    //   needed later during decoding.
+    const bool                  fHasLengthAndPosition;
+    // When fHasLengthAndPosition is true, we do not need to actually buffer
+    // inside buffer(). We'll buffer inside get(). This keeps track of how many
+    // bytes we've buffered inside get(), for the (non-existent) case of:
+    //  buffer(n)
+    //  get()
+    //  buffer(n + u)
+    //  get()
+    // The second call to get() needs to only truly buffer the part that was
+    // not already buffered.
+    mutable size_t              fTrulyBuffered;
+    // Only used if !fHasLengthAndPosition. In that case, markPosition will
+    // copy into an SkData, stored here.
+    SkTHashMap<size_t, SkData*> fMarkedData;
 };
 #endif // SkStreamBuffer_DEFINED