From f59b0341538307e4a217a1f68aae855a2edcf4a1 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Tue, 4 Mar 2025 17:36:36 -0800 Subject: [PATCH] [naga] Centralize naming of anonymous entry point return types. When an entry point's return type is anonymous, have `naga::proc::Namer` assign it a name based on its shader stage. Remove bespoke logic for this from the WGSL backend. Fixes #7263. --- naga/src/back/wgsl/writer.rs | 40 ++----------------- naga/src/proc/namer.rs | 30 +++++++++++++- .../in/glsl/anonymous-entry-point-type.frag | 9 +++++ .../tests/out/glsl/quad-vert.main.Vertex.glsl | 4 +- naga/tests/out/hlsl/quad-vert.hlsl | 12 +++--- naga/tests/out/msl/quad-vert.msl | 4 +- .../wgsl/anonymous-entry-point-type.frag.wgsl | 21 ++++++++++ 7 files changed, 73 insertions(+), 47 deletions(-) create mode 100644 naga/tests/in/glsl/anonymous-entry-point-type.frag create mode 100644 naga/tests/out/wgsl/anonymous-entry-point-type.frag.wgsl diff --git a/naga/src/back/wgsl/writer.rs b/naga/src/back/wgsl/writer.rs index c12a6e6ee6..4fd0493f91 100644 --- a/naga/src/back/wgsl/writer.rs +++ b/naga/src/back/wgsl/writer.rs @@ -75,7 +75,6 @@ pub struct Writer { names: crate::FastHashMap, namer: proc::Namer, named_expressions: crate::NamedExpressions, - ep_results: Vec<(ShaderStage, Handle)>, required_polyfills: crate::FastIndexSet, } @@ -87,7 +86,6 @@ impl Writer { names: crate::FastHashMap::default(), namer: proc::Namer::default(), named_expressions: crate::NamedExpressions::default(), - ep_results: vec![], required_polyfills: crate::FastIndexSet::default(), } } @@ -104,7 +102,6 @@ impl Writer { &mut self.names, ); self.named_expressions.clear(); - self.ep_results.clear(); self.required_polyfills.clear(); } @@ -125,13 +122,6 @@ impl Writer { self.reset(module); - // Save all ep result types - for ep in &module.entry_points { - if let Some(ref result) = ep.function.result { - self.ep_results.push((ep.stage, result.ty)); - } - } - // Write all structs for (handle, ty) in module.types.iter() { if let TypeInner::Struct { ref members, .. } = ty.inner { @@ -224,29 +214,6 @@ impl Writer { Ok(()) } - /// Helper method used to write struct name - /// - /// # Notes - /// Adds no trailing or leading whitespace - fn write_struct_name(&mut self, module: &Module, handle: Handle) -> BackendResult { - if module.types[handle].name.is_none() { - if let Some(&(stage, _)) = self.ep_results.iter().find(|&&(_, ty)| ty == handle) { - let name = match stage { - ShaderStage::Compute => "ComputeOutput", - ShaderStage::Fragment => "FragmentOutput", - ShaderStage::Vertex => "VertexOutput", - }; - - write!(self.out, "{name}")?; - return Ok(()); - } - } - - write!(self.out, "{}", self.names[&NameKey::Type(handle)])?; - - Ok(()) - } - /// Helper method used to write /// [functions](https://gpuweb.github.io/gpuweb/wgsl/#functions) /// @@ -407,8 +374,7 @@ impl Writer { handle: Handle, members: &[crate::StructMember], ) -> BackendResult { - write!(self.out, "struct ")?; - self.write_struct_name(module, handle)?; + write!(self.out, "struct {}", self.names[&NameKey::Type(handle)])?; write!(self.out, " {{")?; writeln!(self.out)?; for (index, member) in members.iter().enumerate() { @@ -439,7 +405,9 @@ impl Writer { fn write_type(&mut self, module: &Module, ty: Handle) -> BackendResult { let inner = &module.types[ty].inner; match *inner { - TypeInner::Struct { .. } => self.write_struct_name(module, ty)?, + TypeInner::Struct { .. } => { + write!(self.out, "{}", self.names[&NameKey::Type(ty)])?; + } ref other => self.write_value_type(module, other)?, } diff --git a/naga/src/proc/namer.rs b/naga/src/proc/namer.rs index 8550d7b54b..53bb39ecab 100644 --- a/naga/src/proc/namer.rs +++ b/naga/src/proc/namer.rs @@ -181,10 +181,38 @@ impl Namer { .map(|string| (AsciiUniCase(*string))), ); + // Choose fallback names for anonymous entry point return types. + let mut entrypoint_type_fallbacks = FastHashMap::default(); + for ep in &module.entry_points { + if let Some(ref result) = ep.function.result { + if let crate::Type { + name: None, + inner: crate::TypeInner::Struct { .. }, + } = module.types[result.ty] + { + let label = match ep.stage { + crate::ShaderStage::Vertex => "VertexOutput", + crate::ShaderStage::Fragment => "FragmentOutput", + crate::ShaderStage::Compute => "ComputeOutput", + }; + entrypoint_type_fallbacks.insert(result.ty, label); + } + } + } + let mut temp = String::new(); for (ty_handle, ty) in module.types.iter() { - let ty_name = self.call_or(&ty.name, "type"); + // If the type is anonymous, check `entrypoint_types` for + // something better than just `"type"`. + let raw_label = match ty.name { + Some(ref given_name) => given_name.as_str(), + None => entrypoint_type_fallbacks + .get(&ty_handle) + .cloned() + .unwrap_or("type"), + }; + let ty_name = self.call(raw_label); output.insert(NameKey::Type(ty_handle), ty_name); if let crate::TypeInner::Struct { ref members, .. } = ty.inner { diff --git a/naga/tests/in/glsl/anonymous-entry-point-type.frag b/naga/tests/in/glsl/anonymous-entry-point-type.frag new file mode 100644 index 0000000000..a070b47dd1 --- /dev/null +++ b/naga/tests/in/glsl/anonymous-entry-point-type.frag @@ -0,0 +1,9 @@ +layout(location = 0) out vec4 o_Target; + +struct FragmentOutput { + float x; +}; + +void main() { + o_Target = vec4(0.0); +} diff --git a/naga/tests/out/glsl/quad-vert.main.Vertex.glsl b/naga/tests/out/glsl/quad-vert.main.Vertex.glsl index 4a48679668..847b89e3bf 100644 --- a/naga/tests/out/glsl/quad-vert.main.Vertex.glsl +++ b/naga/tests/out/glsl/quad-vert.main.Vertex.glsl @@ -9,7 +9,7 @@ struct gen_gl_PerVertex { float gen_gl_ClipDistance[1]; float gen_gl_CullDistance[1]; }; -struct type_4 { +struct VertexOutput { vec2 member; vec4 gen_gl_Position; }; @@ -41,7 +41,7 @@ void main() { main_1(); vec2 _e7 = v_uv; vec4 _e8 = unnamed.gen_gl_Position; - type_4 _tmp_return = type_4(_e7, _e8); + VertexOutput _tmp_return = VertexOutput(_e7, _e8); _vs2fs_location0 = _tmp_return.member; gl_Position = _tmp_return.gen_gl_Position; gl_Position.yz = vec2(-gl_Position.y, gl_Position.z * 2.0 - gl_Position.w); diff --git a/naga/tests/out/hlsl/quad-vert.hlsl b/naga/tests/out/hlsl/quad-vert.hlsl index 20834db423..9b383d5811 100644 --- a/naga/tests/out/hlsl/quad-vert.hlsl +++ b/naga/tests/out/hlsl/quad-vert.hlsl @@ -6,7 +6,7 @@ struct gl_PerVertex { int _end_pad_0; }; -struct type_4 { +struct VertexOutput { float2 member : LOC0; float4 gl_Position : SV_Position; }; @@ -44,8 +44,8 @@ void main_1() return; } -type_4 Constructtype_4(float2 arg0, float4 arg1) { - type_4 ret = (type_4)0; +VertexOutput ConstructVertexOutput(float2 arg0, float4 arg1) { + VertexOutput ret = (VertexOutput)0; ret.member = arg0; ret.gl_Position = arg1; return ret; @@ -58,7 +58,7 @@ VertexOutput_main main(float2 a_uv : LOC1, float2 a_pos : LOC0) main_1(); float2 _e7 = v_uv; float4 _e8 = unnamed.gl_Position; - const type_4 type_4_ = Constructtype_4(_e7, _e8); - const VertexOutput_main type_4_1 = { type_4_.member, type_4_.gl_Position }; - return type_4_1; + const VertexOutput vertexoutput = ConstructVertexOutput(_e7, _e8); + const VertexOutput_main vertexoutput_1 = { vertexoutput.member, vertexoutput.gl_Position }; + return vertexoutput_1; } diff --git a/naga/tests/out/msl/quad-vert.msl b/naga/tests/out/msl/quad-vert.msl index e70df2260d..e96b906fdf 100644 --- a/naga/tests/out/msl/quad-vert.msl +++ b/naga/tests/out/msl/quad-vert.msl @@ -13,7 +13,7 @@ struct gl_PerVertex { type_3 gl_ClipDistance; type_3 gl_CullDistance; }; -struct type_4 { +struct VertexOutput { metal::float2 member; metal::float4 gl_Position; }; @@ -53,6 +53,6 @@ vertex main_Output main_( main_1(v_uv, a_uv_1, unnamed, a_pos_1); metal::float2 _e7 = v_uv; metal::float4 _e8 = unnamed.gl_Position; - const auto _tmp = type_4 {_e7, _e8}; + const auto _tmp = VertexOutput {_e7, _e8}; return main_Output { _tmp.member, _tmp.gl_Position }; } diff --git a/naga/tests/out/wgsl/anonymous-entry-point-type.frag.wgsl b/naga/tests/out/wgsl/anonymous-entry-point-type.frag.wgsl new file mode 100644 index 0000000000..5778897d4c --- /dev/null +++ b/naga/tests/out/wgsl/anonymous-entry-point-type.frag.wgsl @@ -0,0 +1,21 @@ +struct FragmentOutput { + x: f32, +} + +struct FragmentOutput_1 { + @location(0) o_Target: vec4, +} + +var o_Target: vec4; + +fn main_1() { + o_Target = vec4(0f); + return; +} + +@fragment +fn main() -> FragmentOutput_1 { + main_1(); + let _e1 = o_Target; + return FragmentOutput_1(_e1); +}