diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp index 4c83bb638a..2efcf060ff 100644 --- a/src/core/SkColorSpace.cpp +++ b/src/core/SkColorSpace.cpp @@ -380,14 +380,28 @@ bool SkColorSpace::Equals(const SkColorSpace* x, const SkColorSpace* y) { } if (x->hash() == y->hash()) { + #if defined(SK_DEBUG) + // Do these floats function equivalently? + // This returns true more often than simple float comparison (NaN vs. NaN) and, + // also returns true more often than simple bitwise comparison (+0 vs. -0) and, + // even returns true more often than those two OR'd together (two different NaNs). + auto equiv = [](float X, float Y) { + return (X==Y) + || (sk_float_isnan(X) && sk_float_isnan(Y)); + }; + for (int i = 0; i < 7; i++) { - SkASSERT((&x->fTransferFn.g)[i] == (&y->fTransferFn.g)[i] && "Hash collsion"); + float X = (&x->fTransferFn.g)[i], + Y = (&y->fTransferFn.g)[i]; + SkASSERTF(equiv(X,Y), "Hash collision at tf[%d], !equiv(%g,%g)\n", i, X,Y); } - for (int r = 0; r < 3; r++) { - for (int c = 0; c < 3; ++c) { - SkASSERT(x->fToXYZD50.vals[r][c] == y->fToXYZD50.vals[r][c] && "Hash collsion"); - } + for (int r = 0; r < 3; r++) + for (int c = 0; c < 3; c++) { + float X = x->fToXYZD50.vals[r][c], + Y = y->fToXYZD50.vals[r][c]; + SkASSERTF(equiv(X,Y), "Hash collision at toXYZD50[%d][%d], !equiv(%g,%g)\n", r,c, X,Y); } + #endif return true; } return false; diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp index c607738e19..926d8a6e6c 100644 --- a/tests/ColorSpaceTest.cpp +++ b/tests/ColorSpaceTest.cpp @@ -348,3 +348,21 @@ DEF_TEST(ColorSpace_classifyUnderflow, r) { sk_sp bad = SkColorSpace::MakeRGB(fn, SkNamedGamut::kSRGB); REPORTER_ASSERT(r, bad == nullptr); } + +DEF_TEST(ColorSpace_equiv, r) { + skcms_TransferFunction tf = SkNamedTransferFn::kSRGB; + skcms_Matrix3x3 gamut = SkNamedGamut::kSRGB; + + // Previously a NaN anywhere in the tf or gamut would trip up Equals(), + // making us think we'd hit a hash collision where we hadn't. + gamut.vals[1][1] = SK_FloatNaN; + + // There's a quick pointer comparison in SkColorSpace::Equals() we want to get past. + sk_sp x = SkColorSpace::MakeRGB(tf, gamut), + y = SkColorSpace::MakeRGB(tf, gamut); + REPORTER_ASSERT(r, x && y); + REPORTER_ASSERT(r, x.get() != y.get()); + + // Most important to test in debug mode that we don't SkASSERT(). + REPORTER_ASSERT(r, SkColorSpace::Equals(x.get(), y.get())); +}