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

SkColorSpaceXform bug fixes attempt 2

Browse Source 
(1) Clamping

If we're going to clamp (8888 outputs), we need to clamp properly
to alpha (not 1) when we premultiply.  This fix is made in
SkColorSpaceXform_XYZ.

An alternative fix would move all clamping out of the store
functions, to before the gamma encoding.  This generally makes sense,
but the "to 2.2 conversion" may introduce NaNs and always needs a
clamp.  So another fix is to just have an extra clamp in the store 2.2
function.  Since we have two pipelines, let's try this one in
SkColorSpaceXform_Pipeline :).

(2) Correctly handle the memcpy() case.

This is not changed from a previous (reverted) CL.

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:

CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD

Change-Id: I0b59239d2488ce9fdbe11efbd96567e420bb9813
Reviewed-on: https://skia-review.googlesource.com/5464
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Matt Sarett 2016-12-01 17:02:07 -05:00 committed by Skia Commit-Bot
parent 2bb3592868
commit abf8ba34c8
2 changed files with 48 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/core/SkColorSpaceXform.cpp
View File

@ -628,13 +628,23 @@ static AI void translate_gamut_1(const Sk4f& rTgTbT, Sk4f& rgba) {
rgba = rgba + rTgTbT;
}
static AI void premultiply(Sk4f& dr, Sk4f& dg, Sk4f& db, const Sk4f& da) {
static AI void premultiply(Sk4f& dr, Sk4f& dg, Sk4f& db, const Sk4f& da, bool kClamp) {
if (kClamp) {
dr = Sk4f::Max(dr, 1.0f);
dg = Sk4f::Max(dg, 1.0f);
db = Sk4f::Max(db, 1.0f);
}
dr = da * dr;
dg = da * dg;
db = da * db;
}
static AI void premultiply_1(const Sk4f& a, Sk4f& rgba) {
static AI void premultiply_1(const Sk4f& a, Sk4f& rgba, bool kClamp) {
if (kClamp) {
rgba = Sk4f::Max(rgba, 1.0f);
}
rgba = a * rgba;
}
@ -892,12 +902,12 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
const uint8_t* const dstTables[3]) {
LoadFn load;
Load1Fn load_1;
static constexpr bool loadAlpha = (kPremul_SkAlphaType == kAlphaType) ||
(kF16_Linear_DstFormat == kDst) ||
(kF32_Linear_DstFormat == kDst);
const bool kLoadAlpha = (kPremul_SkAlphaType == kAlphaType) ||
(kF16_Linear_DstFormat == kDst) ||
(kF32_Linear_DstFormat == kDst);
switch (kSrc) {
case kRGBA_8888_Linear_SrcFormat:
if (loadAlpha) {
if (kLoadAlpha) {
load = load_rgba_linear<kRGBA_Order>;
load_1 = load_rgba_linear_1<kRGBA_Order>;
} else {
@ -906,7 +916,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
break;
case kRGBA_8888_Table_SrcFormat:
if (loadAlpha) {
if (kLoadAlpha) {
load = load_rgba_from_tables<kRGBA_Order>;
load_1 = load_rgba_from_tables_1<kRGBA_Order>;
} else {
@ -915,7 +925,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
break;
case kBGRA_8888_Linear_SrcFormat:
if (loadAlpha) {
if (kLoadAlpha) {
load = load_rgba_linear<kBGRA_Order>;
load_1 = load_rgba_linear_1<kBGRA_Order>;
} else {
@ -924,7 +934,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
break;
case kBGRA_8888_Table_SrcFormat:
if (loadAlpha) {
if (kLoadAlpha) {
load = load_rgba_from_tables<kBGRA_Order>;
load_1 = load_rgba_from_tables_1<kBGRA_Order>;
} else {
@ -992,6 +1002,14 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
break;
}
// We always clamp before converting to 8888 outputs (because we have to).
// In these cases, we also need a clamp before the premul step to make sure
// R, G, B are not logically greater than A.
const bool kClamp = kRGBA_8888_Linear_DstFormat == kDst ||
kRGBA_8888_SRGB_DstFormat == kDst ||
kRGBA_8888_2Dot2_DstFormat == kDst ||
kRGBA_8888_Table_DstFormat == kDst;
const uint32_t* src = (const uint32_t*) vsrc;
Sk4f rXgXbX, rYgYbY, rZgZbZ, rTgTbT;
load_matrix(matrix, rXgXbX, rYgYbY, rZgZbZ, rTgTbT);
@ -1017,7 +1035,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
if (kPremul_SkAlphaType == kAlphaType) {
premultiply(dr, dg, db, da);
premultiply(dr, dg, db, da, kClamp);
}
load(src, r, g, b, a, srcTables);
@ -1039,7 +1057,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
if (kPremul_SkAlphaType == kAlphaType) {
premultiply(dr, dg, db, da);
premultiply(dr, dg, db, da, kClamp);
}
store(dst, src - 4, dr, dg, db, da, dstTables);
@ -1059,7 +1077,7 @@ static void color_xform_RGBA(void* dst, const void* vsrc, int len,
}
if (kPremul_SkAlphaType == kAlphaType) {
premultiply_1(a, rgba);
premultiply_1(a, rgba, kClamp);
}
store_1(dst, src, rgba, a, dstTables);
@ -1173,26 +1191,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;
}
}
}

2
src/opts/SkRasterPipeline_opts.h
View File

@ -349,7 +349,7 @@ STAGE(to_2dot2) {
x64 = x32.rsqrt(); // x^(+1/64)
// 29 = 32 - 2 - 1
return x2.invert() * x32 * x64.invert();
return SkNf::Max(x2.invert() * x32 * x64.invert(), 0.0f); // Watch out for NaN.
};
r = to_2dot2(r);