Add getYUV8Planes() API to SkCodec

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index bbfa5199e61f06bead0549fad66d11fa40313a23..9645cb0238a4878f11f810febb48e83ad844a0be 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -8,6 +8,7 @@
 #include "DMSrcSink.h"
 #include "SkAndroidCodec.h"
 #include "SkCodec.h"
+#include "SkCodecImageGenerator.h"
 #include "SkCommonFlags.h"
 #include "SkData.h"
 #include "SkDocument.h"
@@ -240,11 +241,9 @@ CodecSrc::CodecSrc(Path path, Mode mode, DstColorType dstColorType, float scale)
 {}
 
 bool CodecSrc::veto(SinkFlags flags) const {
-    // No need to test decoding to non-raster or indirect backend.
-    // TODO: Once we implement GPU paths (e.g. JPEG YUV), we should use a deferred decode to
-    // let the GPU handle it.
-    return flags.type != SinkFlags::kRaster
-        || flags.approach != SinkFlags::kDirect;
+    // Test to direct raster backend.  Also enable CodecImageGenerator tests on gpu.

[scroggo, 2016/01/19 18:37:48]

Does this mean to test CodecImageGenerator on the GPU and nowhere else?

[msarett, 2016/01/19 22:35:27]

I decided it would be interesting to test to 8888, 565, and gpu.

+    return (flags.type != SinkFlags::kRaster || flags.approach != SinkFlags::kDirect) &&

[scroggo, 2016/01/19 18:37:48]

Nit: I think it would be easier to follow this with some if statements instead
of a single return statement.

[msarett, 2016/01/19 22:35:27]

Done.

As side comment, I find the concept of a veto() function <return true if you
want to skip> pretty confusing.

[scroggo, 2016/01/21 17:27:28]

Haha, agreed. It confuses me every time I look at it :P But I do understand the
intent - return true to mean "yes, I want to veto running on this Sink". I think
it would be clearer to have a different name (not sure what exactly - something
that means "shouldIRunOnThisSink") and returns true to run.

+            (kGen_Mode != fMode || flags.type != SinkFlags::kGPU);
 }
 
 bool get_decode_info(SkImageInfo* decodeInfo, const SkImageInfo& defaultInfo,
@@ -274,11 +273,38 @@ bool get_decode_info(SkImageInfo* decodeInfo, const SkImageInfo& defaultInfo,
     return true;
 }
 
+Error test_gen(SkCanvas* canvas, SkData* data) {
+    SkImageGenerator* gen = SkCodecImageGenerator::NewFromEncodedCodec(data);
+    if (!gen) {
+        return "Could not create image generator.";
+    }
+
+    // FIXME: The gpu backend does not draw kGray sources correctly.

[scroggo, 2016/01/19 18:37:48]

Is there a bug filed for this?

[msarett, 2016/01/19 22:35:27]

I'll file one and reference it here.

+    if (kGray_8_SkColorType == gen->getInfo().colorType()) {

[scroggo, 2016/01/19 18:37:48]

Alternatively, we could skip before we ever get here. push_codec_srcs could
check the codec's color type (which we already do for other reasons) and decide
to never create this CodecSrc. (If we go that route, we should put an assert
here, and reference where we avoid this case.)

[msarett, 2016/01/19 22:35:27]

Done.

+        return Error::Nonfatal("Disabling kGray decodes with CodecImageGenerator.");
+    }
+
+    SkBitmap bitmap;
+    if (!SkDEPRECATED_InstallDiscardablePixelRef(gen, &bitmap)) {

[scroggo, 2016/01/19 18:37:48]

I see this is deprecated. Why not call SkImage::NewFromGenerator? Will that (or
some other non-deprecated call path) work?

[msarett, 2016/01/19 22:35:27]

Ah yes of course.

+        return "Could not install codec image generator.";
+    }
+
+    canvas->drawBitmap(bitmap, 0, 0);
+    return "";
+}
+
 Error CodecSrc::draw(SkCanvas* canvas) const {
     SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(fPath.c_str()));
     if (!encoded) {
         return SkStringPrintf("Couldn't read %s.", fPath.c_str());
     }
+
+    // The CodecImageGenerator test does not share much code with the other tests,
+    // so we will handle it in its own function.
+    if (kGen_Mode == fMode) {
+        return test_gen(canvas, encoded);
+    }
+
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
     if (nullptr == codec.get()) {
         return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str());
@@ -509,6 +535,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             }
             return "";
         }
+        default:
+            SkASSERT(false);
+            return "Invalid fMode";
     }
     return "";
 }
@@ -541,8 +570,6 @@ AndroidCodecSrc::AndroidCodecSrc(Path path, Mode mode, CodecSrc::DstColorType ds
 
 bool AndroidCodecSrc::veto(SinkFlags flags) const {
     // No need to test decoding to non-raster or indirect backend.
-    // TODO: Once we implement GPU paths (e.g. JPEG YUV), we should use a deferred decode to

[scroggo, 2016/01/19 18:37:48]

I assume the TODO has changed here? Or maybe it's just uninteresting to test
AndroidCodec with YUV, since AndroidCodec just gives us sampling, which YUV does
not support?

[msarett, 2016/01/19 22:35:27]

Yes YUV does not support sampling or native scaling.  I think testing YUV on
SkAndroidCodec is uninteresting.

-    // let the GPU handle it.
     return flags.type != SinkFlags::kRaster
         || flags.approach != SinkFlags::kDirect;
 }