Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(135)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/core/SkColorSpace.cpp

Issue 2389983002: Refactored SkColorSpace and added in a Lab PCS GM (Closed)
Patch Set: responding to comments Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« include/core/SkColorSpace.h ('K') | « resources/icc_profiles/srgb_lab_pcs.icc ('k') | src/core/SkColorSpaceXform.h » ('j') | src/core/SkColorSpaceXform.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/core/SkColorSpace.cpp
diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index c6bf4b9431300460b4fbe4dd2b6f276de9dda4a2..f577d3a50750935e0bb4310e4b496a85977dc611 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -7,26 +7,13 @@
#include "SkColorSpace.h"
#include "SkColorSpace_Base.h"
+#include "SkColorSpace_A2B0.h"
+#include "SkColorSpace_XYZTRC.h"
#include "SkColorSpacePriv.h"
#include "SkOnce.h"
-SkColorSpace_Base::SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50)
- : fGammaNamed(gammaNamed)
- , fGammas(nullptr)
- , fProfileData(nullptr)
- , fToXYZD50(toXYZD50)
- , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
-{}
-
-SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, SkGammaNamed gammaNamed,
- sk_sp<SkGammas> gammas, const SkMatrix44& toXYZD50,
- sk_sp<SkData> profileData)
- : fColorLUT(std::move(colorLUT))
- , fGammaNamed(gammaNamed)
- , fGammas(std::move(gammas))
- , fProfileData(std::move(profileData))
- , fToXYZD50(toXYZD50)
- , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
+SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkData> profileData)
+ : fProfileData(std::move(profileData))
{}
static constexpr float gSRGB_toXYZD50[] {
@@ -90,8 +77,8 @@ sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(const float values[3], const SkMat
gammas->fRedData.fValue = values[0];
gammas->fGreenData.fValue = values[1];
gammas->fBlueData.fValue = values[2];
- return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, kNonStandard_SkGammaNamed, gammas,
- toXYZD50, nullptr));
+ return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(kNonStandard_SkGammaNamed,
+ gammas, toXYZD50, nullptr));
}
return SkColorSpace_Base::NewRGB(gammaNamed, toXYZD50);
@@ -121,7 +108,7 @@ sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(SkGammaNamed gammaNamed, const SkM
break;
}
- return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50));
+ return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(gammaNamed, toXYZD50));
}
sk_sp<SkColorSpace> SkColorSpace::NewRGB(RenderTargetGamma gamma, const SkMatrix44& toXYZD50) {
@@ -152,7 +139,7 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
// Force the mutable type mask to be computed. This avoids races.
(void)srgbToxyzD50.getType();
- gSRGB = new SkColorSpace_Base(kSRGB_SkGammaNamed, srgbToxyzD50);
+ gSRGB = new SkColorSpace_XYZTRC(kSRGB_SkGammaNamed, srgbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gSRGB);
}
@@ -163,7 +150,7 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
// Force the mutable type mask to be computed. This avoids races.
(void)adobergbToxyzD50.getType();
- gAdobeRGB = new SkColorSpace_Base(k2Dot2Curve_SkGammaNamed, adobergbToxyzD50);
+ gAdobeRGB = new SkColorSpace_XYZTRC(k2Dot2Curve_SkGammaNamed, adobergbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gAdobeRGB);
}
@@ -174,7 +161,7 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
// Force the mutable type mask to be computed. This avoids races.
(void)srgbToxyzD50.getType();
- gSRGBLinear = new SkColorSpace_Base(kLinear_SkGammaNamed, srgbToxyzD50);
+ gSRGBLinear = new SkColorSpace_XYZTRC(kLinear_SkGammaNamed, srgbToxyzD50);
});
return sk_ref_sp<SkColorSpace>(gSRGBLinear);
}
@@ -185,35 +172,17 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
}
sk_sp<SkColorSpace> SkColorSpace::makeLinearGamma() {
- if (this->gammaIsLinear()) {
- return sk_ref_sp(this);
- }
- return SkColorSpace_Base::NewRGB(kLinear_SkGammaNamed, as_CSB(this)->fToXYZD50);
+ return as_CSB(this)->makeLinearGammaImpl();
}
///////////////////////////////////////////////////////////////////////////////////////////////////
bool SkColorSpace::gammaCloseToSRGB() const {
- return kSRGB_SkGammaNamed == as_CSB(this)->fGammaNamed ||
- k2Dot2Curve_SkGammaNamed == as_CSB(this)->fGammaNamed;
+ return as_CSB(this)->gammaCloseToSRGBImpl();
}
bool SkColorSpace::gammaIsLinear() const {
- return kLinear_SkGammaNamed == as_CSB(this)->fGammaNamed;
-}
-
-const SkMatrix44& SkColorSpace_Base::fromXYZD50() const {
- fFromXYZOnce([this] {
- if (!fToXYZD50.invert(&fFromXYZD50)) {
- // If a client gives us a dst gamut with a transform that we can't invert, we will
- // simply give them back a transform to sRGB gamut.
- SkDEBUGFAIL("Non-invertible XYZ matrix, defaulting to sRGB");
- SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
- srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50);
- srgbToxyzD50.invert(&fFromXYZD50);
- }
- });
- return fFromXYZD50;
+ return as_CSB(this)->gammaIsLinearImpl();
}
///////////////////////////////////////////////////////////////////////////////////////////////////
@@ -273,41 +242,42 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
// Start by trying the serialization fast path. If we haven't saved ICC profile data,
// we must have a profile that we can serialize easily.
if (!as_CSB(this)->fProfileData) {
+ // Profile data is mandatory for A2B0 color spaces.
// If we have a named profile, only write the enum.
msarett 2016/10/11 13:40:30 Please move this comment down to just above the "i
+ SkASSERT(as_CSB(this)->type() == SkColorSpace_Base::Type::kXYZTRC);
+ const SkColorSpace_XYZTRC* thisXYZ = static_cast<const SkColorSpace_XYZTRC*>(this);
+ const SkGammaNamed gammaNamed = thisXYZ->gammaNamed();
if (this == gSRGB) {
if (memory) {
*((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, kSRGB_Named,
- as_CSB(this)->fGammaNamed, 0);
+ ColorSpaceHeader::Pack(k0_Version, kSRGB_Named, gammaNamed, 0);
}
return sizeof(ColorSpaceHeader);
} else if (this == gAdobeRGB) {
if (memory) {
*((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named,
- as_CSB(this)->fGammaNamed, 0);
+ ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named, gammaNamed, 0);
}
return sizeof(ColorSpaceHeader);
} else if (this == gSRGBLinear) {
if (memory) {
*((ColorSpaceHeader*)memory) =
- ColorSpaceHeader::Pack(k0_Version, kSRGBLinear_Named,
- as_CSB(this)->fGammaNamed, 0);
+ ColorSpaceHeader::Pack(k0_Version, kSRGBLinear_Named, gammaNamed, 0);
}
return sizeof(ColorSpaceHeader);
}
// If we have a named gamma, write the enum and the matrix.
- switch (as_CSB(this)->fGammaNamed) {
+ switch (gammaNamed) {
case kSRGB_SkGammaNamed:
case k2Dot2Curve_SkGammaNamed:
case kLinear_SkGammaNamed: {
if (memory) {
*((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, 0, as_CSB(this)->fGammaNamed,
+ ColorSpaceHeader::Pack(k0_Version, 0, gammaNamed,
ColorSpaceHeader::kMatrix_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
- as_CSB(this)->fToXYZD50.as3x4RowMajorf((float*) memory);
+ thisXYZ->toXYZD50()->as3x4RowMajorf((float*) memory);
}
return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
}
@@ -315,11 +285,11 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
// Otherwise, write the gamma values and the matrix.
if (memory) {
*((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, 0, as_CSB(this)->fGammaNamed,
+ ColorSpaceHeader::Pack(k0_Version, 0, gammaNamed,
ColorSpaceHeader::kFloatGamma_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
- const SkGammas* gammas = as_CSB(this)->gammas();
+ const SkGammas* gammas = thisXYZ->gammas();
SkASSERT(gammas);
SkASSERT(SkGammas::Type::kValue_Type == gammas->fRedType &&
SkGammas::Type::kValue_Type == gammas->fGreenType &&
@@ -329,7 +299,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
*(((float*) memory) + 2) = gammas->fBlueData.fValue;
memory = SkTAddOffset<void>(memory, 3 * sizeof(float));
- as_CSB(this)->fToXYZD50.as3x4RowMajorf((float*) memory);
+ thisXYZ->toXYZD50()->as3x4RowMajorf((float*) memory);
}
return sizeof(ColorSpaceHeader) + 15 * sizeof(float);
}
@@ -438,6 +408,10 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
if (!src || !dst) {
return false;
}
+
+ if (as_CSB(src)->type() != as_CSB(dst)->type()) {
+ return false;
+ }
SkData* srcData = as_CSB(src)->fProfileData.get();
SkData* dstData = as_CSB(dst)->fProfileData.get();
@@ -451,22 +425,53 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
}
// It's important to check fProfileData before named gammas. Some profiles may have named
- // gammas, but also include other wacky features that cause us to save the data.
- switch (as_CSB(src)->fGammaNamed) {
- case kSRGB_SkGammaNamed:
- case k2Dot2Curve_SkGammaNamed:
- case kLinear_SkGammaNamed:
- return (as_CSB(src)->fGammaNamed == as_CSB(dst)->fGammaNamed) &&
- (as_CSB(src)->fToXYZD50 == as_CSB(dst)->fToXYZD50);
- default:
- if (as_CSB(src)->fGammaNamed != as_CSB(dst)->fGammaNamed) {
+ // gammas, but also include other wacky features that cause us to save the data.
msarett 2016/10/11 13:40:31 I believe you don't need any of this? Won't the f
raftias 2016/10/14 17:10:15 Yeah, I guess there wouldn't be any A2B0 profiles
+ if (as_CSB(src)->type() == SkColorSpace_Base::Type::kXYZTRC) {
+ const SkColorSpace_XYZTRC& srcXYZ = *static_cast<const SkColorSpace_XYZTRC*>(src);
+ const SkColorSpace_XYZTRC& dstXYZ = *static_cast<const SkColorSpace_XYZTRC*>(dst);
+
+ switch (srcXYZ.gammaNamed()) {
+ case kSRGB_SkGammaNamed:
+ case k2Dot2Curve_SkGammaNamed:
+ case kLinear_SkGammaNamed:
+ return (srcXYZ.gammaNamed() == dstXYZ.gammaNamed()) &&
+ (*srcXYZ.toXYZD50() == *dstXYZ.toXYZD50());
+ default:
+ if (srcXYZ.gammaNamed() != dstXYZ.gammaNamed()) {
+ return false;
+ }
+ }
+ // TRC tables still need checking
+ } else {
+ const SkColorSpace_A2B0& srcA2B = *static_cast<const SkColorSpace_A2B0*>(src);
+ const SkColorSpace_A2B0& dstA2B = *static_cast<const SkColorSpace_A2B0*>(dst);
+
+ const SkGammaNamed aNamed = srcA2B.aCurveNamed();
+ const SkGammaNamed bNamed = srcA2B.bCurveNamed();
+ const SkGammaNamed mNamed = srcA2B.mCurveNamed();
+ if ((aNamed != dstA2B.aCurveNamed()) ||
+ (bNamed != dstA2B.bCurveNamed()) ||
+ (mNamed != dstA2B.mCurveNamed())) {
+ return false;
+ }
+ if (aNamed != kNonStandard_SkGammaNamed &&
+ bNamed != kNonStandard_SkGammaNamed &&
+ mNamed != kNonStandard_SkGammaNamed) {
+ // All gammas are the same type and are not tables/parametric, so see if
+ // the matrices differ
+ if ((srcA2B.matrix() != dstA2B.matrix()) ||
+ (srcA2B.matrix() != dstA2B.matrix()) ||
+ (srcA2B.matrix() != dstA2B.matrix())) {
return false;
}
-
- // It is unlikely that we will reach this case.
- sk_sp<SkData> srcData = src->serialize();
- sk_sp<SkData> dstData = dst->serialize();
- return srcData->size() == dstData->size() &&
- 0 == memcmp(srcData->data(), dstData->data(), srcData->size());
+ }
+ // a/b/m-curve tables and CLUT (if applicable) still need checking
}
+
+ // It is unlikely that we will reach this case.
+ sk_sp<SkData> serializedSrcData = src->serialize();
msarett 2016/10/11 13:40:30 Thanks, I should not have shadowed my other variab
+ sk_sp<SkData> serializedDstData = dst->serialize();
+ return serializedSrcData->size() == serializedDstData->size() &&
+ 0 == memcmp(serializedSrcData->data(), serializedDstData->data(),
+ serializedSrcData->size());
}
« include/core/SkColorSpace.h ('K') | « resources/icc_profiles/srgb_lab_pcs.icc ('k') | src/core/SkColorSpaceXform.h » ('j') | src/core/SkColorSpaceXform.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698