Various improvements to CodecSrc testing in dm

dm/DM.cpp

Index: dm/DM.cpp
diff --git a/dm/DM.cpp b/dm/DM.cpp
index f5df692ca66cfce0c700bc02d8afa432f8da083a..f6b1c61bc5bede605c101eb1554afa9a3759a5f6 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -205,6 +205,50 @@ static void push_src(const char* tag, const char* options, Src* s) {
     }
 }
 
+static void push_codec_src(Path path, CodecSrc::Mode mode, CodecSrc::DstColorType dstColorType,
+        float scale) {
+    SkAutoTDelete<SkString> folder(new SkString());
+    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);
+    }
+
+    const char* folderStr = folder->c_str();

[scroggo, 2015/09/01 13:11:44]

Could you add a comment here that explains why this is safe to do?

+    CodecSrc* src = new CodecSrc(path, mode, dstColorType, scale, folder.detach());
+    push_src("image", folderStr, src);

[mtklein, 2015/09/01 14:07:02]

Yes, folderStr will be deleted when this function returns.

[scroggo, 2015/09/01 14:10:04]

?

folderStr points into folder, which is owned by src. src will ultimately be
deleted by its TaggedSrc, at which point folderStr will be pointing nowhere
meaningful, so I think this is safe to do, but tricky.

[mtklein, 2015/09/01 14:18:11]

Oh!  I missed the detach.  This is fine.

+}
+
 static void push_codec_srcs(Path path) {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(path.c_str()));
     if (!encoded) {
@@ -218,7 +262,6 @@ static void push_codec_srcs(Path path) {
     }
 
     // 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 +271,25 @@ static void push_codec_srcs(Path path) {
     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 };

[msarett, 2015/08/31 21:05:29]

Behavior change: We will now "try" to test subset mode for kIndex8 and kGray8. 
It will simply fail non-fatally.

+
+    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);
+            }
+        }
     }
 }