From 9eca2ca0c6bb95e9ba0a23c32b8a30b22744157d Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Mon, 2 Aug 2021 13:04:04 -0400 Subject: [PATCH] Use varyings to implement MatrixEffects applied to DeviceCoord FPs. Modify GrGLSLGP's logic to add a varying for a series of matrix sample usages below a DeviceSpace FP in the FP hierarchy. The varying is based off the GPs position output variable in the VS. Adds a new sample usage type for the DeviceSpace FP. Bug: skia:12198 Change-Id: Ic39f3296c60a073656ad0c72a431a462d5714e92 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435717 Commit-Queue: Brian Salomon Reviewed-by: Brian Osman Reviewed-by: Michael Ludwig --- gm/fp_sample_chaining.cpp | 21 +++++++-- include/private/SkSLSampleUsage.h | 5 +++ src/gpu/GrFragmentProcessor.cpp | 3 +- src/gpu/glsl/GrGLSLFragmentProcessor.cpp | 1 + src/gpu/glsl/GrGLSLGeometryProcessor.cpp | 54 +++++++++++++++++------- src/gpu/glsl/GrGLSLGeometryProcessor.h | 7 +-- 6 files changed, 67 insertions(+), 24 deletions(-) diff --git a/gm/fp_sample_chaining.cpp b/gm/fp_sample_chaining.cpp index 2b90a9ae6e..10e5cdf591 100644 --- a/gm/fp_sample_chaining.cpp +++ b/gm/fp_sample_chaining.cpp @@ -140,15 +140,21 @@ SkBitmap make_test_bitmap() { enum EffectType { kUniform, kExplicit, + kDevice, }; static std::unique_ptr wrap(std::unique_ptr fp, - EffectType effectType) { + EffectType effectType, + int drawX, int drawY) { switch (effectType) { case kUniform: return std::make_unique(std::move(fp)); case kExplicit: return std::make_unique(std::move(fp)); + case kDevice: + // Subtract out upper-left corner of draw so that device is effectively identity. + fp = GrMatrixEffect::Make(SkMatrix::Translate(-drawX, -drawY), std::move(fp)); + return GrFragmentProcessor::DeviceSpace(std::move(fp)); } SkUNREACHABLE; } @@ -157,7 +163,7 @@ static std::unique_ptr wrap(std::unique_ptr GrFragmentProcessor::DeviceSpace( private: DeviceSpace(std::unique_ptr fp) : GrFragmentProcessor(kDeviceSpace_ClassID, fp->optimizationFlags()) { - this->registerChild(std::move(fp), SkSL::SampleUsage::Explicit()); + // Passing FragCoord here is the reason this is a subclass and not a runtime-FP. + this->registerChild(std::move(fp), SkSL::SampleUsage::FragCoord()); } std::unique_ptr clone() const override { diff --git a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp index a4639c0e17..987da060fa 100644 --- a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp +++ b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp @@ -70,6 +70,7 @@ SkString GrGLSLFragmentProcessor::invokeChild(int childIndex, SkASSERT(!childProc->sampleUsage().isUniformMatrix()); if (args.fFragBuilder->getProgramBuilder()->fragmentProcessorHasCoordsParam(childProc)) { + SkASSERT(!childProc->sampleUsage().isFragCoord() || skslCoords == "sk_FragCoord.xy"); // The child's function takes a half4 color and a float2 coordinate invocation.appendf(", %s", skslCoords.empty() ? args.fSampleCoord : skslCoords.c_str()); } diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp index 280c7210c8..85d23d5fc6 100644 --- a/src/gpu/glsl/GrGLSLGeometryProcessor.cpp +++ b/src/gpu/glsl/GrGLSLGeometryProcessor.cpp @@ -26,6 +26,7 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::emitCode(EmitArgs& args.fVaryingHandler, args.fUniformHandler, gpArgs.fLocalCoordVar, + gpArgs.fPositionVar, pipeline); if (args.fGeomProc.willUseTessellationShaders()) { @@ -76,18 +77,22 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms( GrGLSLVaryingHandler* varyingHandler, GrGLSLUniformHandler* uniformHandler, const GrShaderVar& localCoordsVar, + const GrShaderVar& positionVar, const GrPipeline& pipeline) { SkASSERT(localCoordsVar.getType() == kFloat2_GrSLType || localCoordsVar.getType() == kFloat3_GrSLType || localCoordsVar.getType() == kVoid_GrSLType); + SkASSERT(positionVar.getType() == kFloat2_GrSLType || + positionVar.getType() == kFloat3_GrSLType); + + enum class BaseCoord { kNone, kLocal, kPosition }; auto baseLocalCoordFSVar = [&, baseLocalCoord = GrGLSLVarying()]() mutable { SkASSERT(GrSLTypeIsFloatType(localCoordsVar.getType())); if (baseLocalCoord.type() == kVoid_GrSLType) { // Initialize to the GP provided coordinate - SkString baseLocalCoordName = SkStringPrintf("LocalCoord"); baseLocalCoord = GrGLSLVarying(localCoordsVar.getType()); - varyingHandler->addVarying(baseLocalCoordName.c_str(), &baseLocalCoord); + varyingHandler->addVarying("LocalCoord", &baseLocalCoord); vb->codeAppendf("%s = %s;\n", baseLocalCoord.vsOut(), localCoordsVar.getName().c_str()); } return baseLocalCoord.fsInVar(); @@ -97,12 +102,13 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms( // 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'. - auto liftTransforms = [&, traversalIndex = 0](auto& self, - const GrFragmentProcessor& fp, - bool hasPerspective, - const GrFragmentProcessor* lastMatrixFP = nullptr, - int lastMatrixTraversalIndex = -1, - bool inExplicitSubtree = false) mutable -> void { + auto liftTransforms = [&, traversalIndex = 0]( + auto& self, + const GrFragmentProcessor& fp, + bool hasPerspective, + const GrFragmentProcessor* lastMatrixFP = nullptr, + int lastMatrixTraversalIndex = -1, + BaseCoord baseCoord = BaseCoord::kLocal) mutable -> void { ++traversalIndex; switch (fp.sampleUsage().kind()) { case SkSL::SampleUsage::Kind::kNone: @@ -117,15 +123,27 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms( lastMatrixFP = &fp; lastMatrixTraversalIndex = traversalIndex; break; + case SkSL::SampleUsage::Kind::kFragCoord: + hasPerspective = positionVar.getType() == kFloat3_GrSLType; + lastMatrixFP = nullptr; + lastMatrixTraversalIndex = -1; + baseCoord = BaseCoord::kPosition; + break; case SkSL::SampleUsage::Kind::kExplicit: - inExplicitSubtree = true; + baseCoord = BaseCoord::kNone; break; } auto& [varyingFSVar, hasCoordsParam] = result[&fp]; hasCoordsParam = fp.usesSampleCoordsDirectly(); - if (fp.usesSampleCoordsDirectly() && !inExplicitSubtree) { + // We add a varying if we're in a chain of matrices multiplied by local or device coords. + // If the coord is the untransformed local coord we add a varying. We don't if it is + // untransformed device coords since it doesn't save us anything over "sk_FragCoord.xy". Of + // course, if the FP doesn't directly use its coords then we don't add a varying. + if (fp.usesSampleCoordsDirectly() && + (baseCoord == BaseCoord::kLocal || + (baseCoord == BaseCoord::kPosition && lastMatrixFP))) { // 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. @@ -134,13 +152,13 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms( } else { // If there is an already a varying that incorporates all matrices from the root to // lastMatrixFP just use it. Otherwise, we add it. - auto& [varying, localCoord, varyingIdx] = fTransformVaryingsMap[lastMatrixFP]; + auto& [varying, inputCoords, varyingIdx] = fTransformVaryingsMap[lastMatrixFP]; if (varying.type() == kVoid_GrSLType) { varying = GrGLSLVarying(hasPerspective ? kFloat3_GrSLType : kFloat2_GrSLType); SkString strVaryingName = SkStringPrintf("TransformedCoords_%d", lastMatrixTraversalIndex); varyingHandler->addVarying(strVaryingName.c_str(), &varying); - localCoord = localCoordsVar; + inputCoords = baseCoord == BaseCoord::kLocal ? localCoordsVar : positionVar; varyingIdx = lastMatrixTraversalIndex; } SkASSERT(varyingIdx == lastMatrixTraversalIndex); @@ -157,11 +175,12 @@ GrGLSLGeometryProcessor::FPCoordsMap GrGLSLGeometryProcessor::collectTransforms( hasPerspective, lastMatrixFP, lastMatrixTraversalIndex, - inExplicitSubtree); + baseCoord); // 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() && + !child->sampleUsage().isFragCoord() && result[child].hasCoordsParam; } } @@ -199,7 +218,7 @@ void GrGLSLGeometryProcessor::emitTransformCode(GrGLSLVertexBuilder* vb, // If we hit an ancestor with a varying on our walk up then save off the varying as the // input to our accumulated transformExpression. Start off assuming we'll reach the root. - GrShaderVar inputCoords = info.localCoords; + GrShaderVar inputCoords = info.inputCoords; for (const auto* base = fp->parent(); base; base = base->parent()) { if (auto iter = fTransformVaryingsMap.find(base); iter != fTransformVaryingsMap.end()) { @@ -209,13 +228,16 @@ void GrGLSLGeometryProcessor::emitTransformCode(GrGLSLVertexBuilder* vb, inputCoords = iter->second.varying.vsOutVar(); break; } else if (base->sampleUsage().isUniformMatrix()) { - // Accumulate any matrices along the path to either the original local coords or - // a parent varying. Getting here means this FP was sampled with a uniform matrix + // Accumulate any matrices along the path to either the original local/device coords + // or a parent varying. Getting here means this FP was sampled with a uniform matrix // but all uses of coords below here in the FP hierarchy are beneath additional // matrix samples and thus this node wasn't assigned a varying. GrShaderVar uniform = uniformHandler->liftUniformToVertexShader( *base->parent(), SkString(SkSL::SampleUsage::MatrixUniformName())); transformExpression.appendf(" * %s", uniform.getName().c_str()); + } else if (base->sampleUsage().isFragCoord()) { + // Our chain of matrices starts here and is based on the device space position. + break; } else { // This intermediate FP is just a pass through and doesn't need to be built // in to the expression, but we must visit its parents in case they add transforms. diff --git a/src/gpu/glsl/GrGLSLGeometryProcessor.h b/src/gpu/glsl/GrGLSLGeometryProcessor.h index 33c8859ea7..287e9ea20e 100644 --- a/src/gpu/glsl/GrGLSLGeometryProcessor.h +++ b/src/gpu/glsl/GrGLSLGeometryProcessor.h @@ -216,18 +216,19 @@ private: // // 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. + // onEmitCode() returns using the value stored in GpArgs::fLocalCoordVar and + // GpArgs::fPositionVar. FPCoordsMap collectTransforms(GrGLSLVertexBuilder* vb, GrGLSLVaryingHandler* varyingHandler, GrGLSLUniformHandler* uniformHandler, const GrShaderVar& localCoordsVar, + const GrShaderVar& positionVar, const GrPipeline& pipeline); - struct TransformInfo { // The varying that conveys the coordinates to one or more FPs in the FS. GrGLSLVarying varying; // The coordinate to be transformed. varying is computed from this. - GrShaderVar localCoords; + GrShaderVar inputCoords; // Used to sort so that ancestor FP varyings are initialized before descendant FP varyings. int traversalOrder; };