commit 0741818e7ab4e9ea8505b8a8687412f0e3804c65
author Leon Scroggins III <scroggo@google.com> Tue Aug 15 12:24:02 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Aug 17 16:09:31 2017 +0000
tree ac183b3107e97b0717319fd403f6d552fdbd840f
parent de44c2df9469e70f57f05d262cd1a245b2ff5e90

Move calls to conversion_possible to SkCodec

Move common code into the base class, so subclasses need not call
conversion_possible.

Use SkEncodedInfo rather than SkImageInfo, and use the proper frame.

API Changes:
- SkAndroidCodec:
  - Add getEncodedInfo(), for SkBitmapRegionCodec
- SkEncodedInfo:
  - Add opaque() helper
- SkBitmapRegionDecoder:
  - Remove unused conversionSupported

(Split off from skia-review.googlesource.com/c/25746)

Bug: skia:5601
Change-Id: If4a40d4b98a3dd0afde2b6058f92315a393a5baf
Reviewed-on: https://skia-review.googlesource.com/34361
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

src/codec/SkCodec.cpp

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 2b61082..84af227 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -155,6 +155,29 @@
 
 SkCodec::~SkCodec() {}
 
+bool SkCodec::conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor,
+                                  bool srcIsOpaque, const SkColorSpace* srcCS) const {
+    if (!valid_alpha(dst.alphaType(), srcIsOpaque)) {
+        return false;
+    }
+
+    switch (dst.colorType()) {
+        case kRGBA_8888_SkColorType:
+        case kBGRA_8888_SkColorType:
+            return true;
+        case kRGBA_F16_SkColorType:
+            return dst.colorSpace() && dst.colorSpace()->gammaIsLinear();
+        case kRGB_565_SkColorType:
+            return srcIsOpaque;
+        case kGray_8_SkColorType:
+            return SkEncodedInfo::kGray_Color == srcColor && srcIsOpaque &&
+                   !needs_color_xform(dst, srcCS, false);
+        default:
+            return false;
+    }
+
+}
+
 bool SkCodec::rewindIfNeeded() {
     // Store the value of fNeedsRewind so we can update it. Next read will
     // require a rewind.
@@ -194,6 +217,10 @@
                                           const Options& options) {
     const int index = options.fFrameIndex;
     if (0 == index) {
+        if (!this->conversionSupported(info, fEncodedInfo.color(), fEncodedInfo.opaque(),
+                                      fSrcInfo.colorSpace())) {
+            return kInvalidConversion;
+        }
         return kSuccess;
     }
 
@@ -213,6 +240,11 @@
     const auto* frame = frameHolder->getFrame(index);
     SkASSERT(frame);
 
+    if (!this->conversionSupported(info, fEncodedInfo.color(), !frame->hasAlpha(),
+                                   fSrcInfo.colorSpace())) {
+        return kInvalidConversion;
+    }
+
     const int requiredFrame = frame->getRequiredFrame();
     if (requiredFrame == kNone) {
         return kSuccess;
@@ -278,10 +310,6 @@
     if (nullptr == options) {
         options = &optsStorage;
     } else {
-        const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options);
-        if (frameIndexResult != kSuccess) {
-            return frameIndexResult;
-        }
         if (options->fSubset) {
             SkIRect subset(*options->fSubset);
             if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
@@ -292,6 +320,12 @@
         }
     }
 
+    const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes,
+                                                           *options);
+    if (frameIndexResult != kSuccess) {
+        return frameIndexResult;
+    }
+
     // FIXME: Support subsets somehow? Note that this works for SkWebpCodec
     // because it supports arbitrary scaling/subset combinations.
     if (!this->dimensionsSupported(info.dimensions())) {
@@ -350,10 +384,6 @@
     if (nullptr == options) {
         options = &optsStorage;
     } else {
-        const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options);
-        if (frameIndexResult != kSuccess) {
-            return frameIndexResult;
-        }
         if (options->fSubset) {
             SkIRect size = SkIRect::MakeSize(info.dimensions());
             if (!size.contains(*options->fSubset)) {
@@ -368,6 +398,12 @@
         }
     }
 
+    const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes,
+                                                           *options);
+    if (frameIndexResult != kSuccess) {
+        return frameIndexResult;
+    }
+
     if (!this->dimensionsSupported(info.dimensions())) {
         return kInvalidScale;
     }
@@ -418,6 +454,18 @@
         }
     }
 
+    // Scanline decoding only supports decoding the first frame.
+    if (options->fFrameIndex != 0) {
+        return kUnimplemented;
+    }
+
+    // The void* dst and rowbytes in handleFrameIndex or only used for decoding prior
+    // frames, which is not supported here anyway, so it is safe to pass nullptr/0.
+    const Result frameIndexResult = this->handleFrameIndex(info, nullptr, 0, *options);
+    if (frameIndexResult != kSuccess) {
+        return frameIndexResult;
+    }
+
     // FIXME: Support subsets somehow?
     if (!this->dimensionsSupported(info.dimensions())) {
         return kInvalidScale;
@@ -567,7 +615,7 @@
     fXformOnDecode = false;
     bool needsColorCorrectPremul = needs_premul(dstInfo, fEncodedInfo) &&
                                    SkTransferFunctionBehavior::kRespect == premulBehavior;
-    if (needs_color_xform(dstInfo, fSrcInfo, needsColorCorrectPremul)) {
+    if (needs_color_xform(dstInfo, fSrcInfo.colorSpace(), needsColorCorrectPremul)) {
         fColorXform = SkColorSpaceXform_Base::New(fSrcInfo.colorSpace(), dstInfo.colorSpace(),
                                                   premulBehavior);
         if (!fColorXform) {