From d1835d7491223f94b07d48e5af8d29fb6742bf4e Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Mon, 8 May 2017 12:23:46 -0400 Subject: [PATCH] "can we?" -> "do we want to?" for SkRasterPipelineBlitter There has been a supported() function in SkRasterPipelineBlitter.cpp for a long time that's becoming increasingly misnamed. That blitter ought to be able to handle all destination formats. This CL moves that logic outside to the creator of the blitter, changing it from "can we handle this format?" to "do we want to use this blitter for this format?". In other CLs I'm working to make creating a pipeline blitter never fail. Change-Id: Ie59fb8ec6e63d215d1baef439e464e8f0ab3ae4d Reviewed-on: https://skia-review.googlesource.com/15842 Reviewed-by: Mike Reed Commit-Queue: Mike Klein --- src/core/SkBlitter.cpp | 7 +++++-- src/core/SkRasterPipelineBlitter.cpp | 18 ------------------ 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp index 0e8bd2db22..2a34ffcbf5 100644 --- a/src/core/SkBlitter.cpp +++ b/src/core/SkBlitter.cpp @@ -847,8 +847,11 @@ SkBlitter* SkBlitter::Choose(const SkPixmap& device, return alloc->make(device, *paint); } - if (SkBlitter* blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc)) { - return blitter; + // By policy we choose not to handle legacy 8888 with SkRasterPipelineBlitter. + if (device.colorSpace() || device.colorType() != kN32_SkColorType) { + if (SkBlitter* blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc)) { + return blitter; + } } if (nullptr == shader) { diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp index 57a377bb19..9b882c2ca1 100644 --- a/src/core/SkRasterPipelineBlitter.cpp +++ b/src/core/SkRasterPipelineBlitter.cpp @@ -76,16 +76,6 @@ SkBlitter* SkCreateRasterPipelineBlitter(const SkPixmap& dst, return SkRasterPipelineBlitter::Create(dst, paint, ctm, alloc); } -static bool supported(const SkImageInfo& info) { - switch (info.colorType()) { - case kAlpha_8_SkColorType: return true; - case kRGB_565_SkColorType: return true; - case kN32_SkColorType: return info.gammaCloseToSRGB(); - case kRGBA_F16_SkColorType: return true; - default: return false; - } -} - SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst, const SkPaint& paint, const SkMatrix& ctm, @@ -103,11 +93,6 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst, SkShader* shader = paint.getShader(); SkColorFilter* colorFilter = paint.getColorFilter(); - // TODO: all temporary - if (!supported(dst.info())) { - return nullptr; - } - // TODO: Think more about under what conditions we dither: // - if we're drawing anything into 565 and the user has asked us to dither, or // - if we're drawing a gradient into 565 or 8888. @@ -174,8 +159,6 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst, } void SkRasterPipelineBlitter::append_load_d(SkRasterPipeline* p) const { - SkASSERT(supported(fDst.info())); - p->append(SkRasterPipeline::move_src_dst); switch (fDst.info().colorType()) { case kAlpha_8_SkColorType: p->append(SkRasterPipeline::load_a8, &fDstPtr); break; @@ -207,7 +190,6 @@ void SkRasterPipelineBlitter::append_store(SkRasterPipeline* p) const { if (fDst.info().colorType() == kBGRA_8888_SkColorType) { p->append(SkRasterPipeline::swap_rb); } - SkASSERT(supported(fDst.info())); switch (fDst.info().colorType()) { case kAlpha_8_SkColorType: p->append(SkRasterPipeline::store_a8, &fDstPtr); break; case kRGB_565_SkColorType: p->append(SkRasterPipeline::store_565, &fDstPtr); break;