Banish SkShaderBase::isRasterPipelineOnly()
Keeping related heuristics in sync with actual shader capabilities is somewhat tricky,
and overall fragile.
So how about this: instead of an explicit opt-in mechanism, try to instantiate a legacy
shader context and fall back to raster pipeline on failure (null Context => implicit
opt-in for raster pipeline). Shaders can still choose not to draw by returning both a
null Context and failing appendStages().
BUG=skia:7772
Change-Id: I2e76f51af7064853a6cb851b4c30c82eba3ee828
Reviewed-on: https://skia-review.googlesource.com/118383
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 29294a9..edd3da2 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -914,10 +914,6 @@
if (matrix.hasPerspective()) {
return true;
}
- // ... or unless the shader is raster pipeline-only.
- if (paint.getShader() && as_SB(paint.getShader())->isRasterPipelineOnly(matrix)) {
- return true;
- }
// Added support only for shaders (and other constraints) for android
if (device.colorType() == kRGB_565_SkColorType) {
@@ -947,22 +943,25 @@
SkBlendMode mode = origPaint.getBlendMode();
sk_sp<Sk3DShader> shader3D;
- SkTCopyOnFirstWrite<SkPaint> paint(origPaint);
+ // We're going to tweak the original paint in two ways:
+ // 1) tweaks to `commonPaint` affect both raster pipeline and legacy pipeline
+ // 2) (later) tweaks to `legacyPaint` affect only legacy pipeline
+ SkTCopyOnFirstWrite<SkPaint> commonPaint(origPaint);
if (origPaint.getMaskFilter() != nullptr &&
as_MFB(origPaint.getMaskFilter())->getFormat() == SkMask::k3D_Format) {
shader3D = sk_make_sp<Sk3DShader>(sk_ref_sp(shader));
// we know we haven't initialized lazyPaint yet, so just do it
- paint.writable()->setShader(shader3D);
+ commonPaint.writable()->setShader(shader3D);
shader = as_SB(shader3D.get());
}
if (mode != SkBlendMode::kSrcOver) {
bool deviceIsOpaque = kRGB_565_SkColorType == device.colorType();
- switch (SkInterpretXfermode(*paint, deviceIsOpaque)) {
+ switch (SkInterpretXfermode(*commonPaint, deviceIsOpaque)) {
case kSrcOver_SkXfermodeInterpretation:
mode = SkBlendMode::kSrcOver;
- paint.writable()->setBlendMode(mode);
+ commonPaint.writable()->setBlendMode(mode);
break;
case kSkipDrawing_SkXfermodeInterpretation:{
return alloc->make<SkNullBlitter>();
@@ -978,7 +977,7 @@
* will fall into our optimizations for SRC mode.
*/
if (mode == SkBlendMode::kClear) {
- SkPaint* p = paint.writable();
+ SkPaint* p = commonPaint.writable();
p->setShader(nullptr);
shader = nullptr;
p->setColorFilter(nullptr);
@@ -989,32 +988,35 @@
if (kAlpha_8_SkColorType == device.colorType() && drawCoverage) {
SkASSERT(nullptr == shader);
- SkASSERT(paint->isSrcOver());
- return alloc->make<SkA8_Coverage_Blitter>(device, *paint);
+ SkASSERT(commonPaint->isSrcOver());
+ return alloc->make<SkA8_Coverage_Blitter>(device, *commonPaint);
}
- if (paint->isDither() && !SkPaintPriv::ShouldDither(*paint, device.colorType())) {
+ if (commonPaint->isDither() && !SkPaintPriv::ShouldDither(*commonPaint, device.colorType())) {
// Disable dithering when not needed.
- paint.writable()->setDither(false);
+ commonPaint.writable()->setDither(false);
}
- if (UseRasterPipelineBlitter(device, *paint, matrix)) {
- auto blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc);
+ if (UseRasterPipelineBlitter(device, *commonPaint, matrix)) {
+ auto blitter = SkCreateRasterPipelineBlitter(device, *commonPaint, matrix, alloc);
SkASSERT(blitter);
return blitter;
}
+ // Used for the legacy pipeline.
+ SkTCopyOnFirstWrite<SkPaint> legacyPaint(*commonPaint);
+
if (nullptr == shader) {
if (mode != SkBlendMode::kSrcOver) {
// xfermodes (and filters) require shaders for our current blitters
- paint.writable()->setShader(SkShader::MakeColorShader(paint->getColor()));
- paint.writable()->setAlpha(0xFF);
- shader = as_SB(paint->getShader());
+ legacyPaint.writable()->setShader(SkShader::MakeColorShader(legacyPaint->getColor()));
+ legacyPaint.writable()->setAlpha(0xFF);
+ shader = as_SB(legacyPaint->getShader());
} else if (cf) {
// if no shader && no xfermode, we just apply the colorfilter to
// our color and move on.
- SkPaint* writablePaint = paint.writable();
- writablePaint->setColor(cf->filterColor(paint->getColor()));
+ SkPaint* writablePaint = legacyPaint.writable();
+ writablePaint->setColor(cf->filterColor(legacyPaint->getColor()));
writablePaint->setColorFilter(nullptr);
cf = nullptr;
}
@@ -1022,8 +1024,8 @@
if (cf) {
SkASSERT(shader);
- paint.writable()->setShader(shader->makeWithColorFilter(sk_ref_sp(cf)));
- shader = as_SB(paint->getShader());
+ legacyPaint.writable()->setShader(shader->makeWithColorFilter(sk_ref_sp(cf)));
+ shader = as_SB(legacyPaint->getShader());
// blitters should ignore the presence/absence of a filter, since
// if there is one, the shader will take care of it.
}
@@ -1033,15 +1035,17 @@
*/
SkShaderBase::Context* shaderContext = nullptr;
if (shader) {
- const SkShaderBase::ContextRec rec(*paint, matrix, nullptr,
+ const SkShaderBase::ContextRec rec(*legacyPaint, matrix, nullptr,
PreferredShaderDest(device.info()),
device.colorSpace());
// Try to create the ShaderContext
shaderContext = shader->makeContext(rec, alloc);
if (!shaderContext) {
- return alloc->make<SkNullBlitter>();
+ // Fall back to raster pipeline.
+ auto blitter = SkCreateRasterPipelineBlitter(device, *commonPaint, matrix, alloc);
+ SkASSERT(blitter);
+ return blitter;
}
- SkASSERT(shaderContext);
}
SkBlitter* blitter = nullptr;
@@ -1051,20 +1055,20 @@
SkASSERT(!device.colorSpace());
if (shader) {
- blitter = alloc->make<SkARGB32_Shader_Blitter>(device, *paint, shaderContext);
- } else if (paint->getColor() == SK_ColorBLACK) {
- blitter = alloc->make<SkARGB32_Black_Blitter>(device, *paint);
- } else if (paint->getAlpha() == 0xFF) {
- blitter = alloc->make<SkARGB32_Opaque_Blitter>(device, *paint);
+ blitter = alloc->make<SkARGB32_Shader_Blitter>(device, *legacyPaint, shaderContext);
+ } else if (legacyPaint->getColor() == SK_ColorBLACK) {
+ blitter = alloc->make<SkARGB32_Black_Blitter>(device, *legacyPaint);
+ } else if (legacyPaint->getAlpha() == 0xFF) {
+ blitter = alloc->make<SkARGB32_Opaque_Blitter>(device, *legacyPaint);
} else {
- blitter = alloc->make<SkARGB32_Blitter>(device, *paint);
+ blitter = alloc->make<SkARGB32_Blitter>(device, *legacyPaint);
}
break;
case kRGB_565_SkColorType:
- if (shader && SkRGB565_Shader_Blitter::Supports(device, *paint)) {
- blitter = alloc->make<SkRGB565_Shader_Blitter>(device, *paint, shaderContext);
+ if (shader && SkRGB565_Shader_Blitter::Supports(device, *legacyPaint)) {
+ blitter = alloc->make<SkRGB565_Shader_Blitter>(device, *legacyPaint, shaderContext);
} else {
- blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc);
+ blitter = SkCreateRasterPipelineBlitter(device, *legacyPaint, matrix, alloc);
}
break;
diff --git a/src/shaders/SkColorFilterShader.h b/src/shaders/SkColorFilterShader.h
index e771f38..c0083fd 100644
--- a/src/shaders/SkColorFilterShader.h
+++ b/src/shaders/SkColorFilterShader.h
@@ -28,7 +28,6 @@
void flatten(SkWriteBuffer&) const override;
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
bool onAppendStages(const StageRec&) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override { return true; }
private:
sk_sp<SkShader> fShader;
diff --git a/src/shaders/SkComposeShader.h b/src/shaders/SkComposeShader.h
index 7efa06c..ebcc86c 100644
--- a/src/shaders/SkComposeShader.h
+++ b/src/shaders/SkComposeShader.h
@@ -44,8 +44,6 @@
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
bool onAppendStages(const StageRec&) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override { return true; }
-
private:
sk_sp<SkShader> fDst;
sk_sp<SkShader> fSrc;
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index c3e6931..cdd9413 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -69,14 +69,8 @@
return fImage->isOpaque() && fTileModeX != kDecal_TileMode && fTileModeY != kDecal_TileMode;
}
-static bool legacy_shader_can_handle(const SkMatrix& a, const SkMatrix& b) {
- SkMatrix m = SkMatrix::Concat(a, b);
- if (!m.isScaleTranslate()) {
- return false;
- }
-
- SkMatrix inv;
- if (!m.invert(&inv)) {
+static bool legacy_shader_can_handle(const SkMatrix& inv) {
+ if (!inv.isScaleTranslate()) {
return false;
}
@@ -97,37 +91,31 @@
return true;
}
-bool SkImageShader::IsRasterPipelineOnly(const SkMatrix& ctm, SkColorType ct, SkAlphaType at,
- SkShader::TileMode tx, SkShader::TileMode ty,
- const SkMatrix& localM) {
- if (ct != kN32_SkColorType) {
- return true;
- }
- if (at == kUnpremul_SkAlphaType) {
- return true;
- }
-#ifndef SK_SUPPORT_LEGACY_TILED_BITMAPS
- if (tx != ty) {
- return true;
- }
-#endif
- if (tx == kDecal_TileMode || ty == kDecal_TileMode) {
- return true;
- }
- if (!legacy_shader_can_handle(ctm, localM)) {
- return true;
- }
- return false;
-}
-
-bool SkImageShader::onIsRasterPipelineOnly(const SkMatrix& ctm) const {
- SkBitmapProvider provider(fImage.get(), nullptr);
- return IsRasterPipelineOnly(ctm, provider.info().colorType(), provider.info().alphaType(),
- fTileModeX, fTileModeY, this->getLocalMatrix());
-}
-
SkShaderBase::Context* SkImageShader::onMakeContext(const ContextRec& rec,
SkArenaAlloc* alloc) const {
+ const auto info = as_IB(fImage)->onImageInfo();
+
+ if (info.colorType() != kN32_SkColorType) {
+ return nullptr;
+ }
+ if (info.alphaType() == kUnpremul_SkAlphaType) {
+ return nullptr;
+ }
+#ifndef SK_SUPPORT_LEGACY_TILED_BITMAPS
+ if (fTileModeX != fTileModeY) {
+ return nullptr;
+ }
+#endif
+ if (fTileModeX == kDecal_TileMode || fTileModeY == kDecal_TileMode) {
+ return nullptr;
+ }
+
+ SkMatrix inv;
+ if (!this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, &inv) ||
+ !legacy_shader_can_handle(inv)) {
+ return nullptr;
+ }
+
return SkBitmapProcLegacyShader::MakeContext(*this, fTileModeX, fTileModeY,
SkBitmapProvider(fImage.get(), rec.fDstColorSpace),
rec, alloc);
diff --git a/src/shaders/SkImageShader.h b/src/shaders/SkImageShader.h
index a024dc7..78627d5 100644
--- a/src/shaders/SkImageShader.h
+++ b/src/shaders/SkImageShader.h
@@ -30,10 +30,6 @@
std::unique_ptr<GrFragmentProcessor> asFragmentProcessor(const GrFPArgs&) const override;
#endif
- static bool IsRasterPipelineOnly(const SkMatrix& ctm, SkColorType, SkAlphaType,
- SkShader::TileMode tx, SkShader::TileMode ty,
- const SkMatrix& localM);
-
private:
SkImageShader(sk_sp<SkImage>,
SkShader::TileMode tx,
@@ -48,8 +44,6 @@
#endif
SkImage* onIsAImage(SkMatrix*, SkShader::TileMode*) const override;
- bool onIsRasterPipelineOnly(const SkMatrix& ctm) const override;
-
bool onAppendStages(const StageRec&) const override;
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override {
diff --git a/src/shaders/SkLocalMatrixShader.h b/src/shaders/SkLocalMatrixShader.h
index 2d7e313..890482e 100644
--- a/src/shaders/SkLocalMatrixShader.h
+++ b/src/shaders/SkLocalMatrixShader.h
@@ -61,11 +61,6 @@
}
#endif
- bool onIsRasterPipelineOnly(const SkMatrix& ctm) const override {
- return as_SB(fProxyShader)->isRasterPipelineOnly(SkMatrix::Concat(ctm,
- this->getLocalMatrix()));
- }
-
private:
sk_sp<SkShader> fProxyShader;
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index 097a605..21c9e5f 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -280,11 +280,6 @@
return tileShader;
}
-bool SkPictureShader::onIsRasterPipelineOnly(const SkMatrix& ctm) const {
- return SkImageShader::IsRasterPipelineOnly(ctm, kN32_SkColorType, kPremul_SkAlphaType,
- fTmx, fTmy, this->getLocalMatrix());
-}
-
bool SkPictureShader::onAppendStages(const StageRec& rec) const {
// Keep bitmapShader alive by using alloc instead of stack memory
auto& bitmapShader = *rec.fAlloc->make<sk_sp<SkShader>>();
diff --git a/src/shaders/SkPictureShader.h b/src/shaders/SkPictureShader.h
index 3b7cab0..6ad7165 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -41,7 +41,6 @@
bool onAppendStages(const StageRec&) const override;
Context* onMakeContext(const ContextRec&, SkArenaAlloc*) const override;
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override;
private:
SkPictureShader(sk_sp<SkPicture>, TileMode, TileMode, const SkMatrix*, const SkRect*,
diff --git a/src/shaders/SkShader.cpp b/src/shaders/SkShader.cpp
index 2dcba94..195ac29 100644
--- a/src/shaders/SkShader.cpp
+++ b/src/shaders/SkShader.cpp
@@ -91,9 +91,15 @@
}
SkShaderBase::Context* SkShaderBase::makeContext(const ContextRec& rec, SkArenaAlloc* alloc) const {
- return this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, nullptr)
- ? this->onMakeContext(rec, alloc)
- : nullptr;
+ // We always fall back to raster pipeline when perspective is present.
+ if (rec.fMatrix->hasPerspective() ||
+ fLocalMatrix.hasPerspective() ||
+ (rec.fLocalMatrix && rec.fLocalMatrix->hasPerspective()) ||
+ !this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, nullptr)) {
+ return nullptr;
+ }
+
+ return this->onMakeContext(rec, alloc);
}
SkShaderBase::Context* SkShaderBase::makeBurstPipelineContext(const ContextRec& rec,
@@ -114,11 +120,10 @@
SkShaderBase::Context::Context(const SkShaderBase& shader, const ContextRec& rec)
: fShader(shader), fCTM(*rec.fMatrix)
{
- // We should never use a context for RP-only shaders.
- SkASSERT(!shader.isRasterPipelineOnly(*rec.fMatrix));
- // ... or for perspective.
+ // We should never use a context with perspective.
SkASSERT(!rec.fMatrix->hasPerspective());
SkASSERT(!rec.fLocalMatrix || !rec.fLocalMatrix->hasPerspective());
+ SkASSERT(!shader.getLocalMatrix().hasPerspective());
// Because the context parameters must be valid at this point, we know that the matrix is
// invertible.
diff --git a/src/shaders/SkShaderBase.h b/src/shaders/SkShaderBase.h
index f72863e..a4b0607 100644
--- a/src/shaders/SkShaderBase.h
+++ b/src/shaders/SkShaderBase.h
@@ -173,12 +173,6 @@
return this->onMakeColorSpace(xformer);
}
- bool isRasterPipelineOnly(const SkMatrix& ctm) const {
- // We always use RP when perspective is present.
- return ctm.hasPerspective() || fLocalMatrix.hasPerspective()
- || this->onIsRasterPipelineOnly(ctm);
- }
-
struct StageRec {
SkRasterPipeline* fPipeline;
SkArenaAlloc* fAlloc;
@@ -241,8 +235,6 @@
// Default impl creates shadercontext and calls that (not very efficient)
virtual bool onAppendStages(const StageRec&) const;
- virtual bool onIsRasterPipelineOnly(const SkMatrix& ctm) const { return false; }
-
private:
// This is essentially const, but not officially so it can be modified in constructors.
SkMatrix fLocalMatrix;
diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 114b7c5..82ae129 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -419,13 +419,6 @@
return fColorsAreOpaque && (this->getTileMode() != SkShader::kDecal_TileMode);
}
-bool SkGradientShaderBase::onIsRasterPipelineOnly(const SkMatrix& ctm) const {
- if (this->getTileMode() == SkShader::kDecal_TileMode) {
- return true;
- }
- return this->INHERITED::onIsRasterPipelineOnly(ctm);
-}
-
static unsigned rounded_divide(unsigned numer, unsigned denom) {
return (numer + (denom >> 1)) / denom;
}
diff --git a/src/shaders/gradients/SkGradientShaderPriv.h b/src/shaders/gradients/SkGradientShaderPriv.h
index 7fef127..29c8c6e 100644
--- a/src/shaders/gradients/SkGradientShaderPriv.h
+++ b/src/shaders/gradients/SkGradientShaderPriv.h
@@ -94,7 +94,6 @@
void initLinearBitmap(SkBitmap* bitmap, GradientBitmapType) const;
bool onAppendStages(const StageRec&) const override;
- bool onIsRasterPipelineOnly(const SkMatrix& ctm) const override;
virtual void appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* tPipeline,
SkRasterPipeline* postPipeline) const = 0;
diff --git a/src/shaders/gradients/SkLinearGradient.cpp b/src/shaders/gradients/SkLinearGradient.cpp
index 7398221..21338b8 100644
--- a/src/shaders/gradients/SkLinearGradient.cpp
+++ b/src/shaders/gradients/SkLinearGradient.cpp
@@ -55,7 +55,9 @@
SkShaderBase::Context* SkLinearGradient::onMakeContext(
const ContextRec& rec, SkArenaAlloc* alloc) const
{
- return CheckedMakeContext<LinearGradient4fContext>(alloc, *this, rec);
+ return fTileMode != kDecal_TileMode
+ ? CheckedMakeContext<LinearGradient4fContext>(alloc, *this, rec)
+ : nullptr;
}
SkShaderBase::Context* SkLinearGradient::onMakeBurstPipelineContext(
diff --git a/src/shaders/gradients/SkRadialGradient.h b/src/shaders/gradients/SkRadialGradient.h
index 06ba27f..2fac4c6 100644
--- a/src/shaders/gradients/SkRadialGradient.h
+++ b/src/shaders/gradients/SkRadialGradient.h
@@ -30,8 +30,6 @@
void appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* tPipeline,
SkRasterPipeline* postPipeline) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override { return true; }
-
private:
const SkPoint fCenter;
const SkScalar fRadius;
diff --git a/src/shaders/gradients/SkSweepGradient.h b/src/shaders/gradients/SkSweepGradient.h
index c4b5c67..49b789c 100644
--- a/src/shaders/gradients/SkSweepGradient.h
+++ b/src/shaders/gradients/SkSweepGradient.h
@@ -30,8 +30,6 @@
void appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* tPipeline,
SkRasterPipeline* postPipeline) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override { return true; }
-
private:
const SkPoint fCenter;
const SkScalar fTBias,
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.h b/src/shaders/gradients/SkTwoPointConicalGradient.h
index 413a199..59728ef 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.h
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.h
@@ -74,8 +74,6 @@
void appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* tPipeline,
SkRasterPipeline* postPipeline) const override;
- bool onIsRasterPipelineOnly(const SkMatrix&) const override { return true; }
-
private:
SkTwoPointConicalGradient(const SkPoint& c0, SkScalar r0,
const SkPoint& c1, SkScalar r1,