Various improvements to CodecSrc testing in dm
*** Add CodecMode and ScaledCodecMode (in place of
NormalMode), so now we test SkCodec's getPixels() and
SkScaledCodec's getPixels()
*** Don't attempt to test scanline and codec modes using
the dimensions that were recommended for SkScaledCodec.
*** Change tags so that each scale gets its own output
folder.
TODO: Make ScanlineMode and ScanlineSubsetMode support
kOutOfOrder etc. I think this belongs with the gif CL -
I don't want to add test modes that we don't run yet.
BUG=skia:4202
BUG=skia:4238
Review URL: https://codereview.chromium.org/1327433003
diff --git a/dm/DM.cpp b/dm/DM.cpp
index f5df692..2ecba1d 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -175,8 +175,8 @@
/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
struct TaggedSrc : public SkAutoTDelete<Src> {
- const char* tag;
- const char* options;
+ ImplicitString tag;
+ ImplicitString options;
};
struct TaggedSink : public SkAutoTDelete<Sink> {
@@ -193,10 +193,10 @@
return N++ % FLAGS_shards == FLAGS_shard;
}
-static void push_src(const char* tag, const char* options, Src* s) {
+static void push_src(ImplicitString tag, ImplicitString options, Src* s) {
SkAutoTDelete<Src> src(s);
if (in_shard() &&
- FLAGS_src.contains(tag) &&
+ FLAGS_src.contains(tag.c_str()) &&
!SkCommandLineFlags::ShouldSkip(FLAGS_match, src->name().c_str())) {
TaggedSrc& s = gSrcs.push_back();
s.reset(src.detach());
@@ -205,6 +205,49 @@
}
}
+static void push_codec_src(Path path, CodecSrc::Mode mode, CodecSrc::DstColorType dstColorType,
+ float scale) {
+ SkString folder;
+ switch (mode) {
+ case CodecSrc::kCodec_Mode:
+ folder.append("codec");
+ break;
+ case CodecSrc::kScaledCodec_Mode:
+ folder.append("scaled_codec");
+ break;
+ case CodecSrc::kScanline_Mode:
+ folder.append("scanline");
+ break;
+ case CodecSrc::kScanline_Subset_Mode:
+ folder.append("scanline_subset");
+ break;
+ case CodecSrc::kStripe_Mode:
+ folder.append("stripe");
+ break;
+ case CodecSrc::kSubset_Mode:
+ folder.append("subset");
+ break;
+ }
+
+ switch (dstColorType) {
+ case CodecSrc::kGrayscale_Always_DstColorType:
+ folder.append("_kGray8");
+ break;
+ case CodecSrc::kIndex8_Always_DstColorType:
+ folder.append("_kIndex8");
+ break;
+ default:
+ break;
+ }
+
+ if (1.0f != scale) {
+ folder.appendf("_%.3f", scale);
+ }
+
+ CodecSrc* src = new CodecSrc(path, mode, dstColorType, scale);
+ push_src("image", folder, src);
+}
+
static void push_codec_srcs(Path path) {
SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(path.c_str()));
if (!encoded) {
@@ -218,7 +261,6 @@
}
// Choose scales for scaling tests.
- // TODO (msarett): Add more scaling tests as we implement more flexible scaling.
// TODO (msarett): Implement scaling tests for SkImageDecoder in order to compare with these
// tests. SkImageDecoder supports downscales by integer factors.
// SkJpegCodec natively supports scaling to: 0.125, 0.25, 0.375, 0.5, 0.625, 0.75, 0.875
@@ -228,57 +270,25 @@
const float scales[] = { 0.1f, 0.125f, 0.166f, 0.2f, 0.25f, 0.333f, 0.375f, 0.4f, 0.5f, 0.6f,
0.625f, 0.750f, 0.8f, 0.875f, 1.0f };
+ const CodecSrc::Mode modes[] = { CodecSrc::kCodec_Mode, CodecSrc::kScaledCodec_Mode,
+ CodecSrc::kScanline_Mode, CodecSrc::kScanline_Subset_Mode, CodecSrc::kStripe_Mode,
+ CodecSrc::kSubset_Mode };
+
+ const CodecSrc::DstColorType colorTypes[] = { CodecSrc::kGetFromCanvas_DstColorType,
+ CodecSrc::kGrayscale_Always_DstColorType, CodecSrc::kIndex8_Always_DstColorType };
+
for (float scale : scales) {
if (scale != 1.0f && (path.endsWith(".webp") || path.endsWith(".WEBP"))) {
// FIXME: skbug.com/4038 Scaling webp seems to leave some pixels uninitialized/
// compute their colors based on uninitialized values.
continue;
}
- // Build additional test cases for images that decode natively to non-canvas types
- switch(codec->getInfo().colorType()) {
- case kGray_8_SkColorType:
- push_src("image", "codec_kGray8", new CodecSrc(path, CodecSrc::kNormal_Mode,
- CodecSrc::kGrayscale_Always_DstColorType, scale));
- push_src("image", "scanline_kGray8", new CodecSrc(path, CodecSrc::kScanline_Mode,
- CodecSrc::kGrayscale_Always_DstColorType, scale));
- push_src("image", "scanline_subset_kGray8", new CodecSrc(path,
- CodecSrc::kScanline_Subset_Mode, CodecSrc::kGrayscale_Always_DstColorType,
- scale));
- push_src("image", "stripe_kGray8", new CodecSrc(path, CodecSrc::kStripe_Mode,
- CodecSrc::kGrayscale_Always_DstColorType, scale));
- // Intentional fall through
- // FIXME: Is this a long term solution for testing wbmps decodes to kIndex8?
- // Further discussion on this topic is at skbug.com/3683
- case kIndex_8_SkColorType:
- push_src("image", "codec_kIndex8", new CodecSrc(path, CodecSrc::kNormal_Mode,
- CodecSrc::kIndex8_Always_DstColorType, scale));
- push_src("image", "scanline_kIndex8", new CodecSrc(path, CodecSrc::kScanline_Mode,
- CodecSrc::kIndex8_Always_DstColorType, scale));
- push_src("image", "scanline_subset_kIndex8", new CodecSrc(path,
- CodecSrc::kScanline_Subset_Mode, CodecSrc::kIndex8_Always_DstColorType,
- scale));
- push_src("image", "stripe_kIndex8", new CodecSrc(path, CodecSrc::kStripe_Mode,
- CodecSrc::kIndex8_Always_DstColorType, scale));
- break;
- default:
- // Do nothing
- break;
- }
- // Decode all images to the canvas color type
- push_src("image", "codec", new CodecSrc(path, CodecSrc::kNormal_Mode,
- CodecSrc::kGetFromCanvas_DstColorType, scale));
- push_src("image", "scanline", new CodecSrc(path, CodecSrc::kScanline_Mode,
- CodecSrc::kGetFromCanvas_DstColorType, scale));
- push_src("image", "scanline_subset", new CodecSrc(path, CodecSrc::kScanline_Subset_Mode,
- CodecSrc::kGetFromCanvas_DstColorType, scale));
- push_src("image", "stripe", new CodecSrc(path, CodecSrc::kStripe_Mode,
- CodecSrc::kGetFromCanvas_DstColorType, scale));
- // Note: The only codec which supports subsets natively is SkWebpCodec, which will never
- // report kIndex_8 or kGray_8, so there is no need to test kSubset_mode with those color
- // types specifically requested.
- push_src("image", "codec_subset", new CodecSrc(path, CodecSrc::kSubset_Mode,
- CodecSrc::kGetFromCanvas_DstColorType, scale));
+ for (CodecSrc::Mode mode : modes) {
+ for (CodecSrc::DstColorType colorType : colorTypes) {
+ push_codec_src(path, mode, colorType, scale);
+ }
+ }
}
}
@@ -575,8 +585,8 @@
// - this Src / Sink combination is on the blacklist;
// - it's a dry run.
SkString note(task->src->veto(task->sink->flags()) ? " (veto)" : "");
- SkString whyBlacklisted = is_blacklisted(task->sink.tag, task->src.tag,
- task->src.options, name.c_str());
+ SkString whyBlacklisted = is_blacklisted(task->sink.tag, task->src.tag.c_str(),
+ task->src.options.c_str(), name.c_str());
if (!whyBlacklisted.isEmpty()) {
note.appendf(" (--blacklist %s)", whyBlacklisted.c_str());
}
@@ -597,8 +607,8 @@
if (err.isFatal()) {
fail(SkStringPrintf("%s %s %s %s: %s",
task->sink.tag,
- task->src.tag,
- task->src.options,
+ task->src.tag.c_str(),
+ task->src.options.c_str(),
name.c_str(),
err.c_str()));
} else {
@@ -637,13 +647,13 @@
}
if (!FLAGS_readPath.isEmpty() &&
- !gGold.contains(Gold(task->sink.tag, task->src.tag,
- task->src.options, name, md5))) {
+ !gGold.contains(Gold(task->sink.tag, task->src.tag.c_str(),
+ task->src.options.c_str(), name, md5))) {
fail(SkStringPrintf("%s not found for %s %s %s %s in %s",
md5.c_str(),
task->sink.tag,
- task->src.tag,
- task->src.options,
+ task->src.tag.c_str(),
+ task->src.options.c_str(),
name.c_str(),
FLAGS_readPath[0]));
}
@@ -659,7 +669,8 @@
}
}
timer.end();
- done(timer.fWall, task->sink.tag, task->src.tag, task->src.options, name, note, log);
+ done(timer.fWall, task->sink.tag, task->src.tag.c_str(), task->src.options.c_str(), name,
+ note, log);
}
static void WriteToDisk(const Task& task,
@@ -699,10 +710,10 @@
} else {
path = SkOSPath::Join(dir, task.sink.tag);
sk_mkdir(path.c_str());
- path = SkOSPath::Join(path.c_str(), task.src.tag);
+ path = SkOSPath::Join(path.c_str(), task.src.tag.c_str());
sk_mkdir(path.c_str());
- if (strcmp(task.src.options, "") != 0) {
- path = SkOSPath::Join(path.c_str(), task.src.options);
+ if (strcmp(task.src.options.c_str(), "") != 0) {
+ path = SkOSPath::Join(path.c_str(), task.src.options.c_str());
sk_mkdir(path.c_str());
}
path = SkOSPath::Join(path.c_str(), task.src->name().c_str());
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 64c18fc..2c80fac 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -85,14 +85,6 @@
if (nullptr == scanlineDecoder) {
return nullptr;
}
- // DM scanline test assume kTopDown scanline ordering. Other orderings are
- // tested from within SkScaledCodec.
- // TODO (msarett): Redesign the CodecSrc tests to improve our coverage of SkCodec and
- // SkScanlineDecoder functionality. Maybe we should write code to explicitly
- // test kNone, kOutOfOrder, and kBottomUp.
- if (SkScanlineDecoder::kTopDown_SkScanlineOrder != scanlineDecoder->getScanlineOrder()) {
- return nullptr;
- }
if (SkCodec::kSuccess != scanlineDecoder->start(info, NULL, colorPtr, colorCountPtr)) {
return nullptr;
}
@@ -104,13 +96,20 @@
if (!encoded) {
return SkStringPrintf("Couldn't read %s.", fPath.c_str());
}
- SkAutoTDelete<SkCodec> codec(SkScaledCodec::NewFromData(encoded));
- if (nullptr == codec.get()) {
- // scaledCodec not supported, try normal codec
- codec.reset(SkCodec::NewFromData(encoded));
+ SkAutoTDelete<SkCodec> codec(NULL);
+ if (kScaledCodec_Mode == fMode) {
+ codec.reset(SkScaledCodec::NewFromData(encoded));
+ // TODO (msarett): This should fall through to a fatal error once we support scaled
+ // codecs for all image types.
if (nullptr == codec.get()) {
- return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str());
+ return Error::Nonfatal(SkStringPrintf("Couldn't create scaled codec for %s.",
+ fPath.c_str()));
}
+ } else {
+ codec.reset(SkCodec::NewFromData(encoded));
+ }
+ if (nullptr == codec.get()) {
+ return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str());
}
// Choose the color type to decode to
@@ -171,7 +170,8 @@
}
switch (fMode) {
- case kNormal_Mode: {
+ case kScaledCodec_Mode:
+ case kCodec_Mode: {
switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes(), nullptr,
colorPtr, colorCountPtr)) {
case SkCodec::kSuccess:
@@ -192,7 +192,7 @@
SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(
start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
if (nullptr == scanlineDecoder) {
- return Error::Nonfatal("Could not start top-down scanline decoder");
+ return Error::Nonfatal("Could not start scanline decoder");
}
const SkCodec::Result result = scanlineDecoder->getScanlines(
@@ -234,7 +234,8 @@
SkImageInfo largestSubsetDecodeInfo =
decodeInfo.makeWH(subsetWidth + extraX, subsetHeight + extraY);
SkBitmap largestSubsetBm;
- if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, nullptr, colorTable.get())) {
+ if (!largestSubsetBm.tryAllocPixels(largestSubsetDecodeInfo, nullptr,
+ colorTable.get())) {
return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(),
largestSubsetDecodeInfo.width(), largestSubsetDecodeInfo.height());
}
@@ -252,10 +253,11 @@
subsetHeight + extraY : subsetHeight;
const int y = row * subsetHeight;
//create scanline decoder for each subset
- SkAutoTDelete<SkScanlineDecoder> subsetScanlineDecoder(
- start_scanline_decoder(encoded.get(), decodeInfo,
- colorPtr, colorCountPtr));
- if (nullptr == subsetScanlineDecoder) {
+ SkAutoTDelete<SkScanlineDecoder> decoder(start_scanline_decoder(encoded.get(),
+ decodeInfo, colorPtr, colorCountPtr));
+ // TODO (msarett): Support this mode for all scanline orderings.
+ if (nullptr == decoder || SkScanlineDecoder::kTopDown_SkScanlineOrder !=
+ decoder->getScanlineOrder()) {
if (x == 0 && y == 0) {
//first try, image may not be compatible
return Error::Nonfatal("Could not start top-down scanline decoder");
@@ -264,8 +266,7 @@
}
}
//skip to first line of subset
- const SkCodec::Result skipResult =
- subsetScanlineDecoder->skipScanlines(y);
+ const SkCodec::Result skipResult = decoder->skipScanlines(y);
switch (skipResult) {
case SkCodec::kSuccess:
case SkCodec::kIncompleteInput:
@@ -281,7 +282,7 @@
SkAssertResult(largestSubsetBm.extractSubset(&subsetBm, bounds));
SkAutoLockPixels autlockSubsetBm(subsetBm, true);
const SkCodec::Result subsetResult =
- subsetScanlineDecoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
+ decoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
switch (subsetResult) {
case SkCodec::kSuccess:
case SkCodec::kIncompleteInput:
@@ -323,7 +324,11 @@
// Decode odd stripes
SkAutoTDelete<SkScanlineDecoder> decoder(
start_scanline_decoder(encoded.get(), decodeInfo, colorPtr, colorCountPtr));
- if (nullptr == decoder) {
+ if (nullptr == decoder ||
+ SkScanlineDecoder::kTopDown_SkScanlineOrder != decoder->getScanlineOrder()) {
+ // This mode was designed to test the new skip scanlines API in libjpeg-turbo.
+ // Jpegs have kTopDown_SkScanlineOrder, and at this time, it is not interesting
+ // to run this test for image types that do not have this scanline ordering.
return Error::Nonfatal("Could not start top-down scanline decoder");
}
for (int i = 0; i < numStripes; i += 2) {
diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h
index e5f22f8..efae871 100644
--- a/dm/DMSrcSink.h
+++ b/dm/DMSrcSink.h
@@ -24,6 +24,7 @@
struct ImplicitString : public SkString {
template <typename T>
ImplicitString(const T& s) : SkString(s) {}
+ ImplicitString() : SkString("") {}
};
typedef ImplicitString Name;
typedef ImplicitString Path;
@@ -102,7 +103,8 @@
class CodecSrc : public Src {
public:
enum Mode {
- kNormal_Mode,
+ kScaledCodec_Mode,
+ kCodec_Mode,
kScanline_Mode,
kScanline_Subset_Mode,
kStripe_Mode, // Tests the skipping of scanlines
@@ -120,10 +122,10 @@
Name name() const override;
bool veto(SinkFlags) const override;
private:
- Path fPath;
- Mode fMode;
- DstColorType fDstColorType;
- float fScale;
+ Path fPath;
+ Mode fMode;
+ DstColorType fDstColorType;
+ float fScale;
};