Remove position from FrontBufferedStream
When FrontBufferedStream was written, it implemented getPosition()
and returned true for hasPosition(). The CL that introduced it
(crrev.com/23717055) does not have any indication why, but I'm guessing
it was because it was easy to implement. None of our decoders rely on
this (only some tests do).
Now that we have a decoder (SkRawCodec) that expects to be able to seek
a stream if it has a position (which makes sense - SkStream.h
associates that with SkStreamSeekable, and there is no other way to
check to see if a stream is seekable), it is failing because
FrontBufferedStream reports it has a position and the decoder tries to
seek.
Remove FrontBufferedStream::hasPosition() (reverting to the default,
false) and ::getPosition() (so it will return 0).
Fix tests - do not call FrontBufferedStream::getPosition()
Update CodexTest to test using an FrontBufferedStream, like Android
does.
BUG=b/27218441
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1703293002
Review URL: https://codereview.chromium.org/1703293002
diff --git a/src/utils/SkFrontBufferedStream.cpp b/src/utils/SkFrontBufferedStream.cpp
index e0b586a..a0dbeb6 100644
--- a/src/utils/SkFrontBufferedStream.cpp
+++ b/src/utils/SkFrontBufferedStream.cpp
@@ -22,10 +22,6 @@
bool rewind() override;
- bool hasPosition() const override { return true; }
-
- size_t getPosition() const override { return fOffset; }
-
bool hasLength() const override { return fHasLength; }
size_t getLength() const override { return fLength; }
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index c5651ac..a0789fb 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -11,6 +11,7 @@
#include "SkCodec.h"
#include "SkCodecImageGenerator.h"
#include "SkData.h"
+#include "SkFrontBufferedStream.h"
#include "SkImageDecoder.h"
#include "SkMD5.h"
#include "SkRandom.h"
@@ -180,7 +181,7 @@
// Test full image decodes with SkCodec
SkMD5::Digest codecDigest;
- SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
+ const SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
SkBitmap bm;
SkCodec::Result expectedResult = isIncomplete ? SkCodec::kIncompleteInput : SkCodec::kSuccess;
test_codec(r, codec.get(), bm, info, size, expectedResult, &codecDigest, nullptr);
@@ -313,8 +314,8 @@
&codecDigest);
}
- // Test SkCodecImageGenerator
if (!isIncomplete) {
+ // Test SkCodecImageGenerator
SkAutoTDelete<SkStream> stream(resource(path));
SkAutoTUnref<SkData> fullData(SkData::NewFromStream(stream, stream->getLength()));
SkAutoTDelete<SkImageGenerator> gen(SkCodecImageGenerator::NewFromEncodedCodec(fullData));
@@ -323,6 +324,16 @@
SkAutoLockPixels autoLockPixels(bm);
REPORTER_ASSERT(r, gen->getPixels(info, bm.getPixels(), bm.rowBytes()));
compare_to_good_digest(r, codecDigest, bm);
+
+ // Test using SkFrontBufferedStream, as Android does
+ SkStream* bufferedStream = SkFrontBufferedStream::Create(new SkMemoryStream(fullData),
+ SkCodec::MinBufferedBytesNeeded());
+ REPORTER_ASSERT(r, bufferedStream);
+ codec.reset(SkCodec::NewFromStream(bufferedStream));
+ REPORTER_ASSERT(r, codec);
+ if (codec) {
+ test_info(r, codec.get(), info, SkCodec::kSuccess, &codecDigest);
+ }
}
// If we've just tested incomplete decodes, let's run the same test again on full decodes.
diff --git a/tests/FrontBufferedStreamTest.cpp b/tests/FrontBufferedStreamTest.cpp
index 0b161b9..7821c5e 100644
--- a/tests/FrontBufferedStreamTest.cpp
+++ b/tests/FrontBufferedStreamTest.cpp
@@ -68,7 +68,7 @@
test_read(reporter, bufferedStream, gAbcs, bufferSize / 4);
// Now test reading part of what was buffered, and buffering new data.
- test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), bufferSize / 2);
+ test_read(reporter, bufferedStream, gAbcs + bufferSize / 4, bufferSize / 2);
// Now test reading what was buffered, buffering new data, and
// reading directly from the stream.
@@ -96,7 +96,7 @@
test_read(reporter, bufferedStream, gAbcs, bufferSize);
// Read past the size of the buffer. At this point, we cannot return.
- test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), 1);
+ test_read(reporter, bufferedStream, gAbcs + memStream->getPosition(), 1);
test_rewind(reporter, bufferedStream, false);
}
@@ -116,7 +116,7 @@
bufferedStream->skip(bufferSize / 2);
// Test that reading will still work.
- test_read(reporter, bufferedStream, gAbcs + bufferedStream->getPosition(), bufferSize / 4);
+ test_read(reporter, bufferedStream, gAbcs + memStream->getPosition(), bufferSize / 4);
test_rewind(reporter, bufferedStream, true);
test_read(reporter, bufferedStream, gAbcs, bufferSize);
@@ -219,13 +219,12 @@
memStream->skip(arbitraryOffset);
SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(memStream, bufferSize));
- // Since SkMemoryStream has a length and a position, bufferedStream must also.
+ // Since SkMemoryStream has a length, bufferedStream must also.
REPORTER_ASSERT(reporter, bufferedStream->hasLength());
const size_t amountToRead = 10;
const size_t bufferedLength = bufferedStream->getLength();
- size_t currentPosition = bufferedStream->getPosition();
- REPORTER_ASSERT(reporter, 0 == currentPosition);
+ size_t currentPosition = 0;
// Read the stream in chunks. After each read, the position must match currentPosition,
// which sums the amount attempted to read, unless the end of the stream has been reached.
@@ -235,7 +234,7 @@
test_read(reporter, bufferedStream, gAbcs + arbitraryOffset + currentPosition,
amountToRead);
currentPosition = SkTMin(currentPosition + amountToRead, bufferedLength);
- REPORTER_ASSERT(reporter, bufferedStream->getPosition() == currentPosition);
+ REPORTER_ASSERT(reporter, memStream->getPosition() - arbitraryOffset == currentPosition);
}
REPORTER_ASSERT(reporter, bufferedStream->isAtEnd());
REPORTER_ASSERT(reporter, bufferedLength == currentPosition);
diff --git a/tests/StreamTest.cpp b/tests/StreamTest.cpp
index 15ebfcf..2a60a7b 100644
--- a/tests/StreamTest.cpp
+++ b/tests/StreamTest.cpp
@@ -266,12 +266,11 @@
for (size_t start = 0; start <= bufferSize; start++) {
// Skip to the starting point
REPORTER_ASSERT(r, bufferedStream->skip(start) == start);
- REPORTER_ASSERT(r, bufferedStream->getPosition() == start);
const size_t bytesPeeked = bufferedStream->peek(peekStorage.get(), bytesToPeek);
if (0 == bytesPeeked) {
- // Peeking should only fail completely if we have read beyond the buffer.
- REPORTER_ASSERT(r, bufferedStream->getPosition() >= bufferSize);
+ // Peeking should only fail completely if we have read/skipped beyond the buffer.
+ REPORTER_ASSERT(r, start >= bufferSize);
break;
}