Fix SkModeColorFilter in 565
It has been incorrectly interpreting its SkColor as sRGB all the time. Now, we plumb through the destintation color space and some scratch space, letting it decide how to interpret its SkColor later when it knows about the dst color space. The scratch space is blitter scoped, which lets this be thread safe (this is much like SkShader::Context).
This only corrects the gamma transformation for now. I've kept my previous TODO about gamut transformation. Everything assumes sRGB gamut for now.
Shaders will get the same treatement in this pipeline.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4725
Change-Id: I55b0c7d5db9ad8d7dcdd6295c9dac61d10aeaed4
Reviewed-on: https://skia-review.googlesource.com/4725
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
diff --git a/include/core/SkColorFilter.h b/include/core/SkColorFilter.h
index 346e721..a194927 100644
--- a/include/core/SkColorFilter.h
+++ b/include/core/SkColorFilter.h
@@ -17,6 +17,7 @@
class GrFragmentProcessor;
class SkBitmap;
class SkColorSpace;
+class SkFallbackAlloc;
class SkRasterPipeline;
/**
@@ -72,7 +73,8 @@
virtual void filterSpan4f(const SkPM4f src[], int count, SkPM4f result[]) const;
- bool appendStages(SkRasterPipeline*, bool shaderIsOpaque) const;
+ bool appendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*,
+ bool shaderIsOpaque) const;
enum Flags {
/** If set the filter methods will not change the alpha channel of the colors.
@@ -158,7 +160,8 @@
protected:
SkColorFilter() {}
- virtual bool onAppendStages(SkRasterPipeline*, bool shaderIsOpaque) const;
+ virtual bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*,
+ bool shaderIsOpaque) const;
private:
/*
diff --git a/include/core/SkPixmap.h b/include/core/SkPixmap.h
index fe2a0b9..ce13297 100644
--- a/include/core/SkPixmap.h
+++ b/include/core/SkPixmap.h
@@ -70,6 +70,7 @@
int height() const { return fInfo.height(); }
SkColorType colorType() const { return fInfo.colorType(); }
SkAlphaType alphaType() const { return fInfo.alphaType(); }
+ SkColorSpace* colorSpace() const { return fInfo.colorSpace(); }
bool isOpaque() const { return fInfo.isOpaque(); }
SkIRect bounds() const { return SkIRect::MakeWH(this->width(), this->height()); }
diff --git a/include/effects/SkLumaColorFilter.h b/include/effects/SkLumaColorFilter.h
index 69edbc9..5cddec6 100644
--- a/include/effects/SkLumaColorFilter.h
+++ b/include/effects/SkLumaColorFilter.h
@@ -42,7 +42,8 @@
private:
SkLumaColorFilter();
- bool onAppendStages(SkRasterPipeline*, bool shaderIsOpaque) const override;
+ bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*,
+ bool shaderIsOpaque) const override;
typedef SkColorFilter INHERITED;
};
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 6e21beb..664109a 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -6,6 +6,7 @@
*/
#include "SkColorFilter.h"
+#include "SkFixedAlloc.h"
#include "SkReadBuffer.h"
#include "SkRefCnt.h"
#include "SkString.h"
@@ -37,11 +38,14 @@
}
#endif
-bool SkColorFilter::appendStages(SkRasterPipeline* pipeline, bool shaderIsOpaque) const {
- return this->onAppendStages(pipeline, shaderIsOpaque);
+bool SkColorFilter::appendStages(SkRasterPipeline* pipeline,
+ SkColorSpace* dst,
+ SkFallbackAlloc* scratch,
+ bool shaderIsOpaque) const {
+ return this->onAppendStages(pipeline, dst, scratch, shaderIsOpaque);
}
-bool SkColorFilter::onAppendStages(SkRasterPipeline*, bool shaderIsOpaque) const {
+bool SkColorFilter::onAppendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*, bool) const {
return false;
}
diff --git a/src/core/SkColorMatrixFilterRowMajor255.cpp b/src/core/SkColorMatrixFilterRowMajor255.cpp
index b27d40d..c234025 100644
--- a/src/core/SkColorMatrixFilterRowMajor255.cpp
+++ b/src/core/SkColorMatrixFilterRowMajor255.cpp
@@ -232,6 +232,8 @@
//////
bool SkColorMatrixFilterRowMajor255::onAppendStages(SkRasterPipeline* p,
+ SkColorSpace* dst,
+ SkFallbackAlloc* scratch,
bool shaderIsOpaque) const {
bool willStayOpaque = shaderIsOpaque && (fFlags & kAlphaUnchanged_Flag);
bool needsClamp0 = false,
diff --git a/src/core/SkColorMatrixFilterRowMajor255.h b/src/core/SkColorMatrixFilterRowMajor255.h
index 0f1665c..f4312c5 100644
--- a/src/core/SkColorMatrixFilterRowMajor255.h
+++ b/src/core/SkColorMatrixFilterRowMajor255.h
@@ -36,7 +36,8 @@
void flatten(SkWriteBuffer&) const override;
private:
- bool onAppendStages(SkRasterPipeline*, bool shaderIsOpaque) const override;
+ bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*,
+ bool shaderIsOpaque) const override;
SkScalar fMatrix[20];
float fTranspose[20]; // for Sk4s
diff --git a/src/core/SkFixedAlloc.h b/src/core/SkFixedAlloc.h
index 8e2736e..d4e4d8f 100644
--- a/src/core/SkFixedAlloc.h
+++ b/src/core/SkFixedAlloc.h
@@ -37,7 +37,7 @@
// Create the T.
auto ptr = (T*)(fBuffer+fUsed);
- new (ptr) T{std::forward<Args>(args)...};
+ new (ptr) T(std::forward<Args>(args)...);
fUsed += sizeof(T);
// Stamp a footer after the T that we can use to clean it up.
@@ -78,7 +78,7 @@
return ptr;
}
}
- auto ptr = new T{std::forward<Args>(args)...};
+ auto ptr = new T(std::forward<Args>(args)...);
fHeapAllocs.push_back({[](void* ptr) { delete (T*)ptr; }, ptr});
return ptr;
}
diff --git a/src/core/SkModeColorFilter.cpp b/src/core/SkModeColorFilter.cpp
index 760e824..9c76f9e 100644
--- a/src/core/SkModeColorFilter.cpp
+++ b/src/core/SkModeColorFilter.cpp
@@ -9,7 +9,9 @@
#include "SkBlendModePriv.h"
#include "SkColorFilter.h"
#include "SkColorPriv.h"
+#include "SkFixedAlloc.h"
#include "SkModeColorFilter.h"
+#include "SkPM4fPriv.h"
#include "SkRasterPipeline.h"
#include "SkReadBuffer.h"
#include "SkWriteBuffer.h"
@@ -62,9 +64,9 @@
void SkModeColorFilter::filterSpan4f(const SkPM4f shader[], int count, SkPM4f result[]) const {
SkXfermodeProc4f proc = SkXfermode::GetProc4f(fMode);
-
+ auto pm4f = SkColor4f::FromColor(fColor).premul();
for (int i = 0; i < count; i++) {
- result[i] = proc(fPM4f, shader[i]);
+ result[i] = proc(pm4f, shader[i]);
}
}
@@ -76,7 +78,6 @@
void SkModeColorFilter::updateCache() {
fPMColor = SkPreMultiplyColor(fColor);
fProc = SkXfermode::GetProc(fMode);
- fPM4f = SkColor4f::FromColor(fColor).premul();
}
sk_sp<SkFlattenable> SkModeColorFilter::CreateProc(SkReadBuffer& buffer) {
@@ -85,11 +86,14 @@
return SkColorFilter::MakeModeFilter(color, mode);
}
-bool SkModeColorFilter::onAppendStages(SkRasterPipeline* p, bool shaderIsOpaque) const {
- // TODO: For some modes we can cut a stage by loading the fPM4f into dr,dg,db,da
- // and applying the opposite xfermode, e.g. dst-in instead of src-in.
+bool SkModeColorFilter::onAppendStages(SkRasterPipeline* p,
+ SkColorSpace* dst,
+ SkFallbackAlloc* scratch,
+ bool shaderIsOpaque) const {
+ auto color = scratch->make<SkPM4f>(SkPM4f_from_SkColor(fColor, dst));
+
p->append(SkRasterPipeline::move_src_dst);
- p->append(SkRasterPipeline::constant_color, &fPM4f);
+ p->append(SkRasterPipeline::constant_color, color);
auto mode = (SkBlendMode)fMode;
if (!SkBlendMode_AppendStages(mode, p)) {
return false;
diff --git a/src/core/SkModeColorFilter.h b/src/core/SkModeColorFilter.h
index 2c3d4dc..2586c29 100644
--- a/src/core/SkModeColorFilter.h
+++ b/src/core/SkModeColorFilter.h
@@ -45,7 +45,8 @@
void flatten(SkWriteBuffer&) const override;
- bool onAppendStages(SkRasterPipeline*, bool shaderIsOpaque) const override;
+ bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkFallbackAlloc*,
+ bool shaderIsOpaque) const override;
private:
SkColor fColor;
@@ -53,7 +54,6 @@
// cache
SkPMColor fPMColor;
SkXfermodeProc fProc;
- SkPM4f fPM4f;
void updateCache();
diff --git a/src/core/SkPM4fPriv.h b/src/core/SkPM4fPriv.h
index 89a0cae..f70c5c5 100644
--- a/src/core/SkPM4fPriv.h
+++ b/src/core/SkPM4fPriv.h
@@ -9,6 +9,7 @@
#define SkPM4fPriv_DEFINED
#include "SkColorPriv.h"
+#include "SkColorSpace.h"
#include "SkPM4f.h"
#include "SkSRGB.h"
@@ -71,4 +72,17 @@
return linear;
}
+static inline SkPM4f SkPM4f_from_SkColor(SkColor color, SkColorSpace* dst) {
+ SkColor4f color4f;
+ if (dst) {
+ // sRGB gamma, sRGB gamut.
+ color4f = SkColor4f::FromColor(color);
+ // TODO: gamut transform if needed
+ } else {
+ // Linear gamma, dst gamut.
+ swizzle_rb(SkNx_cast<float>(Sk4b::Load(&color)) * (1/255.0f)).store(&color4f);
+ }
+ return color4f.premul();
+}
+
#endif
diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp
index b8c0107..d382eba 100644
--- a/src/core/SkRasterPipelineBlitter.cpp
+++ b/src/core/SkRasterPipelineBlitter.cpp
@@ -9,8 +9,10 @@
#include "SkBlendModePriv.h"
#include "SkColor.h"
#include "SkColorFilter.h"
+#include "SkFixedAlloc.h"
#include "SkOpts.h"
#include "SkPM4f.h"
+#include "SkPM4fPriv.h"
#include "SkRasterPipeline.h"
#include "SkShader.h"
#include "SkXfermode.h"
@@ -24,6 +26,8 @@
: fDst(dst)
, fBlend(blend)
, fPaintColor(paintColor)
+ , fScratchAlloc(fScratch, sizeof(fScratch))
+ , fScratchFallback(&fScratchAlloc)
{}
void blitH (int x, int y, int w) override;
@@ -57,6 +61,11 @@
const void* fMaskPtr = nullptr;
float fConstantCoverage = 0.0f;
+ // Scratch space for shaders and color filters to use.
+ char fScratch[64];
+ SkFixedAlloc fScratchAlloc;
+ SkFallbackAlloc fScratchFallback;
+
typedef SkBlitter INHERITED;
};
@@ -75,29 +84,14 @@
}
}
-template <typename Effect>
-static bool append_effect_stages(const Effect* effect, SkRasterPipeline* pipeline) {
- return !effect || effect->appendStages(pipeline);
-}
-
-static SkPM4f paint_color(const SkPixmap& dst, const SkPaint& paint) {
- auto paintColor = paint.getColor();
- SkColor4f color;
- if (dst.info().colorSpace()) {
- color = SkColor4f::FromColor(paintColor);
- // TODO: transform from sRGB to dst gamut.
- } else {
- swizzle_rb(SkNx_cast<float>(Sk4b::Load(&paintColor)) * (1/255.0f)).store(&color);
- }
- return color.premul();
-}
-
SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
const SkPaint& paint,
SkTBlitterAllocator* alloc) {
- auto blitter = alloc->createT<SkRasterPipelineBlitter>(dst,
- paint.getBlendMode(),
- paint_color(dst, paint));
+ auto blitter = alloc->createT<SkRasterPipelineBlitter>(
+ dst,
+ paint.getBlendMode(),
+ SkPM4f_from_SkColor(paint.getColor(), dst.colorSpace()));
+
auto earlyOut = [&] {
blitter->~SkRasterPipelineBlitter();
alloc->freeLast();
@@ -128,7 +122,8 @@
}
if (colorFilter) {
- if (!colorFilter->appendStages(pipeline, is_opaque)) {
+ if (!colorFilter->appendStages(pipeline, dst.colorSpace(), &blitter->fScratchFallback,
+ is_opaque)) {
return earlyOut();
}
is_opaque = is_opaque && (colorFilter->getFlags() & SkColorFilter::kAlphaUnchanged_Flag);
diff --git a/src/effects/SkLumaColorFilter.cpp b/src/effects/SkLumaColorFilter.cpp
index 3061708..94d800a 100644
--- a/src/effects/SkLumaColorFilter.cpp
+++ b/src/effects/SkLumaColorFilter.cpp
@@ -38,7 +38,10 @@
}
}
-bool SkLumaColorFilter::onAppendStages(SkRasterPipeline* p, bool shaderIsOpaque) const {
+bool SkLumaColorFilter::onAppendStages(SkRasterPipeline* p,
+ SkColorSpace* dst,
+ SkFallbackAlloc* scratch,
+ bool shaderIsOpaque) const {
p->append(SkRasterPipeline::luminance_to_alpha);
return true;
}