From ed7d9de439a92209b723c07c10f647a5bbf0845f Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sat, 30 Mar 2024 10:19:17 +0100 Subject: [PATCH] Fix indexed drawing with RenderBundle (#5441) * enhance vertex_indices test to also run with render bundles * fix render bundle index limit check * changelog entry --- CHANGELOG.md | 1 + tests/tests/vertex_indices/mod.rs | 125 ++++++++++++++++++++---------- wgpu-core/src/command/bundle.rs | 2 +- 3 files changed, 86 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b98734c5e..d9102c58b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,7 @@ By @cwfitzgerald in [#5325](https://github.com/gfx-rs/wgpu/pull/5325). #### General - Fix an issue where command encoders weren't properly freed if an error occurred during command encoding. By @ErichDonGubler in [#5251](https://github.com/gfx-rs/wgpu/pull/5251). +- Fix incorrect validation causing all indexed draws on render bundles to fail. By @wumpf in [#5430](https://github.com/gfx-rs/wgpu/pull/5340). #### Android - Fix linking error when targeting android without `winit`. By @ashdnazg in [#5326](https://github.com/gfx-rs/wgpu/pull/5326). diff --git a/tests/tests/vertex_indices/mod.rs b/tests/tests/vertex_indices/mod.rs index 745eeff8c..e0a2cbae0 100644 --- a/tests/tests/vertex_indices/mod.rs +++ b/tests/tests/vertex_indices/mod.rs @@ -5,9 +5,10 @@ use std::{num::NonZeroU64, ops::Range}; -use wgpu::util::{BufferInitDescriptor, DeviceExt}; +use wgpu::util::{BufferInitDescriptor, DeviceExt, RenderEncoder}; use wgpu_test::{gpu_test, GpuTestConfiguration, TestParameters, TestingContext}; +use wgt::RenderBundleDescriptor; /// Generic struct representing a draw call struct Draw { @@ -19,7 +20,7 @@ struct Draw { impl Draw { /// Directly execute the draw call - fn execute(&self, rpass: &mut wgpu::RenderPass<'_>) { + fn execute(&self, rpass: &mut dyn RenderEncoder<'_>) { if let Some(base_vertex) = self.base_vertex { rpass.draw_indexed(self.vertex.clone(), base_vertex, self.instance.clone()); } else { @@ -64,7 +65,7 @@ impl Draw { /// Execute the draw call from the given indirect buffer fn execute_indirect<'rpass>( &self, - rpass: &mut wgpu::RenderPass<'rpass>, + rpass: &mut dyn RenderEncoder<'rpass>, indirect: &'rpass wgpu::Buffer, offset: &mut u64, ) { @@ -169,10 +170,21 @@ impl DrawCallKind { const ARRAY: [Self; 2] = [Self::Direct, Self::Indirect]; } +#[derive(Debug, Copy, Clone)] +enum EncoderKind { + RenderPass, + RenderBundle, +} + +impl EncoderKind { + const ARRAY: [Self; 2] = [Self::RenderPass, Self::RenderBundle]; +} + struct Test { case: TestCase, id_source: IdSource, draw_call_kind: DrawCallKind, + encoder_kind: EncoderKind, } impl Test { @@ -325,11 +337,14 @@ async fn vertex_index_common(ctx: TestingContext) { for case in TestCase::ARRAY { for id_source in IdSource::ARRAY { for draw_call_kind in DrawCallKind::ARRAY { - tests.push(Test { - case, - id_source, - draw_call_kind, - }) + for encoder_kind in EncoderKind::ARRAY { + tests.push(Test { + case, + id_source, + draw_call_kind, + encoder_kind, + }) + } } } } @@ -373,6 +388,7 @@ async fn vertex_index_common(ctx: TestingContext) { .device .create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); + let render_bundle; let indirect_buffer; let mut rpass = encoder1.begin_render_pass(&wgpu::RenderPassDescriptor { label: None, @@ -386,34 +402,64 @@ async fn vertex_index_common(ctx: TestingContext) { occlusion_query_set: None, }); - rpass.set_vertex_buffer(0, identity_buffer.slice(..)); - rpass.set_vertex_buffer(1, identity_buffer.slice(..)); - rpass.set_index_buffer(identity_buffer.slice(..), wgpu::IndexFormat::Uint32); - rpass.set_pipeline(pipeline); - rpass.set_bind_group(0, &bg, &[]); + { + // Need to scope render_bundle_encoder since it's not Send and would otherwise + // infect the function if not going out of scope before an await call. + // (it is dropped via `take` + `finish` earlier, but compiler does not take this into account) + let mut render_bundle_encoder = match test.encoder_kind { + EncoderKind::RenderPass => None, + EncoderKind::RenderBundle => Some(ctx.device.create_render_bundle_encoder( + &wgpu::RenderBundleEncoderDescriptor { + label: Some("test renderbundle encoder"), + color_formats: &[Some(wgpu::TextureFormat::Rgba8Unorm)], + depth_stencil: None, + sample_count: 1, + multiview: None, + }, + )), + }; - let draws = test.case.draws(); + let render_encoder: &mut dyn RenderEncoder = render_bundle_encoder + .as_mut() + .map(|r| r as &mut dyn RenderEncoder) + .unwrap_or(&mut rpass); - match test.draw_call_kind { - DrawCallKind::Direct => { - for draw in draws { - draw.execute(&mut rpass); + render_encoder.set_vertex_buffer(0, identity_buffer.slice(..)); + render_encoder.set_vertex_buffer(1, identity_buffer.slice(..)); + render_encoder.set_index_buffer(identity_buffer.slice(..), wgpu::IndexFormat::Uint32); + render_encoder.set_pipeline(pipeline); + render_encoder.set_bind_group(0, &bg, &[]); + + let draws = test.case.draws(); + + match test.draw_call_kind { + DrawCallKind::Direct => { + for draw in draws { + draw.execute(render_encoder); + } + } + DrawCallKind::Indirect => { + let mut indirect_bytes = Vec::new(); + for draw in draws { + draw.add_to_buffer(&mut indirect_bytes, features); + } + indirect_buffer = ctx.device.create_buffer_init(&BufferInitDescriptor { + label: Some("indirect"), + contents: &indirect_bytes, + usage: wgpu::BufferUsages::INDIRECT, + }); + let mut offset = 0; + for draw in draws { + draw.execute_indirect(render_encoder, &indirect_buffer, &mut offset); + } } } - DrawCallKind::Indirect => { - let mut indirect_bytes = Vec::new(); - for draw in draws { - draw.add_to_buffer(&mut indirect_bytes, features); - } - indirect_buffer = ctx.device.create_buffer_init(&BufferInitDescriptor { - label: Some("indirect"), - contents: &indirect_bytes, - usage: wgpu::BufferUsages::INDIRECT, + + if let Some(render_bundle_encoder) = render_bundle_encoder.take() { + render_bundle = render_bundle_encoder.finish(&RenderBundleDescriptor { + label: Some("test renderbundle"), }); - let mut offset = 0; - for draw in draws { - draw.execute_indirect(&mut rpass, &indirect_buffer, &mut offset); - } + rpass.execute_bundles([&render_bundle]); } } @@ -439,21 +485,18 @@ async fn vertex_index_common(ctx: TestingContext) { .panic_on_timeout(); let data: Vec = bytemuck::cast_slice(&slice.get_mapped_range()).to_vec(); + let case_name = format!( + "Case {:?} getting indices from {:?} using {:?} draw calls, encoded with a {:?}", + test.case, test.id_source, test.draw_call_kind, test.encoder_kind + ); if data != expected { eprintln!( - "Failed: Got: {:?} Expected: {:?} - Case {:?} getting indices from {:?} using {:?} draw calls", - data, - expected, - test.case, - test.id_source, - test.draw_call_kind + "Failed: Got: {:?} Expected: {:?} - {case_name}", + data, expected, ); failed = true; } else { - eprintln!( - "Passed: Case {:?} getting indices from {:?} using {:?} draw calls", - test.case, test.id_source, test.draw_call_kind - ); + eprintln!("Passed: {case_name}"); } } diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index e2848f737..47beda8ec 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -166,7 +166,7 @@ fn validate_indexed_draw( ) -> Result<(), DrawError> { let last_index = first_index as u64 + index_count as u64; let index_limit = index_state.limit(); - if last_index <= index_limit { + if last_index > index_limit { return Err(DrawError::IndexBeyondLimit { last_index, index_limit,