From 1436735efae140e8121f2b14d1a42b094d97b973 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Sat, 28 May 2022 22:23:38 -0400 Subject: [PATCH 1/4] Implement submission indexes --- deno_webgpu/src/buffer.rs | 2 +- player/src/bin/play.rs | 4 +-- player/tests/test.rs | 2 +- wgpu-core/src/device/life.rs | 2 ++ wgpu-core/src/device/mod.rs | 28 ++++++++++++--- wgpu-core/src/device/queue.rs | 23 ++++++++---- wgpu-hal/src/lib.rs | 1 + wgpu/examples/capture/main.rs | 25 +++++++++---- wgpu/examples/framework.rs | 2 +- wgpu/examples/hello-compute/main.rs | 2 +- wgpu/examples/mipmap/main.rs | 2 +- wgpu/src/backend/direct.rs | 17 +++++---- wgpu/src/backend/web.rs | 5 ++- wgpu/src/lib.rs | 35 ++++++++++++++----- wgpu/tests/vertex_indices/mod.rs | 2 +- wgpu/tests/zero_init_texture_after_discard.rs | 2 +- 16 files changed, 110 insertions(+), 44 deletions(-) diff --git a/deno_webgpu/src/buffer.rs b/deno_webgpu/src/buffer.rs index 45eab483d1..1eb331863e 100644 --- a/deno_webgpu/src/buffer.rs +++ b/deno_webgpu/src/buffer.rs @@ -128,7 +128,7 @@ pub async fn op_webgpu_buffer_get_map_async( { let state = state.borrow(); let instance = state.borrow::(); - gfx_select!(device => instance.device_poll(device, false)).unwrap(); + gfx_select!(device => instance.device_poll(device, false, None)).unwrap(); } tokio::time::sleep(Duration::from_millis(10)).await; } diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs index 524c859430..f05e66f56d 100644 --- a/player/src/bin/play.rs +++ b/player/src/bin/play.rs @@ -95,7 +95,7 @@ fn main() { } gfx_select!(device => global.device_stop_capture(device)); - gfx_select!(device => global.device_poll(device, true)).unwrap(); + gfx_select!(device => global.device_poll(device, true, None)).unwrap(); } #[cfg(feature = "winit")] { @@ -181,7 +181,7 @@ fn main() { }, Event::LoopDestroyed => { log::info!("Closing"); - gfx_select!(device => global.device_poll(device, true)).unwrap(); + gfx_select!(device => global.device_poll(device, true, None)).unwrap(); } _ => {} } diff --git a/player/tests/test.rs b/player/tests/test.rs index 6fa3395ae2..fd96f32173 100644 --- a/player/tests/test.rs +++ b/player/tests/test.rs @@ -120,7 +120,7 @@ impl Test<'_> { } println!("\t\t\tWaiting..."); - wgc::gfx_select!(device => global.device_poll(device, true)).unwrap(); + wgc::gfx_select!(device => global.device_poll(device, true, None)).unwrap(); for expect in self.expectations { println!("\t\t\tChecking {}", expect.name); diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 8c6705276f..27f1708860 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -224,6 +224,8 @@ struct ActiveSubmission { pub enum WaitIdleError { #[error(transparent)] Device(#[from] DeviceError), + #[error("Tried to wait using a submission index from the wrong device. Submission index is from device {0:?}. Called poll on device {1:?}.")] + WrongSubmissionIndex(id::QueueId, id::DeviceId), #[error("GPU got stuck :(")] StuckGpu, } diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 24fa72e571..495c62d299 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -440,6 +440,7 @@ impl Device { &'this self, hub: &Hub, force_wait: bool, + submission_index: Option, token: &mut Token<'token, Self>, ) -> Result<(UserClosures, bool), WaitIdleError> { profiling::scope!("maintain", "Device"); @@ -464,13 +465,20 @@ impl Device { life_tracker.triage_mapped(hub, token); let last_done_index = if force_wait { - let current_index = self.active_submission_index; + let index_to_wait_for = match submission_index { + Some(submission_index) => { + // We don't need to check to see if the queue id matches + // as we already checked this from inside the poll call. + submission_index.index + } + None => self.active_submission_index, + }; unsafe { self.raw - .wait(&self.fence, current_index, CLEANUP_WAIT_MS) + .wait(&self.fence, index_to_wait_for, CLEANUP_WAIT_MS) .map_err(DeviceError::from)? }; - current_index + index_to_wait_for } else { unsafe { self.raw @@ -4968,15 +4976,25 @@ impl Global { &self, device_id: id::DeviceId, force_wait: bool, + submission_index: Option, ) -> Result { let (closures, queue_empty) = { + if let Some(submission_index) = submission_index { + if submission_index.queue_id != device_id { + return Err(WaitIdleError::WrongSubmissionIndex( + submission_index.queue_id, + device_id, + )); + } + } + let hub = A::hub(self); let mut token = Token::root(); let (device_guard, mut token) = hub.devices.read(&mut token); device_guard .get(device_id) .map_err(|_| DeviceError::Invalid)? - .maintain(hub, force_wait, &mut token)? + .maintain(hub, force_wait, submission_index, &mut token)? }; unsafe { closures.fire(); @@ -5004,7 +5022,7 @@ impl Global { let (device_guard, mut token) = hub.devices.read(&mut token); for (id, device) in device_guard.iter(A::VARIANT) { - let (cbs, queue_empty) = device.maintain(hub, force_wait, &mut token)?; + let (cbs, queue_empty) = device.maintain(hub, force_wait, None, &mut token)?; all_queue_empty = all_queue_empty && queue_empty; // If the device's own `RefCount` clone is the only one left, and diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index e76255f772..7246199c2b 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -12,7 +12,7 @@ use crate::{ id, init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange}, resource::{BufferAccessError, BufferMapState, TextureInner}, - track, FastHashSet, + track, FastHashSet, SubmissionIndex, }; use hal::{CommandEncoder as _, Device as _, Queue as _}; @@ -39,6 +39,13 @@ pub struct SubmittedWorkDoneClosure { unsafe impl Send for SubmittedWorkDoneClosure {} unsafe impl Sync for SubmittedWorkDoneClosure {} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct WrappedSubmissionIndex { + pub queue_id: id::QueueId, + pub index: SubmissionIndex, +} + struct StagingData { buffer: A::Buffer, } @@ -580,10 +587,10 @@ impl Global { &self, queue_id: id::QueueId, command_buffer_ids: &[id::CommandBufferId], - ) -> Result<(), QueueSubmitError> { + ) -> Result { profiling::scope!("submit", "Queue"); - let callbacks = { + let (submit_index, callbacks) = { let hub = A::hub(self); let mut token = Token::root(); @@ -918,24 +925,28 @@ impl Global { // This will schedule destruction of all resources that are no longer needed // by the user but used in the command stream, among other things. - let (closures, _) = match device.maintain(hub, false, &mut token) { + let (closures, _) = match device.maintain(hub, false, None, &mut token) { Ok(closures) => closures, Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)), Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu), + Err(WaitIdleError::WrongSubmissionIndex(..)) => unreachable!(), }; device.pending_writes.temp_resources = pending_write_resources; device.temp_suspected.clear(); device.lock_life(&mut token).post_submit(); - closures + (submit_index, closures) }; // the closures should execute with nothing locked! unsafe { callbacks.fire(); } - Ok(()) + Ok(WrappedSubmissionIndex { + queue_id, + index: submit_index, + }) } pub fn queue_get_timestamp_period( diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 82a3243986..0737053514 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -304,6 +304,7 @@ pub trait Device: Send + Sync { unsafe fn create_fence(&self) -> Result; unsafe fn destroy_fence(&self, fence: A::Fence); unsafe fn get_fence_value(&self, fence: &A::Fence) -> Result; + /// Calling wait with a lower value than the current fence value will immediately return. unsafe fn wait( &self, fence: &A::Fence, diff --git a/wgpu/examples/capture/main.rs b/wgpu/examples/capture/main.rs index 2f4b2a56a5..52aa8fad2c 100644 --- a/wgpu/examples/capture/main.rs +++ b/wgpu/examples/capture/main.rs @@ -5,7 +5,7 @@ use std::env; use std::fs::File; use std::io::Write; use std::mem::size_of; -use wgpu::{Buffer, Device}; +use wgpu::{Buffer, Device, SubmissionIndex}; async fn run(png_output_path: &str) { let args: Vec<_> = env::args().collect(); @@ -20,14 +20,22 @@ async fn run(png_output_path: &str) { return; } }; - let (device, buffer, buffer_dimensions) = create_red_image_with_dimensions(width, height).await; - create_png(png_output_path, device, buffer, &buffer_dimensions).await; + let (device, buffer, buffer_dimensions, submission_index) = + create_red_image_with_dimensions(width, height).await; + create_png( + png_output_path, + device, + buffer, + &buffer_dimensions, + submission_index, + ) + .await; } async fn create_red_image_with_dimensions( width: usize, height: usize, -) -> (Device, Buffer, BufferDimensions) { +) -> (Device, Buffer, BufferDimensions, SubmissionIndex) { let adapter = wgpu::Instance::new( wgpu::util::backend_bits_from_env().unwrap_or_else(wgpu::Backends::all), ) @@ -114,8 +122,8 @@ async fn create_red_image_with_dimensions( encoder.finish() }; - queue.submit(Some(command_buffer)); - (device, output_buffer, buffer_dimensions) + let index = queue.submit(Some(command_buffer)); + (device, output_buffer, buffer_dimensions, index) } async fn create_png( @@ -123,6 +131,7 @@ async fn create_png( device: Device, output_buffer: Buffer, buffer_dimensions: &BufferDimensions, + submission_index: SubmissionIndex, ) { // Note that we're not calling `.await` here. let buffer_slice = output_buffer.slice(..); @@ -131,7 +140,9 @@ async fn create_png( // Poll the device in a blocking manner so that our future resolves. // In an actual application, `device.poll(...)` should // be called in an event loop or on another thread. - device.poll(wgpu::Maintain::Wait); + // + // We pass our submission index so we don't need to wait for any other possible submissions. + device.poll(wgpu::Maintain::Wait(Some(submission_index))); // If a file system is available, write the buffer as a PNG let has_file_system_available = cfg!(not(target_arch = "wasm32")); if !has_file_system_available { diff --git a/wgpu/examples/framework.rs b/wgpu/examples/framework.rs index 0ed135f1cd..ddab7ce68f 100644 --- a/wgpu/examples/framework.rs +++ b/wgpu/examples/framework.rs @@ -518,7 +518,7 @@ pub fn test(mut params: FrameworkRefTest) { let dst_buffer_slice = dst_buffer.slice(..); let _ = dst_buffer_slice.map_async(wgpu::MapMode::Read); - ctx.device.poll(wgpu::Maintain::Wait); + ctx.device.poll(wgpu::Maintain::Wait(None)); let bytes = dst_buffer_slice.get_mapped_range().to_vec(); test_common::image::compare_image_output( diff --git a/wgpu/examples/hello-compute/main.rs b/wgpu/examples/hello-compute/main.rs index bbff9130f4..c5d58921ab 100644 --- a/wgpu/examples/hello-compute/main.rs +++ b/wgpu/examples/hello-compute/main.rs @@ -153,7 +153,7 @@ async fn execute_gpu_inner( // Poll the device in a blocking manner so that our future resolves. // In an actual application, `device.poll(...)` should // be called in an event loop or on another thread. - device.poll(wgpu::Maintain::Wait); + device.poll(wgpu::Maintain::Wait(None)); // Awaits until `buffer_future` can be read from if let Ok(()) = buffer_future.await { diff --git a/wgpu/examples/mipmap/main.rs b/wgpu/examples/mipmap/main.rs index 9603895d57..3570bdaf46 100644 --- a/wgpu/examples/mipmap/main.rs +++ b/wgpu/examples/mipmap/main.rs @@ -386,7 +386,7 @@ impl framework::Example for Example { .slice(..) .map_async(wgpu::MapMode::Read); // Wait for device to be done rendering mipmaps - device.poll(wgpu::Maintain::Wait); + device.poll(wgpu::Maintain::Wait(None)); // This is guaranteed to be ready. let timestamp_view = query_sets .data_buffer diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index fdd599e57b..72dd696759 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -800,6 +800,7 @@ impl crate::Context for Context { type SurfaceId = Surface; type SurfaceOutputDetail = SurfaceOutputDetail; + type SubmissionIndex = wgc::device::queue::WrappedSubmissionIndex; type RequestAdapterFuture = Ready>; #[allow(clippy::type_complexity)] @@ -1571,7 +1572,7 @@ impl crate::Context for Context { #[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))] { - match wgc::gfx_select!(device.id => global.device_poll(device.id, true)) { + match wgc::gfx_select!(device.id => global.device_poll(device.id, true, None)) { Ok(_) => (), Err(err) => self.handle_error_fatal(err, "Device::drop"), } @@ -1582,12 +1583,14 @@ impl crate::Context for Context { fn device_poll(&self, device: &Self::DeviceId, maintain: crate::Maintain) -> bool { let global = &self.0; + let (wait, index) = match maintain { + crate::Maintain::Poll => (false, None), + crate::Maintain::Wait(index) => (true, index.map(|i| i.0)), + }; match wgc::gfx_select!(device.id => global.device_poll( device.id, - match maintain { - crate::Maintain::Poll => false, - crate::Maintain::Wait => true, - } + wait, + index )) { Ok(queue_empty) => queue_empty, Err(err) => self.handle_error_fatal(err, "Device::poll"), @@ -2190,12 +2193,12 @@ impl crate::Context for Context { &self, queue: &Self::QueueId, command_buffers: I, - ) { + ) -> Self::SubmissionIndex { let temp_command_buffers = command_buffers.collect::>(); let global = &self.0; match wgc::gfx_select!(*queue => global.queue_submit(*queue, &temp_command_buffers)) { - Ok(()) => (), + Ok(index) => index, Err(err) => self.handle_error_fatal(err, "Queue::submit"), } } diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 18b84cb25c..48738f4f18 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -995,6 +995,7 @@ impl crate::Context for Context { type SurfaceId = Sendable; type SurfaceOutputDetail = SurfaceOutputDetail; + type SubmissionIndex = (); type RequestAdapterFuture = MakeSendFuture< wasm_bindgen_futures::JsFuture, @@ -2200,10 +2201,12 @@ impl crate::Context for Context { &self, queue: &Self::QueueId, command_buffers: I, - ) { + ) -> Self::SubmissionIndex { let temp_command_buffers = command_buffers.map(|i| i.0).collect::(); queue.0.submit(&temp_command_buffers); + + // SubmissionIndex is (), so just let this function end } fn queue_get_timestamp_period(&self, _queue: &Self::QueueId) -> f32 { diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 15408d8a7d..899cc1a925 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -187,6 +187,7 @@ trait Context: Debug + Send + Sized + Sync { type SurfaceId: Debug + Send + Sync + 'static; type SurfaceOutputDetail: Send; + type SubmissionIndex: Debug + Copy + Clone + Send + 'static; type RequestAdapterFuture: Future> + Send; type RequestDeviceFuture: Future> @@ -490,7 +491,7 @@ trait Context: Debug + Send + Sized + Sync { &self, queue: &Self::QueueId, command_buffers: I, - ); + ) -> Self::SubmissionIndex; fn queue_get_timestamp_period(&self, queue: &Self::QueueId) -> f32; fn queue_on_submitted_work_done( &self, @@ -550,13 +551,24 @@ pub struct Device { id: ::DeviceId, } -/// Passed to [`Device::poll`] to control if it should block or not. This has no effect on -/// the web. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +/// Identifier for a particular call to [`Queue::submit`]. Can be used +/// as part of an argument to [`Device::poll`] to block for a particular +/// submission to finish. +#[derive(Debug, Copy, Clone)] +pub struct SubmissionIndex(::SubmissionIndex); + +/// Passed to [`Device::poll`] to control how and if it should block. +#[derive(Clone)] pub enum Maintain { - /// Block - Wait, - /// Don't block + /// Block until the callbacks for the given submission will resolve on their own. + /// + /// If the submission index is None it will wait for the most recent submission. + /// + /// On native this will block the thread until the submission is finished. + /// + /// On web this is a no-op but all the callbacks will automatically fire. + Wait(Option), + /// Check the device for a single time without blocking. Poll, } @@ -3366,14 +3378,19 @@ impl Queue { } /// Submits a series of finished command buffers for execution. - pub fn submit>(&self, command_buffers: I) { - Context::queue_submit( + pub fn submit>( + &self, + command_buffers: I, + ) -> SubmissionIndex { + let raw = Context::queue_submit( &*self.context, &self.id, command_buffers .into_iter() .map(|mut comb| comb.id.take().unwrap()), ); + + SubmissionIndex(raw) } /// Gets the amount of nanoseconds each tick of a timestamp query represents. diff --git a/wgpu/tests/vertex_indices/mod.rs b/wgpu/tests/vertex_indices/mod.rs index fa85ae62d9..1bc362f7db 100644 --- a/wgpu/tests/vertex_indices/mod.rs +++ b/wgpu/tests/vertex_indices/mod.rs @@ -124,7 +124,7 @@ fn pulling_common( ctx.queue.submit(Some(encoder.finish())); let slice = buffer.slice(..); let _ = slice.map_async(wgpu::MapMode::Read); - ctx.device.poll(wgpu::Maintain::Wait); + ctx.device.poll(wgpu::Maintain::Wait(None)); let data: Vec = bytemuck::cast_slice(&*slice.get_mapped_range()).to_vec(); assert_eq!(data, expected); diff --git a/wgpu/tests/zero_init_texture_after_discard.rs b/wgpu/tests/zero_init_texture_after_discard.rs index ec77e909f0..488778aff1 100644 --- a/wgpu/tests/zero_init_texture_after_discard.rs +++ b/wgpu/tests/zero_init_texture_after_discard.rs @@ -283,7 +283,7 @@ fn assert_buffer_is_zero(readback_buffer: &wgpu::Buffer, device: &wgpu::Device) { let buffer_slice = readback_buffer.slice(..); let _ = buffer_slice.map_async(wgpu::MapMode::Read); - device.poll(wgpu::Maintain::Wait); + device.poll(wgpu::Maintain::Wait(None)); let buffer_view = buffer_slice.get_mapped_range(); assert!( From b7178b8ca7456d381a5699af6c88cff2f160a2d2 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Sun, 29 May 2022 03:00:09 -0400 Subject: [PATCH 2/4] Write some unit tests for poll --- wgpu/tests/common/mod.rs | 11 ++++ wgpu/tests/poll.rs | 108 +++++++++++++++++++++++++++++++++++++++ wgpu/tests/root.rs | 1 + 3 files changed, 120 insertions(+) create mode 100644 wgpu/tests/poll.rs diff --git a/wgpu/tests/common/mod.rs b/wgpu/tests/common/mod.rs index a98de71ac6..8daa0b71c6 100644 --- a/wgpu/tests/common/mod.rs +++ b/wgpu/tests/common/mod.rs @@ -121,6 +121,17 @@ impl TestParameters { self } + /// Mark the test as always failing and needing to be skipped, equivilant to specific_failure(None, None, None) + pub fn skip(mut self) -> Self { + self.failures.push(FailureCase { + backends: None, + vendor: None, + adapter: None, + skip: true, + }); + self + } + /// Mark the test as always failing on a specific backend, equivilant to specific_failure(backend, None, None) pub fn backend_failure(mut self, backends: wgpu::Backends) -> Self { self.failures.push(FailureCase { diff --git a/wgpu/tests/poll.rs b/wgpu/tests/poll.rs new file mode 100644 index 0000000000..7409a96ffc --- /dev/null +++ b/wgpu/tests/poll.rs @@ -0,0 +1,108 @@ +use std::num::NonZeroU64; + +use wgpu::{ + BindGroupDescriptor, BindGroupEntry, BindGroupLayoutDescriptor, BindGroupLayoutEntry, + BindingResource, BindingType, BufferBindingType, BufferDescriptor, BufferUsages, CommandBuffer, + CommandEncoderDescriptor, ComputePassDescriptor, Maintain, ShaderStages, +}; + +use crate::common::{initialize_test, TestParameters, TestingContext}; + +fn generate_dummy_work(ctx: &TestingContext) -> CommandBuffer { + let buffer = ctx.device.create_buffer(&BufferDescriptor { + label: None, + size: 16, + usage: BufferUsages::UNIFORM, + mapped_at_creation: false, + }); + + let bind_group_layout = ctx + .device + .create_bind_group_layout(&BindGroupLayoutDescriptor { + label: None, + entries: &[BindGroupLayoutEntry { + binding: 0, + visibility: ShaderStages::COMPUTE, + ty: BindingType::Buffer { + ty: BufferBindingType::Uniform, + has_dynamic_offset: false, + min_binding_size: Some(NonZeroU64::new(16).unwrap()), + }, + count: None, + }], + }); + + let bind_group = ctx.device.create_bind_group(&BindGroupDescriptor { + label: None, + layout: &bind_group_layout, + entries: &[BindGroupEntry { + binding: 0, + resource: BindingResource::Buffer(buffer.as_entire_buffer_binding()), + }], + }); + + let mut cmd_buf = ctx + .device + .create_command_encoder(&CommandEncoderDescriptor::default()); + + let mut cpass = cmd_buf.begin_compute_pass(&ComputePassDescriptor::default()); + cpass.set_bind_group(0, &bind_group, &[]); + drop(cpass); + + cmd_buf.finish() +} + +#[test] +fn wait() { + initialize_test(TestParameters::default().skip(), |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait(None)); + }) +} + +#[test] +fn double_wait() { + initialize_test(TestParameters::default().skip(), |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait(None)); + ctx.device.poll(Maintain::Wait(None)); + }) +} + +#[test] +fn wait_on_submission() { + initialize_test(TestParameters::default().skip(), |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + let index = ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait(Some(index))); + }) +} + +#[test] +fn double_wait_on_submission() { + initialize_test(TestParameters::default().skip(), |ctx| { + let cmd_buf = generate_dummy_work(&ctx); + + let index = ctx.queue.submit(Some(cmd_buf)); + ctx.device.poll(Maintain::Wait(Some(index))); + ctx.device.poll(Maintain::Wait(Some(index))); + }) +} + +#[test] +fn wait_out_of_order() { + initialize_test(TestParameters::default().skip(), |ctx| { + let cmd_buf1 = generate_dummy_work(&ctx); + let cmd_buf2 = generate_dummy_work(&ctx); + + let index1 = ctx.queue.submit(Some(cmd_buf1)); + let index2 = ctx.queue.submit(Some(cmd_buf2)); + ctx.device.poll(Maintain::Wait(Some(index2))); + ctx.device.poll(Maintain::Wait(Some(index1))); + }) +} diff --git a/wgpu/tests/root.rs b/wgpu/tests/root.rs index 95bf28bf34..49b7a33872 100644 --- a/wgpu/tests/root.rs +++ b/wgpu/tests/root.rs @@ -5,5 +5,6 @@ mod clear_texture; mod device; mod example_wgsl; mod instance; +mod poll; mod vertex_indices; mod zero_init_texture_after_discard; From 882602f1497ce00c1efc54c9e3477e92bba858ec Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Sat, 4 Jun 2022 01:56:31 -0400 Subject: [PATCH 3/4] Update wgpu/src/lib.rs Co-authored-by: Jim Blandy --- wgpu/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 899cc1a925..2bc59906b7 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -560,13 +560,13 @@ pub struct SubmissionIndex(::SubmissionIndex); /// Passed to [`Device::poll`] to control how and if it should block. #[derive(Clone)] pub enum Maintain { - /// Block until the callbacks for the given submission will resolve on their own. + /// On native backends, block until the given submission has + /// completed execution, and any callbacks have been invoked. + /// + /// On the web, this has no effect. Callbacks are invoked from the + /// window event loop. /// - /// If the submission index is None it will wait for the most recent submission. - /// - /// On native this will block the thread until the submission is finished. - /// - /// On web this is a no-op but all the callbacks will automatically fire. + /// If the submission index is `None`, wait for the most recent submission. Wait(Option), /// Check the device for a single time without blocking. Poll, From 9894497c1d9773b42ace08d3c36ec9c69f416941 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Sat, 4 Jun 2022 02:26:36 -0400 Subject: [PATCH 4/4] Unify Maintain in both wgc and wgpu --- deno_webgpu/src/buffer.rs | 2 +- player/src/bin/play.rs | 2 +- player/tests/test.rs | 2 +- wgpu-core/src/device/mod.rs | 28 ++++++++------ wgpu-core/src/device/queue.rs | 2 +- wgpu-types/src/lib.rs | 37 +++++++++++++++++++ wgpu/examples/framework.rs | 2 +- wgpu/examples/hello-compute/main.rs | 2 +- wgpu/examples/mipmap/main.rs | 2 +- wgpu/src/backend/direct.rs | 11 ++---- wgpu/src/lib.rs | 18 ++------- wgpu/tests/common/mod.rs | 6 +-- wgpu/tests/poll.rs | 16 ++++---- wgpu/tests/vertex_indices/mod.rs | 2 +- wgpu/tests/zero_init_texture_after_discard.rs | 2 +- 15 files changed, 81 insertions(+), 53 deletions(-) diff --git a/deno_webgpu/src/buffer.rs b/deno_webgpu/src/buffer.rs index 1eb331863e..23ba5ede9f 100644 --- a/deno_webgpu/src/buffer.rs +++ b/deno_webgpu/src/buffer.rs @@ -128,7 +128,7 @@ pub async fn op_webgpu_buffer_get_map_async( { let state = state.borrow(); let instance = state.borrow::(); - gfx_select!(device => instance.device_poll(device, false, None)).unwrap(); + gfx_select!(device => instance.device_poll(device, wgpu_types::Maintain::Wait)).unwrap(); } tokio::time::sleep(Duration::from_millis(10)).await; } diff --git a/player/src/bin/play.rs b/player/src/bin/play.rs index f05e66f56d..9b5722490c 100644 --- a/player/src/bin/play.rs +++ b/player/src/bin/play.rs @@ -95,7 +95,7 @@ fn main() { } gfx_select!(device => global.device_stop_capture(device)); - gfx_select!(device => global.device_poll(device, true, None)).unwrap(); + gfx_select!(device => global.device_poll(device, wgt::Maintain::Wait)).unwrap(); } #[cfg(feature = "winit")] { diff --git a/player/tests/test.rs b/player/tests/test.rs index fd96f32173..7d1c156a26 100644 --- a/player/tests/test.rs +++ b/player/tests/test.rs @@ -120,7 +120,7 @@ impl Test<'_> { } println!("\t\t\tWaiting..."); - wgc::gfx_select!(device => global.device_poll(device, true, None)).unwrap(); + wgc::gfx_select!(device => global.device_poll(device, wgt::Maintain::Wait)).unwrap(); for expect in self.expectations { println!("\t\t\tChecking {}", expect.name); diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 495c62d299..35ae13e7e7 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -428,6 +428,9 @@ impl Device { /// Check this device for completed commands. /// + /// The `maintain` argument tells how the maintence function should behave, either + /// blocking or just polling the current state of the gpu. + /// /// Return a pair `(closures, queue_empty)`, where: /// /// - `closures` is a list of actions to take: mapping buffers, notifying the user @@ -439,8 +442,7 @@ impl Device { fn maintain<'this, 'token: 'this, G: GlobalIdentityHandlerFactory>( &'this self, hub: &Hub, - force_wait: bool, - submission_index: Option, + maintain: wgt::Maintain, token: &mut Token<'token, Self>, ) -> Result<(UserClosures, bool), WaitIdleError> { profiling::scope!("maintain", "Device"); @@ -464,14 +466,14 @@ impl Device { ); life_tracker.triage_mapped(hub, token); - let last_done_index = if force_wait { - let index_to_wait_for = match submission_index { - Some(submission_index) => { + let last_done_index = if maintain.is_wait() { + let index_to_wait_for = match maintain { + wgt::Maintain::WaitForSubmissionIndex(submission_index) => { // We don't need to check to see if the queue id matches // as we already checked this from inside the poll call. submission_index.index } - None => self.active_submission_index, + _ => self.active_submission_index, }; unsafe { self.raw @@ -4975,11 +4977,10 @@ impl Global { pub fn device_poll( &self, device_id: id::DeviceId, - force_wait: bool, - submission_index: Option, + maintain: wgt::Maintain, ) -> Result { let (closures, queue_empty) = { - if let Some(submission_index) = submission_index { + if let wgt::Maintain::WaitForSubmissionIndex(submission_index) = maintain { if submission_index.queue_id != device_id { return Err(WaitIdleError::WrongSubmissionIndex( submission_index.queue_id, @@ -4994,7 +4995,7 @@ impl Global { device_guard .get(device_id) .map_err(|_| DeviceError::Invalid)? - .maintain(hub, force_wait, submission_index, &mut token)? + .maintain(hub, maintain, &mut token)? }; unsafe { closures.fire(); @@ -5022,7 +5023,12 @@ impl Global { let (device_guard, mut token) = hub.devices.read(&mut token); for (id, device) in device_guard.iter(A::VARIANT) { - let (cbs, queue_empty) = device.maintain(hub, force_wait, None, &mut token)?; + let maintain = if force_wait { + wgt::Maintain::Wait + } else { + wgt::Maintain::Poll + }; + let (cbs, queue_empty) = device.maintain(hub, maintain, &mut token)?; all_queue_empty = all_queue_empty && queue_empty; // If the device's own `RefCount` clone is the only one left, and diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 7246199c2b..8b1a662272 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -925,7 +925,7 @@ impl Global { // This will schedule destruction of all resources that are no longer needed // by the user but used in the command stream, among other things. - let (closures, _) = match device.maintain(hub, false, None, &mut token) { + let (closures, _) = match device.maintain(hub, wgt::Maintain::Wait, &mut token) { Ok(closures) => closures, Err(WaitIdleError::Device(err)) => return Err(QueueSubmitError::Queue(err)), Err(WaitIdleError::StuckGpu) => return Err(QueueSubmitError::StuckGpu), diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 7a74f71334..88e2a01205 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -2273,6 +2273,43 @@ impl Default for ColorWrites { } } +/// Passed to `Device::poll` to control how and if it should block. +#[derive(Clone)] +pub enum Maintain { + /// On native backends, block until the given submission has + /// completed execution, and any callbacks have been invoked. + /// + /// On the web, this has no effect. Callbacks are invoked from the + /// window event loop. + WaitForSubmissionIndex(T), + /// Same as WaitForSubmissionIndex but waits for the most recent submission. + Wait, + /// Check the device for a single time without blocking. + Poll, +} + +impl Maintain { + /// This maintain represents a wait of some kind. + pub fn is_wait(&self) -> bool { + match *self { + Self::WaitForSubmissionIndex(..) | Self::Wait => true, + Self::Poll => false, + } + } + + /// Map on the wait index type. + pub fn map_index(self, func: F) -> Maintain + where + F: FnOnce(T) -> U, + { + match self { + Self::WaitForSubmissionIndex(i) => Maintain::WaitForSubmissionIndex(func(i)), + Self::Wait => Maintain::Wait, + Self::Poll => Maintain::Poll, + } + } +} + /// State of the stencil operation (fixed-pipeline stage). /// /// For use in [`DepthStencilState`]. diff --git a/wgpu/examples/framework.rs b/wgpu/examples/framework.rs index ddab7ce68f..0ed135f1cd 100644 --- a/wgpu/examples/framework.rs +++ b/wgpu/examples/framework.rs @@ -518,7 +518,7 @@ pub fn test(mut params: FrameworkRefTest) { let dst_buffer_slice = dst_buffer.slice(..); let _ = dst_buffer_slice.map_async(wgpu::MapMode::Read); - ctx.device.poll(wgpu::Maintain::Wait(None)); + ctx.device.poll(wgpu::Maintain::Wait); let bytes = dst_buffer_slice.get_mapped_range().to_vec(); test_common::image::compare_image_output( diff --git a/wgpu/examples/hello-compute/main.rs b/wgpu/examples/hello-compute/main.rs index c5d58921ab..bbff9130f4 100644 --- a/wgpu/examples/hello-compute/main.rs +++ b/wgpu/examples/hello-compute/main.rs @@ -153,7 +153,7 @@ async fn execute_gpu_inner( // Poll the device in a blocking manner so that our future resolves. // In an actual application, `device.poll(...)` should // be called in an event loop or on another thread. - device.poll(wgpu::Maintain::Wait(None)); + device.poll(wgpu::Maintain::Wait); // Awaits until `buffer_future` can be read from if let Ok(()) = buffer_future.await { diff --git a/wgpu/examples/mipmap/main.rs b/wgpu/examples/mipmap/main.rs index 3570bdaf46..9603895d57 100644 --- a/wgpu/examples/mipmap/main.rs +++ b/wgpu/examples/mipmap/main.rs @@ -386,7 +386,7 @@ impl framework::Example for Example { .slice(..) .map_async(wgpu::MapMode::Read); // Wait for device to be done rendering mipmaps - device.poll(wgpu::Maintain::Wait(None)); + device.poll(wgpu::Maintain::Wait); // This is guaranteed to be ready. let timestamp_view = query_sets .data_buffer diff --git a/wgpu/src/backend/direct.rs b/wgpu/src/backend/direct.rs index 72dd696759..e62ada1a00 100644 --- a/wgpu/src/backend/direct.rs +++ b/wgpu/src/backend/direct.rs @@ -1572,7 +1572,8 @@ impl crate::Context for Context { #[cfg(any(not(target_arch = "wasm32"), feature = "emscripten"))] { - match wgc::gfx_select!(device.id => global.device_poll(device.id, true, None)) { + match wgc::gfx_select!(device.id => global.device_poll(device.id, wgt::Maintain::Wait)) + { Ok(_) => (), Err(err) => self.handle_error_fatal(err, "Device::drop"), } @@ -1583,14 +1584,10 @@ impl crate::Context for Context { fn device_poll(&self, device: &Self::DeviceId, maintain: crate::Maintain) -> bool { let global = &self.0; - let (wait, index) = match maintain { - crate::Maintain::Poll => (false, None), - crate::Maintain::Wait(index) => (true, index.map(|i| i.0)), - }; + let maintain_inner = maintain.map_index(|i| i.0); match wgc::gfx_select!(device.id => global.device_poll( device.id, - wait, - index + maintain_inner )) { Ok(queue_empty) => queue_empty, Err(err) => self.handle_error_fatal(err, "Device::poll"), diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 2bc59906b7..b645927cad 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -557,21 +557,6 @@ pub struct Device { #[derive(Debug, Copy, Clone)] pub struct SubmissionIndex(::SubmissionIndex); -/// Passed to [`Device::poll`] to control how and if it should block. -#[derive(Clone)] -pub enum Maintain { - /// On native backends, block until the given submission has - /// completed execution, and any callbacks have been invoked. - /// - /// On the web, this has no effect. Callbacks are invoked from the - /// window event loop. - /// - /// If the submission index is `None`, wait for the most recent submission. - Wait(Option), - /// Check the device for a single time without blocking. - Poll, -} - /// The main purpose of this struct is to resolve mapped ranges (convert sizes /// to end points), and to ensure that the sub-ranges don't intersect. #[derive(Debug)] @@ -1263,6 +1248,9 @@ pub type TextureDescriptor<'a> = wgt::TextureDescriptor>; /// Corresponds to [WebGPU `GPUQuerySetDescriptor`]( /// https://gpuweb.github.io/gpuweb/#dictdef-gpuquerysetdescriptor). pub type QuerySetDescriptor<'a> = wgt::QuerySetDescriptor>; +pub use wgt::Maintain as MaintainBase; +/// Passed to [`Device::poll`] to control how and if it should block. +pub type Maintain = wgt::Maintain; /// Describes a [`TextureView`]. /// diff --git a/wgpu/tests/common/mod.rs b/wgpu/tests/common/mod.rs index 8daa0b71c6..b0df54d2f1 100644 --- a/wgpu/tests/common/mod.rs +++ b/wgpu/tests/common/mod.rs @@ -110,7 +110,7 @@ impl TestParameters { self } - /// Mark the test as always failing, equivilant to specific_failure(None, None, None) + /// Mark the test as always failing, equivalent to specific_failure(None, None, None) pub fn failure(mut self) -> Self { self.failures.push(FailureCase { backends: None, @@ -121,7 +121,7 @@ impl TestParameters { self } - /// Mark the test as always failing and needing to be skipped, equivilant to specific_failure(None, None, None) + /// Mark the test as always failing and needing to be skipped, equivalent to specific_failure(None, None, None) pub fn skip(mut self) -> Self { self.failures.push(FailureCase { backends: None, @@ -132,7 +132,7 @@ impl TestParameters { self } - /// Mark the test as always failing on a specific backend, equivilant to specific_failure(backend, None, None) + /// Mark the test as always failing on a specific backend, equivalent to specific_failure(backend, None, None) pub fn backend_failure(mut self, backends: wgpu::Backends) -> Self { self.failures.push(FailureCase { backends: Some(backends), diff --git a/wgpu/tests/poll.rs b/wgpu/tests/poll.rs index 7409a96ffc..6113436d0b 100644 --- a/wgpu/tests/poll.rs +++ b/wgpu/tests/poll.rs @@ -58,7 +58,7 @@ fn wait() { let cmd_buf = generate_dummy_work(&ctx); ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait(None)); + ctx.device.poll(Maintain::Wait); }) } @@ -68,8 +68,8 @@ fn double_wait() { let cmd_buf = generate_dummy_work(&ctx); ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait(None)); - ctx.device.poll(Maintain::Wait(None)); + ctx.device.poll(Maintain::Wait); + ctx.device.poll(Maintain::Wait); }) } @@ -79,7 +79,7 @@ fn wait_on_submission() { let cmd_buf = generate_dummy_work(&ctx); let index = ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait(Some(index))); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); }) } @@ -89,8 +89,8 @@ fn double_wait_on_submission() { let cmd_buf = generate_dummy_work(&ctx); let index = ctx.queue.submit(Some(cmd_buf)); - ctx.device.poll(Maintain::Wait(Some(index))); - ctx.device.poll(Maintain::Wait(Some(index))); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index)); }) } @@ -102,7 +102,7 @@ fn wait_out_of_order() { let index1 = ctx.queue.submit(Some(cmd_buf1)); let index2 = ctx.queue.submit(Some(cmd_buf2)); - ctx.device.poll(Maintain::Wait(Some(index2))); - ctx.device.poll(Maintain::Wait(Some(index1))); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index2)); + ctx.device.poll(Maintain::WaitForSubmissionIndex(index1)); }) } diff --git a/wgpu/tests/vertex_indices/mod.rs b/wgpu/tests/vertex_indices/mod.rs index 1bc362f7db..fa85ae62d9 100644 --- a/wgpu/tests/vertex_indices/mod.rs +++ b/wgpu/tests/vertex_indices/mod.rs @@ -124,7 +124,7 @@ fn pulling_common( ctx.queue.submit(Some(encoder.finish())); let slice = buffer.slice(..); let _ = slice.map_async(wgpu::MapMode::Read); - ctx.device.poll(wgpu::Maintain::Wait(None)); + ctx.device.poll(wgpu::Maintain::Wait); let data: Vec = bytemuck::cast_slice(&*slice.get_mapped_range()).to_vec(); assert_eq!(data, expected); diff --git a/wgpu/tests/zero_init_texture_after_discard.rs b/wgpu/tests/zero_init_texture_after_discard.rs index 488778aff1..ec77e909f0 100644 --- a/wgpu/tests/zero_init_texture_after_discard.rs +++ b/wgpu/tests/zero_init_texture_after_discard.rs @@ -283,7 +283,7 @@ fn assert_buffer_is_zero(readback_buffer: &wgpu::Buffer, device: &wgpu::Device) { let buffer_slice = readback_buffer.slice(..); let _ = buffer_slice.map_async(wgpu::MapMode::Read); - device.poll(wgpu::Maintain::Wait(None)); + device.poll(wgpu::Maintain::Wait); let buffer_view = buffer_slice.get_mapped_range(); assert!(