Merge #754

754: Implement MultiDrawIndirect Extensions r=kvark a=cwfitzgerald

**Connections**

Closes #742.

**Description**

These extensions, especially when combined with binding indexing, allow the creation of extremely cpu-efficient gpu powered pipelines.

Adds two extensions allowing various types of multi-draw-indirect
- MULTI_DRAW_INDIRECT (giving `multi_draw_indirect` and `multi_draw_indexed_indirect`)
- MULTI_DRAW_INDIRECT_COUNT (giving `multi_draw_indirect_count` and `multi_draw_indexed_indirect_count`)

This adds what I believe to be an extra restriction on the `*count` family of functions when compared to the underlying api. The buffer _must_ be large enough to draw `max_count` draws, even if that many draws are never drawn. This makes these operations no more unsafe than indirect would be, which is currently safe.

I did not implement these for renderbundles, but there's no reason they couldn't work, so those branches are marked with `unimplemented` as opposed to `unreachable`.

Additional Changes:
- Added some validation to the normal `draw_*_indirect` functions to prevent buffer overruns.
- The DX12 gfx-hal backend requires the strides to _always_ be non-zero, so I modified the normal indirect draws to use explicit strides.
- Made device limits and features `pub(crate)` as they need to be checked in random places in the code.

**Testing**

The change was tested using a modified version of wgpu-rs's texture-array example using a variety of permutations. I have not been able to test regular MULTI_DRAW_INDIRECT on mac, but I see no reason why that wouldn't work.

https://github.com/gfx-rs/wgpu-rs/pull/414

Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>

Cargo.lock

@@ -58,11 +58,11 @@ dependencies = [
 
 [[package]]
 name = "ash"
-version = "0.30.0"
+version = "0.31.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "69daec0742947f33a85931fa3cb0ce5f07929159dcbd1f0cbb5b2912e2978509"
+checksum = "c69a8137596e84c22d57f3da1b5de1d4230b1742a710091c85f4d7ce50f00f38"
 dependencies = [
- "libloading 0.5.2",
+ "libloading 0.6.2",
 ]
 
 [[package]]
@@ -417,9 +417,9 @@ dependencies = [
 
 [[package]]
 name = "gfx-backend-dx12"
-version = "0.5.7"
+version = "0.5.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6e1a979a793023717bcaa7511c8cbb449bab550c093737c98674a659a2bbaf73"
+checksum = "05218b5c94539f22ac7d6feb4b2482431b89f6cc897132494701ac48619218d7"
 dependencies = [
  "bitflags",
  "d3d12",
@@ -472,9 +472,9 @@ dependencies = [
 
 [[package]]
 name = "gfx-backend-vulkan"
-version = "0.5.8"
+version = "0.5.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "04af900c2597587b35e801e9d3f91fd8078cc06067421a22caa640ad2b1bc53e"
+checksum = "5f2e8bb53e5bea0bfec7035462a75717cd04d733963a225c816339a671ef108b"
 dependencies = [
  "arrayvec",
  "ash",
@@ -502,9 +502,9 @@ dependencies = [
 
 [[package]]
 name = "gfx-hal"
-version = "0.5.2"
+version = "0.5.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f13e8fd6aaa8f50146b9519999432e097da43466749d8863c53409322c705339"
+checksum = "a18534b23d4c262916231511309bc1f307c74cda8dcb68b93a10ca213a22814b"
 dependencies = [
  "bitflags",
  "raw-window-handle",

wgpu-core/Cargo.toml

@@ -28,7 +28,7 @@ bitflags = "1.0"
 copyless = "0.1"
 fxhash = "0.2"
 log = "0.4"
-hal = { package = "gfx-hal", version = "0.5.2" }
+hal = { package = "gfx-hal", version = "0.5.3" }
 gfx-backend-empty = "0.5"
 parking_lot = "0.10"
 raw-window-handle = { version = "0.3", optional = true }
@@ -60,15 +60,15 @@ version = "0.5"
 
 [target.'cfg(any(target_os = "ios", target_os = "macos"))'.dependencies]
 gfx-backend-metal = { version = "0.5.4" }
-gfx-backend-vulkan = { version = "0.5.6", optional = true }
+gfx-backend-vulkan = { version = "0.5.9", optional = true }
 
 [target.'cfg(all(unix, not(target_os = "ios"), not(target_os = "macos")))'.dependencies]
-gfx-backend-vulkan = { version = "0.5.6" }
+gfx-backend-vulkan = { version = "0.5.9" }
 
 [target.'cfg(windows)'.dependencies]
-gfx-backend-dx12 = { version = "0.5.6" }
+gfx-backend-dx12 = { version = "0.5.8" }
 gfx-backend-dx11 = { version = "0.5" }
-gfx-backend-vulkan = { version = "0.5.8" }
+gfx-backend-vulkan = { version = "0.5.9" }
 
 [target.'cfg(any(target_os = "linux", target_os = "macos", target_os = "windows", target_os = "dragonfly", target_os = "freebsd"))'.dependencies]
 battery = { version = "0.7", optional = true }

wgpu-core/src/command/bundle.rs

@@ -203,14 +203,26 @@ impl RenderBundle {
                         first_instance..first_instance + instance_count,
                     );
                 }
-                RenderCommand::DrawIndirect { buffer_id, offset } => {
+                RenderCommand::MultiDrawIndirect {
+                    buffer_id,
+                    offset,
+                    count: None,
+                    indexed: false,
+                } => {
                     let buffer = &buffer_guard[buffer_id];
                     comb.draw_indirect(&buffer.raw, offset, 1, 0);
                 }
-                RenderCommand::DrawIndexedIndirect { buffer_id, offset } => {
+                RenderCommand::MultiDrawIndirect {
+                    buffer_id,
+                    offset,
+                    count: None,
+                    indexed: true,
+                } => {
                     let buffer = &buffer_guard[buffer_id];
                     comb.draw_indexed_indirect(&buffer.raw, offset, 1, 0);
                 }
+                RenderCommand::MultiDrawIndirect { .. }
+                | RenderCommand::MultiDrawIndirectCount { .. } => unimplemented!(),
                 RenderCommand::PushDebugGroup { color: _, len: _ } => unimplemented!(),
                 RenderCommand::InsertDebugMarker { color: _, len: _ } => unimplemented!(),
                 RenderCommand::PopDebugGroup => unimplemented!(),
@@ -652,9 +664,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         commands.extend(state.flush_binds());
                         commands.push(command);
                     }
-                    RenderCommand::DrawIndirect {
+                    RenderCommand::MultiDrawIndirect {
                         buffer_id,
                         offset: _,
+                        count: None,
+                        indexed: false,
                     } => {
                         let buffer = state
                             .trackers
@@ -671,9 +685,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         commands.extend(state.flush_binds());
                         commands.push(command);
                     }
-                    RenderCommand::DrawIndexedIndirect {
+                    RenderCommand::MultiDrawIndirect {
                         buffer_id,
                         offset: _,
+                        count: None,
+                        indexed: true,
                     } => {
                         let buffer = state
                             .trackers
@@ -691,6 +707,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         commands.extend(state.flush_binds());
                         commands.push(command);
                     }
+                    RenderCommand::MultiDrawIndirect { .. }
+                    | RenderCommand::MultiDrawIndirectCount { .. } => unimplemented!(),
                     RenderCommand::PushDebugGroup { color: _, len: _ } => unimplemented!(),
                     RenderCommand::InsertDebugMarker { color: _, len: _ } => unimplemented!(),
                     RenderCommand::PopDebugGroup => unimplemented!(),
@@ -872,10 +890,12 @@ pub mod bundle_ffi {
         offset: BufferAddress,
     ) {
         span!(_guard, DEBUG, "RenderBundle::draw_indirect");
-        bundle
-            .base
-            .commands
-            .push(RenderCommand::DrawIndirect { buffer_id, offset });
+        bundle.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: None,
+            indexed: false,
+        });
     }
 
     #[no_mangle]
@@ -885,10 +905,12 @@ pub mod bundle_ffi {
         offset: BufferAddress,
     ) {
         span!(_guard, DEBUG, "RenderBundle::draw_indexed_indirect");
-        bundle
-            .base
-            .commands
-            .push(RenderCommand::DrawIndexedIndirect { buffer_id, offset });
+        bundle.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: None,
+            indexed: true,
+        });
     }
 
     #[no_mangle]

wgpu-core/src/command/render.rs

@@ -123,13 +123,20 @@ pub enum RenderCommand {
         base_vertex: i32,
         first_instance: u32,
     },
-    DrawIndirect {
+    MultiDrawIndirect {
         buffer_id: id::BufferId,
         offset: BufferAddress,
+        /// Count of `None` represents a non-multi call.
+        count: Option<u32>,
+        indexed: bool,
     },
-    DrawIndexedIndirect {
+    MultiDrawIndirectCount {
         buffer_id: id::BufferId,
         offset: BufferAddress,
+        count_buffer_id: id::BufferId,
+        count_buffer_offset: BufferAddress,
+        max_count: u32,
+        indexed: bool,
     },
     PushDebugGroup {
         color: u32,
@@ -1180,30 +1187,168 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                         );
                     }
                 }
-                RenderCommand::DrawIndirect { buffer_id, offset } => {
+                RenderCommand::MultiDrawIndirect {
+                    buffer_id,
+                    offset,
+                    count,
+                    indexed,
+                } => {
                     state.is_ready().unwrap();
 
+                    let name = match (count, indexed) {
+                        (None, false) => "drawIndirect",
+                        (None, true) => "drawIndexedIndirect",
+                        (Some(..), false) => "multiDrawIndirect",
+                        (Some(..), true) => "multiDrawIndexedIndirect",
+                    };
+
+                    let stride = match indexed {
+                        false => 16,
+                        true => 20,
+                    };
+
+                    if count.is_some() {
+                        assert!(
+                            device.features.contains(wgt::Features::MULTI_DRAW_INDIRECT),
+                            "The feature MULTI_DRAW_INDIRECT must be enabled to use {}",
+                            name
+                        );
+                    }
+
                     let buffer = trackers
                         .buffers
                         .use_extend(&*buffer_guard, buffer_id, (), BufferUse::INDIRECT)
                         .unwrap();
-                    assert!(buffer.usage.contains(BufferUsage::INDIRECT), "An invalid drawIndirect call has been made. The buffer usage is {:?} which does not contain required usage INDIRECT", buffer.usage);
+                    assert!(
+                        buffer.usage.contains(BufferUsage::INDIRECT),
+                        "An invalid {} call has been made. The indirect buffer usage is {:?} which does not contain required usage INDIRECT",
+                        name,
+                        buffer.usage,
+                    );
 
-                    unsafe {
-                        raw.draw_indirect(&buffer.raw, offset, 1, 0);
+                    let actual_count = count.unwrap_or(1);
+
+                    let begin_offset = offset;
+                    let end_offset = offset + stride * actual_count as u64;
+                    assert!(
+                        end_offset <= buffer.size,
+                        "{} with offset {}{} uses bytes {}..{} which overruns indirect buffer of size {}",
+                        name,
+                        offset,
+                        count.map_or_else(String::new, |v| format!(" and count {}", v)),
+                        begin_offset,
+                        end_offset,
+                        buffer.size
+                    );
+
+                    match indexed {
+                        false => unsafe {
+                            raw.draw_indirect(&buffer.raw, offset, actual_count, stride as u32);
+                        },
+                        true => unsafe {
+                            raw.draw_indexed_indirect(
+                                &buffer.raw,
+                                offset,
+                                actual_count,
+                                stride as u32,
+                            );
+                        },
                     }
                 }
-                RenderCommand::DrawIndexedIndirect { buffer_id, offset } => {
+                RenderCommand::MultiDrawIndirectCount {
+                    buffer_id,
+                    offset,
+                    count_buffer_id,
+                    count_buffer_offset,
+                    max_count,
+                    indexed,
+                } => {
                     state.is_ready().unwrap();
 
+                    let name = match indexed {
+                        false => "multiDrawIndirectCount",
+                        true => "multiDrawIndexedIndirectCount",
+                    };
+
+                    let stride = match indexed {
+                        false => 16,
+                        true => 20,
+                    };
+
+                    assert!(
+                        device
+                            .features
+                            .contains(wgt::Features::MULTI_DRAW_INDIRECT_COUNT),
+                        "The feature MULTI_DRAW_INDIRECT_COUNT must be enabled to use {}",
+                        name
+                    );
+
                     let buffer = trackers
                         .buffers
                         .use_extend(&*buffer_guard, buffer_id, (), BufferUse::INDIRECT)
                         .unwrap();
-                    assert!(buffer.usage.contains(BufferUsage::INDIRECT), "An invalid drawIndexedIndirect call has been made. The buffer usage is {:?} which does not contain required usage INDIRECT", buffer.usage);
+                    assert!(
+                        buffer.usage.contains(BufferUsage::INDIRECT),
+                        "An invalid {} call has been made. The indirect buffer usage is {:?} which does not contain required usage INDIRECT",
+                        name,
+                        buffer.usage
+                    );
+                    let count_buffer = trackers
+                        .buffers
+                        .use_extend(&*buffer_guard, count_buffer_id, (), BufferUse::INDIRECT)
+                        .unwrap();
+                    assert!(
+                        count_buffer.usage.contains(BufferUsage::INDIRECT),
+                        "An invalid {} call has been made. The count buffer usage is {:?} which does not contain required usage INDIRECT",
+                        name,
+                        count_buffer.usage
+                    );
 
-                    unsafe {
-                        raw.draw_indexed_indirect(&buffer.raw, offset, 1, 0);
+                    let begin_offset = offset;
+                    let end_offset = offset + stride * max_count as u64;
+                    assert!(
+                        end_offset <= buffer.size,
+                        "{} with offset {} and max_count {} uses bytes {}..{} which overruns indirect buffer of size {}",
+                        name,
+                        offset,
+                        max_count,
+                        begin_offset,
+                        end_offset,
+                        buffer.size
+                    );
+
+                    let begin_count_offset = count_buffer_offset;
+                    let end_count_offset = count_buffer_offset + 4;
+                    assert!(
+                        end_count_offset <= count_buffer.size,
+                        "{} uses bytes {}..{} which overruns count buffer of size {}",
+                        name,
+                        begin_count_offset,
+                        end_count_offset,
+                        count_buffer.size
+                    );
+
+                    match indexed {
+                        false => unsafe {
+                            raw.draw_indirect_count(
+                                &buffer.raw,
+                                offset,
+                                &count_buffer.raw,
+                                count_buffer_offset,
+                                max_count,
+                                stride as u32,
+                            );
+                        },
+                        true => unsafe {
+                            raw.draw_indexed_indirect_count(
+                                &buffer.raw,
+                                offset,
+                                &count_buffer.raw,
+                                count_buffer_offset,
+                                max_count,
+                                stride as u32,
+                            );
+                        },
                     }
                 }
                 RenderCommand::PushDebugGroup { color, len } => {
@@ -1476,9 +1621,12 @@ pub mod render_ffi {
         offset: BufferAddress,
     ) {
         span!(_guard, DEBUG, "RenderPass::draw_indirect");
-        pass.base
-            .commands
-            .push(RenderCommand::DrawIndirect { buffer_id, offset });
+        pass.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: None,
+            indexed: false,
+        });
     }
 
     #[no_mangle]
@@ -1488,9 +1636,92 @@ pub mod render_ffi {
         offset: BufferAddress,
     ) {
         span!(_guard, DEBUG, "RenderPass::draw_indexed_indirect");
+        pass.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: None,
+            indexed: true,
+        });
+    }
+
+    #[no_mangle]
+    pub extern "C" fn wgpu_render_pass_multi_draw_indirect(
+        pass: &mut RenderPass,
+        buffer_id: id::BufferId,
+        offset: BufferAddress,
+        count: u32,
+    ) {
+        span!(_guard, DEBUG, "RenderPass::multi_draw_indirect");
+        pass.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: Some(count),
+            indexed: false,
+        });
+    }
+
+    #[no_mangle]
+    pub extern "C" fn wgpu_render_pass_multi_draw_indexed_indirect(
+        pass: &mut RenderPass,
+        buffer_id: id::BufferId,
+        offset: BufferAddress,
+        count: u32,
+    ) {
+        span!(_guard, DEBUG, "RenderPass::multi_draw_indexed_indirect");
+        pass.base.commands.push(RenderCommand::MultiDrawIndirect {
+            buffer_id,
+            offset,
+            count: Some(count),
+            indexed: true,
+        });
+    }
+
+    #[no_mangle]
+    pub extern "C" fn wgpu_render_pass_multi_draw_indirect_count(
+        pass: &mut RenderPass,
+        buffer_id: id::BufferId,
+        offset: BufferAddress,
+        count_buffer_id: id::BufferId,
+        count_buffer_offset: BufferAddress,
+        max_count: u32,
+    ) {
+        span!(_guard, DEBUG, "RenderPass::multi_draw_indirect_count");
         pass.base
             .commands
-            .push(RenderCommand::DrawIndexedIndirect { buffer_id, offset });
+            .push(RenderCommand::MultiDrawIndirectCount {
+                buffer_id,
+                offset,
+                count_buffer_id,
+                count_buffer_offset,
+                max_count,
+                indexed: false,
+            });
+    }
+
+    #[no_mangle]
+    pub extern "C" fn wgpu_render_pass_multi_draw_indexed_indirect_count(
+        pass: &mut RenderPass,
+        buffer_id: id::BufferId,
+        offset: BufferAddress,
+        count_buffer_id: id::BufferId,
+        count_buffer_offset: BufferAddress,
+        max_count: u32,
+    ) {
+        span!(
+            _guard,
+            DEBUG,
+            "RenderPass::multi_draw_indexed_indirect_count"
+        );
+        pass.base
+            .commands
+            .push(RenderCommand::MultiDrawIndirectCount {
+                buffer_id,
+                offset,
+                count_buffer_id,
+                count_buffer_offset,
+                max_count,
+                indexed: true,
+            });
     }
 
     #[no_mangle]

wgpu-core/src/device/mod.rs

@@ -196,8 +196,8 @@ pub struct Device<B: hal::Backend> {
     temp_suspected: life::SuspectedResources,
     pub(crate) hal_limits: hal::Limits,
     pub(crate) private_features: PrivateFeatures,
-    limits: wgt::Limits,
-    features: wgt::Features,
+    pub(crate) limits: wgt::Limits,
+    pub(crate) features: wgt::Features,
     //TODO: move this behind another mutex. This would allow several methods to switch
     // to borrow Device immutably, such as `write_buffer`, `write_texture`, and `buffer_unmap`.
     pending_writes: queue::PendingWrites<B>,

wgpu-core/src/instance.rs

@@ -151,6 +151,14 @@ impl<B: hal::Backend> Adapter<B> {
             wgt::Features::UNSIZED_BINDING_ARRAY,
             adapter_features.contains(hal::Features::UNSIZED_DESCRIPTOR_ARRAY),
         );
+        features.set(
+            wgt::Features::MULTI_DRAW_INDIRECT,
+            adapter_features.contains(hal::Features::MULTI_DRAW_INDIRECT),
+        );
+        features.set(
+            wgt::Features::MULTI_DRAW_INDIRECT_COUNT,
+            adapter_features.contains(hal::Features::DRAW_INDIRECT_COUNT),
+        );
         if unsafe_features.allowed() {
             // Unsafe features go here
         }
@@ -622,6 +630,18 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
                     .features
                     .contains(wgt::Features::UNSIZED_BINDING_ARRAY),
             );
+            enabled_features.set(
+                hal::Features::MULTI_DRAW_INDIRECT,
+                adapter
+                    .features
+                    .contains(wgt::Features::MULTI_DRAW_INDIRECT),
+            );
+            enabled_features.set(
+                hal::Features::DRAW_INDIRECT_COUNT,
+                adapter
+                    .features
+                    .contains(wgt::Features::MULTI_DRAW_INDIRECT_COUNT),
+            );
 
             let family = adapter
                 .raw

wgpu-types/src/lib.rs

@@ -126,8 +126,10 @@ impl NonExhaustive {
 }
 
 bitflags::bitflags! {
-    /// Features that are not guarenteed to be supported. These are either part of the webgpu standard,
-    /// or are extension features supported by wgpu when targeting native.
+    /// Features that are not guaranteed to be supported.
+    ///
+    /// These are either part of the webgpu standard, or are extension features supported by
+    /// wgpu when targeting native.
     ///
     /// If you want to use a feature, you need to first verify that the adapter supports
     /// the feature. If the adapter does not support the feature, requesting a device with it enabled
@@ -209,6 +211,27 @@ bitflags::bitflags! {
         ///
         /// This is a native only feature.
         const UNSIZED_BINDING_ARRAY = 0x0000_0000_0010_0000;
+        /// Allows the user to call [`RenderPass::multi_draw_indirect`] and [`RenderPass::multi_draw_indexed_indirect`].
+        ///
+        /// Allows multiple indirect calls to be dispatched from a single buffer.
+        ///
+        /// Supported platforms:
+        /// - DX12
+        /// - Metal
+        /// - Vulkan
+        ///
+        /// This is a native only feature.
+        const MULTI_DRAW_INDIRECT = 0x0000_0000_0020_0000;
+        /// Allows the user to call [`RenderPass::multi_draw_indirect_count`] and [`RenderPass::multi_draw_indexed_indirect_count`].
+        ///
+        /// This allows the use of a buffer containing the actual number of draw calls.
+        ///
+        /// Supported platforms:
+        /// - DX12
+        /// - Vulkan 1.2+ (or VK_KHR_draw_indirect_count)
+        ///
+        /// This is a native only feature.
+        const MULTI_DRAW_INDIRECT_COUNT = 0x0000_0000_0040_0000;
         /// Features which are part of the upstream webgpu standard
         const ALL_WEBGPU = 0x0000_0000_0000_FFFF;
         /// Features that require activating the unsafe feature flag