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/glsl/GrGLSLGeometryProcessor.cpp b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
index 0d63776..334b35b 100644
--- a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
@@ -20,6 +20,13 @@
GrGPArgs gpArgs;
this->onEmitCode(args, &gpArgs);
+ // FIXME This must always be called at the moment, even when fLocalCoordVar is uninitialized
+ // and void because collectTransforms registers the uniforms for legacy coord transforms, which
+ // still need to be added even if the FPs are sampled explicitly. When they are gone, we only
+ // need to call this if the local coord isn't void (plus verify that FPs really don't need it).
+ this->collectTransforms(args.fVertBuilder, args.fVaryingHandler, args.fUniformHandler,
+ gpArgs.fLocalCoordVar, args.fFPCoordTransformHandler);
+
if (args.fGP.willUseTessellationShaders()) {
// Tessellation shaders are temporarily responsible for integrating their own code strings
// while we work out full support.
@@ -62,12 +69,11 @@
}
}
-void GrGLSLGeometryProcessor::emitTransforms(GrGLSLVertexBuilder* vb,
- GrGLSLVaryingHandler* varyingHandler,
- GrGLSLUniformHandler* uniformHandler,
- const GrShaderVar& localCoordsVar,
- const SkMatrix& localMatrix,
- FPCoordTransformHandler* handler) {
+void GrGLSLGeometryProcessor::collectTransforms(GrGLSLVertexBuilder* vb,
+ GrGLSLVaryingHandler* varyingHandler,
+ GrGLSLUniformHandler* uniformHandler,
+ const GrShaderVar& localCoordsVar,
+ FPCoordTransformHandler* handler) {
// We only require localCoordsVar to be valid if there is a coord transform that needs
// it. CTs on FPs called with explicit coords do not require a local coord.
auto getLocalCoords = [&localCoordsVar,
@@ -120,8 +126,7 @@
if (!fp.isSampledWithExplicitCoords()) {
auto [localCoordsStr, localCoordLength] = getLocalCoords();
GrGLSLVarying v(kFloat2_GrSLType);
- if (localMatrix.hasPerspective() || coordTransform.matrix().hasPerspective() ||
- localCoordLength == 3) {
+ if (coordTransform.matrix().hasPerspective() || localCoordLength == 3) {
v = GrGLSLVarying(kFloat3_GrSLType);
}
SkString strVaryingName;
@@ -182,18 +187,12 @@
}
}
-void GrGLSLGeometryProcessor::setTransformDataHelper(const SkMatrix& localMatrix,
- const GrGLSLProgramDataManager& pdman,
+void GrGLSLGeometryProcessor::setTransformDataHelper(const GrGLSLProgramDataManager& pdman,
const CoordTransformRange& transformRange) {
int i = 0;
for (auto [transform, fp] : transformRange) {
if (fInstalledTransforms[i].fHandle.isValid()) {
- SkMatrix m;
- if (fp.isSampledWithExplicitCoords()) {
- m = GetTransformMatrix(transform, SkMatrix::I());
- } else {
- m = GetTransformMatrix(transform, localMatrix);
- }
+ SkMatrix m = GetTransformMatrix(transform, SkMatrix::I());
if (!SkMatrixPriv::CheapEqual(fInstalledTransforms[i].fCurrentValue, m)) {
if (fInstalledTransforms[i].fType == kFloat4_GrSLType) {
float values[4] = {m.getScaleX(), m.getTranslateX(),
@@ -213,11 +212,94 @@
SkASSERT(i == fInstalledTransforms.count());
}
+void GrGLSLGeometryProcessor::setTransform(const GrGLSLProgramDataManager& pdman,
+ const UniformHandle& uniform,
+ const SkMatrix& matrix,
+ SkMatrix* state) const {
+ if (!uniform.isValid() || (state && SkMatrixPriv::CheapEqual(*state, matrix))) {
+ // No update needed
+ return;
+ }
+ if (state) {
+ *state = matrix;
+ }
+ if (matrix.isScaleTranslate()) {
+ // ComputeMatrixKey and writeX() assume the uniform is a float4 (can't assert since nothing
+ // is exposed on a handle, but should be caught lower down).
+ float values[4] = {matrix.getScaleX(), matrix.getTranslateX(),
+ matrix.getScaleY(), matrix.getTranslateY()};
+ pdman.set4fv(uniform, 1, values);
+ } else {
+ pdman.setSkMatrix(uniform, matrix);
+ }
+}
+
+static void write_vertex_position(GrGLSLVertexBuilder* vertBuilder,
+ GrGLSLUniformHandler* uniformHandler,
+ const GrShaderVar& inPos,
+ const SkMatrix& matrix,
+ const char* matrixName,
+ GrShaderVar* outPos,
+ GrGLSLGeometryProcessor::UniformHandle* matrixUniform) {
+ SkASSERT(inPos.getType() == kFloat3_GrSLType || inPos.getType() == kFloat2_GrSLType);
+ SkString outName = vertBuilder->newTmpVarName(inPos.getName().c_str());
+
+ if (matrix.isIdentity()) {
+ // Direct assignment, we won't use a uniform for the matrix.
+ outPos->set(inPos.getType(), outName.c_str());
+ vertBuilder->codeAppendf("float%d %s = %s;", GrSLTypeVecLength(inPos.getType()),
+ outName.c_str(), inPos.getName().c_str());
+ } else {
+ SkASSERT(matrixUniform);
+
+ bool useCompactTransform = matrix.isScaleTranslate();
+ const char* mangledMatrixName;
+ *matrixUniform = uniformHandler->addUniform(nullptr,
+ kVertex_GrShaderFlag,
+ useCompactTransform ? kFloat4_GrSLType
+ : kFloat3x3_GrSLType,
+ matrixName,
+ &mangledMatrixName);
+
+ if (inPos.getType() == kFloat3_GrSLType) {
+ // A float3 stays a float3 whether or not the matrix adds perspective
+ if (useCompactTransform) {
+ vertBuilder->codeAppendf("float3 %s = %s.xz1 * %s + %s.yw0;\n",
+ outName.c_str(), mangledMatrixName,
+ inPos.getName().c_str(), mangledMatrixName);
+ } else {
+ vertBuilder->codeAppendf("float3 %s = %s * %s;\n", outName.c_str(),
+ mangledMatrixName, inPos.getName().c_str());
+ }
+ outPos->set(kFloat3_GrSLType, outName.c_str());
+ } else if (matrix.hasPerspective()) {
+ // A float2 is promoted to a float3 if we add perspective via the matrix
+ SkASSERT(!useCompactTransform);
+ vertBuilder->codeAppendf("float3 %s = (%s * %s.xy1);",
+ outName.c_str(), mangledMatrixName, inPos.getName().c_str());
+ outPos->set(kFloat3_GrSLType, outName.c_str());
+ } else {
+ if (useCompactTransform) {
+ vertBuilder->codeAppendf("float2 %s = %s.xz * %s + %s.yw;\n",
+ outName.c_str(), mangledMatrixName,
+ inPos.getName().c_str(), mangledMatrixName);
+ } else {
+ vertBuilder->codeAppendf("float2 %s = (%s * %s.xy1).xy;\n",
+ outName.c_str(), mangledMatrixName,
+ inPos.getName().c_str());
+ }
+ outPos->set(kFloat2_GrSLType, outName.c_str());
+ }
+ }
+}
+
void GrGLSLGeometryProcessor::writeOutputPosition(GrGLSLVertexBuilder* vertBuilder,
GrGPArgs* gpArgs,
const char* posName) {
- gpArgs->fPositionVar.set(kFloat2_GrSLType, "pos2");
- vertBuilder->codeAppendf("float2 %s = %s;", gpArgs->fPositionVar.c_str(), posName);
+ // writeOutputPosition assumes the incoming pos name points to a float2 variable
+ GrShaderVar inPos(posName, kFloat2_GrSLType);
+ write_vertex_position(vertBuilder, nullptr, inPos, SkMatrix::I(), "viewMatrix",
+ &gpArgs->fPositionVar, nullptr);
}
void GrGLSLGeometryProcessor::writeOutputPosition(GrGLSLVertexBuilder* vertBuilder,
@@ -226,24 +308,17 @@
const char* posName,
const SkMatrix& mat,
UniformHandle* viewMatrixUniform) {
- if (mat.isIdentity()) {
- gpArgs->fPositionVar.set(kFloat2_GrSLType, "pos2");
- vertBuilder->codeAppendf("float2 %s = %s;", gpArgs->fPositionVar.c_str(), posName);
- } else {
- const char* viewMatrixName;
- *viewMatrixUniform = uniformHandler->addUniform(nullptr,
- kVertex_GrShaderFlag,
- kFloat3x3_GrSLType,
- "uViewM",
- &viewMatrixName);
- if (!mat.hasPerspective()) {
- gpArgs->fPositionVar.set(kFloat2_GrSLType, "pos2");
- vertBuilder->codeAppendf("float2 %s = (%s * float3(%s, 1)).xy;",
- gpArgs->fPositionVar.c_str(), viewMatrixName, posName);
- } else {
- gpArgs->fPositionVar.set(kFloat3_GrSLType, "pos3");
- vertBuilder->codeAppendf("float3 %s = %s * float3(%s, 1);",
- gpArgs->fPositionVar.c_str(), viewMatrixName, posName);
- }
- }
+ GrShaderVar inPos(posName, kFloat2_GrSLType);
+ write_vertex_position(vertBuilder, uniformHandler, inPos, mat, "viewMatrix",
+ &gpArgs->fPositionVar, viewMatrixUniform);
+}
+
+void GrGLSLGeometryProcessor::writeLocalCoord(GrGLSLVertexBuilder* vertBuilder,
+ GrGLSLUniformHandler* uniformHandler,
+ GrGPArgs* gpArgs,
+ GrShaderVar localVar,
+ const SkMatrix& localMatrix,
+ UniformHandle* localMatrixUniform) {
+ write_vertex_position(vertBuilder, uniformHandler, localVar, localMatrix, "localMatrix",
+ &gpArgs->fLocalCoordVar, localMatrixUniform);
}
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.h b/src/gpu/glsl/GrGLSLGeometryProcessor.h
index f23bd0e..10e2137 100644
--- a/src/gpu/glsl/GrGLSLGeometryProcessor.h
+++ b/src/gpu/glsl/GrGLSLGeometryProcessor.h
@@ -22,41 +22,33 @@
/* Any general emit code goes in the base class emitCode. Subclasses override onEmitCode */
void emitCode(EmitArgs&) final;
-protected:
- // A helper which subclasses can use to upload coord transform matrices in setData().
- void setTransformDataHelper(const SkMatrix& localMatrix,
- const GrGLSLProgramDataManager& pdman,
- const CoordTransformRange&);
-
- // Emit transformed local coords from the vertex shader as a uniform matrix and varying per
- // coord-transform. localCoordsVar must be a 2- or 3-component vector. If it is 3 then it is
- // assumed to be a 2D homogeneous coordinate.
- void emitTransforms(GrGLSLVertexBuilder*,
- GrGLSLVaryingHandler*,
- GrGLSLUniformHandler*,
- const GrShaderVar& localCoordsVar,
- const SkMatrix& localMatrix,
- FPCoordTransformHandler*);
-
- // Version of above that assumes identity for the local matrix.
- void emitTransforms(GrGLSLVertexBuilder* vb,
- GrGLSLVaryingHandler* varyingHandler,
- GrGLSLUniformHandler* uniformHandler,
- const GrShaderVar& localCoordsVar,
- FPCoordTransformHandler* handler) {
- this->emitTransforms(vb, varyingHandler, uniformHandler, localCoordsVar, SkMatrix::I(),
- handler);
- }
-
- // TODO: doc
+ // Generate the final code for assigning transformed coordinates to the varyings recorded in
+ // the call to collectTransforms(). This must happen after FP code emission so that it has
+ // access to any uniforms the FPs registered for const/uniform sample matrix invocations.
void emitTransformCode(GrGLSLVertexBuilder* vb,
GrGLSLUniformHandler* uniformHandler) override;
+protected:
+ // A helper which subclasses can use to upload coord transform matrices in setData().
+ void setTransformDataHelper(const GrGLSLProgramDataManager& pdman,
+ const CoordTransformRange&);
+
+ // A helper for setting the matrix on a uniform handle initialized through
+ // writeOutputPosition or writeLocalCoord. Automatically handles elided uniforms,
+ // scale+translate matrices, and state tracking (if provided state pointer is non-null).
+ void setTransform(const GrGLSLProgramDataManager& pdman, const UniformHandle& uniform,
+ const SkMatrix& matrix, SkMatrix* state=nullptr) const;
+
struct GrGPArgs {
// Used to specify the output variable used by the GP to store its device position. It can
// either be a float2 or a float3 (in order to handle perspective). The subclass sets this
// in its onEmitCode().
GrShaderVar fPositionVar;
+ // Used to specify the variable storing the draw's local coordinates. It can be either a
+ // float2, float3, or void. It can only be void when no FP needs local coordinates. This
+ // variable can be an attribute or local variable, but should not itself be a varying.
+ // GrGLSLGeometryProcessor automatically determines if this must be passed to a FS.
+ GrShaderVar fLocalCoordVar;
};
// Helpers for adding code to write the transformed vertex position. The first simple version
@@ -73,19 +65,54 @@
const SkMatrix& mat,
UniformHandle* viewMatrixUniform);
- static uint32_t ComputePosKey(const SkMatrix& mat) {
+ // Helper to transform an existing variable by a given local matrix (e.g. the inverse view
+ // matrix). It will declare the transformed local coord variable and will set
+ // GrGPArgs::fLocalCoordVar.
+ void writeLocalCoord(GrGLSLVertexBuilder*, GrGLSLUniformHandler*, GrGPArgs*,
+ GrShaderVar localVar, const SkMatrix& localMatrix,
+ UniformHandle* localMatrixUniform);
+
+ // GPs that use writeOutputPosition and/or writeLocalCoord must incorporate the matrix type
+ // into their key, and should use this function or one of the other related helpers.
+ static uint32_t ComputeMatrixKey(const SkMatrix& mat) {
if (mat.isIdentity()) {
- return 0x0;
+ return 0b00;
+ } else if (mat.isScaleTranslate()) {
+ return 0b01;
} else if (!mat.hasPerspective()) {
- return 0x01;
+ return 0b10;
} else {
- return 0x02;
+ return 0b11;
}
}
+ static uint32_t ComputeMatrixKeys(const SkMatrix& viewMatrix, const SkMatrix& localMatrix) {
+ return (ComputeMatrixKey(viewMatrix) << kMatrixKeyBits) | ComputeMatrixKey(localMatrix);
+ }
+ static uint32_t AddMatrixKeys(uint32_t flags, const SkMatrix& viewMatrix,
+ const SkMatrix& localMatrix) {
+ // Shifting to make room for the matrix keys shouldn't lose bits
+ SkASSERT(((flags << (2 * kMatrixKeyBits)) >> (2 * kMatrixKeyBits)) == flags);
+ return (flags << (2 * kMatrixKeyBits)) | ComputeMatrixKeys(viewMatrix, localMatrix);
+ }
+ static constexpr int kMatrixKeyBits = 2;
private:
virtual void onEmitCode(EmitArgs&, GrGPArgs*) = 0;
+ // Iterates over the FPs in 'handler' to register additional varyings and uniforms to support
+ // VS-promoted local coord evaluation for the FPs. Subclasses must call this with
+ // 'localCoordsVar' set to an SkSL variable expression of type 'float2' or 'float3' representing
+ // the original local coordinates of the draw.
+ //
+ // This must happen before FP code emission so that the FPs can find the appropriate varying
+ // handles they use in place of explicit coord sampling; it is automatically called after
+ // onEmitCode() returns using the value stored in GpArgs::fLocalCoordVar.
+ void collectTransforms(GrGLSLVertexBuilder* vb,
+ GrGLSLVaryingHandler* varyingHandler,
+ GrGLSLUniformHandler* uniformHandler,
+ const GrShaderVar& localCoordsVar,
+ FPCoordTransformHandler* handler);
+
struct TransformUniform {
UniformHandle fHandle;
GrSLType fType = kVoid_GrSLType;
diff --git a/src/gpu/glsl/GrGLSLShaderBuilder.cpp b/src/gpu/glsl/GrGLSLShaderBuilder.cpp
index 5b27431..e5a9c14 100644
--- a/src/gpu/glsl/GrGLSLShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLShaderBuilder.cpp
@@ -20,7 +20,8 @@
, fOutputs(GrGLSLProgramBuilder::kVarsPerBlock)
, fFeaturesAddedMask(0)
, fCodeIndex(kCode)
- , fFinalized(false) {
+ , fFinalized(false)
+ , fTmpVariableCounter(0) {
// We push back some dummy pointers which will later become our header
for (int i = 0; i <= kCode; i++) {
fShaderStrings.push_back();
diff --git a/src/gpu/glsl/GrGLSLShaderBuilder.h b/src/gpu/glsl/GrGLSLShaderBuilder.h
index b69abe8..b766280 100644
--- a/src/gpu/glsl/GrGLSLShaderBuilder.h
+++ b/src/gpu/glsl/GrGLSLShaderBuilder.h
@@ -84,6 +84,13 @@
void declareGlobal(const GrShaderVar&);
+ // Generates a unique variable name for holding the result of a temporary expression when it's
+ // not reasonable to just add a new block for scoping. Does not declare anything.
+ SkString newTmpVarName(const char* suffix) {
+ int tmpIdx = fTmpVariableCounter++;
+ return SkStringPrintf("_tmp_%d_%s", tmpIdx, suffix);
+ }
+
/**
* Called by GrGLSLProcessors to add code to one of the shaders.
*/
@@ -238,6 +245,9 @@
int fCodeIndex;
bool fFinalized;
+ // Counter for generating unique scratch variable names in a shader.
+ int fTmpVariableCounter;
+
friend class GrCCCoverageProcessor; // to access code().
friend class GrGLSLProgramBuilder;
friend class GrGLProgramBuilder;