Fix various SkColorSpace bugs
(1) Fixes serialization/deserialization of wacky SkColorSpaces
(2) Fix gamma equals checking
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2194903002
Review-Url: https://codereview.chromium.org/2194903002
diff --git a/bench/ColorCodecBench.cpp b/bench/ColorCodecBench.cpp
index 5079b48..8481422 100644
--- a/bench/ColorCodecBench.cpp
+++ b/bench/ColorCodecBench.cpp
@@ -140,7 +140,7 @@
SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(fEncoded.get()));
fSrcData = codec->getICCData();
sk_sp<SkData> dstData = SkData::MakeFromFileName(
- GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+ GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
SkASSERT(dstData);
fDstSpace = nullptr;
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index c03a8c4..f4c8956 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -869,7 +869,7 @@
// Load the dst ICC profile. This particular dst is fairly similar to Adobe RGB.
sk_sp<SkData> dstData = SkData::MakeFromFileName(
- GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+ GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
if (!dstData) {
return "Cannot read monitor profile. Is the resource path set correctly?";
}
diff --git a/resources/icc_profiles/HP_Z32x.icc b/resources/icc_profiles/HP_Z32x.icc
new file mode 100644
index 0000000..3d532b8
--- /dev/null
+++ b/resources/icc_profiles/HP_Z32x.icc
Binary files differ
diff --git a/resources/monitor_profiles/HP_ZR30w.icc b/resources/icc_profiles/HP_ZR30w.icc
similarity index 100%
rename from resources/monitor_profiles/HP_ZR30w.icc
rename to resources/icc_profiles/HP_ZR30w.icc
Binary files differ
diff --git a/resources/icc_profiles/upperLeft.icc b/resources/icc_profiles/upperLeft.icc
new file mode 100644
index 0000000..8c521a5
--- /dev/null
+++ b/resources/icc_profiles/upperLeft.icc
Binary files differ
diff --git a/resources/icc_profiles/upperRight.icc b/resources/icc_profiles/upperRight.icc
new file mode 100644
index 0000000..44ffa87
--- /dev/null
+++ b/resources/icc_profiles/upperRight.icc
Binary files differ
diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index 6c34277..3874f0e 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -201,49 +201,52 @@
};
size_t SkColorSpace::writeToMemory(void* memory) const {
- // If we have a named profile, only write the enum.
- switch (fNamed) {
- case kSRGB_Named:
- case kAdobeRGB_Named: {
- if (memory) {
- *((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
+ // 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) {
+ // If we have a named profile, only write the enum.
+ switch (fNamed) {
+ case kSRGB_Named:
+ case kAdobeRGB_Named: {
+ if (memory) {
+ *((ColorSpaceHeader*) memory) =
+ ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
+ }
+ return sizeof(ColorSpaceHeader);
}
- return sizeof(ColorSpaceHeader);
+ default:
+ break;
}
- default:
- break;
+
+ // If we have a named gamma, write the enum and the matrix.
+ switch (fGammaNamed) {
+ case kSRGB_GammaNamed:
+ case k2Dot2Curve_GammaNamed:
+ case kLinear_GammaNamed: {
+ if (memory) {
+ *((ColorSpaceHeader*) memory) =
+ ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
+ ColorSpaceHeader::kMatrix_Flag);
+ memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
+ fToXYZD50.as4x3ColMajorf((float*) memory);
+ }
+ return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
+ }
+ default:
+ SkASSERT(false);
+ return 0;
+ }
}
- // If we have a named gamma, write the enum and the matrix.
- switch (fGammaNamed) {
- case kSRGB_GammaNamed:
- case k2Dot2Curve_GammaNamed:
- case kLinear_GammaNamed: {
- if (memory) {
- *((ColorSpaceHeader*) memory) =
- ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
- ColorSpaceHeader::kMatrix_Flag);
- memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
- fToXYZD50.as4x3ColMajorf((float*) memory);
- }
- return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
- }
- default:
- break;
- }
-
- // If we do not have a named gamma, this must have been created from an ICC profile.
- // Since we were unable to recognize the gamma, we will have saved the ICC data.
- SkASSERT(as_CSB(this)->fProfileData);
-
+ // Otherwise, serialize the ICC data.
size_t profileSize = as_CSB(this)->fProfileData->size();
if (SkAlign4(profileSize) != (uint32_t) SkAlign4(profileSize)) {
return 0;
}
if (memory) {
- *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
+ *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, kUnknown_Named,
+ kNonStandard_GammaNamed,
ColorSpaceHeader::kICC_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
@@ -316,7 +319,8 @@
bool SkColorSpace::gammasAreMatching() const {
const SkGammas* gammas = as_CSB(this)->gammas();
SkASSERT(gammas);
- return gammas->fRedData == gammas->fGreenData && gammas->fGreenData == gammas->fBlueData;
+ return gammas->fRedType == gammas->fGreenType && gammas->fGreenType == gammas->fBlueType &&
+ gammas->fRedData == gammas->fGreenData && gammas->fGreenData == gammas->fBlueData;
}
bool SkColorSpace::gammasAreNamed() const {
diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index 4fbfa6c..4a7f175 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -404,7 +404,7 @@
// share the same table pointer. This should almost always be true.
// I've never seen a profile where all three gamma curves didn't match.
// But it is possible that they won't.
- if (gammas->data(0) == gammas->data(i)) {
+ if (gammas->type(0) == gammas->type(i) && gammas->data(0) == gammas->data(i)) {
outGammaTables[i] = outGammaTables[0];
continue;
}
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 6cfbb8c..f450773 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -70,38 +70,57 @@
};
bool isNamed(int i) const {
- SkASSERT(0 <= i && i < 3);
- return (&fRedType)[i] == Type::kNamed_Type;
+ return Type::kNamed_Type == this->type(i);
}
bool isValue(int i) const {
- SkASSERT(0 <= i && i < 3);
- return (&fRedType)[i] == Type::kValue_Type;
+ return Type::kValue_Type == this->type(i);
}
bool isTable(int i) const {
- SkASSERT(0 <= i && i < 3);
- return (&fRedType)[i] == Type::kTable_Type;
+ return Type::kTable_Type == this->type(i);
}
bool isParametric(int i) const {
- SkASSERT(0 <= i && i < 3);
- return (&fRedType)[i] == Type::kParam_Type;
+ return Type::kParam_Type == this->type(i);
}
const Data& data(int i) const {
- SkASSERT(0 <= i && i < 3);
- return (&fRedData)[i];
+ switch (i) {
+ case 0:
+ return fRedData;
+ case 1:
+ return fGreenData;
+ case 2:
+ return fBlueData;
+ default:
+ SkASSERT(false);
+ return fRedData;
+ }
}
const float* table(int i) const {
SkASSERT(isTable(i));
- return (&fRedData)[i].fTable.table(this);
+ return this->data(i).fTable.table(this);
}
const Params& params(int i) const {
SkASSERT(isParametric(i));
- return (&fRedData)[i].params(this);
+ return this->data(i).params(this);
+ }
+
+ Type type(int i) const {
+ switch (i) {
+ case 0:
+ return fRedType;
+ case 1:
+ return fGreenType;
+ case 2:
+ return fBlueType;
+ default:
+ SkASSERT(false);
+ return fRedType;
+ }
}
SkGammas()
diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index f9593d9..62a898c 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -125,7 +125,7 @@
// Test saving the original ICC data
sk_sp<SkData> monitorData = SkData::MakeFromFileName(
- GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+ GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
REPORTER_ASSERT(r, monitorData);
if (!monitorData) {
return;
@@ -211,7 +211,13 @@
test_serialize(r, SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named).get(), true);
sk_sp<SkData> monitorData = SkData::MakeFromFileName(
- GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+ GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
+ test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+ monitorData = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str());
+ test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+ monitorData = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str());
+ test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+ monitorData = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str());
test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
}