missed a place to clamp

We have a fast path where we can use a memset for constant colors.
That wasn't being gamut-clamped until now.

Some refactoring to allow append_color_pipeline() to be called
without a this pointer.

Change-Id: I8a10e537d579235e80633a5e480f1e38c7932014
Reviewed-on: https://skia-review.googlesource.com/152583
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
Mike Klein 2018-09-07 11:18:37 -04:00 committed by Skia Commit-Bot
parent 41b67730a1
commit c471151721

View File

@ -50,7 +50,6 @@ public:
void blitV (int x, int y, int height, SkAlpha alpha) override;
private:
void append_color_pipeline(SkRasterPipeline*) const;
void append_load_dst (SkRasterPipeline*) const;
void append_store (SkRasterPipeline*) const;
@ -87,6 +86,23 @@ private:
typedef SkBlitter INHERITED;
};
static void append_color_pipeline(SkRasterPipeline* p,
const SkRasterPipeline& colorPipeline,
SkImageInfo dstInfo) {
p->extend(colorPipeline);
// 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)
{
// TODO: this will be common enough that we may want to fuse into ::clamp_premul.
p->append(SkRasterPipeline::clamp_0);
p->append(SkRasterPipeline::clamp_a);
}
}
SkBlitter* SkCreateRasterPipelineBlitter(const SkPixmap& dst,
const SkPaint& paint,
const SkMatrix& ctm,
@ -224,7 +240,7 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
// Run our color pipeline all the way through to produce what we'd memset when we can.
// Not all blits can memset, so we need to keep colorPipeline too.
SkRasterPipeline_<256> p;
p.extend(*colorPipeline);
append_color_pipeline(&p, *colorPipeline, dst.info());
blitter->fDstPtr = SkJumper_MemoryCtx{&blitter->fMemsetColor, 0};
blitter->append_store(&p);
p.run(0,0,1,1);
@ -240,21 +256,6 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
return blitter;
}
void SkRasterPipelineBlitter::append_color_pipeline(SkRasterPipeline* p) const {
p->extend(fColorPipeline);
// 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 (fDst.info().colorType() != kRGBA_F16_SkColorType &&
fDst.info().colorType() != kRGBA_F32_SkColorType &&
fDst.info().alphaType() == kPremul_SkAlphaType)
{
// TODO: this will be common enough that we may want to fuse into ::clamp_premul.
p->append(SkRasterPipeline::clamp_0);
p->append(SkRasterPipeline::clamp_a);
}
}
void SkRasterPipelineBlitter::append_load_dst(SkRasterPipeline* p) const {
const void* ctx = &fDstPtr;
switch (fDst.info().colorType()) {
@ -340,7 +341,7 @@ void SkRasterPipelineBlitter::blitRect(int x, int y, int w, int h) {
if (!fBlitRect) {
SkRasterPipeline p(fAlloc);
this->append_color_pipeline(&p);
append_color_pipeline(&p, fColorPipeline, fDst.info());
if (fBlend == SkBlendMode::kSrcOver
&& (fDst.info().colorType() == kRGBA_8888_SkColorType ||
fDst.info().colorType() == kBGRA_8888_SkColorType)
@ -376,7 +377,7 @@ void SkRasterPipelineBlitter::blitRect(int x, int y, int w, int h) {
void SkRasterPipelineBlitter::blitAntiH(int x, int y, const SkAlpha aa[], const int16_t runs[]) {
if (!fBlitAntiH) {
SkRasterPipeline p(fAlloc);
this->append_color_pipeline(&p);
append_color_pipeline(&p, fColorPipeline, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/false)) {
p.append(SkRasterPipeline::scale_1_float, &fCurrentCoverage);
this->append_load_dst(&p);
@ -460,7 +461,7 @@ void SkRasterPipelineBlitter::blitMask(const SkMask& mask, const SkIRect& clip)
// Lazily build whichever pipeline we need, specialized for each mask format.
if (effectiveMaskFormat == SkMask::kA8_Format && !fBlitMaskA8) {
SkRasterPipeline p(fAlloc);
this->append_color_pipeline(&p);
append_color_pipeline(&p, fColorPipeline, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/false)) {
p.append(SkRasterPipeline::scale_u8, &fMaskPtr);
this->append_load_dst(&p);
@ -475,7 +476,7 @@ void SkRasterPipelineBlitter::blitMask(const SkMask& mask, const SkIRect& clip)
}
if (effectiveMaskFormat == SkMask::kLCD16_Format && !fBlitMaskLCD16) {
SkRasterPipeline p(fAlloc);
this->append_color_pipeline(&p);
append_color_pipeline(&p, fColorPipeline, fDst.info());
if (SkBlendMode_ShouldPreScaleCoverage(fBlend, /*rgb_coverage=*/true)) {
// Somewhat unusually, scale_565 needs dst loaded first.
this->append_load_dst(&p);