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>
diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp
index 0ef37a8..0ae6df3 100644
--- a/src/core/SkRasterPipelineBlitter.cpp
+++ b/src/core/SkRasterPipelineBlitter.cpp
@@ -50,7 +50,6 @@
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 @@
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 @@
// 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 @@
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 @@
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::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 @@
// 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 @@
}
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);