Various improvements to CodecSrc testing in dm

dm/DM.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 "CrashHandler.h"
 #include "DMJsonWriter.h"
 #include "DMSrcSink.h"
@@ skipping 187 matching lines @@
     if (in_shard() &&
         FLAGS_src.contains(tag) &&
         !SkCommandLineFlags::ShouldSkip(FLAGS_match, src->name().c_str())) {
         TaggedSrc& s = gSrcs.push_back();
         s.reset(src.detach());
         s.tag = tag;
         s.options = options;
     }
 }
 
+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) {
         SkDebugf("Couldn't read %s.", path.c_str());
         return;
     }
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
     if (nullptr == codec.get()) {
         SkDebugf("Couldn't create codec for %s.", path.c_str());
         return;
     }
 
     // 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
     // 0.1, 0.16, 0.2 etc allow us to test SkScaledCodec with sampleSize 10, 6, 5, etc
     // 0.4, 0.7 etc allow to test what happens when the client requests a scale that
     // does not exactly match a sampleSize or native scaling capability
     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;
+
+        for (CodecSrc::Mode mode : modes) {
+            for (CodecSrc::DstColorType colorType : colorTypes) {
+                push_codec_src(path, mode, colorType, scale);
+            }
         }
-
-        // 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));
     }
 }
 
 static bool codec_supported(const char* ext) {
     // FIXME: Once other versions of SkCodec are available, we can add them to this
     // list (and eventually we can remove this check once they are all supported).
     static const char* const exts[] = {
         "bmp", "gif", "jpg", "jpeg", "png", "ico", "wbmp", "webp",
         "BMP", "GIF", "JPG", "JPEG", "PNG", "ICO", "WBMP", "WEBP",
     };
@@ skipping 621 matching lines @@
     }
     return 0;
 }
 
 #if !defined(SK_BUILD_FOR_IOS)
 int main(int argc, char** argv) {
     SkCommandLineFlags::Parse(argc, argv);
     return dm_main();
 }
 #endif