Revert "clamp gamut if needed in SkConvertPixels"

This reverts commit b8fef7c986.

Reason for revert: serialize-8888?

Original change's description:
> clamp gamut if needed in SkConvertPixels
> 
> We need to clamp here for all the same reasons we need to clamp when
> blitting.  I've centralized the clamp's implementation to help that.
> 
> I've allowed any --config to run this GM.  --config 565 was actually
> pointing out this problem by asserting, and now looks fine.
> 
> Change-Id: Icc066792fc53747b161302d200abdd8dc4669f7f
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
> Reviewed-on: https://skia-review.googlesource.com/158721
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>

TBR=mtklein@google.com,brianosman@google.com

Change-Id: Id888656b313619ab2652a3387bdbc5208e192ec1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://skia-review.googlesource.com/c/158980
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2018-10-02 17:51:51 +00:00 committed by Skia Commit-Bot
parent 6e2d607334
commit 6a76dab257
5 changed files with 22 additions and 20 deletions

View File

@ -124,6 +124,11 @@ protected:
}
void onDraw(SkCanvas* canvas) override {
if (!canvas->imageInfo().colorSpace()) {
// This gm is only interesting in color correct modes.
return;
}
const SkAlphaType alphaTypes[] = {
kUnpremul_SkAlphaType,
kPremul_SkAlphaType,

View File

@ -177,8 +177,6 @@ static void convert_with_pipeline(const SkImageInfo& dstInfo, void* dstRow, size
pipeline.append_load(srcInfo.colorType(), &src);
steps.apply(&pipeline);
pipeline.append_gamut_clamp_if_normalized(dstInfo);
// We'll dither if we're decreasing precision below 32-bit.
float dither_rate = 0.0f;
if (srcInfo.bytesPerPixel() > dstInfo.bytesPerPixel()) {

View File

@ -23,7 +23,6 @@ void SkRasterPipeline::append(StockStage stage, void* ctx) {
SkASSERT(stage != unbounded_uniform_color); // Please use append_constant_color().
SkASSERT(stage != set_rgb); // Please use append_set_rgb().
SkASSERT(stage != unbounded_set_rgb); // Please use append_set_rgb().
SkASSERT(stage != clamp_gamut); // Please use append_gamut_clamp_if_normalized().
this->unchecked_append(stage, ctx);
}
void SkRasterPipeline::unchecked_append(StockStage stage, void* ctx) {
@ -267,12 +266,3 @@ void SkRasterPipeline::append_store(SkColorType ct, const SkJumper_MemoryCtx* ct
break;
}
}
void SkRasterPipeline::append_gamut_clamp_if_normalized(const SkImageInfo& dstInfo) {
if (dstInfo.colorType() != kRGBA_F16_SkColorType &&
dstInfo.colorType() != kRGBA_F32_SkColorType &&
dstInfo.alphaType() == kPremul_SkAlphaType)
{
this->unchecked_append(SkRasterPipeline::clamp_gamut, nullptr);
}
}

View File

@ -156,8 +156,6 @@ public:
void append_load_dst(SkColorType, const SkJumper_MemoryCtx*);
void append_store (SkColorType, const SkJumper_MemoryCtx*);
void append_gamut_clamp_if_normalized(const SkImageInfo&);
bool empty() const { return fStages == nullptr; }

View File

@ -86,6 +86,17 @@ private:
typedef SkBlitter INHERITED;
};
static void append_gamut_clamp(SkRasterPipeline* p, SkImageInfo dstInfo) {
// TODO: can we refine this condition further to avoid clamps when we're known in-gamut?
// When opaque we could _probably_ get away without a clamp, but for consistency we keep it.
if (dstInfo.colorType() != kRGBA_F16_SkColorType &&
dstInfo.colorType() != kRGBA_F32_SkColorType &&
dstInfo.alphaType() == kPremul_SkAlphaType)
{
p->append(SkRasterPipeline::clamp_gamut);
}
}
SkBlitter* SkCreateRasterPipelineBlitter(const SkPixmap& dst,
const SkPaint& paint,
const SkMatrix& ctm,
@ -203,7 +214,7 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
if (is_constant) {
SkColor4f constantColor;
SkJumper_MemoryCtx constantColorPtr = { &constantColor, 0 };
colorPipeline->append_gamut_clamp_if_normalized(dst.info());
append_gamut_clamp(colorPipeline, dst.info());
colorPipeline->append(SkRasterPipeline::store_f32, &constantColorPtr);
colorPipeline->run(0,0,1,1);
colorPipeline->reset();
@ -225,7 +236,7 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
// Not all blits can memset, so we need to keep colorPipeline too.
SkRasterPipeline_<256> p;
p.extend(*colorPipeline);
p.append_gamut_clamp_if_normalized(dst.info());
append_gamut_clamp(&p, dst.info());
blitter->fDstPtr = SkJumper_MemoryCtx{&blitter->fMemsetColor, 0};
blitter->append_store(&p);
p.run(0,0,1,1);
@ -290,7 +301,7 @@ void SkRasterPipelineBlitter::blitRect(int x, int y, int w, int h) {
if (!fBlitRect) {
SkRasterPipeline p(fAlloc);
p.extend(fColorPipeline);
p.append_gamut_clamp_if_normalized(fDst.info());
append_gamut_clamp(&p, fDst.info());
if (fBlend == SkBlendMode::kSrcOver
&& (fDst.info().colorType() == kRGBA_8888_SkColorType ||
fDst.info().colorType() == kBGRA_8888_SkColorType)
@ -327,7 +338,7 @@ void SkRasterPipelineBlitter::blitAntiH(int x, int y, const SkAlpha aa[], const
if (!fBlitAntiH) {
SkRasterPipeline p(fAlloc);
p.extend(fColorPipeline);
p.append_gamut_clamp_if_normalized(fDst.info());
append_gamut_clamp(&p, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/false)) {
p.append(SkRasterPipeline::scale_1_float, &fCurrentCoverage);
this->append_load_dst(&p);
@ -412,7 +423,7 @@ void SkRasterPipelineBlitter::blitMask(const SkMask& mask, const SkIRect& clip)
if (effectiveMaskFormat == SkMask::kA8_Format && !fBlitMaskA8) {
SkRasterPipeline p(fAlloc);
p.extend(fColorPipeline);
p.append_gamut_clamp_if_normalized(fDst.info());
append_gamut_clamp(&p, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/false)) {
p.append(SkRasterPipeline::scale_u8, &fMaskPtr);
this->append_load_dst(&p);
@ -428,7 +439,7 @@ void SkRasterPipelineBlitter::blitMask(const SkMask& mask, const SkIRect& clip)
if (effectiveMaskFormat == SkMask::kLCD16_Format && !fBlitMaskLCD16) {
SkRasterPipeline p(fAlloc);
p.extend(fColorPipeline);
p.append_gamut_clamp_if_normalized(fDst.info());
append_gamut_clamp(&p, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/true)) {
// Somewhat unusually, scale_565 needs dst loaded first.
this->append_load_dst(&p);