Perform color correction on png decodes

src/codec/SkPngCodec.h

 /*
  * 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 "SkCodec.h"
+#include "SkColorSpaceXform.h"
 #include "SkColorTable.h"
 #include "SkPngChunkReader.h"
 #include "SkEncodedFormat.h"
 #include "SkImageInfo.h"
 #include "SkRefCnt.h"
 #include "SkSwizzler.h"
 
 #include "png.h"
 
 class SkStream;
 
 class SkPngCodec : public SkCodec {
 public:
     static bool IsPng(const char*, size_t);
 
     // Assume IsPng was called and returned true.
     static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
 
     virtual ~SkPngCodec();
 
 protected:
     Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*, int*)
             override;
     SkEncodedFormat onGetEncodedFormat() const override { return kPNG_SkEncodedFormat; }
     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; }
+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;
+    std::unique_ptr<SkColorSpaceXform> fColorXform;
+    SkAutoTMalloc<uint8_t>             fStorage;
+    uint8_t*                           fSwizzlerSrcRow;
+    uint32_t*                          fColorXformSrcRow;
+    size_t                             fSrcRowBytes;
+
+    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:
-    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;
-
-    const int                       fNumberPasses;
-    int                             fBitDepth;
-
     bool createColorTable(SkColorType dstColorType, bool premultiply, int* ctableCount);
     void destroyReadStruct();
 
     typedef SkCodec INHERITED;
 };