AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

more careful SkColorSpace hash collision detection

Browse Source 
The new unit test SkASSERT()s a hash collision at head
(but passes in release builds) and does not SkASSERT()
with this patch to SkColorSpace.cpp.

Bug: chromium:1113865
Change-Id: Ibc05af4145a92bbd15c7d5e06ece9d269bd7a242
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310110
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Mike Klein 2020-08-13 13:08:29 -05:00 committed by Skia Commit-Bot
parent 8c91c93dce
commit 9e77f20864
2 changed files with 37 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/core/SkColorSpace.cpp
View File

@ -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");
}
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");
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++) {
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;

18
tests/ColorSpaceTest.cpp
View File

@ -348,3 +348,21 @@ DEF_TEST(ColorSpace_classifyUnderflow, r) {
sk_sp<SkColorSpace> 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<SkColorSpace> 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()));
}