From: Matt Sarett Date: Thu, 9 Feb 2017 18:50:45 +0000 (-0500) Subject: Pixel conversion refactors: use raster pipeline for 565 and gray X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~55^2~410 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5101448ebb28cdd8104436ee25ce34c7d5dfe334;p=platform%2Fupstream%2FlibSkiaSharp.git Pixel conversion refactors: use raster pipeline for 565 and gray I'm trying not to do too much in one CL. But, in general, I hope to drop (non-performance important/optimized) special cases and use the pipeline. BUG=skia: CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD Change-Id: I724d3982f1467f6232371360b860484f13b1ede8 Reviewed-on: https://skia-review.googlesource.com/8271 Reviewed-by: Mike Klein Commit-Queue: Matt Sarett --- diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp index 10bed27..cae2eef 100644 --- a/src/core/SkConfig8888.cpp +++ b/src/core/SkConfig8888.cpp @@ -32,59 +32,44 @@ static inline bool can_memcpy(const SkImageInfo& dstInfo, const SkImageInfo& src return false; } - return !dstInfo.colorSpace() || !srcInfo.colorSpace() || + return !dstInfo.colorSpace() || SkColorSpace::Equals(dstInfo.colorSpace(), srcInfo.colorSpace()); } -// For now disable 565 in the pipeline. Its (higher) quality is so different its too much to -// rebase (for now) -// -//#define PIPELINE_HANDLES_565 - -static bool is_srgb(const SkImageInfo& info) { - return info.colorSpace() && info.colorSpace()->gammaCloseToSRGB(); -} - +// Default: Use the pipeline. static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_t dstRB, const SkImageInfo& srcInfo, const void* srcRow, size_t srcRB, - SkColorTable* ctable) { - SkASSERT(srcInfo.width() == dstInfo.width()); - SkASSERT(srcInfo.height() == dstInfo.height()); - - bool src_srgb = is_srgb(srcInfo); - const bool dst_srgb = is_srgb(dstInfo); - if (!dstInfo.colorSpace()) { - src_srgb = false; // untagged dst means ignore tags on src - } - + bool isColorAware) { SkRasterPipeline pipeline; - switch (srcInfo.colorType()) { case kRGBA_8888_SkColorType: + pipeline.append(SkRasterPipeline::load_8888, &srcRow); + break; case kBGRA_8888_SkColorType: pipeline.append(SkRasterPipeline::load_8888, &srcRow); - if (src_srgb) { - pipeline.append_from_srgb(srcInfo.alphaType()); - } - if (kBGRA_8888_SkColorType == srcInfo.colorType()) { - pipeline.append(SkRasterPipeline::swap_rb); - } + pipeline.append(SkRasterPipeline::swap_rb); break; -#ifdef PIPELINE_HANDLES_565 case kRGB_565_SkColorType: pipeline.append(SkRasterPipeline::load_565, &srcRow); break; -#endif case kRGBA_F16_SkColorType: pipeline.append(SkRasterPipeline::load_f16, &srcRow); break; + case kGray_8_SkColorType: + pipeline.append(SkRasterPipeline::load_g8, &srcRow); + break; default: - return false; // src colortype unsupported + return false; + } + + if (isColorAware && srcInfo.gammaCloseToSRGB()) { + pipeline.append_from_srgb(srcInfo.alphaType()); } float matrix[12]; - if (!append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(), dstInfo.colorSpace())) { - return false; + if (isColorAware) { + SkAssertResult(append_gamut_transform(&pipeline, matrix, srcInfo.colorSpace(), + dstInfo.colorSpace())); } SkAlphaType sat = srcInfo.alphaType(); @@ -95,27 +80,26 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_ pipeline.append(SkRasterPipeline::premul); } + if (isColorAware && dstInfo.gammaCloseToSRGB()) { + pipeline.append(SkRasterPipeline::to_srgb); + } + switch (dstInfo.colorType()) { case kRGBA_8888_SkColorType: + pipeline.append(SkRasterPipeline::store_8888, &dstRow); + break; case kBGRA_8888_SkColorType: - if (kBGRA_8888_SkColorType == dstInfo.colorType()) { - pipeline.append(SkRasterPipeline::swap_rb); - } - if (dst_srgb) { - pipeline.append(SkRasterPipeline::to_srgb); - } + pipeline.append(SkRasterPipeline::swap_rb); pipeline.append(SkRasterPipeline::store_8888, &dstRow); break; -#ifdef PIPELINE_HANDLES_565 case kRGB_565_SkColorType: pipeline.append(SkRasterPipeline::store_565, &dstRow); break; -#endif case kRGBA_F16_SkColorType: pipeline.append(SkRasterPipeline::store_f16, &dstRow); break; default: - return false; // dst colortype unsupported + return false; } auto p = pipeline.compile(); @@ -124,8 +108,8 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_ p(0,srcInfo.width()); // The pipeline has pointers to srcRow and dstRow, so we just need to update them in the // loop to move between rows of src/dst. - srcRow = (const char*)srcRow + srcRB; - dstRow = (char*)dstRow + dstRB; + dstRow = SkTAddOffset(dstRow, dstRB); + srcRow = SkTAddOffset(srcRow, srcRB); } return true; } @@ -231,19 +215,6 @@ bool SkSrcPixelInfo::convertPixelsTo(SkDstPixelInfo* dst, int width, int height) return true; } -static void copy_g8_to_32(void* dst, size_t dstRB, const void* src, size_t srcRB, int w, int h) { - uint32_t* dst32 = (uint32_t*)dst; - const uint8_t* src8 = (const uint8_t*)src; - - for (int y = 0; y < h; ++y) { - for (int x = 0; x < w; ++x) { - dst32[x] = SkPackARGB32(0xFF, src8[x], src8[x], src8[x]); - } - dst32 = (uint32_t*)((char*)dst32 + dstRB); - src8 += srcRB; - } -} - static bool extract_alpha(void* dst, size_t dstRB, const void* src, size_t srcRB, const SkImageInfo& srcInfo, SkColorTable* ctable) { uint8_t* SK_RESTRICT dst8 = (uint8_t*)dst; @@ -384,7 +355,8 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t return true; } - const bool isColorAware = srcInfo.colorSpace() && dstInfo.colorSpace(); + const bool isColorAware = dstInfo.colorSpace(); + SkASSERT(srcInfo.colorSpace() || !isColorAware); // Handle fancy alpha swizzling if both are ARGB32 if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel() && !isColorAware) { @@ -413,11 +385,6 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t * are supported. */ - if (kGray_8_SkColorType == srcInfo.colorType() && 4 == dstInfo.bytesPerPixel()) { - copy_g8_to_32(dstPixels, dstRB, srcPixels, srcRB, width, height); - return true; - } - if (kAlpha_8_SkColorType == dstInfo.colorType() && extract_alpha(dstPixels, dstRB, srcPixels, srcRB, srcInfo, ctable)) { return true; diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h index a3e449a..81d7070 100644 --- a/src/core/SkImageInfoPriv.h +++ b/src/core/SkImageInfoPriv.h @@ -39,10 +39,11 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) { /** * Returns true if Skia has defined a pixel conversion from the |src| to the |dst|. * Returns false otherwise. Some discussion of false cases: - * We will not convert to kIndex8 when the |src| is not kIndex8. - * We do not convert to kGray8 when the |src| is not kGray8. We may add this - * feature - it just requires some work to convert to luminance while handling color - * spaces correctly. Currently no one is asking for this. + * We will not convert to kIndex8 when the |src| is not kIndex8 in the same color space + * (color tables are immutable). + * We do not convert to kGray8 when the |src| is not kGray8 in the same color space. + * We may add this feature - it just requires some work to convert to luminance while + * handling color spaces correctly. Currently no one is asking for this. * We will not convert from kAlpha8 when the |dst| is not kAlpha8. This would require * inventing color information. * We will not convert to kOpaque when the |src| is not kOpaque. This could be @@ -58,11 +59,15 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm return false; } - if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) { + if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType() && + dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace())) + { return false; } - if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType()) { + if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType() && + dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace())) + { return false; } diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h index 2dd2c32..e507b28 100644 --- a/src/core/SkRasterPipeline.h +++ b/src/core/SkRasterPipeline.h @@ -66,6 +66,7 @@ M(from_2dot2) M(to_2dot2) \ M(constant_color) M(seed_shader) M(store_f32) \ M(load_a8) M(store_a8) \ + M(load_g8) \ M(load_565) M(store_565) \ M(load_f16) M(store_f16) \ M(load_8888) M(store_8888) \ diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h index dec81d9..4f01fd5 100644 --- a/src/opts/SkRasterPipeline_opts.h +++ b/src/opts/SkRasterPipeline_opts.h @@ -520,6 +520,12 @@ STAGE_CTX(store_a8, uint8_t**) { store(tail, SkNx_cast(SkNf_round(255.0f, a)), ptr); } +STAGE_CTX(load_g8, const uint8_t**) { + auto ptr = *ctx + x; + r = g = b = SkNf_from_byte(load(tail, ptr)); + a = 1.0f; +} + STAGE_CTX(load_565, const uint16_t**) { auto ptr = *ctx + x; from_565(load(tail, ptr), &r,&g,&b);