Fix issue where GrQuadPerEdgeAA GP doesn't emit coord transforms
This happens when all the FPs are sampled with explicit coords.
The GrGLSLGeometryProcessor::emitTransforms() helper required a valid VS
var for local coords even when all FPs use explicit coords and wouldn't
use a local coords var. It now only requires the variable be valid if
there is an FP with a coord transform that is not explicitly sampled.
Change-Id: I8008e854c738a17b207c82bb791e230ef4d5364d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272459
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
index 9fae493..0eba2cd 100644
--- a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
@@ -62,53 +62,68 @@
const GrShaderVar& localCoordsVar,
const SkMatrix& localMatrix,
FPCoordTransformHandler* handler) {
- SkASSERT(GrSLTypeIsFloatType(localCoordsVar.getType()));
- SkASSERT(2 == GrSLTypeVecLength(localCoordsVar.getType()) ||
- 3 == GrSLTypeVecLength(localCoordsVar.getType()));
+ // 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,
+ localCoords = SkString(),
+ localCoordLength = int()]() mutable {
+ if (localCoords.isEmpty()) {
+ localCoordLength = GrSLTypeVecLength(localCoordsVar.getType());
+ SkASSERT(GrSLTypeIsFloatType(localCoordsVar.getType()));
+ SkASSERT(localCoordLength == 2 || localCoordLength == 3);
+ if (localCoordLength == 3) {
+ localCoords = localCoordsVar.getName();
+ } else {
+ localCoords.printf("float3(%s, 1)", localCoordsVar.c_str());
+ }
+ }
+ return std::make_tuple(localCoords, localCoordLength);
+ };
- bool threeComponentLocalCoords = 3 == GrSLTypeVecLength(localCoordsVar.getType());
- SkString localCoords;
- if (threeComponentLocalCoords) {
- localCoords = localCoordsVar.getName();
- } else {
- localCoords.printf("float3(%s, 1)", localCoordsVar.c_str());
- }
for (int i = 0; *handler; ++*handler, ++i) {
auto [coordTransform, fp] = handler->get();
- if (coordTransform.isNoOp() && fp.isSampledWithExplicitCoords()) {
- handler->omitCoordsForCurrCoordTransform();
- fInstalledTransforms.push_back();
- } else {
+ // Add uniform for coord transform matrix.
+ const char* matrixName;
+ if (!fp.isSampledWithExplicitCoords() || !coordTransform.isNoOp()) {
SkString strUniName;
strUniName.printf("CoordTransformMatrix_%d", i);
- const char* uniName;
fInstalledTransforms.push_back().fHandle = uniformHandler
->addUniform(kVertex_GrShaderFlag,
kFloat3x3_GrSLType,
strUniName.c_str(),
- &uniName)
+ &matrixName)
.toIndex();
- GrSLType varyingType = kFloat2_GrSLType;
+ } else {
+ // Install a coord transform that will be skipped.
+ fInstalledTransforms.push_back();
+ handler->omitCoordsForCurrCoordTransform();
+ continue;
+ }
+
+ // Add varying if required and register varying and matrix uniform.
+ if (!fp.isSampledWithExplicitCoords()) {
+ auto [localCoordsStr, localCoordLength] = getLocalCoords();
+ GrGLSLVarying v(kFloat2_GrSLType);
if (localMatrix.hasPerspective() || coordTransform.matrix().hasPerspective() ||
- threeComponentLocalCoords) {
- varyingType = kFloat3_GrSLType;
+ localCoordLength == 3) {
+ v = GrGLSLVarying(kFloat3_GrSLType);
}
SkString strVaryingName;
strVaryingName.printf("TransformedCoords_%d", i);
- GrGLSLVarying v(varyingType);
- if (!fp.isSampledWithExplicitCoords()) {
- varyingHandler->addVarying(strVaryingName.c_str(), &v);
+ varyingHandler->addVarying(strVaryingName.c_str(), &v);
- if (kFloat2_GrSLType == varyingType) {
- vb->codeAppendf("%s = (%s * %s).xy;", v.vsOut(), uniName, localCoords.c_str());
- } else {
- vb->codeAppendf("%s = %s * %s;", v.vsOut(), uniName, localCoords.c_str());
- }
+ if (v.type() == kFloat2_GrSLType) {
+ vb->codeAppendf("%s = (%s * %s).xy;", v.vsOut(), matrixName,
+ localCoordsStr.c_str());
+ } else {
+ vb->codeAppendf("%s = %s * %s;", v.vsOut(), matrixName, localCoordsStr.c_str());
}
- handler->specifyCoordsForCurrCoordTransform(
- SkString(uniName),
- fInstalledTransforms.back().fHandle,
- GrShaderVar(SkString(v.fsIn()), varyingType));
+ GrShaderVar fsVar(SkString(v.fsIn()), v.type(), GrShaderVar::kIn_TypeModifier);
+ handler->specifyCoordsForCurrCoordTransform(SkString(matrixName),
+ fInstalledTransforms.back().fHandle, fsVar);
+ } else {
+ handler->specifyCoordsForCurrCoordTransform(SkString(matrixName),
+ fInstalledTransforms.back().fHandle);
}
}
}
diff --git a/src/gpu/glsl/GrGLSLPrimitiveProcessor.h b/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
index f4ca1f3..b4d3935 100644
--- a/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
+++ b/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
@@ -30,10 +30,12 @@
struct TransformVar {
TransformVar() = default;
- TransformVar(SkString matrixCode, UniformHandle uniformMatrix, GrShaderVar varyingPoint)
- : fMatrixCode(std::move(matrixCode))
- , fUniformMatrix(uniformMatrix)
- , fVaryingPoint(varyingPoint) {}
+ TransformVar(SkString matrixCode,
+ UniformHandle uniformMatrix,
+ GrShaderVar varyingPoint = {})
+ : fMatrixCode(std::move(matrixCode))
+ , fUniformMatrix(uniformMatrix)
+ , fVaryingPoint(varyingPoint) {}
// a string of SkSL code which resolves to the transformation matrix
SkString fMatrixCode;
diff --git a/src/gpu/ops/GrQuadPerEdgeAA.cpp b/src/gpu/ops/GrQuadPerEdgeAA.cpp
index 66b84a9..ee2a694 100644
--- a/src/gpu/ops/GrQuadPerEdgeAA.cpp
+++ b/src/gpu/ops/GrQuadPerEdgeAA.cpp
@@ -575,9 +575,7 @@
void setData(const GrGLSLProgramDataManager& pdman, const GrPrimitiveProcessor& proc,
const CoordTransformRange& transformRange) override {
const auto& gp = proc.cast<QuadPerEdgeAAGeometryProcessor>();
- if (gp.fLocalCoord.isInitialized()) {
- this->setTransformDataHelper(SkMatrix::I(), pdman, transformRange);
- }
+ this->setTransformDataHelper(SkMatrix::I(), pdman, transformRange);
fTextureColorSpaceXformHelper.setData(pdman, gp.fTextureColorSpaceXform.get());
}
@@ -610,17 +608,18 @@
gpArgs->fPositionVar = gp.fPosition.asShaderVar();
}
- // Handle local coordinates if they exist
- if (gp.fLocalCoord.isInitialized()) {
- // NOTE: If the only usage of local coordinates is for the inline texture fetch
- // before FPs, then there are no registered FPCoordTransforms and this ends up
- // emitting nothing, so there isn't a duplication of local coordinates
- this->emitTransforms(args.fVertBuilder,
- args.fVaryingHandler,
- args.fUniformHandler,
- gp.fLocalCoord.asShaderVar(),
- args.fFPCoordTransformHandler);
- }
+ // Handle local coordinates if they exist. This is required even when the op
+ // isn't providing local coords but there are FPs called with explicit coords.
+ // It installs the uniforms that transform their coordinates in the fragment
+ // shader.
+ // NOTE: If the only usage of local coordinates is for the inline texture fetch
+ // before FPs, then there are no registered FPCoordTransforms and this ends up
+ // emitting nothing, so there isn't a duplication of local coordinates
+ this->emitTransforms(args.fVertBuilder,
+ args.fVaryingHandler,
+ args.fUniformHandler,
+ gp.fLocalCoord.asShaderVar(),
+ args.fFPCoordTransformHandler);
// Solid color before any texturing gets modulated in
if (gp.fColor.isInitialized()) {
