SkSL: Salt inline identifiers in PipelineStage and FragmentProcessor passes

Fixes a bug with name conflicts in the final SkSL.

Bug: skia:10526
Change-Id: Ic238f89dd778c186e775ecbaabfbaed9e426274f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Brian Osman 2020-07-23 13:28:14 -04:00 committed by Skia Commit-Bot
parent 6b050f6bef
commit b5f0f52831
5 changed files with 40 additions and 14 deletions

View File

@ -42,12 +42,12 @@ public:
fragBuilder->codeAppendf(
R"SkSL(half4 inputColor = %s;
@if (%s) {
half4 inlineResult0;
half4 inlineArg1_0 = inputColor;
half4 inlineResult_fp_0;
half4 inlineArg_fp_1_0 = inputColor;
{
inlineResult0 = half4(inlineArg1_0.xyz / max(inlineArg1_0.w, 9.9999997473787516e-05), inlineArg1_0.w);
inlineResult_fp_0 = half4(inlineArg_fp_1_0.xyz / max(inlineArg_fp_1_0.w, 9.9999997473787516e-05), inlineArg_fp_1_0.w);
}
inputColor = inlineResult0;
inputColor = inlineResult_fp_0;
}
%s = %s * inputColor + %s;

View File

@ -51,12 +51,12 @@ return t < 0.16666666666666666 ? p + ((q - p) * 6.0) * t : (t < 0.5 ? q : (t < 0
fragBuilder->codeAppendf(
R"SkSL(
half4 inColor = %s;
half4 inlineResult0;
half4 inlineArg1_0 = inColor;
half4 inlineResult_fp_0;
half4 inlineArg_fp_1_0 = inColor;
{
inlineResult0 = half4(inlineArg1_0.xyz / max(inlineArg1_0.w, 9.9999997473787516e-05), inlineArg1_0.w);
inlineResult_fp_0 = half4(inlineArg_fp_1_0.xyz / max(inlineArg_fp_1_0.w, 9.9999997473787516e-05), inlineArg_fp_1_0.w);
}
half4 color = inlineResult0;
half4 color = inlineResult_fp_0;
@if (%s) {
color.xyz = color.xyz * color.xyz;

View File

@ -2199,11 +2199,23 @@ std::unique_ptr<Expression> IRGenerator::inlineCall(
// order guarantees. Since we can't use gotos (which are normally used to replace return
// statements), we wrap the whole function in a loop and use break statements to jump to the
// end.
// Use unique variable names during the PipelineStage and FragmentProcessor passes. Otherwise,
// the SkSL we make in the resulting FP will have variables with names like 'inlineResult0'.
// If that FP's code is then inlined again when the combined fragment shader SkSL is compiled,
// we'll reuse those names (the counter is reset), creating a conflict. (skbug.com/10526)
const char* inlineSalt = "";
switch (fKind) {
case Program::kPipelineStage_Kind: inlineSalt = "_ps_"; break;
case Program::kFragmentProcessor_Kind: inlineSalt = "_fp_"; break;
default: break;
}
Variable* resultVar;
if (function.fDeclaration.fReturnType != *fContext.fVoid_Type) {
std::unique_ptr<String> name(new String());
int varIndex = fInlineVarCounter++;
name->appendf("inlineResult%d", varIndex);
name->appendf("inlineResult%s%d", inlineSalt, varIndex);
String* namePtr = (String*) fSymbolTable->takeOwnership(std::move(name));
resultVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
new Variable(-1, Modifiers(), namePtr->c_str(),
@ -2225,7 +2237,7 @@ std::unique_ptr<Expression> IRGenerator::inlineCall(
int argIndex = fInlineVarCounter++;
for (int i = 0; i < (int) arguments.size(); ++i) {
std::unique_ptr<String> argName(new String());
argName->appendf("inlineArg%d_%d", argIndex, i);
argName->appendf("inlineArg%s%d_%d", inlineSalt, argIndex, i);
String* argNamePtr = (String*) fSymbolTable->takeOwnership(std::move(argName));
Variable* argVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
new Variable(-1, Modifiers(),

View File

@ -196,6 +196,10 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrContext* context
effect.test(0xFF000000, 0xFF00007F, 0xFF007F00, 0xFF007F7F,
[](SkCanvas* canvas, SkPaint*) { canvas->rotate(45.0f); });
//
// Sampling children
//
// Sampling a null child should return the paint color
effect.build("in shader child;",
"color = sample(child);");
@ -222,6 +226,16 @@ static void test_RuntimeEffect_Shaders(skiatest::Reporter* r, GrContext* context
"color = sample(child, float3x3(0, 1, 0, 1, 0, 0, 0, 0, 1));");
effect.child("child") = rgbwShader;
effect.test(0xFF0000FF, 0xFFFF0000, 0xFF00FF00, 0xFFFFFFFF);
//
// Helper functions
//
// Test case for inlining in the pipeline-stage and fragment-shader passes (skbug.com/10526):
effect.build("float2 helper(float2 x) { return x + 1; }",
"float2 v = helper(p);"
"color = half4(half2(v), 0, 1);");
effect.test(0xFF00FFFF);
}
DEF_TEST(SkRuntimeEffectSimple, r) {

View File

@ -830,12 +830,12 @@ DEF_TEST(SkSLFPFunction, r) {
"R\"SkSL(return c.wzyx;\n"
")SkSL\", &flip_name);",
"fragBuilder->codeAppendf(\n"
"R\"SkSL(half4 inlineResult0;\n"
"half4 inlineArg1_0 = %s;\n"
"R\"SkSL(half4 inlineResult_fp_0;\n"
"half4 inlineArg_fp_1_0 = %s;\n"
"{\n"
" inlineResult0 = inlineArg1_0.wzyx;\n"
" inlineResult_fp_0 = inlineArg_fp_1_0.wzyx;\n"
"}\n"
"%s = inlineResult0;\n"
"%s = inlineResult_fp_0;\n"
"\n"
")SkSL\"\n"
", args.fInputColor, args.fOutputColor);"