Remove explicit sample flag from GrFP.
Instead expand the map that is computed by GrGLSLGP to include a bool
that indicates whether each FP requires coords or not.
Now GrGLSLGP is solely responsible for determining which FPs take coords
and inserting any varyings. The rest of the system follows its lead when
generating FP functions and call sites.
Bug: skia:12198
Change-Id: I3471867fb64e94c7775c0b6209998159abeb6714
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435018
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/include/private/SkSLSampleUsage.h b/include/private/SkSLSampleUsage.h
index 776be5d..c3c3774 100644
--- a/include/private/SkSLSampleUsage.h
+++ b/include/private/SkSLSampleUsage.h
@@ -17,7 +17,8 @@
/**
* Represents all of the ways that a fragment processor is sampled by its parent.
*/
-struct SampleUsage {
+class SampleUsage {
+public:
enum class Kind {
// Child is never sampled
kNone,
@@ -32,14 +33,17 @@
// Make a SampleUsage that corresponds to no sampling of the child at all
SampleUsage() = default;
+ SampleUsage(Kind kind, bool hasPerspective) : fKind(kind), fHasPerspective(hasPerspective) {
+ if (kind != Kind::kUniformMatrix) {
+ SkASSERT(!fHasPerspective);
+ }
+ }
+
// Child is sampled with a matrix whose value is uniform. The name is fixed.
static SampleUsage UniformMatrix(bool hasPerspective) {
return SampleUsage(Kind::kUniformMatrix, hasPerspective);
}
- // Arbitrary name used by all uniform sampling matrices
- static const char* MatrixUniformName() { return "matrix"; }
-
static SampleUsage Explicit() {
return SampleUsage(Kind::kExplicit, false);
}
@@ -48,23 +52,31 @@
return SampleUsage(Kind::kPassThrough, false);
}
+ bool operator==(const SampleUsage& that) const {
+ return fKind == that.fKind && fHasPerspective == that.fHasPerspective;
+ }
+
+ bool operator!=(const SampleUsage& that) const { return !(*this == that); }
+
+ // Arbitrary name used by all uniform sampling matrices
+ static const char* MatrixUniformName() { return "matrix"; }
+
SampleUsage merge(const SampleUsage& other);
+ Kind kind() const { return fKind; }
+
+ bool hasPerspective() const { return fHasPerspective; }
+
bool isSampled() const { return fKind != Kind::kNone; }
bool isPassThrough() const { return fKind == Kind::kPassThrough; }
bool isExplicit() const { return fKind == Kind::kExplicit; }
bool isUniformMatrix() const { return fKind == Kind::kUniformMatrix; }
+ std::string constructor() const;
+
+private:
Kind fKind = Kind::kNone;
bool fHasPerspective = false; // Only valid if fKind is kUniformMatrix
-
- SampleUsage(Kind kind, bool hasPerspective) : fKind(kind), fHasPerspective(hasPerspective) {
- if (kind != Kind::kUniformMatrix) {
- SkASSERT(!fHasPerspective);
- }
- }
-
- std::string constructor() const;
};
} // namespace SkSL
diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp
index a95e7d4..5a042f4 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -21,7 +21,7 @@
if (this->classID() != that.classID()) {
return false;
}
- if (this->usesVaryingCoordsDirectly() != that.usesVaryingCoordsDirectly()) {
+ if (this->sampleUsage() != that.sampleUsage()) {
return false;
}
if (!this->onIsEqual(that)) {
@@ -153,31 +153,21 @@
// The child should not have been attached to another FP already and not had any sampling
// strategy set on it.
- SkASSERT(!child->fParent && !child->sampleUsage().isSampled() &&
- !child->isSampledWithExplicitCoords());
+ SkASSERT(!child->fParent && !child->sampleUsage().isSampled());
// Configure child's sampling state first
child->fUsage = sampleUsage;
- if (sampleUsage.isExplicit()) {
- child->addAndPushFlagToChildren(kSampledWithExplicitCoords_Flag);
- }
-
// Propagate the "will read dest-color" flag up to parent FPs.
if (child->willReadDstColor()) {
this->setWillReadDstColor();
}
- // If the child is not sampled explicitly and not already accessing sample coords directly
- // (through reference or variable matrix expansion), then mark that this FP tree relies on
- // coordinates at a lower level. If the child is sampled with explicit coordinates and
- // there isn't any other direct reference to the sample coords, we halt the upwards propagation
- // because it means this FP is determining coordinates on its own.
- if (!child->isSampledWithExplicitCoords()) {
- if ((child->fFlags & kUsesSampleCoordsDirectly_Flag ||
- child->fFlags & kUsesSampleCoordsIndirectly_Flag)) {
- fFlags |= kUsesSampleCoordsIndirectly_Flag;
- }
+ // If this child receives passthrough or matrix transformed coords from its parent then note
+ // that the parent's coords are used indirectly to ensure that they aren't omitted.
+ if ((sampleUsage.isPassThrough() || sampleUsage.isUniformMatrix()) &&
+ child->usesSampleCoords()) {
+ fFlags |= kUsesSampleCoordsIndirectly_Flag;
}
fRequestedFeatures |= child->fRequestedFeatures;
@@ -188,7 +178,7 @@
fChildProcessors.push_back(std::move(child));
// Validate: our sample strategy comes from a parent we shouldn't have yet.
- SkASSERT(!this->isSampledWithExplicitCoords() && !fUsage.isSampled() && !fParent);
+ SkASSERT(!fUsage.isSampled() && !fParent);
}
void GrFragmentProcessor::cloneAndRegisterAllChildProcessors(const GrFragmentProcessor& src) {
diff --git a/src/gpu/GrFragmentProcessor.h b/src/gpu/GrFragmentProcessor.h
index a1f5b13..c03dc00 100644
--- a/src/gpu/GrFragmentProcessor.h
+++ b/src/gpu/GrFragmentProcessor.h
@@ -216,32 +216,6 @@
SkDEBUGCODE(bool isInstantiated() const;)
- /**
- * Does this FP require local coordinates to be produced by the primitive processor? This only
- * returns true if this FP will directly read those local coordinates. FPs that are sampled
- * explicitly do not require primitive-generated local coordinates (because the sample
- * coordinates are supplied by the parent FP).
- *
- * If the root of an FP tree does not provide explicit coordinates, the geometry processor
- * provides the original local coordinates to start. This may be implicit as part of vertex
- * shader-lifted varyings, or by providing the base local coordinate to the fragment shader.
- */
- bool usesVaryingCoordsDirectly() const {
- return SkToBool(fFlags & kUsesSampleCoordsDirectly_Flag) &&
- !SkToBool(fFlags & kSampledWithExplicitCoords_Flag);
- }
-
- /**
- * Do any of the FPs in this tree require local coordinates to be produced by the primitive
- * processor? This can return true even if this FP does not refer to sample coordinates, but
- * true if a descendant FP uses them.
- */
- bool usesVaryingCoords() const {
- return (SkToBool(fFlags & kUsesSampleCoordsDirectly_Flag) ||
- SkToBool(fFlags & kUsesSampleCoordsIndirectly_Flag)) &&
- !SkToBool(fFlags & kSampledWithExplicitCoords_Flag);
- }
-
/** Do any of the FPs in this tree read back the color from the destination surface? */
bool willReadDstColor() const {
return SkToBool(fFlags & kWillReadDstColor_Flag);
@@ -252,26 +226,27 @@
return SkToBool(fFlags & kIsBlendFunction_Flag);
}
- /**
+ /**
* True if this FP refers directly to the sample coordinate parameter of its function
- * (e.g. uses EmitArgs::fSampleCoord in emitCode()). This also returns true if the
- * coordinate reference comes from autogenerated code invoking 'sample(matrix)' expressions.
- *
- * Unlike usesVaryingCoords(), this can return true whether or not the FP is explicitly
- * sampled, and does not change based on how the FP is composed. This property is specific to
- * the FP's function and not the entire program.
+ * (e.g. uses EmitArgs::fSampleCoord in emitCode()). This is decided at FP-tree construction
+ * time and is not affected by lifting coords to varyings.
*/
- bool referencesSampleCoords() const {
+ bool usesSampleCoordsDirectly() const {
return SkToBool(fFlags & kUsesSampleCoordsDirectly_Flag);
}
- // True if this FP's parent invokes it with 'sample(float2)'
- bool isSampledWithExplicitCoords() const {
- return SkToBool(fFlags & kSampledWithExplicitCoords_Flag);
+ /**
+ * True if this FP uses its input coordinates or if any descendant FP uses them through a chain
+ * of non-explicit sample usages. (e.g. uses EmitArgs::fSampleCoord in emitCode()). This is
+ * decided at FP-tree construction time and is not affected by lifting coords to varyings.
+ */
+ bool usesSampleCoords() const {
+ return SkToBool(fFlags & (kUsesSampleCoordsDirectly_Flag |
+ kUsesSampleCoordsIndirectly_Flag));
}
- // The SampleUsage describing how this FP is invoked by its parent using 'sample(matrix)'
- // This only reflects the immediate sampling from parent to this FP
+ // The SampleUsage describing how this FP is invoked by its parent. This only reflects the
+ // immediate sampling from parent to this FP.
const SkSL::SampleUsage& sampleUsage() const {
return fUsage;
}
@@ -496,26 +471,27 @@
/**
* Subclass implements this to support isEqual(). It will only be called if it is known that
- * the two processors are of the same subclass (i.e. they return the same object from
- * getFactory()).
+ * the two processors are of the same subclass (i.e. have the same ClassID).
*/
virtual bool onIsEqual(const GrFragmentProcessor&) const = 0;
enum PrivateFlags {
kFirstPrivateFlag = kAll_OptimizationFlags + 1,
- // Propagates up the FP tree to the root
+ // Propagates up the FP tree to either root or first explicit sample usage.
kUsesSampleCoordsIndirectly_Flag = kFirstPrivateFlag,
- // Does not propagate at all
+ // Does not propagate at all. It means this FP uses its input sample coords in some way.
+ // Note passthrough and matrix sampling of children don't count as a usage of the coords.
+ // Because indirect sampling stops at an explicit sample usage it is imperative that a FP
+ // that calculates explicit coords for its children using its own sample coords sets this.
kUsesSampleCoordsDirectly_Flag = kFirstPrivateFlag << 1,
+
+ // Does not propagate at all.
kIsBlendFunction_Flag = kFirstPrivateFlag << 2,
- // Propagates down the FP to all its leaves
- kSampledWithExplicitCoords_Flag = kFirstPrivateFlag << 3,
-
- // Propagates up the FP tree to the root
- kWillReadDstColor_Flag = kFirstPrivateFlag << 5,
+ // Propagates up the FP tree to the root.
+ kWillReadDstColor_Flag = kFirstPrivateFlag << 3,
};
void addAndPushFlagToChildren(PrivateFlags flag);
diff --git a/src/gpu/GrGeometryProcessor.cpp b/src/gpu/GrGeometryProcessor.cpp
index 65ed14b..04d2cb7 100644
--- a/src/gpu/GrGeometryProcessor.cpp
+++ b/src/gpu/GrGeometryProcessor.cpp
@@ -9,18 +9,6 @@
#include "src/gpu/GrFragmentProcessor.h"
-/**
- * We specialize the vertex or fragment coord transform code for these matrix types, and where
- * the transform code is applied.
- */
-enum SampleFlag {
- kExplicitlySampled_Flag = 0b0001, // GrFP::isSampledWithExplicitCoords()
- kUniform_SampleMatrix_Flag = 0b0010, // GrFP::sampleUsage()::isUniformMatrix()
-
- // Currently, sample(matrix) only specializes on no-perspective or general.
- // FIXME add new flags as more matrix types are supported.
- kPersp_Matrix_Flag = 0b0100, // GrFP::sampleUsage()::fHasPerspective
-};
GrGeometryProcessor::GrGeometryProcessor(ClassID classID) : GrProcessor(classID) {}
@@ -31,18 +19,11 @@
uint32_t GrGeometryProcessor::ComputeCoordTransformsKey(const GrFragmentProcessor& fp) {
// This is highly coupled with the code in GrGLSLGeometryProcessor::collectTransforms().
-
- uint32_t key = 0;
- if (fp.isSampledWithExplicitCoords()) {
- key |= kExplicitlySampled_Flag;
+ uint32_t key = static_cast<uint32_t>(fp.sampleUsage().kind()) << 1;
+ // This needs to be updated if GP starts specializing varyings on additional matrix types.
+ if (fp.sampleUsage().hasPerspective()) {
+ key |= 0b1;
}
- if (fp.sampleUsage().isUniformMatrix()) {
- key |= kUniform_SampleMatrix_Flag;
- }
- if (fp.sampleUsage().fHasPerspective) {
- key |= kPersp_Matrix_Flag;
- }
-
return key;
}
diff --git a/src/gpu/GrPaint.h b/src/gpu/GrPaint.h
index 15788c3..9c247c9 100644
--- a/src/gpu/GrPaint.h
+++ b/src/gpu/GrPaint.h
@@ -94,9 +94,10 @@
GrFragmentProcessor* getCoverageFragmentProcessor() const {
return fCoverageFragmentProcessor.get();
}
- bool usesVaryingCoords() const {
- return (fColorFragmentProcessor && fColorFragmentProcessor->usesVaryingCoords()) ||
- (fCoverageFragmentProcessor && fCoverageFragmentProcessor->usesVaryingCoords());
+ bool usesLocalCoords() const {
+ // The sample coords for the top level FPs are implicitly the GP's local coords.
+ return (fColorFragmentProcessor && fColorFragmentProcessor->usesSampleCoords()) ||
+ (fCoverageFragmentProcessor && fCoverageFragmentProcessor->usesSampleCoords());
}
/**
diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h
index 7e5a986..7d99cfa 100644
--- a/src/gpu/GrPipeline.h
+++ b/src/gpu/GrPipeline.h
@@ -105,9 +105,10 @@
bool isColorFragmentProcessor(int idx) const { return idx < fNumColorProcessors; }
bool isCoverageFragmentProcessor(int idx) const { return idx >= fNumColorProcessors; }
- bool usesVaryingCoords() const {
+ bool usesLocalCoords() const {
+ // The sample coords for the top level FPs are implicitly the GP's local coords.
for (const auto& fp : fFragmentProcessors) {
- if (fp->usesVaryingCoords()) {
+ if (fp->usesSampleCoords()) {
return true;
}
}
diff --git a/src/gpu/GrProcessorAnalysis.cpp b/src/gpu/GrProcessorAnalysis.cpp
index 72285e6..71f7823 100644
--- a/src/gpu/GrProcessorAnalysis.cpp
+++ b/src/gpu/GrProcessorAnalysis.cpp
@@ -40,7 +40,7 @@
if (fCompatibleWithCoverageAsAlpha && !fp->compatibleWithCoverageAsAlpha()) {
fCompatibleWithCoverageAsAlpha = false;
}
- if (fp->usesVaryingCoords()) {
+ if (fp->usesSampleCoords()) {
fUsesLocalCoords = true;
}
if (fp->willReadDstColor()) {
diff --git a/src/gpu/GrProcessorSet.cpp b/src/gpu/GrProcessorSet.cpp
index 43b3a84..e2726b2 100644
--- a/src/gpu/GrProcessorSet.cpp
+++ b/src/gpu/GrProcessorSet.cpp
@@ -129,13 +129,13 @@
if (!fCoverageFragmentProcessor->compatibleWithCoverageAsAlpha()) {
analysis.fCompatibleWithCoverageAsAlpha = false;
}
- coverageUsesLocalCoords |= fCoverageFragmentProcessor->usesVaryingCoords();
+ coverageUsesLocalCoords |= fCoverageFragmentProcessor->usesSampleCoords();
}
if (clip && clip->hasCoverageFragmentProcessor()) {
hasCoverageFP = true;
const GrFragmentProcessor* clipFP = clip->coverageFragmentProcessor();
analysis.fCompatibleWithCoverageAsAlpha &= clipFP->compatibleWithCoverageAsAlpha();
- coverageUsesLocalCoords |= clipFP->usesVaryingCoords();
+ coverageUsesLocalCoords |= clipFP->usesSampleCoords();
}
int colorFPsToEliminate = colorAnalysis.initialProcessorsToEliminate(overrideInputColor);
analysis.fInputColorType = static_cast<Analysis::PackedInputColorType>(
diff --git a/src/gpu/effects/GrSkSLFP.cpp b/src/gpu/effects/GrSkSLFP.cpp
index d631bb8..9583a0e 100644
--- a/src/gpu/effects/GrSkSLFP.cpp
+++ b/src/gpu/effects/GrSkSLFP.cpp
@@ -204,7 +204,7 @@
// parameter. fSampleCoord could be a varying, so writes to it would be illegal.
const char* coords = "float2(0)";
SkString coordsVarName;
- if (fp.referencesSampleCoords()) {
+ if (fp.usesSampleCoordsDirectly()) {
coordsVarName = args.fFragBuilder->newTmpVarName("coords");
coords = coordsVarName.c_str();
args.fFragBuilder->codeAppendf("float2 %s = %s;\n", coords, args.fSampleCoord);
diff --git a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
index 1de6760..a4639c0 100644
--- a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
@@ -5,11 +5,13 @@
* found in the LICENSE file.
*/
+#include "src/gpu/glsl/GrGLSLFragmentProcessor.h"
+
#include "src/gpu/GrFragmentProcessor.h"
#include "src/gpu/GrProcessor.h"
#include "src/gpu/GrShaderCaps.h"
-#include "src/gpu/glsl/GrGLSLFragmentProcessor.h"
#include "src/gpu/glsl/GrGLSLFragmentShaderBuilder.h"
+#include "src/gpu/glsl/GrGLSLProgramBuilder.h"
#include "src/gpu/glsl/GrGLSLUniformHandler.h"
void GrGLSLFragmentProcessor::setData(const GrGLSLProgramDataManager& pdman,
@@ -63,14 +65,13 @@
invocation.appendf(", %s", destColor);
}
- if (childProc->isSampledWithExplicitCoords()) {
+ // Assert that the child has no sample matrix. A uniform matrix sample call would go through
+ // invokeChildWithMatrix, not here.
+ SkASSERT(!childProc->sampleUsage().isUniformMatrix());
+
+ if (args.fFragBuilder->getProgramBuilder()->fragmentProcessorHasCoordsParam(childProc)) {
// The child's function takes a half4 color and a float2 coordinate
invocation.appendf(", %s", skslCoords.empty() ? args.fSampleCoord : skslCoords.c_str());
- } else {
- // Assert that the child has no sample matrix and skslCoords matches the default. (A uniform
- // matrix sample call would go through invokeChildWithMatrix, not here.)
- SkASSERT(skslCoords.empty() || skslCoords == args.fSampleCoord);
- SkASSERT(childProc->sampleUsage().isPassThrough());
}
invocation.append(")");
@@ -117,10 +118,10 @@
//
// In all other cases, we need to insert sksl to compute matrix * parent coords and then invoke
// the function.
- if (childProc->isSampledWithExplicitCoords()) {
+ if (args.fFragBuilder->getProgramBuilder()->fragmentProcessorHasCoordsParam(childProc)) {
// Only check perspective for this specific matrix transform, not the aggregate FP property.
// Any parent perspective will have already been applied when evaluated in the FS.
- if (childProc->sampleUsage().fHasPerspective) {
+ if (childProc->sampleUsage().hasPerspective()) {
invocation.appendf(", proj((%s) * %s.xy1)", matrixName.c_str(), args.fSampleCoord);
} else if (args.fShaderCaps->nonsquareMatrixSupport()) {
invocation.appendf(", float3x2(%s) * %s.xy1", matrixName.c_str(), args.fSampleCoord);
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index 514d2ec..1e44460 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -37,17 +37,18 @@
params[numParams++] = GrShaderVar(args.fDestColor, kHalf4_GrSLType);
}
- if (args.fFp.isSampledWithExplicitCoords()) {
- // Functions sampled with explicit coordinates take a float2 coordinate as input.
+ if (fProgramBuilder->fragmentProcessorHasCoordsParam(&args.fFp)) {
params[numParams++] = GrShaderVar(args.fSampleCoord, kFloat2_GrSLType);
-
- } else if (args.fFp.referencesSampleCoords()) {
- // Sampled through a chain of passthrough/matrix samples usages. The actual transformation
- // code is emitted in the vertex shader, so this only has to access it. Add a float2 _coords
- // variable that maps to the associated varying and replaces the absent 2nd argument to the
- // fp's function.
+ } else {
+ // Either doesn't use coords at all or sampled through a chain of passthrough/matrix
+ // samples usages. In the latter case the coords are emitted in the vertex shader as a
+ // varying, so this only has to access it. Add a float2 _coords variable that maps to the
+ // associated varying and replaces the absent 2nd argument to the fp's function.
GrShaderVar varying = fProgramBuilder->varyingCoordsForFragmentProcessor(&args.fFp);
switch(varying.getType()) {
+ case kVoid_GrSLType:
+ SkASSERT(!args.fFp.usesSampleCoordsDirectly());
+ break;
case kFloat2_GrSLType:
// Just point the local coords to the varying
args.fSampleCoord = varying.getName().c_str();
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
index ff85206..280c721 100644
--- a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp
@@ -17,20 +17,16 @@
#include <queue>
-GrGLSLGeometryProcessor::FPToVaryingCoordsMap GrGLSLGeometryProcessor::emitCode(
- EmitArgs& args,
- const GrPipeline& pipeline) {
+GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::emitCode(EmitArgs& args,
+ const GrPipeline& pipeline) {
GrGPArgs gpArgs;
this->onEmitCode(args, &gpArgs);
- FPToVaryingCoordsMap transformMap;
- if (gpArgs.fLocalCoordVar.getType() != kVoid_GrSLType) {
- transformMap = this->collectTransforms(args.fVertBuilder,
- args.fVaryingHandler,
- args.fUniformHandler,
- gpArgs.fLocalCoordVar,
- pipeline);
- }
+ FPCoordsMap transformMap = this->collectTransforms(args.fVertBuilder,
+ args.fVaryingHandler,
+ args.fUniformHandler,
+ gpArgs.fLocalCoordVar,
+ pipeline);
if (args.fGeomProc.willUseTessellationShaders()) {
// Tessellation shaders are temporarily responsible for integrating their own code strings
@@ -75,16 +71,17 @@
return transformMap;
}
-GrGLSLGeometryProcessor::FPToVaryingCoordsMap GrGLSLGeometryProcessor::collectTransforms(
+GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms(
GrGLSLVertexBuilder* vb,
GrGLSLVaryingHandler* varyingHandler,
GrGLSLUniformHandler* uniformHandler,
const GrShaderVar& localCoordsVar,
const GrPipeline& pipeline) {
SkASSERT(localCoordsVar.getType() == kFloat2_GrSLType ||
- localCoordsVar.getType() == kFloat3_GrSLType);
+ localCoordsVar.getType() == kFloat3_GrSLType ||
+ localCoordsVar.getType() == kVoid_GrSLType);
- auto baseLocalCoordVarying = [&, baseLocalCoord = GrGLSLVarying()]() mutable {
+ auto baseLocalCoordFSVar = [&, baseLocalCoord = GrGLSLVarying()]() mutable {
SkASSERT(GrSLTypeIsFloatType(localCoordsVar.getType()));
if (baseLocalCoord.type() == kVoid_GrSLType) {
// Initialize to the GP provided coordinate
@@ -96,7 +93,7 @@
return baseLocalCoord.fsInVar();
};
- FPToVaryingCoordsMap result;
+ FPCoordsMap result;
// Performs a pre-order traversal of FP hierarchy rooted at fp and identifies FPs that are
// sampled with a series of matrices applied to local coords. For each such FP a varying is
// added to the varying handler and added to 'result'.
@@ -104,9 +101,10 @@
const GrFragmentProcessor& fp,
bool hasPerspective,
const GrFragmentProcessor* lastMatrixFP = nullptr,
- int lastMatrixTraversalIndex = -1) mutable {
+ int lastMatrixTraversalIndex = -1,
+ bool inExplicitSubtree = false) mutable -> void {
++traversalIndex;
- switch (fp.sampleUsage().fKind) {
+ switch (fp.sampleUsage().kind()) {
case SkSL::SampleUsage::Kind::kNone:
// This should only happen at the root. Otherwise how did this FP get added?
SkASSERT(!fp.parent());
@@ -115,20 +113,24 @@
break;
case SkSL::SampleUsage::Kind::kUniformMatrix:
// Update tracking of last matrix and matrix props.
- hasPerspective |= fp.sampleUsage().fHasPerspective;
+ hasPerspective |= fp.sampleUsage().hasPerspective();
lastMatrixFP = &fp;
lastMatrixTraversalIndex = traversalIndex;
break;
case SkSL::SampleUsage::Kind::kExplicit:
- // Nothing in this subtree can be lifted.
- return;
+ inExplicitSubtree = true;
+ break;
}
- if (fp.usesVaryingCoordsDirectly()) {
+
+ auto& [varyingFSVar, hasCoordsParam] = result[&fp];
+ hasCoordsParam = fp.usesSampleCoordsDirectly();
+
+ if (fp.usesSampleCoordsDirectly() && !inExplicitSubtree) {
// Associate the varying with the highest possible node in the FP tree that shares the
// same coordinates so that multiple FPs in a subtree can share. If there are no matrix
// sample nodes on the way up the tree then directly use the local coord.
if (!lastMatrixFP) {
- result[&fp] = baseLocalCoordVarying();
+ varyingFSVar = baseLocalCoordFSVar();
} else {
// If there is an already a varying that incorporates all matrices from the root to
// lastMatrixFP just use it. Otherwise, we add it.
@@ -143,16 +145,24 @@
}
SkASSERT(varyingIdx == lastMatrixTraversalIndex);
// The FP will use the varying in the fragment shader as its coords.
- result[&fp] = varying.fsInVar();
+ varyingFSVar = varying.fsInVar();
}
- } else if (!fp.usesVaryingCoords()) {
- // Early-out. No point in continuing to traversal down from here.
- return;
+ hasCoordsParam = false;
}
for (int c = 0; c < fp.numChildProcessors(); ++c) {
- if (auto child = fp.childProcessor(c)) {
- self(self, *child, hasPerspective, lastMatrixFP, lastMatrixTraversalIndex);
+ if (auto* child = fp.childProcessor(c)) {
+ self(self,
+ *child,
+ hasPerspective,
+ lastMatrixFP,
+ lastMatrixTraversalIndex,
+ inExplicitSubtree);
+ // If we have a varying then we never need a param. Otherwise, if one of our
+ // children takes a non-explicit coord then we'll need our coord.
+ hasCoordsParam |= varyingFSVar.getType() == kVoid_GrSLType &&
+ !child->sampleUsage().isExplicit() &&
+ result[child].hasCoordsParam;
}
}
};
diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.h b/src/gpu/glsl/GrGLSLGeometryProcessor.h
index dd3edbc..33c8859 100644
--- a/src/gpu/glsl/GrGLSLGeometryProcessor.h
+++ b/src/gpu/glsl/GrGLSLGeometryProcessor.h
@@ -32,9 +32,15 @@
*/
class GrGLSLGeometryProcessor {
public:
- using UniformHandle = GrGLSLProgramDataManager::UniformHandle;
- using SamplerHandle = GrGLSLUniformHandler::SamplerHandle;
- using FPToVaryingCoordsMap = std::unordered_map<const GrFragmentProcessor*, GrShaderVar>;
+ using UniformHandle = GrGLSLProgramDataManager::UniformHandle;
+ using SamplerHandle = GrGLSLUniformHandler::SamplerHandle;
+ /**
+ * Struct of optional varying that replaces the input coords and bool indicating whether the FP
+ * should take a coord param as an argument. The latter may be false if the coords are simply
+ * unused or if the GP has lifted their computation to a varying emitted by the VS.
+ */
+ struct FPCoords {GrShaderVar coordsVarying; bool hasCoordsParam;};
+ using FPCoordsMap = std::unordered_map<const GrFragmentProcessor*, FPCoords>;
virtual ~GrGLSLGeometryProcessor() {}
@@ -77,7 +83,7 @@
* has its input coords implemented by the GP as a varying, the varying will be accessible in
* the returned map and should be used when the FP code is emitted.
**/
- FPToVaryingCoordsMap emitCode(EmitArgs&, const GrPipeline& pipeline);
+ FPCoordsMap emitCode(EmitArgs&, const GrPipeline& pipeline);
/**
* Called after all effect emitCode() functions, to give the processor a chance to write out
@@ -211,11 +217,12 @@
// 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.
- FPToVaryingCoordsMap collectTransforms(GrGLSLVertexBuilder* vb,
- GrGLSLVaryingHandler* varyingHandler,
- GrGLSLUniformHandler* uniformHandler,
- const GrShaderVar& localCoordsVar,
- const GrPipeline& pipeline);
+ FPCoordsMap collectTransforms(GrGLSLVertexBuilder* vb,
+ GrGLSLVaryingHandler* varyingHandler,
+ GrGLSLUniformHandler* uniformHandler,
+ const GrShaderVar& localCoordsVar,
+ const GrPipeline& pipeline);
+
struct TransformInfo {
// The varying that conveys the coordinates to one or more FPs in the FS.
GrGLSLVarying varying;
diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
index aa4db49..f90c428 100644
--- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
@@ -127,7 +127,7 @@
outputColor->c_str(),
outputCoverage->c_str(),
texSamplers.get());
- fFPCoordVaryings = fGeometryProcessor->emitCode(args, this->pipeline());
+ fFPCoordsMap = fGeometryProcessor->emitCode(args, this->pipeline());
// We have to check that effects and the code they emit are consistent, ie if an effect
// asks for dst color, then the emit code needs to follow suit
@@ -388,8 +388,11 @@
}
GrShaderVar GrGLSLProgramBuilder::varyingCoordsForFragmentProcessor(const GrFragmentProcessor* fp) {
- auto iter = fFPCoordVaryings.find(fp);
- return iter == fFPCoordVaryings.end() ? GrShaderVar() : iter->second;
+ return fFPCoordsMap[fp].coordsVarying;
+}
+
+bool GrGLSLProgramBuilder::fragmentProcessorHasCoordsParam(const GrFragmentProcessor* fp) {
+ return fFPCoordsMap[fp].hasCoordsParam;
}
void GrGLSLProgramBuilder::finalizeShaders() {
diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.h b/src/gpu/glsl/GrGLSLProgramBuilder.h
index 7cda822..97e41f7 100644
--- a/src/gpu/glsl/GrGLSLProgramBuilder.h
+++ b/src/gpu/glsl/GrGLSLProgramBuilder.h
@@ -88,6 +88,12 @@
*/
GrShaderVar varyingCoordsForFragmentProcessor(const GrFragmentProcessor*);
+ /**
+ * If the FP's coords are unused or all uses have been lifted to interpolated varyings then
+ * don't put coords in the FP's function signature or call sites.
+ */
+ bool fragmentProcessorHasCoordsParam(const GrFragmentProcessor*);
+
virtual GrGLSLUniformHandler* uniformHandler() = 0;
virtual const GrGLSLUniformHandler* uniformHandler() const = 0;
virtual GrGLSLVaryingHandler* varyingHandler() = 0;
@@ -176,7 +182,7 @@
// These are used to check that we don't excede the allowable number of resources in a shader.
int fNumFragmentSamplers;
- GrGLSLGeometryProcessor::FPToVaryingCoordsMap fFPCoordVaryings;
+ GrGLSLGeometryProcessor::FPCoordsMap fFPCoordsMap;
};
#endif
diff --git a/src/gpu/tessellate/GrPathCurveTessellator.cpp b/src/gpu/tessellate/GrPathCurveTessellator.cpp
index 3d56746..97bfc52 100644
--- a/src/gpu/tessellate/GrPathCurveTessellator.cpp
+++ b/src/gpu/tessellate/GrPathCurveTessellator.cpp
@@ -174,7 +174,7 @@
GrPathTessellationShader* shader;
if (caps.shaderCaps()->tessellationSupport() &&
caps.shaderCaps()->infinitySupport() && // The hw tessellation shaders use infinity.
- !pipeline.usesVaryingCoords() && // Our tessellation back door doesn't handle varyings.
+ !pipeline.usesLocalCoords() && // Our tessellation back door doesn't handle varyings.
numPathVerbs >= caps.minPathVerbsForHwTessellation()) {
shader = GrPathTessellationShader::MakeHardwareTessellationShader(arena, viewMatrix, color,
PatchType::kCurves);
diff --git a/src/gpu/tessellate/GrPathWedgeTessellator.cpp b/src/gpu/tessellate/GrPathWedgeTessellator.cpp
index e215605..bd1455d 100644
--- a/src/gpu/tessellate/GrPathWedgeTessellator.cpp
+++ b/src/gpu/tessellate/GrPathWedgeTessellator.cpp
@@ -268,7 +268,7 @@
GrPathTessellationShader* shader;
if (caps.shaderCaps()->tessellationSupport() &&
caps.shaderCaps()->infinitySupport() && // The hw tessellation shaders use infinity.
- !pipeline.usesVaryingCoords() && // Our tessellation back door doesn't handle varyings.
+ !pipeline.usesLocalCoords() && // Our tessellation back door doesn't handle varyings.
numPathVerbs >= caps.minPathVerbsForHwTessellation()) {
shader = GrPathTessellationShader::MakeHardwareTessellationShader(arena, viewMatrix, color,
PatchType::kWedges);
diff --git a/src/gpu/tessellate/GrStrokeTessellateOp.cpp b/src/gpu/tessellate/GrStrokeTessellateOp.cpp
index d83670d..05fdafa 100644
--- a/src/gpu/tessellate/GrStrokeTessellateOp.cpp
+++ b/src/gpu/tessellate/GrStrokeTessellateOp.cpp
@@ -151,7 +151,7 @@
!caps.shaderCaps()->infinitySupport() /* The hw tessellation shaders use infinity. */) {
return false;
}
- if (pipeline.usesVaryingCoords()) {
+ if (pipeline.usesLocalCoords()) {
// Our back door for HW tessellation shaders isn't currently capable of passing varyings to
// the fragment shader, so if the processors have varyings, we need to use instanced draws
// instead.
diff --git a/src/gpu/v1/SurfaceDrawContext.cpp b/src/gpu/v1/SurfaceDrawContext.cpp
index fb0e41c..315b256 100644
--- a/src/gpu/v1/SurfaceDrawContext.cpp
+++ b/src/gpu/v1/SurfaceDrawContext.cpp
@@ -822,7 +822,7 @@
SkRect croppedRect, croppedLocal{};
const GrClip* optimizedClip = clip;
if (clip && viewMatrix.isScaleTranslate() && quad.fDevice.asRect(&croppedRect) &&
- (!paint.usesVaryingCoords() || quad.fLocal.asRect(&croppedLocal))) {
+ (!paint.usesLocalCoords() || quad.fLocal.asRect(&croppedLocal))) {
// The cropped quad is still a rect, and our view matrix preserves rects. Map it back
// to pre-matrix space.
SkMatrix inverse;
diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp
index a6f8188..01be63a 100644
--- a/tests/ProcessorTest.cpp
+++ b/tests/ProcessorTest.cpp
@@ -806,9 +806,9 @@
"\n%s", fp.dumpTreeInfo().c_str());
REPORTER_ASSERT(reporter, fp.numChildProcessors() == clone.numChildProcessors(),
"\n%s", fp.dumpTreeInfo().c_str());
- REPORTER_ASSERT(reporter, fp.usesVaryingCoords() == clone.usesVaryingCoords(),
+ REPORTER_ASSERT(reporter, fp.sampleUsage() == clone.sampleUsage(),
"\n%s", fp.dumpTreeInfo().c_str());
- REPORTER_ASSERT(reporter, fp.referencesSampleCoords() == clone.referencesSampleCoords(),
+ REPORTER_ASSERT(reporter, fp.usesSampleCoords() == clone.usesSampleCoords(),
"\n%s", fp.dumpTreeInfo().c_str());
}
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index c139588..d5409e4 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -893,8 +893,8 @@
"child", std::move(child));
REPORTER_ASSERT(r, fp);
- REPORTER_ASSERT(r, fp->childProcessor(0)->isSampledWithExplicitCoords() == expectExplicit);
- REPORTER_ASSERT(r, fp->referencesSampleCoords() == expectReferencesSampleCoords);
+ REPORTER_ASSERT(r, fp->childProcessor(0)->sampleUsage().isExplicit() == expectExplicit);
+ REPORTER_ASSERT(r, fp->usesSampleCoords() == expectReferencesSampleCoords);
};
// Cases where our optimization is valid, and works: