Reland "Centralize geometry processor vertex shader transform code"
This is a reland of 0426947243e1d6b470830c4d3b1c5704ed1f23e1
Original change's description:
> Centralize geometry processor vertex shader transform code
>
> GrGLSLGeometryProcessors no longer have to call emitTransforms() in
> their onEmitCode() function. Instead, the GpArgs struct allows them to
> set a GrShaderVar that holds the computed or explicitly provided local
> coordinates in the vertex shader.
>
> The base GrGLSLGeometryProcessor now automatically uses that to collect
> all of the transforms that can then be lifted out of FPs to the vertex
> shader, and base their computation on the GP provided local coordinate.
>
> As part of this, there is no more built-in magic concatenation of a
> local matrix / inverse view matrix to these coordinate transforms. GP
> implementations that relied on this now manage their own uniform for this
> matrix and compute the local coordinate before assigning to GpArgs.
>
> The base GrGLSLGeometryProcessor is updated to provide helpers for this
> pattern.
>
> Bug: skia:10396
> Change-Id: I56afb3fff4b806f6015ab13626ac1afde9ef5c2b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/297027
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:10396
Change-Id: If1347bcacb7c405a66f9d4c5b0059e9d735b3f9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298062
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/effects/GrBezierEffect.cpp b/src/gpu/effects/GrBezierEffect.cpp
index cbd8baa..e127ab4 100644
--- a/src/gpu/effects/GrBezierEffect.cpp
+++ b/src/gpu/effects/GrBezierEffect.cpp
@@ -28,12 +28,8 @@
const CoordTransformRange& transformRange) override {
const GrConicEffect& ce = primProc.cast<GrConicEffect>();
- if (!ce.viewMatrix().isIdentity() &&
- !SkMatrixPriv::CheapEqual(fViewMatrix, ce.viewMatrix()))
- {
- fViewMatrix = ce.viewMatrix();
- pdman.setSkMatrix(fViewMatrixUniform, fViewMatrix);
- }
+ this->setTransform(pdman, fViewMatrixUniform, ce.viewMatrix(), &fViewMatrix);
+ this->setTransform(pdman, fLocalMatrixUniform, ce.localMatrix(), &fLocalMatrix);
if (ce.color() != fColor) {
pdman.set4fv(fColorUniform, 1, ce.color().vec());
@@ -44,24 +40,27 @@
pdman.set1f(fCoverageScaleUniform, GrNormalizeByteToFloat(ce.coverageScale()));
fCoverageScale = ce.coverageScale();
}
- this->setTransformDataHelper(ce.localMatrix(), pdman, transformRange);
+ this->setTransformDataHelper(pdman, transformRange);
}
private:
SkMatrix fViewMatrix;
+ SkMatrix fLocalMatrix;
SkPMColor4f fColor;
uint8_t fCoverageScale;
UniformHandle fColorUniform;
UniformHandle fCoverageScaleUniform;
UniformHandle fViewMatrixUniform;
+ UniformHandle fLocalMatrixUniform;
typedef GrGLSLGeometryProcessor INHERITED;
};
GrGLConicEffect::GrGLConicEffect(const GrGeometryProcessor& processor)
- : fViewMatrix(SkMatrix::InvalidMatrix())
- , fColor(SK_PMColor4fILLEGAL)
- , fCoverageScale(0xff) {}
+ : fViewMatrix(SkMatrix::InvalidMatrix())
+ , fLocalMatrix(SkMatrix::InvalidMatrix())
+ , fColor(SK_PMColor4fILLEGAL)
+ , fCoverageScale(0xff) {}
void GrGLConicEffect::onEmitCode(EmitArgs& args, GrGPArgs* gpArgs) {
GrGLSLVertexBuilder* vertBuilder = args.fVertBuilder;
@@ -87,14 +86,10 @@
gp.inPosition().name(),
gp.viewMatrix(),
&fViewMatrixUniform);
-
- // emit transforms with position
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- gp.inPosition().asShaderVar(),
- gp.localMatrix(),
- args.fFPCoordTransformHandler);
+ if (gp.usesLocalCoords()) {
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs, gp.inPosition().asShaderVar(),
+ gp.localMatrix(), &fLocalMatrixUniform);
+ }
// TODO: we should check on the number of bits float and half provide and use the smallest one
// that suffices. Additionally we should assert that the upstream code only lets us get here if
@@ -165,8 +160,9 @@
const GrConicEffect& ce = gp.cast<GrConicEffect>();
uint32_t key = ce.isAntiAliased() ? (ce.isFilled() ? 0x0 : 0x1) : 0x2;
key |= 0xff != ce.coverageScale() ? 0x8 : 0x0;
- key |= ce.usesLocalCoords() && ce.localMatrix().hasPerspective() ? 0x10 : 0x0;
- key |= ComputePosKey(ce.viewMatrix()) << 5;
+ key |= ce.usesLocalCoords() ? 0x10 : 0x0;
+ key = AddMatrixKeys(key, ce.viewMatrix(), ce.usesLocalCoords() ? ce.localMatrix()
+ : SkMatrix::I());
b->add32(key);
}
@@ -227,12 +223,8 @@
const CoordTransformRange& transformRange) override {
const GrQuadEffect& qe = primProc.cast<GrQuadEffect>();
- if (!qe.viewMatrix().isIdentity() &&
- !SkMatrixPriv::CheapEqual(fViewMatrix, qe.viewMatrix()))
- {
- fViewMatrix = qe.viewMatrix();
- pdman.setSkMatrix(fViewMatrixUniform, fViewMatrix);
- }
+ this->setTransform(pdman, fViewMatrixUniform, qe.viewMatrix(), &fViewMatrix);
+ this->setTransform(pdman, fLocalMatrixUniform, qe.localMatrix(), &fLocalMatrix);
if (qe.color() != fColor) {
pdman.set4fv(fColorUniform, 1, qe.color().vec());
@@ -243,24 +235,28 @@
pdman.set1f(fCoverageScaleUniform, GrNormalizeByteToFloat(qe.coverageScale()));
fCoverageScale = qe.coverageScale();
}
- this->setTransformDataHelper(qe.localMatrix(), pdman, transformRange);
+ this->setTransformDataHelper(pdman, transformRange);
}
private:
SkMatrix fViewMatrix;
+ SkMatrix fLocalMatrix;
SkPMColor4f fColor;
uint8_t fCoverageScale;
+
UniformHandle fColorUniform;
UniformHandle fCoverageScaleUniform;
UniformHandle fViewMatrixUniform;
+ UniformHandle fLocalMatrixUniform;
typedef GrGLSLGeometryProcessor INHERITED;
};
GrGLQuadEffect::GrGLQuadEffect(const GrGeometryProcessor& processor)
- : fViewMatrix(SkMatrix::InvalidMatrix())
- , fColor(SK_PMColor4fILLEGAL)
- , fCoverageScale(0xff) {}
+ : fViewMatrix(SkMatrix::InvalidMatrix())
+ , fLocalMatrix(SkMatrix::InvalidMatrix())
+ , fColor(SK_PMColor4fILLEGAL)
+ , fCoverageScale(0xff) {}
void GrGLQuadEffect::onEmitCode(EmitArgs& args, GrGPArgs* gpArgs) {
GrGLSLVertexBuilder* vertBuilder = args.fVertBuilder;
@@ -286,14 +282,10 @@
gp.inPosition().name(),
gp.viewMatrix(),
&fViewMatrixUniform);
-
- // emit transforms with position
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- gp.inPosition().asShaderVar(),
- gp.localMatrix(),
- args.fFPCoordTransformHandler);
+ if (gp.usesLocalCoords()) {
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs, gp.inPosition().asShaderVar(),
+ gp.localMatrix(), &fLocalMatrixUniform);
+ }
fragBuilder->codeAppendf("half edgeAlpha;");
@@ -329,8 +321,9 @@
const GrQuadEffect& ce = gp.cast<GrQuadEffect>();
uint32_t key = ce.isAntiAliased() ? (ce.isFilled() ? 0x0 : 0x1) : 0x2;
key |= ce.coverageScale() != 0xff ? 0x8 : 0x0;
- key |= ce.usesLocalCoords() && ce.localMatrix().hasPerspective() ? 0x10 : 0x0;
- key |= ComputePosKey(ce.viewMatrix()) << 5;
+ key |= ce.usesLocalCoords()? 0x10 : 0x0;
+ key = AddMatrixKeys(key, ce.viewMatrix(), ce.usesLocalCoords() ? ce.localMatrix()
+ : SkMatrix::I());
b->add32(key);
}
diff --git a/src/gpu/effects/GrBitmapTextGeoProc.cpp b/src/gpu/effects/GrBitmapTextGeoProc.cpp
index 8761212..4a9f94e 100644
--- a/src/gpu/effects/GrBitmapTextGeoProc.cpp
+++ b/src/gpu/effects/GrBitmapTextGeoProc.cpp
@@ -20,7 +20,10 @@
class GrGLBitmapTextGeoProc : public GrGLSLGeometryProcessor {
public:
- GrGLBitmapTextGeoProc() : fColor(SK_PMColor4fILLEGAL), fAtlasDimensions{0,0} {}
+ GrGLBitmapTextGeoProc()
+ : fColor(SK_PMColor4fILLEGAL)
+ , fAtlasDimensions{0,0}
+ , fLocalMatrix(SkMatrix::InvalidMatrix()) {}
void onEmitCode(EmitArgs& args, GrGPArgs* gpArgs) override {
const GrBitmapTextGeoProc& btgp = args.fGP.cast<GrBitmapTextGeoProc>();
@@ -53,14 +56,8 @@
// Setup position
gpArgs->fPositionVar = btgp.inPosition().asShaderVar();
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- btgp.inPosition().asShaderVar(),
- btgp.localMatrix(),
- args.fFPCoordTransformHandler);
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs, btgp.inPosition().asShaderVar(),
+ btgp.localMatrix(), &fLocalMatrixUniform);
fragBuilder->codeAppend("half4 texColor;");
append_multitexture_lookup(args, btgp.numTextureSamplers(),
@@ -92,7 +89,9 @@
1.0f / atlasDimensions.fHeight);
fAtlasDimensions = atlasDimensions;
}
- this->setTransformDataHelper(btgp.localMatrix(), pdman, transformRange);
+
+ this->setTransform(pdman, fLocalMatrixUniform, btgp.localMatrix(), &fLocalMatrix);
+ this->setTransformDataHelper(pdman, transformRange);
}
static inline void GenKey(const GrGeometryProcessor& proc,
@@ -102,6 +101,7 @@
uint32_t key = 0;
key |= btgp.usesW() ? 0x1 : 0x0;
key |= btgp.maskFormat() << 1;
+ key |= ComputeMatrixKey(btgp.localMatrix()) << 2;
b->add32(key);
b->add32(btgp.numTextureSamplers());
}
@@ -113,6 +113,9 @@
SkISize fAtlasDimensions;
UniformHandle fAtlasDimensionsInvUniform;
+ SkMatrix fLocalMatrix;
+ UniformHandle fLocalMatrixUniform;
+
typedef GrGLSLGeometryProcessor INHERITED;
};
diff --git a/src/gpu/effects/GrDistanceFieldGeoProc.cpp b/src/gpu/effects/GrDistanceFieldGeoProc.cpp
index 3bf74e1..19c75f3 100644
--- a/src/gpu/effects/GrDistanceFieldGeoProc.cpp
+++ b/src/gpu/effects/GrDistanceFieldGeoProc.cpp
@@ -57,14 +57,8 @@
// Setup position
gpArgs->fPositionVar = dfTexEffect.inPosition().asShaderVar();
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- dfTexEffect.inPosition().asShaderVar(),
- dfTexEffect.localMatrix(),
- args.fFPCoordTransformHandler);
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs, gpArgs->fPositionVar,
+ dfTexEffect.localMatrix(), &fLocalMatrixUniform);
// add varyings
GrGLSLVarying uv(kFloat2_GrSLType);
@@ -185,7 +179,8 @@
1.0f / atlasDimensions.fHeight);
fAtlasDimensions = atlasDimensions;
}
- this->setTransformDataHelper(dfa8gp.localMatrix(), pdman, transformRange);
+ this->setTransformDataHelper(pdman, transformRange);
+ this->setTransform(pdman, fLocalMatrixUniform, dfa8gp.localMatrix(), &fLocalMatrix);
}
static inline void GenKey(const GrGeometryProcessor& gp,
@@ -193,6 +188,7 @@
GrProcessorKeyBuilder* b) {
const GrDistanceFieldA8TextGeoProc& dfTexEffect = gp.cast<GrDistanceFieldA8TextGeoProc>();
uint32_t key = dfTexEffect.getFlags();
+ key |= ComputeMatrixKey(dfTexEffect.localMatrix()) << 16;
b->add32(key);
b->add32(dfTexEffect.numTextureSamplers());
}
@@ -202,9 +198,12 @@
float fDistanceAdjust = -1.f;
UniformHandle fDistanceAdjustUni;
#endif
- SkISize fAtlasDimensions = {0, 0};
+ SkISize fAtlasDimensions = {0, 0};
UniformHandle fAtlasDimensionsInvUniform;
+ SkMatrix fLocalMatrix = SkMatrix::InvalidMatrix();
+ UniformHandle fLocalMatrixUniform;
+
typedef GrGLSLGeometryProcessor INHERITED;
};
@@ -354,31 +353,20 @@
varyingHandler->addPassThroughAttribute(dfPathEffect.inColor(), args.fOutputColor);
if (dfPathEffect.matrix().hasPerspective()) {
- // Setup position
+ // Setup position (output position is transformed, local coords are pass through)
this->writeOutputPosition(vertBuilder,
uniformHandler,
gpArgs,
dfPathEffect.inPosition().name(),
dfPathEffect.matrix(),
&fMatrixUniform);
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- dfPathEffect.inPosition().asShaderVar(),
- args.fFPCoordTransformHandler);
+ gpArgs->fLocalCoordVar = dfPathEffect.inPosition().asShaderVar();
} else {
- // Setup position
+ // Setup position (output position is pass through, local coords are transformed)
this->writeOutputPosition(vertBuilder, gpArgs, dfPathEffect.inPosition().name());
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- dfPathEffect.inPosition().asShaderVar(),
- dfPathEffect.matrix(),
- args.fFPCoordTransformHandler);
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs,
+ dfPathEffect.inPosition().asShaderVar(), dfPathEffect.matrix(),
+ &fMatrixUniform);
}
// Use highp to work around aliasing issues
@@ -463,10 +451,9 @@
const CoordTransformRange& transformRange) override {
const GrDistanceFieldPathGeoProc& dfpgp = proc.cast<GrDistanceFieldPathGeoProc>();
- if (dfpgp.matrix().hasPerspective() && !SkMatrixPriv::CheapEqual(fMatrix, dfpgp.matrix())) {
- fMatrix = dfpgp.matrix();
- pdman.setSkMatrix(fMatrixUniform, fMatrix);
- }
+ // We always set the matrix uniform; it's either used to transform from local to device
+ // for the output position, or from device to local for the local coord variable.
+ this->setTransform(pdman, fMatrixUniform, dfpgp.matrix(), &fMatrix);
const SkISize& atlasDimensions = dfpgp.atlasDimensions();
SkASSERT(SkIsPow2(atlasDimensions.fWidth) && SkIsPow2(atlasDimensions.fHeight));
@@ -477,11 +464,7 @@
fAtlasDimensions = atlasDimensions;
}
- if (dfpgp.matrix().hasPerspective()) {
- this->setTransformDataHelper(SkMatrix::I(), pdman, transformRange);
- } else {
- this->setTransformDataHelper(dfpgp.matrix(), pdman, transformRange);
- }
+ this->setTransformDataHelper(pdman, transformRange);
}
static inline void GenKey(const GrGeometryProcessor& gp,
@@ -490,7 +473,7 @@
const GrDistanceFieldPathGeoProc& dfTexEffect = gp.cast<GrDistanceFieldPathGeoProc>();
uint32_t key = dfTexEffect.getFlags();
- key |= ComputePosKey(dfTexEffect.matrix()) << 16;
+ key |= ComputeMatrixKey(dfTexEffect.matrix()) << 16;
b->add32(key);
b->add32(dfTexEffect.matrix().hasPerspective());
b->add32(dfTexEffect.numTextureSamplers());
@@ -605,7 +588,9 @@
class GrGLDistanceFieldLCDTextGeoProc : public GrGLSLGeometryProcessor {
public:
- GrGLDistanceFieldLCDTextGeoProc() : fAtlasDimensions({0, 0}) {
+ GrGLDistanceFieldLCDTextGeoProc()
+ : fAtlasDimensions({0, 0})
+ , fLocalMatrix(SkMatrix::InvalidMatrix()) {
fDistanceAdjust = GrDistanceFieldLCDTextGeoProc::DistanceAdjust::Make(1.0f, 1.0f, 1.0f);
}
@@ -634,14 +619,9 @@
// Setup position
gpArgs->fPositionVar = dfTexEffect.inPosition().asShaderVar();
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- dfTexEffect.inPosition().asShaderVar(),
- dfTexEffect.localMatrix(),
- args.fFPCoordTransformHandler);
+ this->writeLocalCoord(vertBuilder, uniformHandler, gpArgs,
+ dfTexEffect.inPosition().asShaderVar(), dfTexEffect.localMatrix(),
+ &fLocalMatrixUniform);
// set up varyings
GrGLSLVarying uv(kFloat2_GrSLType);
@@ -801,7 +781,8 @@
1.0f / atlasDimensions.fHeight);
fAtlasDimensions = atlasDimensions;
}
- this->setTransformDataHelper(dflcd.localMatrix(), pdman, transformRange);
+ this->setTransformDataHelper(pdman, transformRange);
+ this->setTransform(pdman, fLocalMatrixUniform, dflcd.localMatrix(), &fLocalMatrix);
}
static inline void GenKey(const GrGeometryProcessor& gp,
@@ -809,7 +790,8 @@
GrProcessorKeyBuilder* b) {
const GrDistanceFieldLCDTextGeoProc& dfTexEffect = gp.cast<GrDistanceFieldLCDTextGeoProc>();
- uint32_t key = dfTexEffect.getFlags();
+ uint32_t key = (dfTexEffect.getFlags() << 16) |
+ ComputeMatrixKey(dfTexEffect.localMatrix());
b->add32(key);
b->add32(dfTexEffect.numTextureSamplers());
}
@@ -821,6 +803,9 @@
SkISize fAtlasDimensions;
UniformHandle fAtlasDimensionsInvUniform;
+ SkMatrix fLocalMatrix;
+ UniformHandle fLocalMatrixUniform;
+
typedef GrGLSLGeometryProcessor INHERITED;
};
diff --git a/src/gpu/effects/GrShadowGeoProc.cpp b/src/gpu/effects/GrShadowGeoProc.cpp
index de6db26..d129a7d 100644
--- a/src/gpu/effects/GrShadowGeoProc.cpp
+++ b/src/gpu/effects/GrShadowGeoProc.cpp
@@ -22,7 +22,6 @@
const GrRRectShadowGeoProc& rsgp = args.fGP.cast<GrRRectShadowGeoProc>();
GrGLSLVertexBuilder* vertBuilder = args.fVertBuilder;
GrGLSLVaryingHandler* varyingHandler = args.fVaryingHandler;
- GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
// emit attributes
@@ -35,13 +34,7 @@
// Setup position
this->writeOutputPosition(vertBuilder, gpArgs, rsgp.inPosition().name());
-
- // emit transforms
- this->emitTransforms(vertBuilder,
- varyingHandler,
- uniformHandler,
- rsgp.inPosition().asShaderVar(),
- args.fFPCoordTransformHandler);
+ // No need for local coordinates, this GP does not combine with fragment processors
fragBuilder->codeAppend("half d = length(shadowParams.xy);");
fragBuilder->codeAppend("float2 uv = float2(shadowParams.z * (1.0 - d), 0.5);");
@@ -53,7 +46,7 @@
void setData(const GrGLSLProgramDataManager& pdman, const GrPrimitiveProcessor& proc,
const CoordTransformRange& transformRange) override {
- this->setTransformDataHelper(SkMatrix::I(), pdman, transformRange);
+ this->setTransformDataHelper(pdman, transformRange);
}
private: