From b211b79a5cd402294227af317a984a409042ec85 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Wed, 17 Aug 2022 01:01:23 +0900 Subject: [PATCH] internal/shader: use a return statement in a fragment shader entrypoint Updates #2247 --- internal/shader/shader.go | 11 +--- internal/shader/testdata/for5.expected.fs | 37 +++++------ .../shader/testdata/issue1238.expected.fs | 14 ++-- .../shader/testdata/issue1245.expected.fs | 17 ++--- .../shader/testdata/issue1701.expected.fs | 11 ++-- .../testdata/vertex_fragment.expected.fs | 11 ++-- .../testdata/vertex_fragment.expected.metal | 4 +- internal/shaderir/glsl/glsl.go | 64 +++++++------------ internal/shaderir/hlsl/hlsl.go | 14 +--- internal/shaderir/ir_test.go | 19 +++--- internal/shaderir/msl/msl.go | 16 ++--- internal/shaderir/program.go | 1 - internal/shaderir/type.go | 4 +- 13 files changed, 79 insertions(+), 144 deletions(-) diff --git a/internal/shader/shader.go b/internal/shader/shader.go index 8db6df157..60d9e76ad 100644 --- a/internal/shader/shader.go +++ b/internal/shader/shader.go @@ -704,6 +704,8 @@ func (cs *compileState) parseFunc(block *block, d *ast.FuncDecl) (function, bool // For the vertex entry, a parameter (variable) is used as a returning value. // For example, GLSL doesn't treat gl_Position as a returning value. + // TODO: This can be resolved by having an indirect function like what the fragment entry already does. + // See internal/shaderir/glsl.adjustProgram. if len(outParams) == 0 { outParams = append(outParams, variable{ typ: shaderir.Type{Main: shaderir.Vec4}, @@ -735,14 +737,7 @@ func (cs *compileState) parseFunc(block *block, d *ast.FuncDecl) (function, bool return function{}, false } - // For the fragment entry, a parameter (variable) is used as a returning value. - // For example, GLSL doesn't treat gl_FragColor as a returning value. - if len(outParams) == 0 { - outParams = append(outParams, variable{ - typ: shaderir.Type{Main: shaderir.Vec4}, - }) - } - if len(outParams) != 1 || outParams[0].typ.Main != shaderir.Vec4 { + if len(outParams) != 0 || returnType.Main != shaderir.Vec4 { cs.addError(d.Pos(), fmt.Sprintf("fragment entry point must have one returning vec4 value for a color")) return function{}, false } diff --git a/internal/shader/testdata/for5.expected.fs b/internal/shader/testdata/for5.expected.fs index b91df9f8a..3bc18a139 100644 --- a/internal/shader/testdata/for5.expected.fs +++ b/internal/shader/testdata/for5.expected.fs @@ -3,34 +3,31 @@ uniform float U1; uniform float U2; int F0(in int l0); -void F1(in vec4 l0, out vec4 l1); +vec4 F1(in vec4 l0); int F0(in int l0) { return l0; } -void F1(in vec4 l0, out vec4 l1) { - int l2 = 0; - int l4 = 0; - l2 = 0; - for (int l3 = 0; l3 < 10; l3++) { - int l4 = 0; - l4 = F0(l3); - l2 = (l2) + (l4); - for (int l5 = 0; l5 < 10; l5++) { - int l6 = 0; - l6 = F0(l5); - l2 = (l2) + (l6); +vec4 F1(in vec4 l0) { + int l1 = 0; + int l3 = 0; + l1 = 0; + for (int l2 = 0; l2 < 10; l2++) { + int l3 = 0; + l3 = F0(l2); + l1 = (l1) + (l3); + for (int l4 = 0; l4 < 10; l4++) { + int l5 = 0; + l5 = F0(l4); + l1 = (l1) + (l5); } } - l4 = 0; - l2 = (l2) + (l4); - l1 = vec4(l2); - return; + l3 = 0; + l1 = (l1) + (l3); + return vec4(l1); } void main(void) { - vec4 l0 = vec4(0); - F1(gl_FragCoord, l0); - gl_FragColor = l0; + gl_FragColor = F1(gl_FragCoord); } diff --git a/internal/shader/testdata/issue1238.expected.fs b/internal/shader/testdata/issue1238.expected.fs index 5455afc2a..e8918713d 100644 --- a/internal/shader/testdata/issue1238.expected.fs +++ b/internal/shader/testdata/issue1238.expected.fs @@ -1,16 +1,12 @@ -void F0(in vec4 l0, out vec4 l1); +vec4 F0(in vec4 l0); -void F0(in vec4 l0, out vec4 l1) { +vec4 F0(in vec4 l0) { if (true) { - l1 = l0; - return; + return l0; } - l1 = l0; - return; + return l0; } void main(void) { - vec4 l0 = vec4(0); - F0(gl_FragCoord, l0); - gl_FragColor = l0; + gl_FragColor = F0(gl_FragCoord); } diff --git a/internal/shader/testdata/issue1245.expected.fs b/internal/shader/testdata/issue1245.expected.fs index 3514ba8b1..fd429e654 100644 --- a/internal/shader/testdata/issue1245.expected.fs +++ b/internal/shader/testdata/issue1245.expected.fs @@ -1,16 +1,13 @@ -void F0(in vec4 l0, out vec4 l1); +vec4 F0(in vec4 l0); -void F0(in vec4 l0, out vec4 l1) { - vec4 l2 = vec4(0); - for (float l3 = 0.0; l3 < 4.0; l3++) { - (l2).x = ((l2).x) + ((l3) * (1.0000000000e-02)); +vec4 F0(in vec4 l0) { + vec4 l1 = vec4(0); + for (float l2 = 0.0; l2 < 4.0; l2++) { + (l1).x = ((l1).x) + ((l2) * (1.0000000000e-02)); } - l1 = l2; - return; + return l1; } void main(void) { - vec4 l0 = vec4(0); - F0(gl_FragCoord, l0); - gl_FragColor = l0; + gl_FragColor = F0(gl_FragCoord); } diff --git a/internal/shader/testdata/issue1701.expected.fs b/internal/shader/testdata/issue1701.expected.fs index edde27125..efc4c17db 100644 --- a/internal/shader/testdata/issue1701.expected.fs +++ b/internal/shader/testdata/issue1701.expected.fs @@ -1,6 +1,6 @@ void F2(void); void F3(void); -void F5(in vec4 l0, out vec4 l1); +vec4 F5(in vec4 l0); void F2(void) { } @@ -9,14 +9,11 @@ void F3(void) { F2(); } -void F5(in vec4 l0, out vec4 l1) { +vec4 F5(in vec4 l0) { F3(); - l1 = vec4(0.0); - return; + return vec4(0.0); } void main(void) { - vec4 l0 = vec4(0); - F5(gl_FragCoord, l0); - gl_FragColor = l0; + gl_FragColor = F5(gl_FragCoord); } diff --git a/internal/shader/testdata/vertex_fragment.expected.fs b/internal/shader/testdata/vertex_fragment.expected.fs index 63dd73c09..f885a4426 100644 --- a/internal/shader/testdata/vertex_fragment.expected.fs +++ b/internal/shader/testdata/vertex_fragment.expected.fs @@ -2,15 +2,12 @@ uniform vec2 U0; varying vec2 V0; varying vec4 V1; -void F0(in vec4 l0, in vec2 l1, in vec4 l2, out vec4 l3); +vec4 F0(in vec4 l0, in vec2 l1, in vec4 l2); -void F0(in vec4 l0, in vec2 l1, in vec4 l2, out vec4 l3) { - l3 = vec4((l0).x, (l1).y, (l2).z, 1.0); - return; +vec4 F0(in vec4 l0, in vec2 l1, in vec4 l2) { + return vec4((l0).x, (l1).y, (l2).z, 1.0); } void main(void) { - vec4 l0 = vec4(0); - F0(gl_FragCoord, V0, V1, l0); - gl_FragColor = l0; + gl_FragColor = F0(gl_FragCoord, V0, V1); } diff --git a/internal/shader/testdata/vertex_fragment.expected.metal b/internal/shader/testdata/vertex_fragment.expected.metal index e1cefdf06..71bd5a290 100644 --- a/internal/shader/testdata/vertex_fragment.expected.metal +++ b/internal/shader/testdata/vertex_fragment.expected.metal @@ -26,7 +26,5 @@ vertex Varyings Vertex( fragment float4 Fragment( Varyings varyings [[stage_in]], constant float2& U0 [[buffer(1)]]) { - float4 out = float4(0); - out = float4((varyings.Position).x, (varyings.M0).y, (varyings.M1).z, 1.0); - return out; + return float4((varyings.Position).x, (varyings.M0).y, (varyings.M1).z, 1.0); } diff --git a/internal/shaderir/glsl/glsl.go b/internal/shaderir/glsl/glsl.go index dd7a745ee..2f6595fca 100644 --- a/internal/shaderir/glsl/glsl.go +++ b/internal/shaderir/glsl/glsl.go @@ -430,13 +430,8 @@ func (c *compileContext) localVariableName(p *shaderir.Program, topBlock *shader return "gl_FragCoord" case idx < nv+1: return fmt.Sprintf("V%d", idx-1) - case idx == nv+1: - if c.version == GLSLVersionES300 { - return "fragColor" - } - return "gl_FragColor" default: - return fmt.Sprintf("l%d", idx-(nv+2)) + return fmt.Sprintf("l%d", idx-(nv+1)) } default: return fmt.Sprintf("l%d", idx) @@ -613,13 +608,22 @@ func (c *compileContext) block(p *shaderir.Program, topBlock, block *shaderir.Bl case shaderir.Break: lines = append(lines, idt+"break;") case shaderir.Return: - if len(s.Exprs) == 0 { + switch { + case topBlock == p.FragmentFunc.Block: + token := "gl_FragColor" + if c.version == GLSLVersionES300 { + token = "fragColor" + } + lines = append(lines, fmt.Sprintf("%s%s = %s;", idt, token, expr(&s.Exprs[0]))) + // The 'return' statement is not required so far, as the fragment entrypoint has only one sentence so far. See adjustProgram implementation. + case len(s.Exprs) == 0: lines = append(lines, idt+"return;") - } else { + default: lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, expr(&s.Exprs[0]))) } case shaderir.Discard: - lines = append(lines, idt+"discard;") + // 'discard' is invoked only in the fragment shader entry point. + lines = append(lines, idt+"discard;", idt+"return vec4(0.0);") default: lines = append(lines, fmt.Sprintf("%s?(unexpected stmt: %d)", idt, s.Type)) } @@ -661,17 +665,14 @@ func adjustProgram(p *shaderir.Program) *shaderir.Program { } copy(inParams[1:], newP.Varyings) - outParams := make([]shaderir.Type, 1) - outParams[0] = shaderir.Type{ - Main: shaderir.Vec4, // gl_FragColor - } - newP.Funcs = append(newP.Funcs, shaderir.Func{ Index: funcIdx, InParams: inParams, - OutParams: outParams, - Return: shaderir.Type{}, - Block: newP.FragmentFunc.Block, + OutParams: nil, + Return: shaderir.Type{ + Main: shaderir.Vec4, + }, + Block: newP.FragmentFunc.Block, }) // Create an AST to call the new function. @@ -687,45 +688,24 @@ func adjustProgram(p *shaderir.Program) *shaderir.Program { Index: i, }) } - call = append(call, shaderir.Expr{ - Type: shaderir.LocalVariable, - Index: 1 + len(newP.Varyings) + 1, // l0 as an out parameter. - }) // Replace the entry point with just calling the new function. stmts := []shaderir.Stmt{ { - Type: shaderir.ExprStmt, + // Return: This will be replaced with assignment to gl_FragColor. + Type: shaderir.Return, Exprs: []shaderir.Expr{ + // The function call { Type: shaderir.Call, Exprs: call, }, }, }, - { - Type: shaderir.Assign, - Exprs: []shaderir.Expr{ - // gl_FragColor - { - Type: shaderir.LocalVariable, - Index: 1 + len(newP.Varyings), - }, - // l0 - { - Type: shaderir.LocalVariable, - Index: 1 + len(newP.Varyings) + 1, - }, - }, - }, } newP.FragmentFunc = shaderir.FragmentFunc{ Block: &shaderir.Block{ - LocalVars: []shaderir.Type{ - { - Main: shaderir.Vec4, - }, - }, + LocalVars: nil, LocalVarIndexOffset: 1 + len(newP.Varyings) + 1, Stmts: stmts, }, diff --git a/internal/shaderir/hlsl/hlsl.go b/internal/shaderir/hlsl/hlsl.go index a8c31d38b..374d8bfe4 100644 --- a/internal/shaderir/hlsl/hlsl.go +++ b/internal/shaderir/hlsl/hlsl.go @@ -26,7 +26,6 @@ import ( const ( vsOut = "varyings" - psOut = "psOut" ) type compileContext struct { @@ -147,11 +146,7 @@ func Compile(p *shaderir.Program) (string, []int) { if p.FragmentFunc.Block != nil && len(p.FragmentFunc.Block.Stmts) > 0 { lines = append(lines, "") lines = append(lines, fmt.Sprintf("float4 PSMain(Varyings %s) : SV_TARGET {", vsOut)) - lines = append(lines, fmt.Sprintf("\tfloat4 %s = 0.0;", psOut)) lines = append(lines, c.block(p, p.FragmentFunc.Block, p.FragmentFunc.Block, 0)...) - if last := fmt.Sprintf("\treturn %s;", psOut); lines[len(lines)-1] != last { - lines = append(lines, last) - } lines = append(lines, "}") } @@ -311,10 +306,8 @@ func (c *compileContext) localVariableName(p *shaderir.Program, topBlock *shader return fmt.Sprintf("%s.Position", vsOut) case idx < nv+1: return fmt.Sprintf("%s.M%d", vsOut, idx-1) - case idx == nv+1: - return psOut default: - return fmt.Sprintf("l%d", idx-(nv+2)) + return fmt.Sprintf("l%d", idx-(nv+1)) } default: return fmt.Sprintf("l%d", idx) @@ -525,15 +518,14 @@ func (c *compileContext) block(p *shaderir.Program, topBlock, block *shaderir.Bl switch { case topBlock == p.VertexFunc.Block: lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, vsOut)) - case topBlock == p.FragmentFunc.Block: - lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, psOut)) case len(s.Exprs) == 0: lines = append(lines, idt+"return;") default: lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, expr(&s.Exprs[0]))) } case shaderir.Discard: - lines = append(lines, idt+"discard;") + // 'discard' is invoked only in the fragment shader entry point. + lines = append(lines, idt+"discard;", idt+"return float4(0.0, 0.0, 0.0, 0.0);") default: lines = append(lines, fmt.Sprintf("%s?(unexpected stmt: %d)", idt, s.Type)) } diff --git a/internal/shaderir/ir_test.go b/internal/shaderir/ir_test.go index 0faaf4b1a..6b2874c98 100644 --- a/internal/shaderir/ir_test.go +++ b/internal/shaderir/ir_test.go @@ -985,7 +985,7 @@ varying vec2 V1;`, {Main: shaderir.Float}, {Main: shaderir.Vec2}, }, - 3+1, + 3, assignStmt( localVariableExpr(3), localVariableExpr(0), @@ -994,8 +994,7 @@ varying vec2 V1;`, localVariableExpr(4), localVariableExpr(1), ), - assignStmt( - localVariableExpr(5), + returnStmt( localVariableExpr(2), ), ), @@ -1019,20 +1018,18 @@ uniform float U0; varying float V0; varying vec2 V1; -void F0(in vec4 l0, in float l1, in vec2 l2, out vec4 l3); +vec4 F0(in vec4 l0, in float l1, in vec2 l2); -void F0(in vec4 l0, in float l1, in vec2 l2, out vec4 l3) { - float l4 = float(0); - vec2 l5 = vec2(0); +vec4 F0(in vec4 l0, in float l1, in vec2 l2) { + float l3 = float(0); + vec2 l4 = vec2(0); l3 = l0; l4 = l1; - l5 = l2; + return l2; } void main(void) { - vec4 l0 = vec4(0); - F0(gl_FragCoord, V0, V1, l0); - gl_FragColor = l0; + gl_FragColor = F0(gl_FragCoord, V0, V1); }`, }, } diff --git a/internal/shaderir/msl/msl.go b/internal/shaderir/msl/msl.go index dbffbcaa0..19572a150 100644 --- a/internal/shaderir/msl/msl.go +++ b/internal/shaderir/msl/msl.go @@ -25,8 +25,7 @@ import ( ) const ( - vertexOut = "varyings" - fragmentOut = "out" + vertexOut = "varyings" ) type compileContext struct { @@ -137,11 +136,7 @@ func Compile(p *shaderir.Program, vertex, fragment string) (shader string) { lines = append(lines, fmt.Sprintf("\ttexture2d T%[1]d [[texture(%[1]d)]]", i)) } lines[len(lines)-1] += ") {" - lines = append(lines, fmt.Sprintf("\tfloat4 %s = float4(0);", fragmentOut)) lines = append(lines, c.block(p, p.FragmentFunc.Block, p.FragmentFunc.Block, 0)...) - if last := fmt.Sprintf("\treturn %s;", fragmentOut); lines[len(lines)-1] != last { - lines = append(lines, last) - } lines = append(lines, "}") } @@ -306,10 +301,8 @@ func localVariableName(p *shaderir.Program, topBlock *shaderir.Block, idx int) s return fmt.Sprintf("%s.Position", vertexOut) case idx < nv+1: return fmt.Sprintf("%s.M%d", vertexOut, idx-1) - case idx == nv+1: - return fragmentOut default: - return fmt.Sprintf("l%d", idx-(nv+2)) + return fmt.Sprintf("l%d", idx-(nv+1)) } default: return fmt.Sprintf("l%d", idx) @@ -495,15 +488,14 @@ func (c *compileContext) block(p *shaderir.Program, topBlock, block *shaderir.Bl switch { case topBlock == p.VertexFunc.Block: lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, vertexOut)) - case topBlock == p.FragmentFunc.Block: - lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, fragmentOut)) case len(s.Exprs) == 0: lines = append(lines, idt+"return;") default: lines = append(lines, fmt.Sprintf("%sreturn %s;", idt, expr(&s.Exprs[0]))) } case shaderir.Discard: - lines = append(lines, idt+"discard_fragment();") + // 'discard' is invoked only in the fragment shader entry point. + lines = append(lines, idt+"discard_fragment();", idt+"return float4(0.0);") default: lines = append(lines, fmt.Sprintf("%s?(unexpected stmt: %d)", idt, s.Type)) } diff --git a/internal/shaderir/program.go b/internal/shaderir/program.go index ff416c6ec..a78309abd 100644 --- a/internal/shaderir/program.go +++ b/internal/shaderir/program.go @@ -52,7 +52,6 @@ type VertexFunc struct { // FragmentFunc takes pseudo params, and the number is len(varyings) + 2. // If index == 0, the param represents the coordinate of the fragment (gl_FragCoord in GLSL). // If 0 < index <= len(varyings), the param represents (index-1)th verying variable. -// If index == len(varyings)+1, the param is an out-param representing the color of the pixel (gl_FragColor in GLSL). type FragmentFunc struct { Block *Block } diff --git a/internal/shaderir/type.go b/internal/shaderir/type.go index d4b2a35cb..b177412eb 100644 --- a/internal/shaderir/type.go +++ b/internal/shaderir/type.go @@ -193,10 +193,8 @@ func (p *Program) LocalVariableType(topBlock, block *Block, idx int) Type { return Type{Main: Vec4} case idx < nv+1: return p.Varyings[idx-1] - case idx == nv+1: - return Type{Main: Vec4} default: - return localVariableType(p, topBlock, block, idx-(nv+2)) + return localVariableType(p, topBlock, block, idx-(nv+1)) } default: return localVariableType(p, topBlock, block, idx)