From 5734d2f283ac3f015f1192a8698e0b24cbae7e41 Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Sat, 19 Apr 2025 10:07:20 +0200 Subject: [PATCH 1/2] [naga glsl-out] Differentiate between support for `std140` and `std430` layout, and emit `std140` in Uniforms when possible --- CHANGELOG.md | 4 + naga/src/back/glsl/mod.rs | 86 +++++++++++++------ .../out/glsl/wgsl-access.foo_vert.Vertex.glsl | 4 +- .../out/glsl/wgsl-boids.main.Compute.glsl | 2 +- .../out/glsl/wgsl-globals.main.Compute.glsl | 10 +-- .../out/glsl/wgsl-padding.vertex.Vertex.glsl | 6 +- .../wgsl-phony_assignment.main.Compute.glsl | 2 +- .../glsl/wgsl-shadow.fs_main.Fragment.glsl | 4 +- ...adow.fs_main_without_storage.Fragment.glsl | 6 +- .../out/glsl/wgsl-shadow.vs_main.Vertex.glsl | 4 +- ...uct-layout.needs_padding_comp.Compute.glsl | 2 +- ...struct-layout.no_padding_comp.Compute.glsl | 2 +- 12 files changed, 85 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afbbcdab41b..3d9b5cceb4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,10 @@ Bottom level categories: - Add support for rendering to slices of 3D texture views and single layered 2D-Array texture views (this requires `VK_KHR_maintenance1` which should be widely available on newer drivers). By @teoxoy in [#7596](https://github.com/gfx-rs/wgpu/pull/7596) +#### Naga + +- When emitting GLSL, Uniform and Storage Buffer memory layouts are now emitted even if no explicit binding is given. By @cloone8 in [#7579](https://github.com/gfx-rs/wgpu/pull/7579). + ### Bug Fixes #### Naga diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index de9ca1793cc..83d56401a9c 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -211,6 +211,10 @@ impl Version { *self >= Version::Desktop(130) || *self >= Version::new_gles(310) } + fn supports_std140_layout(&self) -> bool { + *self >= Version::Desktop(140) || *self >= Version::new_gles(300) + } + fn supports_std430_layout(&self) -> bool { *self >= Version::Desktop(430) || *self >= Version::new_gles(310) } @@ -1186,47 +1190,77 @@ impl<'a, W: Write> Writer<'a, W> { Ok(()) } - /// Helper method used to write non images/sampler globals + /// Helper method used by [Self::write_global] to write just the layout part of + /// a non image/sampler global variable, if applicable. /// /// # Notes - /// Adds a newline /// - /// # Panics - /// If the global has type sampler - fn write_global( - &mut self, - handle: Handle, - global: &crate::GlobalVariable, - ) -> BackendResult { + /// Adds trailing whitespace if any layout qualifier is written + fn write_global_layout(&mut self, global: &crate::GlobalVariable) -> BackendResult { + // Determine which of the `std` layouts we can use (if any) + let layout = match global.space { + crate::AddressSpace::Uniform => { + if self.options.version.supports_std140_layout() { + Some("std140") + } else { + None + } + } + crate::AddressSpace::Storage { .. } => { + if self.options.version.supports_std430_layout() { + Some("std430") + } else if self.options.version.supports_std140_layout() { + Some("std140") + } else { + None + } + } + _ => None, + }; + + // If our version supports explicit layouts, we can also output the explicit binding + // if we have it if self.options.version.supports_explicit_locations() { if let Some(ref br) = global.binding { match self.options.binding_map.get(br) { Some(binding) => { - let layout = match global.space { - crate::AddressSpace::Storage { .. } => { - if self.options.version.supports_std430_layout() { - "std430, " - } else { - "std140, " - } - } - crate::AddressSpace::Uniform => "std140, ", - _ => "", - }; - write!(self.out, "layout({layout}binding = {binding}) ")? + let layout_padded = + layout.map(|l| format!("{}, ", l)).unwrap_or("".to_string()); + + write!(self.out, "layout({layout_padded}binding = {binding}) ")?; + + return Ok(()); } None => { log::debug!("unassigned binding for {:?}", global.name); - if let crate::AddressSpace::Storage { .. } = global.space { - if self.options.version.supports_std430_layout() { - write!(self.out, "layout(std430) ")? - } - } } } } } + // Either no explicit bindings are supported or we didn't have any. + // Write just the memory layout. + if let Some(layout) = layout { + write!(self.out, "layout({}) ", layout)?; + } + + Ok(()) + } + + /// Helper method used to write non images/sampler globals + /// + /// # Notes + /// Adds a newline + /// + /// # Panics + /// If the global has type sampler + fn write_global( + &mut self, + handle: Handle, + global: &crate::GlobalVariable, + ) -> BackendResult { + self.write_global_layout(global)?; + if let crate::AddressSpace::Storage { access } = global.space { self.write_storage_access(access)?; } diff --git a/naga/tests/out/glsl/wgsl-access.foo_vert.Vertex.glsl b/naga/tests/out/glsl/wgsl-access.foo_vert.Vertex.glsl index c710043d23a..5158a3dab71 100644 --- a/naga/tests/out/glsl/wgsl-access.foo_vert.Vertex.glsl +++ b/naga/tests/out/glsl/wgsl-access.foo_vert.Vertex.glsl @@ -39,11 +39,11 @@ layout(std430) buffer Bar_block_0Vertex { AlignedWrapper data[]; } _group_0_binding_0_vs; -uniform Baz_block_1Vertex { Baz _group_0_binding_1_vs; }; +layout(std140) uniform Baz_block_1Vertex { Baz _group_0_binding_1_vs; }; layout(std430) buffer type_13_block_2Vertex { ivec2 _group_0_binding_2_vs; }; -uniform MatCx2InArray_block_3Vertex { MatCx2InArray _group_0_binding_3_vs; }; +layout(std140) uniform MatCx2InArray_block_3Vertex { MatCx2InArray _group_0_binding_3_vs; }; void test_matrix_within_struct_accesses() { diff --git a/naga/tests/out/glsl/wgsl-boids.main.Compute.glsl b/naga/tests/out/glsl/wgsl-boids.main.Compute.glsl index c42358bfef9..ac46f1404d5 100644 --- a/naga/tests/out/glsl/wgsl-boids.main.Compute.glsl +++ b/naga/tests/out/glsl/wgsl-boids.main.Compute.glsl @@ -20,7 +20,7 @@ struct SimParams { }; const uint NUM_PARTICLES = 1500u; -uniform SimParams_block_0Compute { SimParams _group_0_binding_0_cs; }; +layout(std140) uniform SimParams_block_0Compute { SimParams _group_0_binding_0_cs; }; layout(std430) readonly buffer Particles_block_1Compute { Particle particles[]; diff --git a/naga/tests/out/glsl/wgsl-globals.main.Compute.glsl b/naga/tests/out/glsl/wgsl-globals.main.Compute.glsl index bc20d6415f5..a97c90716c6 100644 --- a/naga/tests/out/glsl/wgsl-globals.main.Compute.glsl +++ b/naga/tests/out/glsl/wgsl-globals.main.Compute.glsl @@ -19,15 +19,15 @@ layout(std430) buffer FooStruct_block_0Compute { FooStruct _group_0_binding_1_cs layout(std430) readonly buffer type_6_block_1Compute { vec2 _group_0_binding_2_cs[]; }; -uniform type_8_block_2Compute { vec4 _group_0_binding_3_cs[20]; }; +layout(std140) uniform type_8_block_2Compute { vec4 _group_0_binding_3_cs[20]; }; -uniform type_4_block_3Compute { vec3 _group_0_binding_4_cs; }; +layout(std140) uniform type_4_block_3Compute { vec3 _group_0_binding_4_cs; }; -uniform type_9_block_4Compute { mat3x2 _group_0_binding_5_cs; }; +layout(std140) uniform type_9_block_4Compute { mat3x2 _group_0_binding_5_cs; }; -uniform type_12_block_5Compute { mat2x4 _group_0_binding_6_cs[2][2]; }; +layout(std140) uniform type_12_block_5Compute { mat2x4 _group_0_binding_6_cs[2][2]; }; -uniform type_15_block_6Compute { mat4x2 _group_0_binding_7_cs[2][2]; }; +layout(std140) uniform type_15_block_6Compute { mat4x2 _group_0_binding_7_cs[2][2]; }; void test_msl_packed_vec3_as_arg(vec3 arg) { diff --git a/naga/tests/out/glsl/wgsl-padding.vertex.Vertex.glsl b/naga/tests/out/glsl/wgsl-padding.vertex.Vertex.glsl index 82d8d31fa9f..87749643381 100644 --- a/naga/tests/out/glsl/wgsl-padding.vertex.Vertex.glsl +++ b/naga/tests/out/glsl/wgsl-padding.vertex.Vertex.glsl @@ -18,11 +18,11 @@ struct Test3_ { mat4x3 a; float b; }; -uniform Test_block_0Vertex { Test _group_0_binding_0_vs; }; +layout(std140) uniform Test_block_0Vertex { Test _group_0_binding_0_vs; }; -uniform Test2_block_1Vertex { Test2_ _group_0_binding_1_vs; }; +layout(std140) uniform Test2_block_1Vertex { Test2_ _group_0_binding_1_vs; }; -uniform Test3_block_2Vertex { Test3_ _group_0_binding_2_vs; }; +layout(std140) uniform Test3_block_2Vertex { Test3_ _group_0_binding_2_vs; }; void main() { diff --git a/naga/tests/out/glsl/wgsl-phony_assignment.main.Compute.glsl b/naga/tests/out/glsl/wgsl-phony_assignment.main.Compute.glsl index 1bf2a8f3944..73e11b697fb 100644 --- a/naga/tests/out/glsl/wgsl-phony_assignment.main.Compute.glsl +++ b/naga/tests/out/glsl/wgsl-phony_assignment.main.Compute.glsl @@ -5,7 +5,7 @@ precision highp int; layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; -uniform type_block_0Compute { float _group_0_binding_0_cs; }; +layout(std140) uniform type_block_0Compute { float _group_0_binding_0_cs; }; int five() { diff --git a/naga/tests/out/glsl/wgsl-shadow.fs_main.Fragment.glsl b/naga/tests/out/glsl/wgsl-shadow.fs_main.Fragment.glsl index ab7214380f1..fc0b13634d4 100644 --- a/naga/tests/out/glsl/wgsl-shadow.fs_main.Fragment.glsl +++ b/naga/tests/out/glsl/wgsl-shadow.fs_main.Fragment.glsl @@ -24,9 +24,9 @@ struct Light { const vec3 c_ambient = vec3(0.05, 0.05, 0.05); const uint c_max_lights = 10u; -uniform Globals_block_0Fragment { Globals _group_0_binding_0_fs; }; +layout(std140) uniform Globals_block_0Fragment { Globals _group_0_binding_0_fs; }; -uniform Entity_block_1Fragment { Entity _group_1_binding_0_fs; }; +layout(std140) uniform Entity_block_1Fragment { Entity _group_1_binding_0_fs; }; layout(std430) readonly buffer type_8_block_2Fragment { Light _group_0_binding_1_fs[]; }; diff --git a/naga/tests/out/glsl/wgsl-shadow.fs_main_without_storage.Fragment.glsl b/naga/tests/out/glsl/wgsl-shadow.fs_main_without_storage.Fragment.glsl index a9fcf31d560..b9ac10e6ee1 100644 --- a/naga/tests/out/glsl/wgsl-shadow.fs_main_without_storage.Fragment.glsl +++ b/naga/tests/out/glsl/wgsl-shadow.fs_main_without_storage.Fragment.glsl @@ -24,11 +24,11 @@ struct Light { const vec3 c_ambient = vec3(0.05, 0.05, 0.05); const uint c_max_lights = 10u; -uniform Globals_block_0Fragment { Globals _group_0_binding_0_fs; }; +layout(std140) uniform Globals_block_0Fragment { Globals _group_0_binding_0_fs; }; -uniform Entity_block_1Fragment { Entity _group_1_binding_0_fs; }; +layout(std140) uniform Entity_block_1Fragment { Entity _group_1_binding_0_fs; }; -uniform type_9_block_2Fragment { Light _group_0_binding_1_fs[10]; }; +layout(std140) uniform type_9_block_2Fragment { Light _group_0_binding_1_fs[10]; }; uniform highp sampler2DArrayShadow _group_0_binding_2_fs; diff --git a/naga/tests/out/glsl/wgsl-shadow.vs_main.Vertex.glsl b/naga/tests/out/glsl/wgsl-shadow.vs_main.Vertex.glsl index 631c412f2a3..95e365409b3 100644 --- a/naga/tests/out/glsl/wgsl-shadow.vs_main.Vertex.glsl +++ b/naga/tests/out/glsl/wgsl-shadow.vs_main.Vertex.glsl @@ -24,9 +24,9 @@ struct Light { const vec3 c_ambient = vec3(0.05, 0.05, 0.05); const uint c_max_lights = 10u; -uniform Globals_block_0Vertex { Globals _group_0_binding_0_vs; }; +layout(std140) uniform Globals_block_0Vertex { Globals _group_0_binding_0_vs; }; -uniform Entity_block_1Vertex { Entity _group_1_binding_0_vs; }; +layout(std140) uniform Entity_block_1Vertex { Entity _group_1_binding_0_vs; }; layout(location = 0) in ivec4 _p2vs_location0; layout(location = 1) in ivec4 _p2vs_location1; diff --git a/naga/tests/out/glsl/wgsl-struct-layout.needs_padding_comp.Compute.glsl b/naga/tests/out/glsl/wgsl-struct-layout.needs_padding_comp.Compute.glsl index 7fe97f29829..487405f823a 100644 --- a/naga/tests/out/glsl/wgsl-struct-layout.needs_padding_comp.Compute.glsl +++ b/naga/tests/out/glsl/wgsl-struct-layout.needs_padding_comp.Compute.glsl @@ -14,7 +14,7 @@ struct NeedsPadding { vec3 v3_needs_padding; float f3_; }; -uniform NeedsPadding_block_0Compute { NeedsPadding _group_0_binding_2_cs; }; +layout(std140) uniform NeedsPadding_block_0Compute { NeedsPadding _group_0_binding_2_cs; }; layout(std430) buffer NeedsPadding_block_1Compute { NeedsPadding _group_0_binding_3_cs; }; diff --git a/naga/tests/out/glsl/wgsl-struct-layout.no_padding_comp.Compute.glsl b/naga/tests/out/glsl/wgsl-struct-layout.no_padding_comp.Compute.glsl index c7bbcf7b70a..e318fb785ea 100644 --- a/naga/tests/out/glsl/wgsl-struct-layout.no_padding_comp.Compute.glsl +++ b/naga/tests/out/glsl/wgsl-struct-layout.no_padding_comp.Compute.glsl @@ -14,7 +14,7 @@ struct NeedsPadding { vec3 v3_needs_padding; float f3_; }; -uniform NoPadding_block_0Compute { NoPadding _group_0_binding_0_cs; }; +layout(std140) uniform NoPadding_block_0Compute { NoPadding _group_0_binding_0_cs; }; layout(std430) buffer NoPadding_block_1Compute { NoPadding _group_0_binding_1_cs; }; From ad3e487584b3e34a51116d65f441fed813c2f5e3 Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Sat, 26 Apr 2025 13:21:54 +0200 Subject: [PATCH 2/2] [naga glsl-out] Remove storage buffer std140 layout fallback, and error when we are unable to assign an explicit memory layout for uniform and storage globals Co-authored-by: teoxoy <28601907+teoxoy@users.noreply.github.com> --- naga/src/back/glsl/mod.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index 83d56401a9c..cfb8260b984 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -1197,23 +1197,25 @@ impl<'a, W: Write> Writer<'a, W> { /// /// Adds trailing whitespace if any layout qualifier is written fn write_global_layout(&mut self, global: &crate::GlobalVariable) -> BackendResult { - // Determine which of the `std` layouts we can use (if any) + // Determine which (if any) explicit memory layout to use, and whether we support it let layout = match global.space { crate::AddressSpace::Uniform => { - if self.options.version.supports_std140_layout() { - Some("std140") - } else { - None + if !self.options.version.supports_std140_layout() { + return Err(Error::Custom( + "Uniform address space requires std140 layout support".to_string(), + )); } + + Some("std140") } crate::AddressSpace::Storage { .. } => { - if self.options.version.supports_std430_layout() { - Some("std430") - } else if self.options.version.supports_std140_layout() { - Some("std140") - } else { - None + if !self.options.version.supports_std430_layout() { + return Err(Error::Custom( + "Storage address space requires std430 layout support".to_string(), + )); } + + Some("std430") } _ => None, }; @@ -1224,10 +1226,13 @@ impl<'a, W: Write> Writer<'a, W> { if let Some(ref br) = global.binding { match self.options.binding_map.get(br) { Some(binding) => { - let layout_padded = - layout.map(|l| format!("{}, ", l)).unwrap_or("".to_string()); + write!(self.out, "layout(")?; + + if let Some(layout) = layout { + write!(self.out, "{}, ", layout)?; + } - write!(self.out, "layout({layout_padded}binding = {binding}) ")?; + write!(self.out, "binding = {binding}) ")?; return Ok(()); }