Perform color correction on png decodes

src/codec/SkPngCodec.h

Index: src/codec/SkPngCodec.h
diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 0a8878395a967c7d7b4a8733b03447bcc1827107..d2a75e658c047eeec2b209ee76704537514ea7ab 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -6,6 +6,7 @@
  */
 
 #include "SkCodec.h"
+#include "SkColorSpaceXform.h"
 #include "SkColorTable.h"
 #include "SkPngChunkReader.h"
 #include "SkEncodedFormat.h"
@@ -33,33 +34,39 @@ protected:
     bool onRewind() override;
     uint32_t onGetFillValue(SkColorType) const override;
 
-    // Helper to set up swizzler and color table. Also calls png_read_update_info.
-    Result initializeSwizzler(const SkImageInfo& requestedInfo, const Options&,
-                              SkPMColor*, int* ctableCount);
+    // Helper to set up swizzler, color xforms, and color table. Also calls png_read_update_info.
+    bool initializeXforms(const SkImageInfo& requestedInfo, const Options&, SkPMColor* colorPtr,
+                          int* colorCount);
     SkSampler* getSampler(bool createIfNecessary) override {
         SkASSERT(fSwizzler);
         return fSwizzler;
     }
+    void allocateStorage(const SkImageInfo& dstInfo);
+
+    virtual int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count,
+                         int startRow) = 0;
 
     SkPngCodec(int width, int height, const SkEncodedInfo&, SkStream*, SkPngChunkReader*,
             png_structp, png_infop, int, int, sk_sp<SkColorSpace>);
 
-    png_structp png_ptr() { return fPng_ptr; }
-    SkSwizzler* swizzler() { return fSwizzler; }
-    int numberPasses() const { return fNumberPasses; }
-
-private:
-    SkAutoTUnref<SkPngChunkReader>  fPngChunkReader;
-    png_structp                     fPng_ptr;
-    png_infop                       fInfo_ptr;
+protected:

[mtklein, 2016/07/29 18:48:56]

You're already protected here.

[msarett, 2016/07/29 20:07:51]

Of course, got it.

+    SkAutoTUnref<SkPngChunkReader>     fPngChunkReader;
+    png_structp                        fPng_ptr;
+    png_infop                          fInfo_ptr;
 
     // These are stored here so they can be used both by normal decoding and scanline decoding.
-    SkAutoTUnref<SkColorTable>      fColorTable;    // May be unpremul.
-    SkAutoTDelete<SkSwizzler>       fSwizzler;
+    SkAutoTUnref<SkColorTable>         fColorTable;    // May be unpremul.
+    SkAutoTDelete<SkSwizzler>          fSwizzler;
+    std::unique_ptr<SkColorSpaceXform> fColorXform;
+    SkAutoTMalloc<uint8_t>             fStorage;
+    uint8_t*                           fSwizzlerSrcRow;
+    uint32_t*                          fColorXformSrcRow;
+    size_t                             fSrcRowBytes;
 
-    const int                       fNumberPasses;
-    int                             fBitDepth;
+    const int                          fNumberPasses;
+    int                                fBitDepth;

[mtklein, 2016/07/29 18:48:56]

I usually find that when data is protected as opposed to public or private,
there's usually a better (flatter) way to design the hierarchy.  

Could you, e.g., make a standalone PngCommon class in the .cpp and a bunch of
SkPngCodecFoo that implement SkCodec directly, each having an instance of
PngCommon?

[msarett, 2016/07/29 20:07:50]

I think you make a good point...

I did my best to refactor with Leon's CL in mind
(https://codereview.chromium.org/1997703003/) to minimize the headache for him
when he gets back.  I think I've managed a similar design.  I'll probably try to
avoid shaking things up any more than I already have.

 
+private:
     bool createColorTable(SkColorType dstColorType, bool premultiply, int* ctableCount);
     void destroyReadStruct();