Add the ability to decode a subset to SkCodec.

src/codec/SkWebpCodec.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 "SkWebpCodec.h"
 #include "SkTemplates.h"
 
@@ skipping 108 matching lines @@
             return MODE_LAST;
     }
 }
 
 // The WebP decoding API allows us to incrementally pass chunks of bytes as we receive them to the
 // decoder with WebPIAppend. In order to do so, we need to read chunks from the SkStream. This size
 // is arbitrary.
 static const size_t BUFFER_SIZE = 4096;
 
 SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
-                                         const Options&, SkPMColor*, int*) {
+                                         const Options& options, SkPMColor*, int*) {
     switch (this->rewindIfNeeded()) {
         case kCouldNotRewind_RewindState:
             return kCouldNotRewind;
         case kRewound_RewindState:
             // Rewound to the beginning. Since creation only does a peek, the stream is at the
             // correct position.
             break;
         case kNoRewindNecessary_RewindState:
             // Already at the right spot for decoding.
             break;
     }
 
     if (!conversion_possible(dstInfo, this->getInfo())) {
         return kInvalidConversion;
     }
 
     WebPDecoderConfig config;
     if (0 == WebPInitDecoderConfig(&config)) {
         // ABI mismatch.
         // FIXME: New enum for this?
         return kInvalidInput;
     }
 
     // Free any memory associated with the buffer. Must be called last, so we declare it first.
     SkAutoTCallVProc<WebPDecBuffer, WebPFreeDecBuffer> autoFree(&(config.output));
 
-    SkISize dimensions = dstInfo.dimensions();
-    if (this->getInfo().dimensions() != dimensions) {
+    SkIRect bounds = SkIRect::MakeSize(this->getInfo().dimensions());
+    if (!options.fSubset.isEmpty()) {
+        // Caller is requesting a subset.
+        if (!bounds.intersect(options.fSubset)) {
+            // The subset is out of bounds.
+            return kInvalidParameters;
+        }
+
+        // This is tricky. libwebp snaps the top and left to even values. Rather than claim we
+        // supported this, when we really did something different, report  that we cannot

[emmaleer, 2015/07/17 22:31:05]

What is the different thing we did, instead of crop the top and left to even
values?
You could add what that different thing is to the comment so it's more clear

[scroggo, 2015/07/20 14:25:42]

Done. Please take a look and let me know if you think this is more clear.

I think this raises a good question about the API contract. As it stands, the
header does not mention any caveats about what kinds of subsets are valid (I
should at least mention that it should intersect with the original bounds,
unless that is obvious...). 

Although we're in control of the main client (BitmapRegionDecoder on Android),
other clients may want to programmatically decide what subsets to use, and only
luck will determine whether they choose an odd top/left. In order to figure out
why they received kInvalidParameters, they'll need to debug or search through
the code.

It does seem weird to have a restriction that depends on a particular
implementation. On the other hand, it is currently the only implementation.
(We've talked about making the default version use the scanline decoder; if we
decided to take that approach, the other implementations would not have this
restriction.) That said, I do not have a better solution, and have updated the
header.

(As a side note, SkImageDecoder's API allows a little bit more flexibility here.
Since it allocates the memory itself, it could always allocate a larger amount
of memory (for the extra row/column that libwebp snapped to) and return an
SkBitmap using SkBitmap::extractSubset or copy the data without the extra
row/column, depending on whether the client was more concerned about the extra
time or extra memory. On the other hand, the API of SkCodec forces the client to
make those decisions, rather than leaving us to guess.)

+        // handle it.
+        if (!SkIsAlign2(bounds.fLeft) || !SkIsAlign2(bounds.fTop)) {
+            return kInvalidParameters;
+        }
+
+        config.options.use_cropping = 1;
+        config.options.crop_left = bounds.fLeft;
+        config.options.crop_top = bounds.fTop;
+        config.options.crop_width = bounds.width();
+        config.options.crop_height = bounds.height();
+    }
+
+    SkISize dstDimensions = dstInfo.dimensions();
+    if (bounds.size() != dstDimensions) {
         // Caller is requesting scaling.
         config.options.use_scaling = 1;
-        config.options.scaled_width = dimensions.width();
-        config.options.scaled_height = dimensions.height();
+        config.options.scaled_width = dstDimensions.width();
+        config.options.scaled_height = dstDimensions.height();

[emmaleer, 2015/07/17 22:31:05]

Should we check that the width and height are being scaled by the same number?
Here we just set the width and height to the dstDimensions. 
But what if the caller didn't correctly scale these values, or scaled width and
height by different values?
Maybe we should do some sort of checks before setting the scaled_width and
scaled_height.

[scroggo, 2015/07/20 14:25:42]

Not sure. As I understand it, libwebp allows scaling width and height by
different factors. Although our other implementations require using the same
factor, I am not sure there's harm in letting libwebp do its thing. I do have a
concern about requiring the same factor though - if we say something like the
following must be true:

origW / scaledW == origH / scaledH

and we rounded differently from the client, we may say they are not using the
same scale factor in each dimension.

     }
 
     config.output.colorspace = webp_decode_mode(dstInfo.colorType(),
             dstInfo.alphaType() == kPremul_SkAlphaType);
     config.output.u.RGBA.rgba = (uint8_t*) dst;
     config.output.u.RGBA.stride = (int) rowBytes;
     config.output.u.RGBA.size = dstInfo.getSafeSize(rowBytes);
     config.output.is_external_memory = 1;
 
     SkAutoTCallVProc<WebPIDecoder, WebPIDelete> idec(WebPIDecode(NULL, 0, &config));
@@ skipping 19 matching lines @@
                 // Break out of the switch statement. Continue the loop.
                 break;
             default:
                 return kInvalidInput;
         }
     }
 }
 
 SkWebpCodec::SkWebpCodec(const SkImageInfo& info, SkStream* stream)
     : INHERITED(info, stream) {}