From 2c35755104c41e47284256abc063d1c6c558f7da Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 4 Sep 2023 21:38:44 -0400 Subject: [PATCH 1/3] wgpu-core: Only produce StageError::InputNotConsumed on DX11/DX12 This error only exists due to an issue with naga's HLSL support: https://github.com/gfx-rs/naga/issues/1945 The WGPU spec itself allows vertex shader outputs that are not consumed by the fragment shader. Until the issue is fixed, we can allow unconsumed outputs on all platforms other than DX11/DX12. --- CHANGELOG.md | 1 + wgpu-core/src/device/resource.rs | 2 +- wgpu-core/src/validation.rs | 15 +++++++++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a54939a25..6526dcbdb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,7 @@ By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) - Derive storage bindings via `naga::StorageAccess` instead of `naga::GlobalUse`. By @teoxoy in [#3985](https://github.com/gfx-rs/wgpu/pull/3985). - `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036). - Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057). +- Only produce `StageError::InputNotConsumed` on DX11/DX12. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116). #### Vulkan - Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772). diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index ba7006d3d5..491bad900e 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1290,7 +1290,7 @@ impl Device { inner: Box::new(inner), }) })?; - let interface = validation::Interface::new(&module, &info, self.limits.clone()); + let interface = validation::Interface::new(&module, &info, self.limits.clone(), A::VARIANT); let hal_shader = hal::ShaderInput::Naga(hal::NagaShader { module, info }); let hal_desc = hal::ShaderModuleDescriptor { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 778cc26cd5..5c8c4df8ef 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -122,6 +122,7 @@ struct EntryPoint { #[derive(Debug)] pub struct Interface { limits: wgt::Limits, + backend: wgt::Backend, resources: naga::Arena, entry_points: FastHashMap<(naga::ShaderStage, String), EntryPoint>, } @@ -831,7 +832,12 @@ impl Interface { list.push(varying); } - pub fn new(module: &naga::Module, info: &naga::valid::ModuleInfo, limits: wgt::Limits) -> Self { + pub fn new( + module: &naga::Module, + info: &naga::valid::ModuleInfo, + limits: wgt::Limits, + backend: wgt::Backend, + ) -> Self { let mut resources = naga::Arena::new(); let mut resource_mapping = FastHashMap::default(); for (var_handle, var) in module.global_variables.iter() { @@ -912,6 +918,7 @@ impl Interface { Self { limits, + backend, resources, entry_points, } @@ -1121,7 +1128,11 @@ impl Interface { } // Check all vertex outputs and make sure the fragment shader consumes them. - if shader_stage == naga::ShaderStage::Fragment { + // This is only needed for HLSL shaders (DX11 and DX12) due to a naga HLSL issue: + // https://github.com/gfx-rs/naga/issues/1945 + if shader_stage == naga::ShaderStage::Fragment + && matches!(self.backend, wgt::Backend::Dx11 | wgt::Backend::Dx12) + { for &index in inputs.keys() { // This is a linear scan, but the count should be low enough // that this should be fine. From e3a9a6c0ed4825472d631c4702514aff6286d8e2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 13 Sep 2023 19:26:39 -0400 Subject: [PATCH 2/3] Add Features::SHADER_UNUSED_VERTEX_OUTPUT to allow disabling check --- CHANGELOG.md | 2 +- deno_webgpu/lib.rs | 7 +++++++ wgpu-core/src/device/resource.rs | 3 ++- wgpu-core/src/validation.rs | 13 +++++++------ wgpu-hal/src/gles/adapter.rs | 1 + wgpu-hal/src/metal/adapter.rs | 1 + wgpu-hal/src/vulkan/adapter.rs | 1 + wgpu-types/src/lib.rs | 8 ++++++++ 8 files changed, 28 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6526dcbdb0..08d20c09fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,7 +124,7 @@ By @wumpf in [#4147](https://github.com/gfx-rs/wgpu/pull/4147) - Derive storage bindings via `naga::StorageAccess` instead of `naga::GlobalUse`. By @teoxoy in [#3985](https://github.com/gfx-rs/wgpu/pull/3985). - `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036). - Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057). -- Only produce `StageError::InputNotConsumed` on DX11/DX12. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116). +- Add `Feature::SHADER_UNUSED_VERTEX_OUTPUT` to allow unused vertex shader outputs. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116). #### Vulkan - Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772). diff --git a/deno_webgpu/lib.rs b/deno_webgpu/lib.rs index 92a6a51334..bf61e42517 100644 --- a/deno_webgpu/lib.rs +++ b/deno_webgpu/lib.rs @@ -365,6 +365,9 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> { if features.contains(wgpu_types::Features::SHADER_EARLY_DEPTH_TEST) { return_features.push("shader-early-depth-test"); } + if features.contains(wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT) { + return_features.push("shader-unused-vertex-output"); + } return_features } @@ -625,6 +628,10 @@ impl From for wgpu_types::Features { wgpu_types::Features::SHADER_EARLY_DEPTH_TEST, required_features.0.contains("shader-early-depth-test"), ); + features.set( + wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT, + required_features.0.contains("shader-unused-vertex-output"), + ); features } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 491bad900e..8acc34acf4 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -1290,7 +1290,8 @@ impl Device { inner: Box::new(inner), }) })?; - let interface = validation::Interface::new(&module, &info, self.limits.clone(), A::VARIANT); + let interface = + validation::Interface::new(&module, &info, self.limits.clone(), self.features); let hal_shader = hal::ShaderInput::Naga(hal::NagaShader { module, info }); let hal_desc = hal::ShaderModuleDescriptor { diff --git a/wgpu-core/src/validation.rs b/wgpu-core/src/validation.rs index 5c8c4df8ef..ef5c65ed00 100644 --- a/wgpu-core/src/validation.rs +++ b/wgpu-core/src/validation.rs @@ -122,7 +122,7 @@ struct EntryPoint { #[derive(Debug)] pub struct Interface { limits: wgt::Limits, - backend: wgt::Backend, + features: wgt::Features, resources: naga::Arena, entry_points: FastHashMap<(naga::ShaderStage, String), EntryPoint>, } @@ -836,7 +836,7 @@ impl Interface { module: &naga::Module, info: &naga::valid::ModuleInfo, limits: wgt::Limits, - backend: wgt::Backend, + features: wgt::Features, ) -> Self { let mut resources = naga::Arena::new(); let mut resource_mapping = FastHashMap::default(); @@ -918,7 +918,7 @@ impl Interface { Self { limits, - backend, + features, resources, entry_points, } @@ -1128,10 +1128,11 @@ impl Interface { } // Check all vertex outputs and make sure the fragment shader consumes them. - // This is only needed for HLSL shaders (DX11 and DX12) due to a naga HLSL issue: - // https://github.com/gfx-rs/naga/issues/1945 + // This requirement is removed if the `SHADER_UNUSED_VERTEX_OUTPUT` feature is enabled. if shader_stage == naga::ShaderStage::Fragment - && matches!(self.backend, wgt::Backend::Dx11 | wgt::Backend::Dx12) + && !self + .features + .contains(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT) { for &index in inputs.keys() { // This is a linear scan, but the count should be low enough diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 3dae58b7c4..cbbcf7399e 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -372,6 +372,7 @@ impl super::Adapter { ver >= (3, 2) || extensions.contains("OES_geometry_shader"), ); features.set(wgt::Features::SHADER_EARLY_DEPTH_TEST, ver >= (3, 1)); + features.set(wgt::Features::SHADER_UNUSED_VERTEX_OUTPUT, true); let gles_bcn_exts = [ "GL_EXT_texture_compression_s3tc_srgb", "GL_EXT_texture_compression_rgtc", diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index da254442bc..c4617deaa0 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -871,6 +871,7 @@ impl super::PrivateCapabilities { features.set(F::ADDRESS_MODE_CLAMP_TO_ZERO, true); features.set(F::RG11B10UFLOAT_RENDERABLE, self.format_rg11b10_all); + features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true); features } diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 78aceeeeef..b515628726 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -522,6 +522,7 @@ impl PhysicalDeviceFeatures { | vk::FormatFeatureFlags::COLOR_ATTACHMENT_BLEND, ); features.set(F::RG11B10UFLOAT_RENDERABLE, rg11b10ufloat_renderable); + features.set(F::SHADER_UNUSED_VERTEX_OUTPUT, true); (features, dl_flags) } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 59c32ec3b8..ebc4d5750f 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -792,6 +792,14 @@ bitflags::bitflags! { /// - Vulkan (with dualSrcBlend) /// - DX12 const DUAL_SOURCE_BLENDING = 1 << 63; + /// Allows vertex shaders to have outputs which are not consumed + /// by the fragment shader. + /// + /// Supported platforms: + /// - Vulkan + /// - Metal + /// - OpenGL + const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 64; } } From c31d0bc34f26e3237e2e52007279358e1b637095 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 20 Sep 2023 19:38:25 -0400 Subject: [PATCH 3/3] Pick an unused feature id --- wgpu-types/src/lib.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index ebc4d5750f..13603fc03f 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -737,6 +737,15 @@ bitflags::bitflags! { /// This is a native only feature. const VERTEX_ATTRIBUTE_64BIT = 1 << 53; + /// Allows vertex shaders to have outputs which are not consumed + /// by the fragment shader. + /// + /// Supported platforms: + /// - Vulkan + /// - Metal + /// - OpenGL + const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 54; + // 54..59 available // Shader: @@ -792,14 +801,6 @@ bitflags::bitflags! { /// - Vulkan (with dualSrcBlend) /// - DX12 const DUAL_SOURCE_BLENDING = 1 << 63; - /// Allows vertex shaders to have outputs which are not consumed - /// by the fragment shader. - /// - /// Supported platforms: - /// - Vulkan - /// - Metal - /// - OpenGL - const SHADER_UNUSED_VERTEX_OUTPUT = 1 << 64; } }