Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indexed drawing with RenderBundle #5441

Merged
merged 3 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
129 changes: 86 additions & 43 deletions tests/tests/vertex_indices/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
})
}
}
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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, &[]);

let draws = test.case.draws();

match test.draw_call_kind {
DrawCallKind::Direct => {
for draw in draws {
draw.execute(&mut rpass);
{
// 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 render_encoder: &mut dyn RenderEncoder = render_bundle_encoder
.as_mut()
.map(|r| r as &mut dyn RenderEncoder)
.unwrap_or(&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);
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);
}
}
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]);
}
}

Expand All @@ -439,21 +485,18 @@ async fn vertex_index_common(ctx: TestingContext) {
.panic_on_timeout();
let data: Vec<u32> = 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}");
}
}

Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ fn validate_indexed_draw<A: HalApi>(
) -> 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,
Expand Down
Loading