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

SkColorSpaceXform bug fixes

Browse Source 
(1) Clamp properly!

Finally came to this realization: clamping in the
store functions (after gamma encoding) is ridiculous.
It is impossible to know how to clamp premul values
to alpha when they are already gamma encoded.

I've moved the clamp out of the store function.
Whew, this actually makes the code look simpler.

And I expect this to fix some buggy images on Gold!

(2) Correctly handle the memcpy() case.

Looks like this only ever worked for RGBA inputs,
never got updated when we added BGRA inputs.

This probably flew under the radar because the
clients are smart enough to avoid performing a
color xform altogether when the color spaces
match.

BUG=skia:

Change-Id: I4870048105efcbecc70b4bd5f77c39537006363e
Reviewed-on: https://skia-review.googlesource.com/5389
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Matt Sarett 2016-11-30 18:49:40 -05:00 committed by Skia Commit-Bot
parent 71b762f2ac
commit d0fdc0f234
2 changed files with 49 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
src/core/SkColorSpaceXform.cpp
View File

@ -645,10 +645,6 @@ static AI void store_srgb(void* dst, const uint32_t* src, Sk4f& dr, Sk4f& dg, Sk
dg = sk_linear_to_srgb_needs_trunc(dg);
db = sk_linear_to_srgb_needs_trunc(db);
dr = sk_clamp_0_255(dr);
dg = sk_clamp_0_255(dg);
db = sk_clamp_0_255(db);
Sk4i da = Sk4i::Load(src) & 0xFF000000;
Sk4i rgba = (SkNx_cast<int>(dr) << kRShift)
@ -662,7 +658,7 @@ template <Order kOrder>
static AI void store_srgb_1(void* dst, const uint32_t* src,
Sk4f& rgba, const Sk4f&,
const uint8_t* const[3]) {
rgba = sk_clamp_0_255(sk_linear_to_srgb_needs_trunc(rgba));
rgba = sk_linear_to_srgb_needs_trunc(rgba);
uint32_t tmp;
SkNx_cast<uint8_t>(SkNx_cast<int32_t>(rgba)).store(&tmp);
@ -693,10 +689,6 @@ static AI void store_2dot2(void* dst, const uint32_t* src, Sk4f& dr, Sk4f& dg, S
dg = linear_to_2dot2(dg);
db = linear_to_2dot2(db);
dr = sk_clamp_0_255(dr);
dg = sk_clamp_0_255(dg);
db = sk_clamp_0_255(db);
Sk4i da = Sk4i::Load(src) & 0xFF000000;
Sk4i rgba = (Sk4f_round(dr) << kRShift)
@ -710,7 +702,7 @@ template <Order kOrder>
static AI void store_2dot2_1(void* dst, const uint32_t* src,
Sk4f& rgba, const Sk4f&,
const uint8_t* const[3]) {
rgba = sk_clamp_0_255(linear_to_2dot2(rgba));
rgba = linear_to_2dot2(rgba);
uint32_t tmp;
SkNx_cast<uint8_t>(Sk4f_round(rgba)).store(&tmp);
@ -727,9 +719,9 @@ static AI void store_linear(void* dst, const uint32_t* src, Sk4f& dr, Sk4f& dg,
const uint8_t* const[3]) {
int kRShift, kGShift = 8, kBShift;
set_rb_shifts(kOrder, &kRShift, &kBShift);
dr = sk_clamp_0_255(255.0f * dr);
dg = sk_clamp_0_255(255.0f * dg);
db = sk_clamp_0_255(255.0f * db);
dr = 255.0f * dr;
dg = 255.0f * dg;
db = 255.0f * db;
Sk4i da = Sk4i::Load(src) & 0xFF000000;
@ -744,7 +736,7 @@ template <Order kOrder>
static AI void store_linear_1(void* dst, const uint32_t* src,
Sk4f& rgba, const Sk4f&,
const uint8_t* const[3]) {
rgba = sk_clamp_0_255(255.0f * rgba);
rgba = 255.0f * rgba;
uint32_t tmp;
SkNx_cast<uint8_t>(Sk4f_round(rgba)).store(&tmp);
@ -811,9 +803,9 @@ static AI void store_generic(void* dst, const uint32_t* src, Sk4f& dr, Sk4f& dg,
const uint8_t* const dstTables[3]) {
int kRShift, kGShift = 8, kBShift;
set_rb_shifts(kOrder, &kRShift, &kBShift);
dr = Sk4f::Min(Sk4f::Max(1023.0f * dr, 0.0f), 1023.0f);
dg = Sk4f::Min(Sk4f::Max(1023.0f * dg, 0.0f), 1023.0f);
db = Sk4f::Min(Sk4f::Max(1023.0f * db, 0.0f), 1023.0f);
dr = 1023.0f * dr;
dg = 1023.0f * dg;
db = 1023.0f * db;
Sk4i ir = Sk4f_round(dr);
Sk4i ig = Sk4f_round(dg);
@ -846,7 +838,7 @@ static AI void store_generic_1(void* dst, const uint32_t* src,
const uint8_t* const dstTables[3]) {
int kRShift, kGShift = 8, kBShift;
set_rb_shifts(kOrder, &kRShift, &kBShift);
rgba = Sk4f::Min(Sk4f::Max(1023.0f * rgba, 0.0f), 1023.0f);
rgba = 1023.0f * rgba;
Sk4i indices = Sk4f_round(rgba);
@ -990,6 +982,9 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
break;
}
static constexpr bool dstIs8888 = kF16_Linear_DstFormat != kDst &&
kF32_Linear_DstFormat != kDst;
const uint32_t* src = (const uint32_t*) vsrc;
Sk4f rXgXbX, rYgYbY, rZgZbZ, rTgTbT;
load_matrix(matrix, rXgXbX, rYgYbY, rZgZbZ, rTgTbT);
@ -1007,6 +1002,12 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
if (kNone_ColorSpaceMatch == kCSM) {
transform_gamut(r, g, b, a, rXgXbX, rYgYbY, rZgZbZ, dr, dg, db, da);
translate_gamut(rTgTbT, dr, dg, db);
if (dstIs8888) {
dr = sk_clamp_0_1(dr);
dg = sk_clamp_0_1(dg);
db = sk_clamp_0_1(db);
}
} else {
dr = r;
dg = g;
@ -1029,6 +1030,12 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
if (kNone_ColorSpaceMatch == kCSM) {
transform_gamut(r, g, b, a, rXgXbX, rYgYbY, rZgZbZ, dr, dg, db, da);
translate_gamut(rTgTbT, dr, dg, db);
if (dstIs8888) {
dr = sk_clamp_0_1(dr);
dg = sk_clamp_0_1(dg);
db = sk_clamp_0_1(db);
}
} else {
dr = r;
dg = g;
@ -1171,26 +1178,23 @@ bool SkColorSpaceXform_XYZ<kSrc, kDst, kCSM>
int len, SkAlphaType alphaType) const
{
if (kFull_ColorSpaceMatch == kCSM) {
switch (alphaType) {
case kPremul_SkAlphaType:
// We can't skip the xform since we need to perform a premultiply in the
// linear space.
break;
default:
switch (dstColorFormat) {
case kRGBA_8888_ColorFormat:
memcpy(dst, src, len * sizeof(uint32_t));
return true;
case kBGRA_8888_ColorFormat:
SkOpts::RGBA_to_BGRA((uint32_t*) dst, src, len);
return true;
case kRGBA_F16_ColorFormat:
case kRGBA_F32_ColorFormat:
// There's still work to do to xform to linear floats.
break;
default:
return false;
}
if (kPremul_SkAlphaType != alphaType) {
if ((kRGBA_8888_ColorFormat == dstColorFormat &&
kRGBA_8888_ColorFormat == srcColorFormat) ||
(kBGRA_8888_ColorFormat == dstColorFormat &&
kBGRA_8888_ColorFormat == srcColorFormat))
{
memcpy(dst, src, len * sizeof(uint32_t));
return true;
}
if ((kRGBA_8888_ColorFormat == dstColorFormat &&
kBGRA_8888_ColorFormat == srcColorFormat) ||
(kBGRA_8888_ColorFormat == dstColorFormat &&
kRGBA_8888_ColorFormat == srcColorFormat))
{
SkOpts::RGBA_to_BGRA((uint32_t*) dst, src, len);
return true;
}
}
}

7
src/core/SkSRGB.h
View File

@ -29,6 +29,13 @@ static inline V sk_clamp_0_255(const V& x) {
return V::Min(V::Max(x, 0.0f), 255.0f);
}
template <typename V>
static inline V sk_clamp_0_1(const V& x) {
// The order of the arguments is important here. We want to make sure that NaN
// clamps to zero. Note that max(NaN, 0) = 0, while max(0, NaN) = NaN.
return V::Min(V::Max(x, 0.0f), 1.0f);
}
// [0.0f, 1.0f] -> [0.0f, 255.xf], for small x. Correct after truncation.
template <typename V>
static inline V sk_linear_to_srgb_needs_trunc(const V& x) {