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>
diff --git a/src/codec/SkStreamBuffer.cpp b/src/codec/SkStreamBuffer.cpp
index 1e60637..754065d 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);
- const size_t bytesBuffered = fStream->read(fBuffer + fBytesBuffered, bytesToBuffer);
- fBytesBuffered += bytesBuffered;
- return bytesBuffered;
+ 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;
+ }
+
+ 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;
}