From 04a10405b3fc2c6cbab469af3d714e4209975378 Mon Sep 17 00:00:00 2001 From: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:10:00 +0100 Subject: [PATCH] [d3d12] fix being able to use `dispatchWorkgroupsIndirect` without validation (#6767) --- wgpu-core/src/device/resource.rs | 9 +- wgpu-core/src/indirect_validation.rs | 2 +- wgpu-hal/src/dx12/command.rs | 16 ++- wgpu-hal/src/dx12/device.rs | 141 ++++++++++++++------------- wgpu-hal/src/dx12/mod.rs | 2 +- wgpu-hal/src/lib.rs | 9 +- 6 files changed, 105 insertions(+), 74 deletions(-) diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 5e04327792..904b5baded 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2612,10 +2612,17 @@ impl Device { .map(|bgl| bgl.raw()) .collect::>(); + let additional_flags = if cfg!(feature = "indirect-validation") { + hal::PipelineLayoutFlags::INDIRECT_BUILTIN_UPDATE + } else { + hal::PipelineLayoutFlags::empty() + }; + let hal_desc = hal::PipelineLayoutDescriptor { label: desc.label.to_hal(self.instance_flags), flags: hal::PipelineLayoutFlags::FIRST_VERTEX_INSTANCE - | hal::PipelineLayoutFlags::NUM_WORK_GROUPS, + | hal::PipelineLayoutFlags::NUM_WORK_GROUPS + | additional_flags, bind_group_layouts: &raw_bind_group_layouts, push_constant_ranges: desc.push_constant_ranges.as_ref(), }; diff --git a/wgpu-core/src/indirect_validation.rs b/wgpu-core/src/indirect_validation.rs index 1ba3ca2362..3045965435 100644 --- a/wgpu-core/src/indirect_validation.rs +++ b/wgpu-core/src/indirect_validation.rs @@ -180,7 +180,7 @@ impl IndirectValidation { let pipeline_layout_desc = hal::PipelineLayoutDescriptor { label: None, - flags: hal::PipelineLayoutFlags::FIRST_VERTEX_INSTANCE, + flags: hal::PipelineLayoutFlags::empty(), bind_group_layouts: &[ dst_bind_group_layout.as_ref(), src_bind_group_layout.as_ref(), diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index d4c60aa528..9296a20393 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -1223,13 +1223,25 @@ impl crate::CommandEncoder for super::CommandEncoder { } unsafe fn dispatch_indirect(&mut self, buffer: &super::Buffer, offset: wgt::BufferAddress) { - self.update_root_elements(); + if self + .pass + .layout + .special_constants + .as_ref() + .and_then(|sc| sc.indirect_cmd_signatures.as_ref()) + .is_some() + { + self.update_root_elements(); + } else { + self.prepare_dispatch([0; 3]); + } + let cmd_signature = &self .pass .layout .special_constants .as_ref() - .map(|sc| &sc.cmd_signatures) + .and_then(|sc| sc.indirect_cmd_signatures.as_ref()) .unwrap_or_else(|| &self.shared.cmd_signatures) .dispatch; unsafe { diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index 94fb021780..a983a92011 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -1122,78 +1122,85 @@ impl crate::Device for super::Device { .into_device_result("Root signature creation")?; let special_constants = if let Some(root_index) = special_constants_root_index { - let constant_indirect_argument_desc = Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { - Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_CONSTANT, - Anonymous: Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC_0 { - Constant: Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC_0_1 { - RootParameterIndex: root_index, - DestOffsetIn32BitValues: 0, - Num32BitValuesToSet: 3, + let cmd_signatures = if desc + .flags + .contains(crate::PipelineLayoutFlags::INDIRECT_BUILTIN_UPDATE) + { + let constant_indirect_argument_desc = Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { + Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_CONSTANT, + Anonymous: Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC_0 { + Constant: Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC_0_1 { + RootParameterIndex: root_index, + DestOffsetIn32BitValues: 0, + Num32BitValuesToSet: 3, + }, }, - }, - }; - let special_constant_buffer_args_len = { - // Hack: construct a dummy value of the special constants buffer value we need to - // fill, and calculate the size of each member. - let super::RootElement::SpecialConstantBuffer { - first_vertex, - first_instance, - other, - } = (super::RootElement::SpecialConstantBuffer { - first_vertex: 0, - first_instance: 0, - other: 0, - }) - else { - unreachable!(); }; - size_of_val(&first_vertex) + size_of_val(&first_instance) + size_of_val(&other) - }; - let cmd_signatures = super::CommandSignatures { - draw: Self::create_command_signature( - &self.raw, - Some(&raw), - special_constant_buffer_args_len + size_of::(), - &[ - constant_indirect_argument_desc, - Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { - Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DRAW, - ..Default::default() - }, - ], - 0, - )?, - draw_indexed: Self::create_command_signature( - &self.raw, - Some(&raw), - special_constant_buffer_args_len + size_of::(), - &[ - constant_indirect_argument_desc, - Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { - Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DRAW_INDEXED, - ..Default::default() - }, - ], - 0, - )?, - dispatch: Self::create_command_signature( - &self.raw, - Some(&raw), - special_constant_buffer_args_len + size_of::(), - &[ - constant_indirect_argument_desc, - Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { - Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DISPATCH, - ..Default::default() - }, - ], - 0, - )?, + let special_constant_buffer_args_len = { + // Hack: construct a dummy value of the special constants buffer value we need to + // fill, and calculate the size of each member. + let super::RootElement::SpecialConstantBuffer { + first_vertex, + first_instance, + other, + } = (super::RootElement::SpecialConstantBuffer { + first_vertex: 0, + first_instance: 0, + other: 0, + }) + else { + unreachable!(); + }; + size_of_val(&first_vertex) + size_of_val(&first_instance) + size_of_val(&other) + }; + Some(super::CommandSignatures { + draw: Self::create_command_signature( + &self.raw, + Some(&raw), + special_constant_buffer_args_len + size_of::(), + &[ + constant_indirect_argument_desc, + Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { + Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DRAW, + ..Default::default() + }, + ], + 0, + )?, + draw_indexed: Self::create_command_signature( + &self.raw, + Some(&raw), + special_constant_buffer_args_len + + size_of::(), + &[ + constant_indirect_argument_desc, + Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { + Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DRAW_INDEXED, + ..Default::default() + }, + ], + 0, + )?, + dispatch: Self::create_command_signature( + &self.raw, + Some(&raw), + special_constant_buffer_args_len + size_of::(), + &[ + constant_indirect_argument_desc, + Direct3D12::D3D12_INDIRECT_ARGUMENT_DESC { + Type: Direct3D12::D3D12_INDIRECT_ARGUMENT_TYPE_DISPATCH, + ..Default::default() + }, + ], + 0, + )?, + }) + } else { + None }; - Some(super::PipelineLayoutSpecialConstants { root_index, - cmd_signatures, + indirect_cmd_signatures: cmd_signatures, }) } else { None diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index a8fa0891ee..e0f27119df 100644 --- a/wgpu-hal/src/dx12/mod.rs +++ b/wgpu-hal/src/dx12/mod.rs @@ -946,7 +946,7 @@ unsafe impl Sync for PipelineLayoutShared {} #[derive(Debug, Clone)] struct PipelineLayoutSpecialConstants { root_index: RootIndex, - cmd_signatures: CommandSignatures, + indirect_cmd_signatures: Option, } unsafe impl Send for PipelineLayoutSpecialConstants {} diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 901442980d..12234d6364 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -1513,10 +1513,15 @@ bitflags!( /// Pipeline layout creation flags. #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct PipelineLayoutFlags: u32 { - /// Include support for `first_vertex` / `first_instance` drawing. + /// D3D12: Add support for `first_vertex` and `first_instance` builtins + /// via push constants for direct execution. const FIRST_VERTEX_INSTANCE = 1 << 0; - /// Include support for num work groups builtin. + /// D3D12: Add support for `num_workgroups` builtins via push constants + /// for direct execution. const NUM_WORK_GROUPS = 1 << 1; + /// D3D12: Add support for the builtins that the other flags enable for + /// indirect execution. + const INDIRECT_BUILTIN_UPDATE = 1 << 2; } );