From ce98801bbb5ac251b374f8b565445add43017aaa Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Fri, 21 Jul 2017 13:11:37 -0400 Subject: when encoding PNG, don't assume F16 values are in range I believe this will fix this batch of broken colorImages: https://gold.skia.org/search?blame=e23e55ef33358b2f6f98fc9cb795c71397c01618&fdiffmax=-1&fref=false&frgbamax=255&frgbamin=0&head=true&include=false&limit=50&match=gamma_correct&match=name&metric=combined&neg=false&offset=0&pos=false&query=source_type%3DcolorImage&sort=desc&unt=true Depending on the instruction set, clamping to logical [0,1] may be implicit or not as we work our way down to bytes. I think that's why these broken images are not more widespread on the bots. Change-Id: Ie9bb937864bf6954301d76d9921a2d3029d12c9a Reviewed-on: https://skia-review.googlesource.com/25742 Reviewed-by: Leon Scroggins Commit-Queue: Mike Klein --- src/images/SkImageEncoderFns.h | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src/images') diff --git a/src/images/SkImageEncoderFns.h b/src/images/SkImageEncoderFns.h index eb6ce8ba02..a17b30b104 100644 --- a/src/images/SkImageEncoderFns.h +++ b/src/images/SkImageEncoderFns.h @@ -265,6 +265,8 @@ static inline void transform_scanline_F16(char* SK_RESTRICT dst, const char* SK_ dst_ctx = { (void*)dst, 0 }; SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, &src_ctx); + p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. + p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); @@ -280,6 +282,8 @@ static inline void transform_scanline_F16_premul(char* SK_RESTRICT dst, const ch SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, &src_ctx); p.append(SkRasterPipeline::unpremul); + p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. + p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_u16_be, &dst_ctx); p.run(0,0, width,1); @@ -295,6 +299,8 @@ static inline void transform_scanline_F16_to_8888(char* SK_RESTRICT dst, dst_ctx = { (void*)dst, 0 }; SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, &src_ctx); + p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. + p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); p.run(0,0, width,1); @@ -311,6 +317,8 @@ static inline void transform_scanline_F16_premul_to_8888(char* SK_RESTRICT dst, SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, &src_ctx); p.append(SkRasterPipeline::unpremul); + p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. + p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); p.run(0,0, width,1); @@ -325,6 +333,8 @@ static inline void transform_scanline_F16_to_premul_8888(char* SK_RESTRICT dst, dst_ctx = { (void*)dst, 0 }; SkRasterPipeline_<256> p; p.append(SkRasterPipeline::load_f16, &src_ctx); + p.append(SkRasterPipeline::clamp_0); // F16 values may be out of [0,1] range, so clamp. + p.append(SkRasterPipeline::clamp_1); p.append(SkRasterPipeline::premul); p.append(SkRasterPipeline::to_srgb); p.append(SkRasterPipeline::store_8888, &dst_ctx); -- cgit v1.2.3