Merge branch 'trunk' into fail-test-msg-arg

gfx-rs · May 21, 2024 · d063f25 · d063f25
2 parents abca4d5 + c7a16b3
commit d063f25
Show file tree

Hide file tree

Showing 5 changed files with 166 additions and 117 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -90,6 +90,10 @@ By @stefnotch in [#5410](https://github.com/gfx-rs/wgpu/pull/5410)
 
 ### Bug Fixes
 
+### General
+
+- Ensure render pipelines have at least 1 target. By @ErichDonGubler in [#5715](https://github.com/gfx-rs/wgpu/pull/5715)
+
 #### Vulkan
 
 - Fix enablement of subgroup ops extension on Vulkan devices that don't support Vulkan 1.3. By @cwfitzgerald in [#5624](https://github.com/gfx-rs/wgpu/pull/5624).

diff --git a/tests/tests/pipeline.rs b/tests/tests/pipeline.rs
@@ -39,3 +39,46 @@ static PIPELINE_DEFAULT_LAYOUT_BAD_MODULE: GpuTestConfiguration = GpuTestConfigu
             None,
         );
     });
+
+const TRIVIAL_VERTEX_SHADER_DESC: wgpu::ShaderModuleDescriptor = wgpu::ShaderModuleDescriptor {
+    label: Some("trivial vertex shader"),
+    source: wgpu::ShaderSource::Wgsl(std::borrow::Cow::Borrowed(
+        "@vertex fn main() -> @builtin(position) vec4<f32> { return vec4<f32>(0); }",
+    )),
+};
+
+#[gpu_test]
+static NO_TARGETLESS_RENDER: GpuTestConfiguration = GpuTestConfiguration::new()
+    .parameters(TestParameters::default())
+    .run_sync(|ctx| {
+        fail(&ctx.device, || {
+            // Testing multisampling is important, because some backends don't behave well if one
+            // tries to compile code in an unsupported multisample count. Failing to validate here
+            // has historically resulted in requesting the back end to compile code.
+            for power_of_two in [1, 2, 4, 8, 16, 32, 64] {
+                ctx.device
+                    .create_render_pipeline(&wgpu::RenderPipelineDescriptor {
+                        label: None,
+                        layout: None,
+                        vertex: wgpu::VertexState {
+                            module: &ctx.device.create_shader_module(TRIVIAL_VERTEX_SHADER_DESC),
+                            entry_point: "main",
+                            compilation_options: Default::default(),
+                            buffers: &[],
+                        },
+                        primitive: Default::default(),
+                        depth_stencil: None,
+                        multisample: wgpu::MultisampleState {
+                            count: power_of_two,
+                            ..Default::default()
+                        },
+                        fragment: None,
+                        multiview: None,
+                        cache: None,
+                    });
+            }
+        })
+        // TODO: concrete error message:
+        // At least one color attachment or depth-stencil attachment was expected, but no
+        // render target for the pipeline was specified.
+    });
diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs
@@ -444,3 +444,92 @@ impl ImplicitPipelineIds<'_> {
         }
     }
 }
+
+/// Create a validator with the given validation flags.
+pub fn create_validator(
+    features: wgt::Features,
+    downlevel: wgt::DownlevelFlags,
+    flags: naga::valid::ValidationFlags,
+) -> naga::valid::Validator {
+    use naga::valid::Capabilities as Caps;
+    let mut caps = Caps::empty();
+    caps.set(
+        Caps::PUSH_CONSTANT,
+        features.contains(wgt::Features::PUSH_CONSTANTS),
+    );
+    caps.set(Caps::FLOAT64, features.contains(wgt::Features::SHADER_F64));
+    caps.set(
+        Caps::PRIMITIVE_INDEX,
+        features.contains(wgt::Features::SHADER_PRIMITIVE_INDEX),
+    );
+    caps.set(
+        Caps::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING,
+        features
+            .contains(wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING),
+    );
+    caps.set(
+        Caps::UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+        features
+            .contains(wgt::Features::UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING),
+    );
+    // TODO: This needs a proper wgpu feature
+    caps.set(
+        Caps::SAMPLER_NON_UNIFORM_INDEXING,
+        features
+            .contains(wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING),
+    );
+    caps.set(
+        Caps::STORAGE_TEXTURE_16BIT_NORM_FORMATS,
+        features.contains(wgt::Features::TEXTURE_FORMAT_16BIT_NORM),
+    );
+    caps.set(Caps::MULTIVIEW, features.contains(wgt::Features::MULTIVIEW));
+    caps.set(
+        Caps::EARLY_DEPTH_TEST,
+        features.contains(wgt::Features::SHADER_EARLY_DEPTH_TEST),
+    );
+    caps.set(
+        Caps::SHADER_INT64,
+        features.contains(wgt::Features::SHADER_INT64),
+    );
+    caps.set(
+        Caps::MULTISAMPLED_SHADING,
+        downlevel.contains(wgt::DownlevelFlags::MULTISAMPLED_SHADING),
+    );
+    caps.set(
+        Caps::DUAL_SOURCE_BLENDING,
+        features.contains(wgt::Features::DUAL_SOURCE_BLENDING),
+    );
+    caps.set(
+        Caps::CUBE_ARRAY_TEXTURES,
+        downlevel.contains(wgt::DownlevelFlags::CUBE_ARRAY_TEXTURES),
+    );
+    caps.set(
+        Caps::SUBGROUP,
+        features.intersects(wgt::Features::SUBGROUP | wgt::Features::SUBGROUP_VERTEX),
+    );
+    caps.set(
+        Caps::SUBGROUP_BARRIER,
+        features.intersects(wgt::Features::SUBGROUP_BARRIER),
+    );
+
+    let mut subgroup_stages = naga::valid::ShaderStages::empty();
+    subgroup_stages.set(
+        naga::valid::ShaderStages::COMPUTE | naga::valid::ShaderStages::FRAGMENT,
+        features.contains(wgt::Features::SUBGROUP),
+    );
+    subgroup_stages.set(
+        naga::valid::ShaderStages::VERTEX,
+        features.contains(wgt::Features::SUBGROUP_VERTEX),
+    );
+
+    let subgroup_operations = if caps.contains(Caps::SUBGROUP) {
+        use naga::valid::SubgroupOperationSet as S;
+        S::BASIC | S::VOTE | S::ARITHMETIC | S::BALLOT | S::SHUFFLE | S::SHUFFLE_RELATIVE
+    } else {
+        naga::valid::SubgroupOperationSet::empty()
+    };
+    let mut validator = naga::valid::Validator::new(flags, caps);
+    validator.subgroup_stages(subgroup_stages);
+    validator.subgroup_operations(subgroup_operations);
+    validator
+}
diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs
@@ -4,7 +4,7 @@ use crate::{
     binding_model::{self, BindGroup, BindGroupLayout, BindGroupLayoutEntryError},
     command, conv,
     device::{
-        bgl,
+        bgl, create_validator,
         life::{LifetimeTracker, WaitIdleError},
         queue::PendingWrites,
         AttachmentData, DeviceLostInvocation, MissingDownlevelFlags, MissingFeatures,
@@ -20,7 +20,7 @@ use crate::{
     },
     instance::Adapter,
     lock::{rank, Mutex, MutexGuard, RwLock},
-    pipeline::{self},
+    pipeline,
     pool::ResourcePool,
     registry::Registry,
     resource::{
@@ -1485,16 +1485,19 @@ impl<A: HalApi> Device<A> {
                 None
             };
 
-        let info = self
-            .create_validator(naga::valid::ValidationFlags::all())
-            .validate(&module)
-            .map_err(|inner| {
-                pipeline::CreateShaderModuleError::Validation(naga::error::ShaderError {
-                    source,
-                    label: desc.label.as_ref().map(|l| l.to_string()),
-                    inner: Box::new(inner),
-                })
-            })?;
+        let info = create_validator(
+            self.features,
+            self.downlevel.flags,
+            naga::valid::ValidationFlags::all(),
+        )
+        .validate(&module)
+        .map_err(|inner| {
+            pipeline::CreateShaderModuleError::Validation(naga::error::ShaderError {
+                source,
+                label: desc.label.as_ref().map(|l| l.to_string()),
+                inner: Box::new(inner),
+            })
+        })?;
 
         let interface =
             validation::Interface::new(&module, &info, self.limits.clone(), self.features);
@@ -1536,111 +1539,6 @@ impl<A: HalApi> Device<A> {
         })
     }
 
-    /// Create a validator with the given validation flags.
-    pub fn create_validator(
-        self: &Arc<Self>,
-        flags: naga::valid::ValidationFlags,
-    ) -> naga::valid::Validator {
-        use naga::valid::Capabilities as Caps;
-        let mut caps = Caps::empty();
-        caps.set(
-            Caps::PUSH_CONSTANT,
-            self.features.contains(wgt::Features::PUSH_CONSTANTS),
-        );
-        caps.set(
-            Caps::FLOAT64,
-            self.features.contains(wgt::Features::SHADER_F64),
-        );
-        caps.set(
-            Caps::PRIMITIVE_INDEX,
-            self.features
-                .contains(wgt::Features::SHADER_PRIMITIVE_INDEX),
-        );
-        caps.set(
-            Caps::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING,
-            self.features.contains(
-                wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING,
-            ),
-        );
-        caps.set(
-            Caps::UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
-            self.features.contains(
-                wgt::Features::UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
-            ),
-        );
-        // TODO: This needs a proper wgpu feature
-        caps.set(
-            Caps::SAMPLER_NON_UNIFORM_INDEXING,
-            self.features.contains(
-                wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING,
-            ),
-        );
-        caps.set(
-            Caps::STORAGE_TEXTURE_16BIT_NORM_FORMATS,
-            self.features
-                .contains(wgt::Features::TEXTURE_FORMAT_16BIT_NORM),
-        );
-        caps.set(
-            Caps::MULTIVIEW,
-            self.features.contains(wgt::Features::MULTIVIEW),
-        );
-        caps.set(
-            Caps::EARLY_DEPTH_TEST,
-            self.features
-                .contains(wgt::Features::SHADER_EARLY_DEPTH_TEST),
-        );
-        caps.set(
-            Caps::SHADER_INT64,
-            self.features.contains(wgt::Features::SHADER_INT64),
-        );
-        caps.set(
-            Caps::MULTISAMPLED_SHADING,
-            self.downlevel
-                .flags
-                .contains(wgt::DownlevelFlags::MULTISAMPLED_SHADING),
-        );
-        caps.set(
-            Caps::DUAL_SOURCE_BLENDING,
-            self.features.contains(wgt::Features::DUAL_SOURCE_BLENDING),
-        );
-        caps.set(
-            Caps::CUBE_ARRAY_TEXTURES,
-            self.downlevel
-                .flags
-                .contains(wgt::DownlevelFlags::CUBE_ARRAY_TEXTURES),
-        );
-        caps.set(
-            Caps::SUBGROUP,
-            self.features
-                .intersects(wgt::Features::SUBGROUP | wgt::Features::SUBGROUP_VERTEX),
-        );
-        caps.set(
-            Caps::SUBGROUP_BARRIER,
-            self.features.intersects(wgt::Features::SUBGROUP_BARRIER),
-        );
-
-        let mut subgroup_stages = naga::valid::ShaderStages::empty();
-        subgroup_stages.set(
-            naga::valid::ShaderStages::COMPUTE | naga::valid::ShaderStages::FRAGMENT,
-            self.features.contains(wgt::Features::SUBGROUP),
-        );
-        subgroup_stages.set(
-            naga::valid::ShaderStages::VERTEX,
-            self.features.contains(wgt::Features::SUBGROUP_VERTEX),
-        );
-
-        let subgroup_operations = if caps.contains(Caps::SUBGROUP) {
-            use naga::valid::SubgroupOperationSet as S;
-            S::BASIC | S::VOTE | S::ARITHMETIC | S::BALLOT | S::SHUFFLE | S::SHUFFLE_RELATIVE
-        } else {
-            naga::valid::SubgroupOperationSet::empty()
-        };
-        let mut validator = naga::valid::Validator::new(flags, caps);
-        validator.subgroup_stages(subgroup_stages);
-        validator.subgroup_operations(subgroup_operations);
-        validator
-    }
-
     #[allow(unused_unsafe)]
     pub(crate) unsafe fn create_shader_module_spirv<'a>(
         self: &Arc<Self>,
@@ -3044,8 +2942,11 @@ impl<A: HalApi> Device<A> {
             );
         }
 
+        let mut target_specified = false;
+
         for (i, cs) in color_targets.iter().enumerate() {
             if let Some(cs) = cs.as_ref() {
+                target_specified = true;
                 let error = loop {
                     if cs.write_mask.contains_invalid_bits() {
                         break Some(pipeline::ColorStateError::InvalidWriteMask(cs.write_mask));
@@ -3073,6 +2974,7 @@ impl<A: HalApi> Device<A> {
                     if !hal::FormatAspects::from(cs.format).contains(hal::FormatAspects::COLOR) {
                         break Some(pipeline::ColorStateError::FormatNotColor(cs.format));
                     }
+
                     if desc.multisample.count > 1
                         && !format_features
                             .flags
@@ -3091,6 +2993,7 @@ impl<A: HalApi> Device<A> {
                                 .supported_sample_counts(),
                         ));
                     }
+
                     if let Some(blend_mode) = cs.blend {
                         for factor in [
                             blend_mode.color.src_factor,
@@ -3130,6 +3033,7 @@ impl<A: HalApi> Device<A> {
         }
 
         if let Some(ds) = depth_stencil_state {
+            target_specified = true;
             let error = loop {
                 let format_features = self.describe_format_features(adapter, ds.format)?;
                 if !format_features
@@ -3180,6 +3084,10 @@ impl<A: HalApi> Device<A> {
             }
         }
 
+        if !target_specified {
+            return Err(pipeline::CreateRenderPipelineError::NoTargetSpecified);
+        }
+
         // Get the pipeline layout from the desc if it is provided.
         let pipeline_layout = match desc.layout {
             Some(pipeline_layout_id) => {

diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs
@@ -495,6 +495,11 @@ pub enum CreateRenderPipelineError {
     PipelineExpectsShaderToUseDualSourceBlending,
     #[error("Shader entry point expects the pipeline to make use of dual-source blending.")]
     ShaderExpectsPipelineToUseDualSourceBlending,
+    #[error("{}", concat!(
+        "At least one color attachment or depth-stencil attachment was expected, ",
+        "but no render target for the pipeline was specified."
+    ))]
+    NoTargetSpecified,
 }
 
 bitflags::bitflags! {