Add some raster pipeline perspective asserts
I meant to add these when removing the guard, but since we landed without
a guard, might as well do it now.
A couple of things exposed by these asserts:
1) we need to also catch perspective in local matrices
2) we need to disallow burst mode with perspective
Also tweak the predicate to hasPerspective() instead of explicit mask
check.
Change-Id: I099e5125fca52dccffca77c60fc800bbdf539b53
Reviewed-on: https://skia-review.googlesource.com/22483
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 24414f3..c79795b 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -792,10 +792,6 @@
if (device.colorSpace()) {
return true;
}
- // ... unless the shader is raster pipeline-only.
- if (paint.getShader() && as_SB(paint.getShader())->isRasterPipelineOnly()) {
- return true;
- }
if (paint.getColorFilter()) {
return true;
}
@@ -804,13 +800,18 @@
return true;
}
#endif
- // ... or unless the blend mode is complicated enough.
+ // ... unless the blend mode is complicated enough.
if (paint.getBlendMode() > SkBlendMode::kLastSeparableMode) {
return true;
}
// ... or unless we have to deal with perspective.
- if (matrix.getType() & SkMatrix::kPerspective_Mask) {
+ if (matrix.hasPerspective()) {
+ return true;
+ }
+
+ // ... or unless the shader is raster pipeline-only.
+ if (paint.getShader() && as_SB(paint.getShader())->isRasterPipelineOnly()) {
return true;
}
diff --git a/src/shaders/SkColorFilterShader.h b/src/shaders/SkColorFilterShader.h
index a3a3c41..001f3cc 100644
--- a/src/shaders/SkColorFilterShader.h
+++ b/src/shaders/SkColorFilterShader.h
@@ -29,9 +29,7 @@
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
bool onAppendStages(SkRasterPipeline*, SkColorSpace* dstCS, SkArenaAlloc*,
const SkMatrix&, const SkPaint&, const SkMatrix* localM) const override;
- bool isRasterPipelineOnly() const override {
- return true;
- }
+ bool onIsRasterPipelineOnly() const override { return true; }
private:
sk_sp<SkShader> fShader;
diff --git a/src/shaders/SkComposeShader.h b/src/shaders/SkComposeShader.h
index a3b5b21..432915d 100644
--- a/src/shaders/SkComposeShader.h
+++ b/src/shaders/SkComposeShader.h
@@ -11,7 +11,7 @@
#include "SkShaderBase.h"
#include "SkBlendMode.h"
-class SkComposeShader : public SkShaderBase {
+class SkComposeShader final : public SkShaderBase {
public:
SkComposeShader(sk_sp<SkShader> dst, sk_sp<SkShader> src, SkBlendMode mode, float lerpT)
: fDst(std::move(dst))
@@ -45,7 +45,7 @@
bool onAppendStages(SkRasterPipeline*, SkColorSpace* dstCS, SkArenaAlloc*,
const SkMatrix&, const SkPaint&, const SkMatrix* localM) const override;
- bool isRasterPipelineOnly() const final { return true; }
+ bool onIsRasterPipelineOnly() const override { return true; }
private:
sk_sp<SkShader> fDst;
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index 064fe7b..f8f0f88 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -75,7 +75,7 @@
return false;
}
-bool SkImageShader::isRasterPipelineOnly() const {
+bool SkImageShader::onIsRasterPipelineOnly() const {
SkBitmapProvider provider(fImage.get(), nullptr);
return IsRasterPipelineOnly(provider.info().colorType(), fTileModeX, fTileModeY);
}
diff --git a/src/shaders/SkImageShader.h b/src/shaders/SkImageShader.h
index e68abb5..a2bd3a6 100644
--- a/src/shaders/SkImageShader.h
+++ b/src/shaders/SkImageShader.h
@@ -19,7 +19,6 @@
const SkMatrix* localMatrix);
bool isOpaque() const override;
- bool isRasterPipelineOnly() const override;
SK_TO_STRING_OVERRIDE()
SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkImageShader)
@@ -40,6 +39,8 @@
#endif
SkImage* onIsAImage(SkMatrix*, TileMode*) const override;
+ bool onIsRasterPipelineOnly() const override;
+
bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*,
const SkMatrix& ctm, const SkPaint&, const SkMatrix*) const override;
diff --git a/src/shaders/SkLocalMatrixShader.h b/src/shaders/SkLocalMatrixShader.h
index 4572e9f..5ae8e03 100644
--- a/src/shaders/SkLocalMatrixShader.h
+++ b/src/shaders/SkLocalMatrixShader.h
@@ -16,7 +16,7 @@
class SkArenaAlloc;
class SkColorSpaceXformer;
-class SkLocalMatrixShader : public SkShaderBase {
+class SkLocalMatrixShader final : public SkShaderBase {
public:
SkLocalMatrixShader(sk_sp<SkShader> proxy, const SkMatrix& localMatrix)
: INHERITED(&localMatrix)
@@ -62,7 +62,7 @@
}
#endif
- bool isRasterPipelineOnly() const final {
+ bool onIsRasterPipelineOnly() const override {
return as_SB(fProxyShader)->isRasterPipelineOnly();
}
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index d80a7da..82439ae 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -265,7 +265,7 @@
return tileShader;
}
-bool SkPictureShader::isRasterPipelineOnly() const {
+bool SkPictureShader::onIsRasterPipelineOnly() const {
return SkImageShader::IsRasterPipelineOnly(kN32_SkColorType, fTmx, fTmy);
}
diff --git a/src/shaders/SkPictureShader.h b/src/shaders/SkPictureShader.h
index 6fd2a3a..55bbebb 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -39,7 +39,7 @@
const SkMatrix&, const SkPaint&, const SkMatrix*) const override;
Context* onMakeContext(const ContextRec&, SkArenaAlloc*) const override;
sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
- bool isRasterPipelineOnly() const override;
+ bool onIsRasterPipelineOnly() 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 bd202c1..5a58410 100644
--- a/src/shaders/SkShader.cpp
+++ b/src/shaders/SkShader.cpp
@@ -101,6 +101,11 @@
SkASSERT(rec.fPreferredDstType == ContextRec::kPM4f_DstType);
+ // Always use vanilla stages for perspective.
+ if (rec.fMatrix->hasPerspective() || fLocalMatrix.hasPerspective()) {
+ return nullptr;
+ }
+
return this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, nullptr)
? this->onMakeBurstPipelineContext(rec, alloc)
: nullptr;
@@ -111,6 +116,9 @@
{
// We should never use a context for RP-only shaders.
SkASSERT(!shader.isRasterPipelineOnly());
+ // ... or for perspective.
+ SkASSERT(!rec.fMatrix->hasPerspective());
+ SkASSERT(!rec.fLocalMatrix || !rec.fLocalMatrix->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 8dc9354..d469442 100644
--- a/src/shaders/SkShaderBase.h
+++ b/src/shaders/SkShaderBase.h
@@ -211,7 +211,10 @@
return this->onMakeColorSpace(xformer);
}
- virtual bool isRasterPipelineOnly() const { return false; }
+ bool isRasterPipelineOnly() const {
+ // We always use RP when perspective is present.
+ return fLocalMatrix.hasPerspective() || this->onIsRasterPipelineOnly();
+ }
// If this returns false, then we draw nothing (do not fall back to shader context)
bool appendStages(SkRasterPipeline*, SkColorSpace* dstCS, SkArenaAlloc*,
@@ -268,6 +271,8 @@
virtual bool onAppendStages(SkRasterPipeline*, SkColorSpace* dstCS, SkArenaAlloc*,
const SkMatrix&, const SkPaint&, const SkMatrix* localM) const;
+ virtual bool onIsRasterPipelineOnly() 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 27b986c..0619ef0 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -538,6 +538,9 @@
fDstToIndexProc = fDstToIndex.getMapXYProc();
fDstToIndexClass = (uint8_t)SkShaderBase::Context::ComputeMatrixClass(fDstToIndex);
+ // TODO: remove all perspective-related gradient code
+ SkASSERT(fDstToIndexClass == kLinear_MatrixClass);
+
// now convert our colors in to PMColors
unsigned paintAlpha = this->getPaintAlpha();
diff --git a/src/shaders/gradients/SkSweepGradient.cpp b/src/shaders/gradients/SkSweepGradient.cpp
index 40cfe68..61ddf5c 100644
--- a/src/shaders/gradients/SkSweepGradient.cpp
+++ b/src/shaders/gradients/SkSweepGradient.cpp
@@ -62,7 +62,7 @@
const SkSweepGradient& shader, const ContextRec& rec)
: INHERITED(shader, rec) {}
-bool SkSweepGradient::isRasterPipelineOnly() const {
+bool SkSweepGradient::onIsRasterPipelineOnly() const {
#ifdef SK_LEGACY_SWEEP_GRADIENT
return false;
#else
diff --git a/src/shaders/gradients/SkSweepGradient.h b/src/shaders/gradients/SkSweepGradient.h
index 599b833..f3932d5 100644
--- a/src/shaders/gradients/SkSweepGradient.h
+++ b/src/shaders/gradients/SkSweepGradient.h
@@ -10,7 +10,7 @@
#include "SkGradientShaderPriv.h"
-class SkSweepGradient : public SkGradientShaderBase {
+class SkSweepGradient final : public SkGradientShaderBase {
public:
SkSweepGradient(SkScalar cx, SkScalar cy, const Descriptor&);
@@ -41,9 +41,9 @@
bool adjustMatrixAndAppendStages(SkArenaAlloc* alloc,
SkMatrix* matrix,
SkRasterPipeline* tPipeline,
- SkRasterPipeline* postPipeline) const final;
+ SkRasterPipeline* postPipeline) const override;
- bool isRasterPipelineOnly() const final;
+ bool onIsRasterPipelineOnly() const override;
private:
const SkPoint fCenter;
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.h b/src/shaders/gradients/SkTwoPointConicalGradient.h
index 3e48a4b..41909c9 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.h
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.h
@@ -44,7 +44,7 @@
SkRasterPipeline* tPipeline,
SkRasterPipeline* postPipeline) const override;
- bool isRasterPipelineOnly() const override { return true; }
+ bool onIsRasterPipelineOnly() const override { return true; }
private:
SkPoint fCenter1;