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>

14 files changed, 39 insertions, 23 deletions

diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 24414f3184..c79795b5fe 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -792,10 +792,6 @@ bool SkBlitter::UseRasterPipelineBlitter(const SkPixmap& device, const SkPaint&
     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 @@ bool SkBlitter::UseRasterPipelineBlitter(const SkPixmap& device, const SkPaint&
         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 a3a3c41d6e..001f3cce70 100644
--- a/src/shaders/SkColorFilterShader.h
+++ b/src/shaders/SkColorFilterShader.h
@@ -29,9 +29,7 @@ protected:
     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 a3b5b21db8..432915dfba 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 @@ protected:
     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 064fe7b975..f8f0f88a21 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -75,7 +75,7 @@ bool SkImageShader::IsRasterPipelineOnly(SkColorType ct, SkShader::TileMode tx,
     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 e68abb5d5a..a2bd3a63f0 100644
--- a/src/shaders/SkImageShader.h
+++ b/src/shaders/SkImageShader.h
@@ -19,7 +19,6 @@ public:
                                 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 @@ protected:
 #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 4572e9fe2e..5ae8e03df4 100644
--- a/src/shaders/SkLocalMatrixShader.h
+++ b/src/shaders/SkLocalMatrixShader.h
@@ -16,7 +16,7 @@ class GrFragmentProcessor;
 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 @@ protected:
     }
 #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 d80a7da1f9..82439aeb62 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -265,7 +265,7 @@ sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix, con
     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 6fd2a3a9f5..55bbebbf81 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -39,7 +39,7 @@ protected:
                         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 bd202c1aed..5a584105b3 100644
--- a/src/shaders/SkShader.cpp
+++ b/src/shaders/SkShader.cpp
@@ -101,6 +101,11 @@ SkShaderBase::Context* SkShaderBase::makeBurstPipelineContext(const ContextRec&
 
     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 @@ SkShaderBase::Context::Context(const SkShaderBase& shader, const ContextRec& rec
 {
     // 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 8dc9354e53..d469442215 100644
--- a/src/shaders/SkShaderBase.h
+++ b/src/shaders/SkShaderBase.h
@@ -211,7 +211,10 @@ public:
         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 @@ protected:
     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 27b986ca50..0619ef0546 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -538,6 +538,9 @@ SkGradientShaderBase::GradientShaderBaseContext::GradientShaderBaseContext(
     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 40cfe68087..61ddf5c39b 100644
--- a/src/shaders/gradients/SkSweepGradient.cpp
+++ b/src/shaders/gradients/SkSweepGradient.cpp
@@ -62,7 +62,7 @@ SkSweepGradient::SweepGradientContext::SweepGradientContext(
         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 599b833868..f3932d53b0 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 @@ protected:
     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 3e48a4b4df..41909c900d 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.h
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.h
@@ -44,7 +44,7 @@ protected:
                                      SkRasterPipeline* tPipeline,
                                      SkRasterPipeline* postPipeline) const override;
 
-    bool isRasterPipelineOnly() const override { return true; }
+    bool onIsRasterPipelineOnly() const override { return true; }
 
 private:
     SkPoint fCenter1;