Refactored SkColorSpace and added in a Lab PCS GM

src/core/SkColorSpace_Base.h

Index: src/core/SkColorSpace_Base.h
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 07fa3832530d3357a45350043a5a3b2d41b41d52..f493f1418100ee9bc2d7320de95c3ae1a6b23166 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -181,16 +181,31 @@ struct SkColorLookUpTable : public SkRefCnt {
 
 class SkColorSpace_Base : public SkColorSpace {
 public:
-
     static sk_sp<SkColorSpace> NewRGB(const float gammas[3], const SkMatrix44& toXYZD50);
 
-    SkGammaNamed gammaNamed() const { return fGammaNamed; }
-    const SkGammas* gammas() const { return fGammas.get(); }
-
-    const SkColorLookUpTable* colorLUT() const { return fColorLUT.get(); }
+    /**
+     * Get the toXYZD50 storage -> PCS matrix

[msarett, 2016/10/07 01:48:36]

How about?

Describes color space gamut as a transformation to XYZ D50.  Returns nullptr if
color gamut cannot be described in terms of XYZ D50.

[raftias, 2016/10/10 20:37:33]

Done.

+     *
+     * @return A pointer to the matrix, or nullptr if an error occured

[msarett, 2016/10/07 01:48:36]

nit: Two spaces after stars (just to be consistent with rest of the file).

[raftias, 2016/10/10 20:37:33]

Done.

+     */
+    virtual const SkMatrix44* requestToXYZD50() const = 0;

[msarett, 2016/10/07 01:48:36]

I think just "toXYZD50" is fine.

[raftias, 2016/10/10 20:37:33]

Done.

+    
+    /**
+     * Get the fromXYZD50 PCS -> screen matrix

[msarett, 2016/10/07 01:48:36]

Same comments on toXYZD50 apply here.

[raftias, 2016/10/10 20:37:33]

Done.

+     *
+     * @return A pointer to the matrix, or nullptr if an error occured
+     */
+    virtual const SkMatrix44* requestFromXYZD50() const = 0;
 
-    const SkMatrix44& toXYZD50() const { return fToXYZD50; }
-    const SkMatrix44& fromXYZD50() const;
+protected:
+    SkColorSpace_Base(sk_sp<SkData> profileData);
+    
+    enum class Type : uint8_t {

[msarett, 2016/10/07 01:48:36]

Skia style would typically be:

enum Type : uint8_t {
    kXYZ_Type,
    kA2B_Type,
}

But I've just read about "enum class" in C++11...  seems cool.
@mtklein does Skia use "enum class"?  In that case, should we drop _Type?

Edit:

I actually think we might not need this - just need more virtual functions.

Maybe we need to keep it around for SkColorSpaceXform?  Or maybe we can just
check for the presence of a matrix (temporary hack)?

[raftias, 2016/10/10 20:37:33]

enum classes seem to be used in other parts of skia (from a brief codesearch)
and follow the chromium guide where _Type is dropped since they are scoped with
Type::. I did if (cs->toXYZD50()) in a lot of the tests, etc, but I kept the
type() call in SkColorSpaceXform, since it seemed a little hacky to just have
that - although it does also increase the amount of friend classes if we keep
type() there as protected.

[msarett, 2016/10/11 13:40:30]

Style sgtm.

I understand what you're saying.  Not feeling great about either option.  SGTM,
as is.

+        kXYZTRC,

[msarett, 2016/10/07 01:48:36]

I don't love these names, though I'm not sure I can suggest anything better. 
Have any ideas?  Maybe just XYZ and A2B?

[raftias, 2016/10/10 20:37:33]

Acknowledged.

+        kA2B0
+    };
+    
+    virtual Type type() const = 0;
 
 private:
 
@@ -205,23 +220,14 @@ private:
 
     static sk_sp<SkColorSpace> NewRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50);
 
-    SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44& toXYZ);
-
-    SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, SkGammaNamed gammaNamed,
-                      sk_sp<SkGammas> gammas, const SkMatrix44& toXYZ, sk_sp<SkData> profileData);
-
-    sk_sp<SkColorLookUpTable> fColorLUT;
-    const SkGammaNamed        fGammaNamed;
-    sk_sp<SkGammas>           fGammas;
-    sk_sp<SkData>             fProfileData;
 
-    const SkMatrix44          fToXYZD50;
-    mutable SkMatrix44        fFromXYZD50;
-    mutable SkOnce            fFromXYZOnce;
+    sk_sp<SkData> fProfileData;
 
     friend class SkColorSpace;
+    friend class SkColorSpaceXform;

[msarett, 2016/10/07 01:48:36]

Note to self: why does this need to be a friend class?

[raftias, 2016/10/10 20:37:33]

Acknowledged.

     friend class ColorSpaceXformTest;
     friend class ColorSpaceTest;
+    friend class LabPCSDemoGM;
     typedef SkColorSpace INHERITED;
 };