SkRuntimeEffect SkSL has a new signature for main()
There is no more 'inout half4 color'. Effects return their output color.
If an effect wants the input color, it must use the (already existing)
approach of sampling a nullptr input shader.
The change is guarded for Chromium (so we can update their runtime color
filters in skia_renderer.cc).
For the GPU backend, FPs can now override usesExplicitReturn to indicate
that their emitCode will generate a return statement. If that's true,
then writeProcessorFunction doesn't inject the automatic return of the
output color, and emitFragProc will *always* wrap that FP in a helper
function, even as a top-level FP. GrSkSLFP opts in to this behavior, so
that the user-supplied return becomes the actual return in the FP's
emitCode.
Adapting the skvm code to this wasn't too bad: It looks fragile (what
happens if there are multiple returns?), but that's not really possible
today, without varying control flow.
Bug: skia:10613
Change-Id: I205b81fd87dd32bab30b6d6d5fc78853485da036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
index a0620aa..d5b12af 100644
--- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
@@ -147,8 +147,6 @@
}
}
-// TODO Processors cannot output zeros because an empty string is all 1s
-// the fix is to allow effects to take the SkString directly
SkString GrGLSLProgramBuilder::emitFragProc(const GrFragmentProcessor& fp,
GrGLSLFragmentProcessor& glslFP,
int transformedCoordVarsIdx,
@@ -159,11 +157,6 @@
AutoStageAdvance adv(this);
this->nameExpression(&output, "output");
- // Enclose custom code in a block to avoid namespace conflicts
- SkString openBrace;
- openBrace.printf("{ // Stage %d, %s\n", fStageIndex, fp.name());
- fFS.codeAppend(openBrace.c_str());
-
int samplerIdx = 0;
for (auto [subFP, subGLSLFP] : GrGLSLFragmentProcessor::ParallelRange(fp, glslFP)) {
if (auto* te = subFP.asTextureEffect()) {
@@ -188,37 +181,49 @@
"_coords",
coords);
- if (fp.referencesSampleCoords()) {
- // The fp's generated code expects a _coords variable, but we're at the root so _coords
- // is just the local coordinates produced by the primitive processor.
- SkASSERT(fp.usesVaryingCoordsDirectly());
+ if (fp.usesExplicitReturn()) {
+ // FPs that explicitly return their output color must be in a helper function
+ args.fInputColor = "_input";
+ args.fOutputColor = "_output";
+ auto name = fFS.writeProcessorFunction(&glslFP, args);
+ fFS.codeAppendf("%s = %s(%s);", output.c_str(), name.c_str(), input.c_str());
+ } else {
+ // Enclose custom code in a block to avoid namespace conflicts
+ fFS.codeAppendf("{ // Stage %d, %s\n", fStageIndex, fp.name());
- const GrShaderVar& varying = coordVars[0];
- switch(varying.getType()) {
- case kFloat2_GrSLType:
- fFS.codeAppendf("float2 %s = %s.xy;\n",
- args.fSampleCoord, varying.getName().c_str());
- break;
- case kFloat3_GrSLType:
- fFS.codeAppendf("float2 %s = %s.xy / %s.z;\n",
- args.fSampleCoord,
- varying.getName().c_str(),
- varying.getName().c_str());
- break;
- default:
- SkDEBUGFAILF("Unexpected type for varying: %d named %s\n",
- (int) varying.getType(), varying.getName().c_str());
- break;
+ if (fp.referencesSampleCoords()) {
+ // The fp's generated code expects a _coords variable, but we're at the root so _coords
+ // is just the local coordinates produced by the primitive processor.
+ SkASSERT(fp.usesVaryingCoordsDirectly());
+
+ const GrShaderVar& varying = coordVars[0];
+ switch(varying.getType()) {
+ case kFloat2_GrSLType:
+ fFS.codeAppendf("float2 %s = %s.xy;\n",
+ args.fSampleCoord, varying.getName().c_str());
+ break;
+ case kFloat3_GrSLType:
+ fFS.codeAppendf("float2 %s = %s.xy / %s.z;\n",
+ args.fSampleCoord,
+ varying.getName().c_str(),
+ varying.getName().c_str());
+ break;
+ default:
+ SkDEBUGFAILF("Unexpected type for varying: %d named %s\n",
+ (int) varying.getType(), varying.getName().c_str());
+ break;
+ }
}
- }
- glslFP.emitCode(args);
+ glslFP.emitCode(args);
+
+ fFS.codeAppend("}");
+ }
// 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
SkDEBUGCODE(verify(fp);)
- fFS.codeAppend("}");
return output;
}