Add scanline decoding to SkCodec.

Add scanline decoding to SkCodec.

Add an interface for decoding scanlines, and implement that interface
in the PNG decoder.

Use a separate method to determine whether an image that used a type
with alpha was actually opaque.


SkScanlineDecoder.h:
New interface for decoding scanlines.

SkCodec.h:
Add getScanlineDecoder.
Add a virtual function (with non-virtual caller) for determining
whether the image truly had alpha. The client can call this to
determine if the image was actually opaque if it reported having alpha.
Remove code to sneakily change the passed in alpha type.

SkCodec_libpng.*:
Split up code onGetPixels into helper functions that can be shared with
the scanline decoder.
Implement scanline decoding.
Implement onReallyHasAlpha.

SkSwizzler.*:
Add a new SrcConfig as a default, which is invalid.
Add a function for setting fDstRow directly.
Assert fDstRow is not NULL.

BUG=skia:3257
Committed: https://skia.googlesource.com/skia/+/05245900bf6d49068b1668da1b38890a41e09bc5

[scroggo, 2015-03-18 20:19:05]

scroggo@google.com changed reviewers:
+ djsollen@google.com, msarett@google.com, reed@google.com

[scroggo, 2015-03-18 20:19:06]

https://codereview.chromium.org/1010903003/diff/40001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1010903003/diff/40001/include/codec/SkCodec.h...
include/codec/SkCodec.h:72: const SkIRect* origSubset = NULL);
I haven't yet handled the subset case, but I wanted to bounce my thoughts of you
all before I start to do it.

We discussed passing the subset in here, but I'm wondering if it's necessary. If
I was going to implement a subset decode, I would probably do something like the
following:

scanlineDecoder->skipLines(origSubset->top());
void* tmpDest = malloc(getInfo()->minRowBytes());
for (int y = origSubset->top(); y < origSubset->bottom(); y++) {
  scanlineDecoder->getNextScanline(tmpDest);
  memcpy(dest, tmpDest + (origSubset->left() * bpp),
         origSubset->width() * bpp);
}
scanlineDecoder->skipLines(getInfo()->height() - origSubset->bottom());

(The scanline implementation looks a bit different, since we don't do it in a
loop, but this is the general idea.)

This reads some extra pixels (maybe a lot, if the caller wanted a narrow
subset), but at least for PNG/JPEG, I think we can only read a row at a time
anyway. I'm guessing BMP could do something smarter, but I imagine it would
greatly complicate the code (WDYT, Matt?).

Should we just leave this to the caller? The only obvious downside is that it
makes things a little bit trickier, but not impossible, to use getScanlines(int
count).

https://codereview.chromium.org/1010903003/diff/40001/include/codec/SkCodec.h...
include/codec/SkCodec.h:81: bool reallyHasAlpha() const {
This maybe should have been a separate change, but it came up as I was
rearranging the PngCodec

[reed1, 2015-03-19 15:31:39]

https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
File src/codec/SkCodec_libpng.cpp (right):

https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:514: SkImageGenerator::Result
onGetNextScanline(void* dst) SK_OVERRIDE {
Is it easy to override the N-scanlines method here, partly to offset the cost of
the setjmp/longjmp?

Do we really need both? Why not just have the N version?

[scroggo, 2015-03-19 17:59:19]

https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
File src/codec/SkCodec_libpng.cpp (right):

https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
src/codec/SkCodec_libpng.cpp:514: SkImageGenerator::Result
onGetNextScanline(void* dst) SK_OVERRIDE {
On 2015/03/19 15:31:39, reed1 wrote:
> Is it easy to override the N-scanlines method here,

Yes.

> partly to offset the cost of
> the setjmp/longjmp?

The N version would offset it some, depending on what the caller supplied as N.

I considered adding the following function:

Result SkScanlineDecoder::setJmp();

Then our longjmp would jump back into that function, and we'd return some bad
result. This would allow us to only call setjmp once. Since not all
implementations need setjmp, and to hide what's going on a little bit from the
caller, we could change the name to something like start() (in keeping with the
call to finish(), at the end).

> 
> Do we really need both? Why not just have the N version?

Maybe not. The 1 version is somewhat simpler, since we don't require the caller
to specify the rowbytes, but certainly it could be implemented/replaced by
calling the N version (and the caller needs to know the rowbytes anyway,
assuming they are using contiguous memory).

[scroggo, 2015-03-24 15:10:39]

On 2015/03/19 17:59:19, scroggo wrote:
>
https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
> File src/codec/SkCodec_libpng.cpp (right):
> 
>
https://codereview.chromium.org/1010903003/diff/40001/src/codec/SkCodec_libpn...
> src/codec/SkCodec_libpng.cpp:514: SkImageGenerator::Result
> onGetNextScanline(void* dst) SK_OVERRIDE {
> On 2015/03/19 15:31:39, reed1 wrote:
> > Is it easy to override the N-scanlines method here,
> 
> Yes.
> 
> > partly to offset the cost of
> > the setjmp/longjmp?
> 
> The N version would offset it some, depending on what the caller supplied as
N.
> 
> I considered adding the following function:
> 
> Result SkScanlineDecoder::setJmp();
> 
> Then our longjmp would jump back into that function, and we'd return some bad
> result. This would allow us to only call setjmp once. Since not all
> implementations need setjmp, and to hide what's going on a little bit from the
> caller, we could change the name to something like start() (in keeping with
the
> call to finish(), at the end).
> 
> > 
> > Do we really need both? Why not just have the N version?
> 
> Maybe not. The 1 version is somewhat simpler, since we don't require the
caller
> to specify the rowbytes, but certainly it could be implemented/replaced by
> calling the N version (and the caller needs to know the rowbytes anyway,
> assuming they are using contiguous memory).

Patch set 11 removes the single line version (though we could easily add it back
in a way that calls the N line version, for convenience).

It also drops subsetting. If we think there's a benefit to adding it, we can add
it back later.

PTAL.

[reed1, 2015-03-24 15:45:20]

https://codereview.chromium.org/1010903003/diff/200001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1010903003/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:75: SkScanlineDecoder* getScanlineDecoder(const
SkImageInfo& dstInfo);
1. Create a new scanline decoder
2. The decoder is owned by the codec

These two seem a little at odds. I was imagining that I could create multiple
scanliners, but now I'm not sure.

[scroggo, 2015-03-24 16:55:47]

https://codereview.chromium.org/1010903003/diff/200001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1010903003/diff/200001/include/codec/SkCodec....
include/codec/SkCodec.h:75: SkScanlineDecoder* getScanlineDecoder(const
SkImageInfo& dstInfo);
On 2015/03/24 15:45:20, reed1 wrote:
> 1. Create a new scanline decoder
> 2. The decoder is owned by the codec
> 
> These two seem a little at odds. I was imagining that I could create multiple
> scanliners, but now I'm not sure.

I have updated the comment. I think it's more clear now.

[reed1, 2015-03-24 17:16:35]

api lgtm w/ ...

Can we get ride of reallyHasAlpha by ...
1. deciding its not important/relevant anymore
2. out-param on getPixels and scanline->finish()
3. keep it, but return an enum so the caller knows when we don't know.

[scroggo, 2015-03-24 21:28:53]

On 2015/03/24 17:16:35, reed1 wrote:
> api lgtm w/ ...
> 
> Can we get ride of reallyHasAlpha by ...
> 1. deciding its not important/relevant anymore
> 2. out-param on getPixels and scanline->finish()
> 3. keep it, but return an enum so the caller knows when we don't know.

I've updated the bug to track this.

Derek, can you review the code?

[djsollen, 2015-03-25 13:08:42]

looking pretty good!

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkCodec....
include/codec/SkCodec.h:67: *  Calling a second time will rewind and replace the
existing one with a
What if the stream cannot be rewound?

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkScanli...
File include/codec/SkScanlineDecoder.h (right):

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkScanli...
include/codec/SkScanlineDecoder.h:86: SkAutoMalloc
storage(fDstInfo.minRowBytes());
do the codecs provide no way to skip this data without actually requiring us to
write it to a temporary buffer?  It seems like wasted time to issue a bunch of
writes to memory that we will be deleting.

https://codereview.chromium.org/1010903003/diff/300001/src/codec/SkCodec_libp...
File src/codec/SkCodec_libpng.h (right):

https://codereview.chromium.org/1010903003/diff/300001/src/codec/SkCodec_libp...
src/codec/SkCodec_libpng.h:41: SkSwizzler::SrcConfig       fSc;
I had a hard time remembering the meaning of this variable in the cpp file. Can
we make it more descriptive? Something like fSrcConfig?

[scroggo, 2015-03-25 13:49:15]

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkCodec....
include/codec/SkCodec.h:67: *  Calling a second time will rewind and replace the
existing one with a
On 2015/03/25 13:08:42, djsollen wrote:
> What if the stream cannot be rewound?

I've added a comment that we will return NULL.

For our methods that return objects, instead of SkImageGenerator::Result, I've
been thinking about using an out parameter Result so we can let the caller know
*why* we didn't create an object. I think Android would appreciate that since it
allows us to throw a Java exception describing the failure better. Thoughts?

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkScanli...
File include/codec/SkScanlineDecoder.h (right):

https://codereview.chromium.org/1010903003/diff/300001/include/codec/SkScanli...
include/codec/SkScanlineDecoder.h:86: SkAutoMalloc
storage(fDstInfo.minRowBytes());
On 2015/03/25 13:08:42, djsollen wrote:
> do the codecs provide no way to skip this data without actually requiring us
to
> write it to a temporary buffer? 

I haven't looked at them all, but to my surprise, png_read_rows actually does
handle a NULL row pointer. Will update. (FWIW, SkImageDecoder_libpng uses a
similar method for skipping rows....)

> It seems like wasted time to issue a bunch of
> writes to memory that we will be deleting.

Agreed. This is the default implementation, which is not necessarily optimal. If
we think that all our decoders can be smart enough to do something better,
perhaps this should be pure virtual.

https://codereview.chromium.org/1010903003/diff/300001/src/codec/SkCodec_libp...
File src/codec/SkCodec_libpng.h (right):

https://codereview.chromium.org/1010903003/diff/300001/src/codec/SkCodec_libp...
src/codec/SkCodec_libpng.h:41: SkSwizzler::SrcConfig       fSc;
On 2015/03/25 13:08:42, djsollen wrote:
> I had a hard time remembering the meaning of this variable in the cpp file.
Can
> we make it more descriptive? Something like fSrcConfig?

Done.

[djsollen, 2015-03-25 14:13:29]

lgtm

[scroggo, 2015-03-25 14:13:49]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-03-25 14:13:50]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1010903003/#ps320001
(title: "Respond to Derek's comments on patch set 16")

[commit-bot: I haz the power, 2015-03-25 14:14:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010903003/320001

[commit-bot: I haz the power, 2015-03-25 14:14:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-25 14:14:13]

Failed to apply patch for src/codec/SkCodec_libpng.cpp:
While running git apply --index -3 -p1;
  error: patch failed: src/codec/SkCodec_libpng.cpp:362
  Falling back to three-way merge...
  <stdin>:352: new blank line at EOF.
  +
  Applied patch to 'src/codec/SkCodec_libpng.cpp' with conflicts.
  U src/codec/SkCodec_libpng.cpp
  warning: 1 line adds whitespace errors.

Patch:       src/codec/SkCodec_libpng.cpp
Index: src/codec/SkCodec_libpng.cpp
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index
57653e74df30b44f379a755a2640f2030b3162a1..14747bcfdf0e9cfa9d7e640b466490da42e41ca2
100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -11,6 +11,7 @@
 #include "SkColorTable.h"
 #include "SkBitmap.h"
 #include "SkMath.h"
+#include "SkScanlineDecoder.h"
 #include "SkSize.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
@@ -114,15 +115,13 @@ typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU
g, U8CPU b);
 
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily
 // the case here.
-SkColorTable* decode_palette(png_structp png_ptr, png_infop info_ptr,
-                             bool premultiply, SkAlphaType* outAlphaType) {
-    SkASSERT(outAlphaType != NULL);
+bool SkPngCodec::decodePalette(bool premultiply) {
     int numPalette;
     png_colorp palette;
     png_bytep trans;
 
-    if (!png_get_PLTE(png_ptr, info_ptr, &palette, &numPalette)) {
-        return NULL;
+    if (!png_get_PLTE(fPng_ptr, fInfo_ptr, &palette, &numPalette)) {
+        return false;
     }
 
     /*  BUGGY IMAGE WORKAROUND
@@ -137,8 +136,8 @@ SkColorTable* decode_palette(png_structp png_ptr, png_infop
info_ptr,
     SkPMColor* colorPtr = colorStorage;
 
     int numTrans;
-    if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
-        png_get_tRNS(png_ptr, info_ptr, &trans, &numTrans, NULL);
+    if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
+        png_get_tRNS(fPng_ptr, fInfo_ptr, &trans, &numTrans, NULL);
     } else {
         numTrans = 0;
     }
@@ -165,11 +164,7 @@ SkColorTable* decode_palette(png_structp png_ptr, png_infop
info_ptr,
         palette++;
     }
 
-    if (transLessThanFF < 0) {
-        *outAlphaType = premultiply ? kPremul_SkAlphaType :
kUnpremul_SkAlphaType;
-    } else {
-        *outAlphaType = kOpaque_SkAlphaType;
-    }
+    fReallyHasAlpha = transLessThanFF < 0;
 
     for (; index < numPalette; index++) {
         *colorPtr++ = SkPackARGB32(0xFF, palette->red, palette->green,
palette->blue);
@@ -181,7 +176,8 @@ SkColorTable* decode_palette(png_structp png_ptr, png_infop
info_ptr,
         *colorPtr = colorPtr[-1];
     }
 
-    return SkNEW_ARGS(SkColorTable, (colorStorage, colorCount));
+    fColorTable.reset(SkNEW_ARGS(SkColorTable, (colorStorage, colorCount)));
+    return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -333,11 +329,16 @@ SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
     return codec;
 }
 
+#define INVALID_NUMBER_PASSES -1
 SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
                        png_structp png_ptr, png_infop info_ptr)
     : INHERITED(info, stream)
     , fPng_ptr(png_ptr)
-    , fInfo_ptr(info_ptr) {}
+    , fInfo_ptr(info_ptr)
+    , fSrcConfig(SkSwizzler::kUnknown)
+    , fNumberPasses(INVALID_NUMBER_PASSES)
+    , fReallyHasAlpha(false)
+{}
 
 SkPngCodec::~SkPngCodec() {
     png_destroy_read_struct(&fPng_ptr, &fInfo_ptr, png_infopp_NULL);
@@ -362,29 +363,9 @@ static bool conversion_possible(const SkImageInfo& dst,
const SkImageInfo& src)
             kUnpremul_SkAlphaType == src.alphaType();
 }
 
-SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
dst,
-                                        size_t rowBytes, const Options&
options,
-                                        SkPMColor ctable[], int* ctableCount) {
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
-    if (requestedInfo.dimensions() != this->getOriginalInfo().dimensions()) {
-        return kInvalidScale;
-    }
-    if (!conversion_possible(requestedInfo, this->getOriginalInfo())) {
-        return kInvalidConversion;
-    }
-
-    SkBitmap decodedBitmap;
-    // If installPixels would have failed, getPixels should have failed before
-    // calling onGetPixels.
-    SkAssertResult(decodedBitmap.installPixels(requestedInfo, dst, rowBytes));
-
-    // Initialize all non-trivial objects before setjmp.
-    SkAutoTUnref<SkColorTable> colorTable;
-    SkAutoTDelete<SkSwizzler> swizzler;
-    SkAutoMalloc storage;                   // Scratch memory for pre-swizzled
rows.
-
+SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo&
requestedInfo,
+                                               void* dst, size_t rowBytes,
+                                               const Options& options) {
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     if (setjmp(png_jmpbuf(fPng_ptr))) {
@@ -398,42 +379,30 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo&
requestedInfo, void*
     png_get_IHDR(fPng_ptr, fInfo_ptr, &origWidth, &origHeight, &bitDepth,
                  &pngColorType, &interlaceType, int_p_NULL, int_p_NULL);
 
-    const int numberPasses = (interlaceType != PNG_INTERLACE_NONE) ?
+    fNumberPasses = (interlaceType != PNG_INTERLACE_NONE) ?
             png_set_interlace_handling(fPng_ptr) : 1;
 
-    SkSwizzler::SrcConfig sc;
-    bool reallyHasAlpha = false;
+    // Set to the default before calling decodePalette, which may change it.
+    fReallyHasAlpha = false;
     if (PNG_COLOR_TYPE_PALETTE == pngColorType) {
-        sc = SkSwizzler::kIndex;
-        SkAlphaType at = requestedInfo.alphaType();
-        colorTable.reset(decode_palette(fPng_ptr, fInfo_ptr,
-                                        kPremul_SkAlphaType == at,
-                                        &at));
-        if (!colorTable) {
+        fSrcConfig = SkSwizzler::kIndex;
+        if (!this->decodePalette(kPremul_SkAlphaType ==
requestedInfo.alphaType())) {
             return kInvalidInput;
         }
-
-        reallyHasAlpha = (at != kOpaque_SkAlphaType);
-
-        if (at != requestedInfo.alphaType()) {
-            // It turns out the image is opaque.
-            SkASSERT(kOpaque_SkAlphaType == at);
-        }
     } else if (kAlpha_8_SkColorType == requestedInfo.colorType()) {
         // Note: we check the destination, since otherwise we would have
         // told png to upscale.
         SkASSERT(PNG_COLOR_TYPE_GRAY == pngColorType);
-        sc = SkSwizzler::kGray;
-    } else if (this->getOriginalInfo().alphaType() == kOpaque_SkAlphaType) {
-        sc = SkSwizzler::kRGBX;
+        fSrcConfig = SkSwizzler::kGray;
+    } else if (this->getInfo().alphaType() == kOpaque_SkAlphaType) {
+        fSrcConfig = SkSwizzler::kRGBX;
     } else {
-        sc = SkSwizzler::kRGBA;
+        fSrcConfig = SkSwizzler::kRGBA;
     }
-    const SkPMColor* colors = colorTable ? colorTable->readColors() : NULL;
-    swizzler.reset(SkSwizzler::CreateSwizzler(sc, colors, requestedInfo,
-                                              dst, rowBytes,
-                                              options.fZeroInitialized));
-    if (!swizzler) {
+    const SkPMColor* colors = fColorTable ? fColorTable->readColors() : NULL;
+    fSwizzler.reset(SkSwizzler::CreateSwizzler(fSrcConfig, colors,
requestedInfo,
+            dst, rowBytes, options.fZeroInitialized));
+    if (!fSwizzler) {
         // FIXME: CreateSwizzler could fail for another reason.
         return kUnimplemented;
     }
@@ -442,16 +411,47 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo&
requestedInfo, void*
     // made in the factory.
     png_read_update_info(fPng_ptr, fInfo_ptr);
 
-    if (numberPasses > 1) {
+    return kSuccess;
+}
+
+SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
dst,
+                                        size_t rowBytes, const Options&
options,
+                                        SkPMColor ctable[], int* ctableCount) {
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+    if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
+        return kInvalidScale;
+    }
+    if (!conversion_possible(requestedInfo, this->getInfo())) {
+        return kInvalidConversion;
+    }
+
+    const Result result = this->initializeSwizzler(requestedInfo, dst,
rowBytes,
+                                                   options);
+    if (result != kSuccess) {
+        return result;
+    }
+
+    // FIXME: Could we use the return value of setjmp to specify the type of
+    // error?
+    if (setjmp(png_jmpbuf(fPng_ptr))) {
+        SkDebugf("setjmp long jump!\n");
+        return kInvalidInput;
+    }
+
+    SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
+    SkAutoMalloc storage;
+    if (fNumberPasses > 1) {
         const int width = requestedInfo.width();
         const int height = requestedInfo.height();
-        const int bpp = SkSwizzler::BytesPerPixel(sc);
+        const int bpp = SkSwizzler::BytesPerPixel(fSrcConfig);
         const size_t rowBytes = width * bpp;
 
         storage.reset(width * height * bpp);
         uint8_t* const base = static_cast<uint8_t*>(storage.get());
 
-        for (int i = 0; i < numberPasses; i++) {
+        for (int i = 0; i < fNumberPasses; i++) {
             uint8_t* row = base;
             for (int y = 0; y < height; y++) {
                 uint8_t* bmRow = row;
@@ -463,27 +463,118 @@ SkCodec::Result SkPngCodec::onGetPixels(const
SkImageInfo& requestedInfo, void*
         // Now swizzle it.
         uint8_t* row = base;
         for (int y = 0; y < height; y++) {
-            reallyHasAlpha |= !SkSwizzler::IsOpaque(swizzler->next(row));
+            fReallyHasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->next(row));
             …
(message too large)

[scroggo, 2015-03-25 14:27:03]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-03-25 14:27:04]

The patchset sent to the CQ was uploaded after l-g-t-m from djsollen@google.com,
reed@google.com
Link to the patchset: https://codereview.chromium.org/1010903003/#ps360001
(title: "Finish rebase")

[commit-bot: I haz the power, 2015-03-25 14:27:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010903003/360001

[commit-bot: I haz the power, 2015-03-25 14:28:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-25 14:28:33]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[scroggo, 2015-03-25 15:55:45]

https://codereview.chromium.org/1010903003/diff/380001/src/codec/SkCodec_libp...
File src/codec/SkCodec_libpng.cpp (right):

https://codereview.chromium.org/1010903003/diff/380001/src/codec/SkCodec_libp...
src/codec/SkCodec_libpng.cpp:512: // Using this local variable will prevent the
clobber error.
I think?

[scroggo, 2015-03-25 17:36:57]

The CQ bit was checked by scroggo@google.com

[scroggo, 2015-03-25 17:36:58]

The patchset sent to the CQ was uploaded after l-g-t-m from djsollen@google.com,
reed@google.com
Link to the patchset: https://codereview.chromium.org/1010903003/#ps420001
(title: "Build with no-clobbered")

[commit-bot: I haz the power, 2015-03-25 17:37:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010903003/420001

[commit-bot: I haz the power, 2015-03-25 18:11:57]

Committed patchset #22 (id:420001) as
https://skia.googlesource.com/skia/+/05245900bf6d49068b1668da1b38890a41e09bc5