From 447e3eee8d135f46f9acc4a14b5a8197dd91aa6f Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Thu, 16 May 2024 18:44:58 -0400 Subject: [PATCH 1/2] fix: ensure render pipelines have at least 1 target --- CHANGELOG.md | 4 ++++ wgpu-core/src/device/resource.rs | 10 ++++++++++ wgpu-core/src/pipeline.rs | 5 +++++ 3 files changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f67e017bdf..e31473abcff 100644 --- 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/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 7ac3878ef8f..3fdb664621b 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -3044,8 +3044,11 @@ impl Device { ); } + 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 +3076,7 @@ impl Device { 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 +3095,7 @@ impl Device { .supported_sample_counts(), )); } + if let Some(blend_mode) = cs.blend { for factor in [ blend_mode.color.src_factor, @@ -3130,6 +3135,7 @@ impl Device { } 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 +3186,10 @@ impl Device { } } + 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 index bfb2c331d8a..ee8f8668c31 100644 --- 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! { From 18b758e3889bdd6ffa769085de15e2b96a0c1eb5 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Wed, 15 May 2024 21:46:52 -0400 Subject: [PATCH 2/2] test: ensure render pipelines have at least 1 target --- tests/tests/pipeline.rs | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/tests/pipeline.rs b/tests/tests/pipeline.rs index 0d725b8f40f..2591bf5d102 100644 --- a/tests/tests/pipeline.rs +++ b/tests/tests/pipeline.rs @@ -35,3 +35,46 @@ static PIPELINE_DEFAULT_LAYOUT_BAD_MODULE: GpuTestConfiguration = GpuTestConfigu pipeline.get_bind_group_layout(0); }); }); + +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 { return vec4(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. + });