Move all knowledge of X sampling into SkScaledCodec

src/codec/SkCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBmpCodec.h"
 #include "SkCodec.h"
 #include "SkData.h"
@@ skipping 134 matching lines @@
     }
 
     if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
 
     // Default options.
     Options optsStorage;
     if (nullptr == options) {
         options = &optsStorage;
+    } else if (options->fSubset) {

[msarett, 2015/10/01 19:34:33]

I'm guessing that, for some reason (maybe the kSubset_Mode in DM), it is
necessary to return kUnimplemented for a subset decode before returning
kInvalidScale for a badly scaled decode.  So you have added this to make sure
that the checks are still in the right order?

[scroggo, 2015/10/01 21:16:57]

That is correct. I don't think this is best long term solution necessarily, but
I think this is good enough until we have subsets in SkScaledCodec.

+        SkIRect subset(*options->fSubset);
+        if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
+            // FIXME: How to differentiate between not supporting subset at all
+            // and not supporting this particular subset?
+            return kUnimplemented;
+        }
     }
+
+    // FIXME: Support subsets somehow? Note that this works for SkWebpCodec

[msarett, 2015/10/01 19:34:33]

There's a couple ways we could go about this, I don't think either of them is
too tricky.

We may eventually want to combine dimensionsSupported() and options->fSubset
into one check.

[scroggo, 2015/10/01 21:16:57]

Yeah, I considered that. I decided to punt that until SkScaledCodec supports
subsets.

[msarett, 2015/10/01 22:53:07]

That makes perfect sense.

+    // because it supports arbitrary scaling/subset combinations.
+    if (!this->dimensionsSupported(info.dimensions())) {
+        return kInvalidScale;
+    }
+
     const Result result = this->onGetPixels(info, pixels, rowBytes, *options, ctable, ctableCount);
 
     if ((kIncompleteInput == result || kSuccess == result) && ctableCount) {
         SkASSERT(*ctableCount >= 0 && *ctableCount <= 256);
     }
     return result;
 }
 
 SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
     return this->getPixels(info, pixels, rowBytes, nullptr, nullptr, nullptr);
@@ skipping 17 matching lines @@
     }
 
     if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
 
     // Set options.
     Options optsStorage;
     if (nullptr == options) {
         options = &optsStorage;
+    } else if (options->fSubset) {
+        SkIRect subset(*options->fSubset);
+        if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
+            // FIXME: How to differentiate between not supporting subset at all
+            // and not supporting this particular subset?
+            return kUnimplemented;
+        }
+    }
+
+    // FIXME: Support subsets somehow?
+    if (!this->dimensionsSupported(dstInfo.dimensions())) {
+        return kInvalidScale;
     }
 
     const Result result = this->onStartScanlineDecode(dstInfo, *options, ctable, ctableCount);
     if (result != SkCodec::kSuccess) {
         return result;
     }
 
     fCurrScanline = 0;
     fDstInfo = dstInfo;
     fOptions = *options;
     return kSuccess;
 }
 
 SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo) {
     return this->startScanlineDecode(dstInfo, nullptr, nullptr, nullptr);
 }
 
 SkCodec::Result SkCodec::getScanlines(void* dst, int countLines, size_t rowBytes) {
     if (fCurrScanline < 0) {
         return kScanlineDecodingNotStarted;
     }
 
     SkASSERT(!fDstInfo.isEmpty());
-    if ((rowBytes < fDstInfo.minRowBytes() && countLines > 1 ) || countLines <= 0
-         || fCurrScanline + countLines > fDstInfo.height()) {
+
+    if (rowBytes < fDstInfo.minRowBytes() && countLines > 1) {
+        // FIXME: This check breaks down when we sample without letting SkCodec

[scroggo, 2015/10/01 15:33:14]

The problem shows up when we want to scale down an image using SkScaledCodec.
(Particularly when using an interlaced PNG, since it uses a countLines > 1.)
fDstInfo has the width of the entire image, while the SkScaledCodec is using the
scaled width.

I have a couple of ideas here:
- We could skip the check entirely, which allows smart clients to work
(slightly) faster, and forces dumb clients to figure out why they're failing
(keeping in mind that we may end up being dumb clients...)
- We could provide some kind of function that tells the minimum row bytes, which
computes it based on a sampler.

[msarett, 2015/10/01 19:34:33]

I've deleted this check in my scaled subsetting CL.  Turns out it is also
annoying when we only want to decode part of a scanline.

So yeah, I'm in favor of getting rid of it.

[scroggo, 2015/10/01 21:16:57]

Done.

+        // know about it.
+        // return kInvalidParameters;
+    }
+
+    if (countLines <= 0 || fCurrScanline + countLines > fDstInfo.height()) {
         return kInvalidParameters;
     }
 
     const Result result = this->onGetScanlines(dst, countLines, rowBytes);
     fCurrScanline += countLines;
     return result;
 }
 
 SkCodec::Result SkCodec::skipScanlines(int countLines) {
     if (fCurrScanline < 0) {
         return kScanlineDecodingNotStarted;
     }
 
     SkASSERT(!fDstInfo.isEmpty());
     if (fCurrScanline + countLines > fDstInfo.height()) {
         // Arguably, we could just skip the scanlines which are remaining,
         // and return kSuccess. We choose to return invalid so the client
         // can catch their bug.
         return SkCodec::kInvalidParameters;
     }
 
     const Result result = this->onSkipScanlines(countLines);
     fCurrScanline += countLines;
     return result;
 }