Use MatrixRec in legacy shader context code
Bug: skia:14076
Change-Id: I750222f18a1705e9e98ee21a6b4ec22d744b2c03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/714818
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index d6bd265..dacec4c 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -770,8 +770,7 @@
if (paint->getShader()) {
shaderContext = as_SB(paint->getShader())
->makeContext({paint->getAlpha(),
- ctm,
- nullptr,
+ SkShaders::MatrixRec(ctm),
device.colorType(),
device.colorSpace(),
props},
diff --git a/src/shaders/SkBitmapProcShader.cpp b/src/shaders/SkBitmapProcShader.cpp
index dd4df85..911ffa5 100644
--- a/src/shaders/SkBitmapProcShader.cpp
+++ b/src/shaders/SkBitmapProcShader.cpp
@@ -79,7 +79,7 @@
{
SkMatrix totalInverse;
// Do this first, so we know the matrix can be inverted.
- if (!shader.computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, &totalInverse)) {
+ if (!rec.fMatrixRec.totalInverse(&totalInverse)) {
return nullptr;
}
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index a1056d1..c0e77b5 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -304,8 +304,7 @@
}
SkMatrix inv;
- if (!this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, &inv) ||
- !legacy_shader_can_handle(inv)) {
+ if (!rec.fMatrixRec.totalInverse(&inv) || !legacy_shader_can_handle(inv)) {
return nullptr;
}
diff --git a/src/shaders/SkLocalMatrixShader.cpp b/src/shaders/SkLocalMatrixShader.cpp
index e984f9c..9c0e164 100644
--- a/src/shaders/SkLocalMatrixShader.cpp
+++ b/src/shaders/SkLocalMatrixShader.cpp
@@ -6,7 +6,6 @@
*/
#include "src/shaders/SkLocalMatrixShader.h"
-#include "src/base/SkTLazy.h"
#include "src/core/SkReadBuffer.h"
#include "src/core/SkWriteBuffer.h"
@@ -63,18 +62,9 @@
}
#ifdef SK_ENABLE_LEGACY_SHADERCONTEXT
-SkShaderBase::Context* SkLocalMatrixShader::onMakeContext(
- const ContextRec& rec, SkArenaAlloc* alloc) const
-{
- SkTCopyOnFirstWrite<SkMatrix> lm(fLocalMatrix);
- if (rec.fLocalMatrix) {
- *lm.writable() = ConcatLocalMatrices(*rec.fLocalMatrix, *lm);
- }
-
- ContextRec newRec(rec);
- newRec.fLocalMatrix = lm;
-
- return as_SB(fWrappedShader)->makeContext(newRec, alloc);
+SkShaderBase::Context* SkLocalMatrixShader::onMakeContext(const ContextRec& rec,
+ SkArenaAlloc* alloc) const {
+ return as_SB(fWrappedShader)->makeContext(ContextRec::Concat(rec, fLocalMatrix), alloc);
}
#endif
diff --git a/src/shaders/SkPerlinNoiseShaderImpl.cpp b/src/shaders/SkPerlinNoiseShaderImpl.cpp
index 374ee56..c41e4bf 100644
--- a/src/shaders/SkPerlinNoiseShaderImpl.cpp
+++ b/src/shaders/SkPerlinNoiseShaderImpl.cpp
@@ -223,18 +223,10 @@
}
#endif
-static inline SkMatrix total_matrix(const SkShaderBase::ContextRec& rec,
- const SkShaderBase& shader) {
- if (rec.fLocalMatrix) {
- return SkMatrix::Concat(*rec.fMatrix, *rec.fLocalMatrix);
- }
- return *rec.fMatrix;
-}
-
SkPerlinNoiseShader::PerlinNoiseShaderContext::PerlinNoiseShaderContext(
const SkPerlinNoiseShader& shader, const ContextRec& rec)
: Context(shader, rec)
- , fMatrix(total_matrix(rec, shader)) // used for temp storage, adjusted below
+ , fMatrix(rec.fMatrixRec.totalMatrix()) // used for temp storage, adjusted below
, fPaintingData(shader.fTileSize,
shader.fSeed,
shader.fBaseFrequencyX,
diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index 3435ea3..56eea11 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -313,13 +313,10 @@
/////////////////////////////////////////////////////////////////////////////////////////
#ifdef SK_ENABLE_LEGACY_SHADERCONTEXT
-SkShaderBase::Context* SkPictureShader::onMakeContext(const ContextRec& rec, SkArenaAlloc* alloc)
-const {
- const auto& vm = *rec.fMatrix;
- const auto* lm = rec.fLocalMatrix;
- const auto totalM = lm ? SkMatrix::Concat(vm, *lm) : vm;
- sk_sp<SkShader> bitmapShader = this->rasterShader(totalM, rec.fDstColorType,
- rec.fDstColorSpace, rec.fProps);
+SkShaderBase::Context* SkPictureShader::onMakeContext(const ContextRec& rec,
+ SkArenaAlloc* alloc) const {
+ sk_sp<SkShader> bitmapShader = this->rasterShader(
+ rec.fMatrixRec.totalMatrix(), rec.fDstColorType, rec.fDstColorSpace, rec.fProps);
if (!bitmapShader) {
return nullptr;
}
diff --git a/src/shaders/SkShaderBase.cpp b/src/shaders/SkShaderBase.cpp
index cfb2b74..0ad9cfb 100644
--- a/src/shaders/SkShaderBase.cpp
+++ b/src/shaders/SkShaderBase.cpp
@@ -113,12 +113,6 @@
void SkShaderBase::flatten(SkWriteBuffer& buffer) const { this->INHERITED::flatten(buffer); }
-bool SkShaderBase::computeTotalInverse(const SkMatrix& ctm,
- const SkMatrix* localMatrix,
- SkMatrix* totalInverse) const {
- return (localMatrix ? SkMatrix::Concat(ctm, *localMatrix) : ctm).invert(totalInverse);
-}
-
bool SkShaderBase::asLuminanceColor(SkColor* colorPtr) const {
SkColor storage;
if (nullptr == colorPtr) {
@@ -134,8 +128,8 @@
SkShaderBase::Context* SkShaderBase::makeContext(const ContextRec& rec, SkArenaAlloc* alloc) const {
#ifdef SK_ENABLE_LEGACY_SHADERCONTEXT
// We always fall back to raster pipeline when perspective is present.
- if (rec.fMatrix->hasPerspective() || (rec.fLocalMatrix && rec.fLocalMatrix->hasPerspective()) ||
- !this->computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, nullptr)) {
+ auto totalMatrix = rec.fMatrixRec.totalMatrix();
+ if (totalMatrix.hasPerspective() || !totalMatrix.invert(nullptr)) {
return nullptr;
}
@@ -146,14 +140,13 @@
}
SkShaderBase::Context::Context(const SkShaderBase& shader, const ContextRec& rec)
- : fShader(shader), fCTM(*rec.fMatrix) {
+ : fShader(shader) {
// We should never use a context with perspective.
- SkASSERT(!rec.fMatrix->hasPerspective());
- SkASSERT(!rec.fLocalMatrix || !rec.fLocalMatrix->hasPerspective());
+ SkASSERT(!rec.fMatrixRec.totalMatrix().hasPerspective());
// Because the context parameters must be valid at this point, we know that the matrix is
// invertible.
- SkAssertResult(fShader.computeTotalInverse(*rec.fMatrix, rec.fLocalMatrix, &fTotalInverse));
+ SkAssertResult(rec.fMatrixRec.totalInverse(&fTotalInverse));
fPaintAlpha = rec.fPaintAlpha;
}
@@ -190,16 +183,7 @@
// SkShader::Context::shadeSpan() handles the paint opacity internally,
// but SkRasterPipelineBlitter applies it as a separate stage.
// We skip the internal shadeSpan() step by forcing the alpha to be opaque.
-
- // We don't have a separate ctm and local matrix at this point. Just pass the combined matrix
- // as the CTM. TODO: thread the MatrixRec through the legacy context system.
- auto tm = mRec.totalMatrix();
- ContextRec cr(SK_AlphaOPAQUE,
- tm,
- nullptr,
- rec.fDstColorType,
- sk_srgb_singleton(),
- rec.fSurfaceProps);
+ ContextRec cr(SK_AlphaOPAQUE, mRec, rec.fDstColorType, sk_srgb_singleton(), rec.fSurfaceProps);
struct CallbackCtx : SkRasterPipeline_CallbackCtx {
sk_sp<const SkShader> shader;
diff --git a/src/shaders/SkShaderBase.h b/src/shaders/SkShaderBase.h
index e6381a4..5936d0f 100644
--- a/src/shaders/SkShaderBase.h
+++ b/src/shaders/SkShaderBase.h
@@ -50,15 +50,16 @@
namespace SkShaders {
/**
* This is used to accumulate matrices, starting with the CTM, when building up
- * SkRasterPipeline, SkVM, and GrFragmentProcessor by walking the SkShader tree. It avoids
+ * SkRasterPipeline, SkVM, or GrFragmentProcessor by walking the SkShader tree. It avoids
* adding a matrix multiply for each individual matrix. It also handles the reverse matrix
* concatenation order required by Android Framework, see b/256873449.
*
- * This also tracks the dubious concept of a "total matrix", which includes all the matrices
- * encountered during traversal to the current shader, including ones that have already been
- * applied. The total matrix represents the transformation from the current shader's coordinate
- * space to device space. It is dubious because it doesn't account for SkShaders that manipulate
- * the coordinates passed to their children, which may not even be representable by a matrix.
+ * This also tracks the dubious concept of a "total matrix", in the legacy Context/shadeSpan system.
+ * That includes all the matrices encountered during traversal to the current shader, including ones
+ * that have already been applied. The total matrix represents the transformation from the current
+ * shader's coordinate space to device space. It is dubious because it doesn't account for SkShaders
+ * that manipulate the coordinates passed to their children, which may not even be representable by
+ * a matrix.
*
* The total matrix is used for mipmap level selection and a filter downgrade optimizations in
* SkImageShader and sizing of the SkImage created by SkPictureShader. If we can remove usages
@@ -290,21 +291,30 @@
* ContextRec acts as a parameter bundle for creating Contexts.
*/
struct ContextRec {
- ContextRec(SkAlpha paintAlpha, const SkMatrix& matrix, const SkMatrix* localM,
- SkColorType dstColorType, SkColorSpace* dstColorSpace, SkSurfaceProps props)
- : fMatrix(&matrix)
- , fLocalMatrix(localM)
- , fDstColorType(dstColorType)
- , fDstColorSpace(dstColorSpace)
- , fProps(props)
- , fPaintAlpha(paintAlpha) {}
+ ContextRec(SkAlpha paintAlpha,
+ const SkShaders::MatrixRec& matrixRec,
+ SkColorType dstColorType,
+ SkColorSpace* dstColorSpace,
+ SkSurfaceProps props)
+ : fMatrixRec(matrixRec)
+ , fDstColorType(dstColorType)
+ , fDstColorSpace(dstColorSpace)
+ , fProps(props)
+ , fPaintAlpha(paintAlpha) {}
- const SkMatrix* fMatrix; // the current matrix in the canvas
- const SkMatrix* fLocalMatrix; // optional local matrix
- SkColorType fDstColorType; // the color type of the dest surface
- SkColorSpace* fDstColorSpace; // the color space of the dest surface (if any)
- SkSurfaceProps fProps; // props of the dest surface
- SkAlpha fPaintAlpha;
+ static ContextRec Concat(const ContextRec& parentRec, const SkMatrix& localM) {
+ return {parentRec.fPaintAlpha,
+ parentRec.fMatrixRec.concat(localM),
+ parentRec.fDstColorType,
+ parentRec.fDstColorSpace,
+ parentRec.fProps};
+ }
+
+ const SkShaders::MatrixRec fMatrixRec;
+ SkColorType fDstColorType; // the color type of the dest surface
+ SkColorSpace* fDstColorSpace; // the color space of the dest surface (if any)
+ SkSurfaceProps fProps; // props of the dest surface
+ SkAlpha fPaintAlpha;
bool isLegacyCompatible(SkColorSpace* shadersColorSpace) const;
};
@@ -337,14 +347,10 @@
uint8_t getPaintAlpha() const { return fPaintAlpha; }
const SkMatrix& getTotalInverse() const { return fTotalInverse; }
- const SkMatrix& getCTM() const { return fCTM; }
private:
- SkMatrix fCTM;
SkMatrix fTotalInverse;
uint8_t fPaintAlpha;
-
- using INHERITED = SkNoncopyable;
};
/**
@@ -379,10 +385,6 @@
*/
virtual bool appendStages(const SkStageRec&, const SkShaders::MatrixRec&) const;
- bool SK_WARN_UNUSED_RESULT computeTotalInverse(const SkMatrix& ctm,
- const SkMatrix* localMatrix,
- SkMatrix* totalInverse) const;
-
virtual SkImage* onIsAImage(SkMatrix*, SkTileMode[2]) const {
return nullptr;
}
