Add a buffered SkStream class.

src/utils/SkBufferedStream.cpp

+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkBufferedStream.h"
+
+SkBufferedStream* SkBufferedStream::Create(SkStream* stream, size_t bufferSize) {
+    if (NULL == stream) {
+        return NULL;
+    }
+    return SkNEW_ARGS(SkBufferedStream, (stream, bufferSize));

[bungeman-skia, 2013/09/17 22:19:11]

One nice thing about this Create maybe being a virtual on SkStream itself is
that if a stream is already rewindable or better then it could just return
itself, avoiding any need to create any buffer and make the extra copy. However,
I'd hate to just keep throwing things onto SkStream. If you look back at
https://codereview.chromium.org/15298009/diff/15001/include/core/SkStream.h#n...
I had proposed an 'Optimizer' for this sort of thing, so that double dispatch
could be used to optimize these sorts of things instead of cluttering up the
SkStream interface.

[scroggo, 2013/09/18 15:15:44]

As Ben and I discussed in person, in my sole use case, I really have a Java
InputStream, and I don't know whether the source can be rewound, so this might
be a bit of overkill. I think that there's no need to put this on SkStream,
since it's a very specialized use case.

+}
+
+SkBufferedStream::SkBufferedStream(SkStream* stream, size_t bufferSize)
+    : fStream(SkRef(stream))
+    , fOffset(0)
+    , fBufferedSoFar(0)
+    , fBufferSize(bufferSize)
+    , fBuffer(bufferSize) {}
+
+bool SkBufferedStream::isAtEnd() const {
+    if (fOffset < fBufferedSoFar) {
+        // Even if the underlying stream is at the end, this stream has been
+        // rewound after buffering, so it is not at the end.
+        return false;
+    }
+
+    return fStream->isAtEnd();
+}
+
+bool SkBufferedStream::rewind() {
+    // Only allow a rewind if we have not exceeded the buffer.
+    if (fOffset <= fBufferSize) {
+        fOffset = 0;
+        return true;
+    }
+    return false;

[bungeman-skia, 2013/09/17 22:19:11]

My reaction was "returning NULL in here!? ...oh, that is in some sense an
error". This should be fine since the contract is that you won't rewind after
reading more than the buffer will hold.

+}
+
+bool SkBufferedStream::hasLength() const {
+    return fStream->hasLength();
+}
+
+size_t SkBufferedStream::getLength() const {
+    return fStream->getLength();
+}
+
+size_t SkBufferedStream::read(void* voidDst, size_t size) {
+    // Cast voidDst to a char* for easy addition.
+    char* dst = reinterpret_cast<char*>(voidDst);
+    SkDEBUGCODE(const size_t totalSize = size;)
+    size_t readSoFar = 0;
+
+    // First, read any data that was previously buffered.
+    if (fOffset < fBufferedSoFar) {
+        // Some data has already been copied to fBuffer. Read up to the
+        // lesser of the size requested and the remainder of the buffered
+        // data.
+        const size_t bytesToCopy = SkTMin(size, fBufferedSoFar - fOffset);
+        if (dst != NULL) {
+            memcpy(dst, this->getBufferAtOffset(), bytesToCopy);
+        }
+
+        // Update fOffset to the new position. It is guaranteed to be
+        // within the buffered data.
+        fOffset += bytesToCopy;
+        SkASSERT(fOffset <= fBufferedSoFar);
+
+        if (bytesToCopy == size) {
+            // The entire requested read was inside the already buffered
+            // data.
+            SkASSERT(fOffset <= fBufferedSoFar);
+            return size;
+        }
+
+        // The requested read extends beyond the buffered data. Update
+        // the remaining number of bytes needed to read, the number of
+        // bytes read so far, and the destination buffer.
+        size -= bytesToCopy;
+        readSoFar += bytesToCopy;
+        SkASSERT(size + readSoFar == totalSize);
+        if (dst != NULL) {
+            dst += bytesToCopy;
+        }
+    }
+
+    // If we got here, we have read everything that was already buffered.
+    SkASSERT(fOffset >= fBufferedSoFar);
+
+    // Buffer any more data that should be buffered, and copy it to the
+    // destination.
+    if (fBufferedSoFar < fBufferSize) {
+        // Data needs to be buffered. Buffer up to the lesser of the size requested
+        // and the remainder of the max buffer size.
+        const size_t bytesToBuffer = SkTMin(size, fBufferSize - fBufferedSoFar);
+        char* buffer = this->getBufferAtOffset();
+        const size_t buffered = fStream->read(buffer, bytesToBuffer);
+        fBufferedSoFar += buffered;
+        fOffset = fBufferedSoFar;
+
+        // Copy the buffer to the destination buffer and update the amount read.
+        if (dst != NULL) {
+            memcpy(dst, buffer, buffered);
+        }
+        readSoFar += buffered;
+
+        if (buffered == size || fStream->isAtEnd()) {
+            // We were able to buffer all of the data requested (or all the data
+            // remaining in the stream) and provide it to the caller.
+            SkASSERT(fBufferedSoFar <= fBufferSize);
+            SkASSERT(totalSize == readSoFar || fStream->isAtEnd());
+            return readSoFar;
+        }
+
+        // The requested read extends beyond the length of the buffer. Update
+        // the remaining number of bytes needed to read and the destination
+        // buffer.
+        size -= buffered;
+        SkASSERT(size + readSoFar == totalSize);
+        if (dst != NULL) {
+            dst += buffered;
+        }
+    }
+
+    // If we get here, we have buffered all that can be buffered.
+    SkASSERT(fBufferSize == fBufferedSoFar && fOffset >= fBufferSize);

[bungeman-skia, 2013/09/17 22:19:11]

It seems like once you get here you can release the memory for fBuffer? Does it
do any good at all to do so early?

If someone were to read more than the buffer size right off, it looks like we
would copy into the buffer and the dst, then copy the remainder into the dst. If
the final offset is going to be past the end of the buffer at the beginning,
does it make sense to just drop the buffer and just do the read? This may not
work because we don't know for sure that we're going to satisfy the entire read
request (we may bail atEnd), but we may or may not want the buffer to be
'cancel-able' in the sense that once we're done with the seeking about at the
beginning and we're ready to rip through, we don't keep buffering when we know
we're not going to need it. This may be completely premature optimization stuff,
but I figured I'd put it here since I thought about it.

[scroggo, 2013/09/18 15:15:44]

Almost. The stream could be at the end, in which case this read would not go
beyond the end of the buffer.
Sure - then the memory could be reused. I actually considered making this a
templated class, in which case we could avoid the extra allocation, but would
not be able to do this early free.
Good point. Maybe this was implied, but if the caller read more than the buffer
size after having buffered some, we'd run into the same situation. If we do want
to preserve the ability to rewind if the stream was shorter than the buffer,
we'd need to special case that as well. At this point I think I' tend to think
that's a premature optimization at the cost of some more complicated code.

+
+    // Read directly from the stream.
+    const size_t bytesReadDirectly = fStream->read(dst, size);
+    fOffset += bytesReadDirectly;
+    return bytesReadDirectly + readSoFar;
+}