From abbd6d5e02832d53a939be15b78de592d88fe9ec Mon Sep 17 00:00:00 2001 From: msarett Date: Mon, 1 Aug 2016 09:43:08 -0700 Subject: [PATCH] Add SkColorSpace::Equals() API BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2196743002 Review-Url: https://codereview.chromium.org/2196743002 --- include/core/SkColorSpace.h | 6 ++++++ include/core/SkImageInfo.h | 6 ++---- src/codec/SkJpegCodec.cpp | 5 ++--- src/core/SkColorSpace.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++ tests/ColorSpaceTest.cpp | 31 +++++++++++++++++++++++++++++++ tests/ImageIsOpaqueTest.cpp | 27 ++++++++++++++++++++++----- 6 files changed, 108 insertions(+), 12 deletions(-) diff --git a/include/core/SkColorSpace.h b/include/core/SkColorSpace.h index 9fc5ccd..4b373f4 100644 --- a/include/core/SkColorSpace.h +++ b/include/core/SkColorSpace.h @@ -104,6 +104,12 @@ public: static sk_sp Deserialize(const void* data, size_t length); + /** + * If both are null, we return true. If one is null and the other is not, we return false. + * If both are non-null, we do a deeper compare. + */ + static bool Equals(const SkColorSpace* src, const SkColorSpace* dst); + protected: SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named); diff --git a/include/core/SkImageInfo.h b/include/core/SkImageInfo.h index f143503..cf50aca 100644 --- a/include/core/SkImageInfo.h +++ b/include/core/SkImageInfo.h @@ -289,12 +289,10 @@ public: bool operator==(const SkImageInfo& other) const { return fWidth == other.fWidth && fHeight == other.fHeight && fColorType == other.fColorType && fAlphaType == other.fAlphaType && - fColorSpace == other.fColorSpace; + SkColorSpace::Equals(fColorSpace.get(), other.fColorSpace.get()); } bool operator!=(const SkImageInfo& other) const { - return fWidth != other.fWidth || fHeight != other.fHeight || - fColorType != other.fColorType || fAlphaType != other.fAlphaType || - fColorSpace != other.fColorSpace; + return !(*this == other); } void unflatten(SkReadBuffer&); diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp index d158b42..2eea46b 100644 --- a/src/codec/SkJpegCodec.cpp +++ b/src/codec/SkJpegCodec.cpp @@ -475,10 +475,9 @@ bool SkJpegCodec::onDimensionsSupported(const SkISize& size) { } static bool needs_color_xform(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo) { - // FIXME (msarett): - // Do a better check for color space equality. return (kRGBA_F16_SkColorType == dstInfo.colorType()) || - (dstInfo.colorSpace() && (dstInfo.colorSpace() != srcInfo.colorSpace())); + (dstInfo.colorSpace() && !SkColorSpace::Equals(srcInfo.colorSpace(), + dstInfo.colorSpace())); } int SkJpegCodec::readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count) { diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp index 3874f0e..e7bd4e8 100644 --- a/src/core/SkColorSpace.cpp +++ b/src/core/SkColorSpace.cpp @@ -316,6 +316,51 @@ sk_sp SkColorSpace::Deserialize(const void* data, size_t length) { return NewICC(data, profileSize); } +bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) { + if (src == dst) { + return true; + } + + if (!src || !dst) { + return false; + } + + switch (src->fNamed) { + case kSRGB_Named: + case kAdobeRGB_Named: + return src->fNamed == dst->fNamed; + case kUnknown_Named: + if (kUnknown_Named != dst->fNamed) { + return false; + } + break; + } + + SkData* srcData = as_CSB(src)->fProfileData.get(); + SkData* dstData = as_CSB(dst)->fProfileData.get(); + if (srcData || dstData) { + if (srcData && dstData) { + return srcData->size() == dstData->size() && + 0 == memcmp(srcData->data(), dstData->data(), srcData->size()); + } + + return false; + } + + // 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 (src->fGammaNamed) { + case kSRGB_GammaNamed: + case k2Dot2Curve_GammaNamed: + case kLinear_GammaNamed: + return (src->fGammaNamed == dst->fGammaNamed) && (src->fToXYZD50 == dst->fToXYZD50); + default: + // If |src| does not have a named gamma, fProfileData should be non-null. + SkASSERT(false); + return false; + } +} + bool SkColorSpace::gammasAreMatching() const { const SkGammas* gammas = as_CSB(this)->gammas(); SkASSERT(gammas); diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp index 62a898c..8837c38 100644 --- a/tests/ColorSpaceTest.cpp +++ b/tests/ColorSpaceTest.cpp @@ -221,3 +221,34 @@ DEF_TEST(ColorSpace_Serialize, r) { test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false); } +DEF_TEST(ColorSpace_Equals, r) { + sk_sp srgb = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named); + sk_sp adobe = SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named); + sk_sp data = SkData::MakeFromFileName( + GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str()); + sk_sp z30 = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str()); + sk_sp z32 = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str()); + sk_sp upperLeft = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str()); + sk_sp upperRight = SkColorSpace::NewICC(data->data(), data->size()); + + REPORTER_ASSERT(r, SkColorSpace::Equals(nullptr, nullptr)); + REPORTER_ASSERT(r, SkColorSpace::Equals(srgb.get(), srgb.get())); + REPORTER_ASSERT(r, SkColorSpace::Equals(adobe.get(), adobe.get())); + REPORTER_ASSERT(r, SkColorSpace::Equals(z30.get(), z30.get())); + REPORTER_ASSERT(r, SkColorSpace::Equals(z32.get(), z32.get())); + REPORTER_ASSERT(r, SkColorSpace::Equals(upperLeft.get(), upperLeft.get())); + REPORTER_ASSERT(r, SkColorSpace::Equals(upperRight.get(), upperRight.get())); + + REPORTER_ASSERT(r, !SkColorSpace::Equals(nullptr, srgb.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(srgb.get(), nullptr)); + REPORTER_ASSERT(r, !SkColorSpace::Equals(adobe.get(), srgb.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(z30.get(), srgb.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(z32.get(), z30.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(upperLeft.get(), srgb.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(upperLeft.get(), upperRight.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(z30.get(), upperRight.get())); + REPORTER_ASSERT(r, !SkColorSpace::Equals(upperRight.get(), adobe.get())); +} diff --git a/tests/ImageIsOpaqueTest.cpp b/tests/ImageIsOpaqueTest.cpp index 0660885..4389a3d 100644 --- a/tests/ImageIsOpaqueTest.cpp +++ b/tests/ImageIsOpaqueTest.cpp @@ -6,6 +6,7 @@ */ #include "SkTypes.h" +#include "Resources.h" #include "Test.h" #if SK_SUPPORT_GPU @@ -17,13 +18,15 @@ #include "SkWriteBuffer.h" static void test_flatten(skiatest::Reporter* reporter, const SkImageInfo& info) { - // just need a safe amount of storage, but ensure that it is 4-byte aligned. - int32_t storage[(sizeof(SkImageInfo)*2) / sizeof(int32_t)]; - SkBinaryWriteBuffer wb(storage, sizeof(storage)); + // Need a safe amount of 4-byte aligned storage. Note that one of the test ICC profiles + // is ~7500 bytes. + const size_t storageBytes = 8000; + SkAutoTMalloc storage(storageBytes / sizeof(uint32_t)); + SkBinaryWriteBuffer wb(storage.get(), storageBytes); info.flatten(wb); - SkASSERT(wb.bytesWritten() < sizeof(storage)); + SkASSERT(wb.bytesWritten() < storageBytes); - SkReadBuffer rb(storage, wb.bytesWritten()); + SkReadBuffer rb(storage.get(), wb.bytesWritten()); // pick a noisy byte pattern, so we ensure that unflatten sets all of our fields SkImageInfo info2 = SkImageInfo::Make(0xB8, 0xB8, (SkColorType) 0xB8, (SkAlphaType) 0xB8); @@ -35,10 +38,24 @@ static void test_flatten(skiatest::Reporter* reporter, const SkImageInfo& info) } DEF_TEST(ImageInfo_flattening, reporter) { + sk_sp data = + SkData::MakeFromFileName(GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str()); + sk_sp space0 = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str()); + sk_sp space1 = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str()); + sk_sp space2 = SkColorSpace::NewICC(data->data(), data->size()); + data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str()); + sk_sp space3 = SkColorSpace::NewICC(data->data(), data->size()); + sk_sp spaces[] = { nullptr, SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named), SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named), + space0, + space1, + space2, + space3, }; for (int ct = 0; ct <= kLastEnum_SkColorType; ++ct) { -- 2.7.4