Focus SkScaledCodec on BitmapRegionDecoder
The primary goal of SkScaledCodec is to replace the current
implementation of BitmapRegionDecoder, which depends on modified
versions of libjpeg and libpng, with an implementation that uses
standard versions of the libaries. Since BitmapRegionDecoder only
supports PNG, WEBP and JPEG, limit SkScaledCodec to those classes.
We will focus on those three until we complete this primary goal.
Then we can continue to make SkScaledCodec work for other formats.
Fix some bugs in SkScaledCodec::NewFromStream:
- Handle a NULL input stream properly
- Ensure that the input stream is deleted as expected on bad data
Add tests for these error cases.
BUG=skia:4428
Review URL: https://codereview.chromium.org/1389053002
diff --git a/src/codec/SkScaledCodec.cpp b/src/codec/SkScaledCodec.cpp
index 65fd93c..fc51613 100644
--- a/src/codec/SkScaledCodec.cpp
+++ b/src/codec/SkScaledCodec.cpp
@@ -12,22 +12,25 @@
SkCodec* SkScaledCodec::NewFromStream(SkStream* stream) {
- bool isWebp = SkWebpCodec::IsWebp(stream);
- if (!stream->rewind()) {
- return nullptr;
- }
- if (isWebp) {
- // Webp codec supports scaling and subsetting natively
- return SkWebpCodec::NewFromStream(stream);
- }
-
SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));
if (nullptr == codec) {
return nullptr;
}
- // wrap in new SkScaledCodec
- return new SkScaledCodec(codec.detach());
+ switch (codec->getEncodedFormat()) {
+ case kWEBP_SkEncodedFormat:
+ // Webp codec supports scaling and subsetting natively
+ return codec.detach();
+ case kPNG_SkEncodedFormat:
+ case kJPEG_SkEncodedFormat:
+ // wrap in new SkScaledCodec
+ return new SkScaledCodec(codec.detach());
+ default:
+ // FIXME: SkScaledCodec is temporarily disabled for other formats
+ // while focusing on the formats that are supported by
+ // BitmapRegionDecoder.
+ return nullptr;
+ }
}
SkCodec* SkScaledCodec::NewFromData(SkData* data) {
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 6363182..12b42e3 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -126,6 +126,22 @@
}
}
+// FIXME: SkScaledCodec is currently only supported for types used by BRD
+// skbug.com/4428
+static bool supports_scaled_codec(const char path[]) {
+ static const char* const exts[] = {
+ "jpg", "jpeg", "png", "webp"
+ "JPG", "JPEG", "PNG", "WEBP"
+ };
+
+ for (uint32_t i = 0; i < SK_ARRAY_COUNT(exts); i++) {
+ if (SkStrEndsWith(path, exts[i])) {
+ return true;
+ }
+ }
+ return false;
+}
+
static void check(skiatest::Reporter* r,
const char path[],
SkISize size,
@@ -227,7 +243,8 @@
}
// SkScaledCodec tests
- if (supportsScanlineDecoding || supportsSubsetDecoding){
+ if ((supportsScanlineDecoding || supportsSubsetDecoding) && supports_scaled_codec(path)) {
+
SkAutoTDelete<SkStream> stream(resource(path));
if (!stream) {
SkDebugf("Missing resource '%s'\n", path);
@@ -385,8 +402,11 @@
}
static void test_invalid_stream(skiatest::Reporter* r, const void* stream, size_t len) {
+ // Neither of these calls should return a codec. Bots should catch us if we leaked anything.
SkCodec* codec = SkCodec::NewFromStream(new SkMemoryStream(stream, len, false));
- // We should not have gotten a codec. Bots should catch us if we leaked anything.
+ REPORTER_ASSERT(r, !codec);
+
+ codec = SkScaledCodec::NewFromStream(new SkMemoryStream(stream, len, false));
REPORTER_ASSERT(r, !codec);
}
@@ -413,6 +433,16 @@
test_invalid_stream(r, emptyGif, sizeof(emptyGif));
}
+DEF_TEST(Codec_null, r) {
+ // Attempting to create an SkCodec or an SkScaledCodec with null should not
+ // crash.
+ SkCodec* codec = SkCodec::NewFromStream(nullptr);
+ REPORTER_ASSERT(r, !codec);
+
+ codec = SkScaledCodec::NewFromStream(nullptr);
+ REPORTER_ASSERT(r, !codec);
+}
+
static void test_dimensions(skiatest::Reporter* r, const char path[]) {
// Create the codec from the resource file
SkAutoTDelete<SkStream> stream(resource(path));