Remove sk_linear_to_srgb_noclamp().

We've just re-noticed this can happen:
  1) we have a properly premultiplied linear color;
  2) we convert that to sRGB;
  3) we convert that back to linear;
  4) that color does not appear to be premultiplied.

Removing sk_linear_to_srgb_noclamp(), and thus always clamping to [0,1] here in linear space, does not fix this problem.  However, it does help keep it from propagating too badly.

Just double-checked: the older Sk4f pipeline (SkXfermode4f, SkPM4fPriv, etc) already use sk_linear_to_srgb() exclusively, so they're already doing this same clamp.

BUG=skia:

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4314

Change-Id: I6352eb0ba969eb25674e8441e43bb51e1e1c0df3
Reviewed-on: https://skia-review.googlesource.com/4314
Reviewed-by: Yuqian Li <liyuqian@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Mike Klein 2016-11-02 14:47:07 -04:00 committed by Skia Commit-Bot
parent 71d9d84d6c
commit 6b059bdc40
2 changed files with 5 additions and 14 deletions

View File

@ -29,7 +29,7 @@ static inline V sk_clamp_0_255(const V& x) {
return V::Min(V::Max(x, 0.0f), 255.0f);
}
// This should probably only be called from sk_linear_to_srgb() or sk_linear_to_srgb_noclamp().
// This should probably only be called from sk_linear_to_srgb().
// It generally doesn't make sense to work with sRGB floats.
template <typename V>
static inline V sk_linear_to_srgb_needs_trunc(const V& x) {
@ -57,15 +57,6 @@ static inline SkNx<N,int> sk_linear_to_srgb(const SkNx<N,float>& x) {
return SkNx_cast<int>(sk_clamp_0_255(f));
}
template <int N>
static inline SkNx<N,int> sk_linear_to_srgb_noclamp(const SkNx<N,float>& x) {
auto f = sk_linear_to_srgb_needs_trunc(x);
for (int i = 0; i < 4; i++) {
SkASSERTF(0.0f <= f[i] && f[i] < 256.0f, "f[%d] was %g, outside [0,256)\n", i, f[i]);
}
return SkNx_cast<int>(f);
}
// sRGB -> linear, using math instead of table lookups, scaling better to larger SIMD vectors.
template <int N>
static inline SkNx<N,float> sk_linear_from_srgb_math(const SkNx<N,int>& s) {

View File

@ -405,10 +405,10 @@ STAGE(load_s_srgb, true) {
STAGE(store_srgb, false) {
auto ptr = *(uint32_t**)ctx + x;
store<kIsTail>(tail, ( sk_linear_to_srgb_noclamp(r) << SK_R32_SHIFT
| sk_linear_to_srgb_noclamp(g) << SK_G32_SHIFT
| sk_linear_to_srgb_noclamp(b) << SK_B32_SHIFT
| SkNx_cast<int>(255.0f * a + 0.5f) << SK_A32_SHIFT ), (int*)ptr);
store<kIsTail>(tail, ( sk_linear_to_srgb(r) << SK_R32_SHIFT
| sk_linear_to_srgb(g) << SK_G32_SHIFT
| sk_linear_to_srgb(b) << SK_B32_SHIFT
| SkNx_cast<int>(0.5f + 255.0f * a) << SK_A32_SHIFT), (int*)ptr);
}
RGBA_XFERMODE(clear) { return 0.0f; }