diff --git a/tests/tests/render_pass_ownership.rs b/tests/tests/render_pass_ownership.rs index 7c2599f6198..04a8c5db4a9 100644 --- a/tests/tests/render_pass_ownership.rs +++ b/tests/tests/render_pass_ownership.rs @@ -12,7 +12,7 @@ use std::num::NonZeroU64; use wgpu::util::DeviceExt as _; -use wgpu_test::{gpu_test, GpuTestConfiguration, TestParameters, TestingContext}; +use wgpu_test::{gpu_test, valid, GpuTestConfiguration, TestParameters, TestingContext}; // Minimal shader with buffer based side effect - only needed to check whether the render pass has executed at all. const SHADER_SRC: &str = " @@ -129,6 +129,7 @@ async fn render_pass_query_set_ownership_pipeline_statistics(ctx: TestingContext bind_group, pipeline, color_attachment_view, + depth_stencil_view, .. } = resource_setup(&ctx); @@ -151,6 +152,14 @@ async fn render_pass_query_set_ownership_pipeline_statistics(ctx: TestingContext resolve_target: None, ops: wgpu::Operations::default(), })], + depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { + view: &depth_stencil_view, + depth_ops: Some(wgpu::Operations { + load: wgpu::LoadOp::Clear(1.0), + store: wgpu::StoreOp::Store, + }), + stencil_ops: None, + }), ..Default::default() }); rpass.set_pipeline(&pipeline); @@ -229,50 +238,65 @@ async fn render_pass_query_timestamps(ctx: TestingContext) { assert_render_pass_executed_normally(encoder, gpu_buffer, cpu_buffer, buffer_size, ctx).await; } -// #[gpu_test] -// static COMPUTE_PASS_KEEP_ENCODER_ALIVE: GpuTestConfiguration = GpuTestConfiguration::new() -// .parameters(TestParameters::default().test_features_limits()) -// .run_async(compute_pass_keep_encoder_alive); - -// async fn compute_pass_keep_encoder_alive(ctx: TestingContext) { -// let ResourceSetup { -// gpu_buffer: _, -// cpu_buffer: _, -// buffer_size: _, -// indirect_buffer, -// bind_group, -// pipeline, -// } = resource_setup(&ctx); - -// let mut encoder = ctx -// .device -// .create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); - -// let rpass = encoder.begin_render_pass(&wgpu::ComputePassDescriptor { -// label: Some("compute_pass"), -// timestamp_writes: None, -// }); - -// // Now drop the encoder - it is kept alive by the compute pass. -// // To do so, we have to make the compute pass forget the lifetime constraint first. -// let mut rpass = rpass.forget_lifetime(); -// drop(encoder); - -// ctx.async_poll(wgpu::Maintain::wait()) -// .await -// .panic_on_timeout(); - -// // Record some draw commands. -// rpass.set_pipeline(&pipeline); -// rpass.set_bind_group(0, &bind_group, &[]); -// rpass.dispatch_workgroups_indirect(&indirect_buffer, 0); - -// // Dropping the pass will still execute the pass, even though there's no way to submit it. -// // Ideally, this would log an error, but the encoder is not dropped until the compute pass is dropped, -// // making this a valid operation. -// // (If instead the encoder was explicitly destroyed or finished, this would be an error.) -// valid(&ctx.device, || drop(rpass)); -// } + +#[gpu_test] +static RENDER_PASS_KEEP_ENCODER_ALIVE: GpuTestConfiguration = GpuTestConfiguration::new() + .parameters(TestParameters::default().test_features_limits()) + .run_async(render_pass_keep_encoder_alive); + +async fn render_pass_keep_encoder_alive(ctx: TestingContext) { + let ResourceSetup { + bind_group, + vertex_buffer, + index_buffer, + pipeline, + color_attachment_view, + depth_stencil_view, + .. + } = resource_setup(&ctx); + + let mut encoder = ctx + .device + .create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); + let rpass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor { + color_attachments: &[Some(wgpu::RenderPassColorAttachment { + view: &color_attachment_view, + resolve_target: None, + ops: wgpu::Operations::default(), + })], + depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachment { + view: &depth_stencil_view, + depth_ops: Some(wgpu::Operations { + load: wgpu::LoadOp::Clear(1.0), + store: wgpu::StoreOp::Store, + }), + stencil_ops: None, + }), + ..Default::default() + }); + + // Now drop the encoder - it is kept alive by the compute pass. + // To do so, we have to make the compute pass forget the lifetime constraint first. + let mut rpass = rpass.forget_lifetime(); + drop(encoder); + + ctx.async_poll(wgpu::Maintain::wait()) + .await + .panic_on_timeout(); + + // Record some a draw command. + rpass.set_pipeline(&pipeline); + rpass.set_bind_group(0, &bind_group, &[]); + rpass.set_vertex_buffer(0, vertex_buffer.slice(..)); + rpass.set_index_buffer(index_buffer.slice(..), wgpu::IndexFormat::Uint32); + rpass.draw(0..3, 0..1); + + // Dropping the pass will still execute the pass, even though there's no way to submit it. + // Ideally, this would log an error, but the encoder is not dropped until the compute pass is dropped, + // making this a valid operation. + // (If instead the encoder was explicitly destroyed or finished, this would be an error.) + valid(&ctx.device, || drop(rpass)); +} async fn assert_render_pass_executed_normally( mut encoder: wgpu::CommandEncoder, diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 6b44f08a8ea..5eaec79eb2c 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -1274,9 +1274,19 @@ impl Drop for CommandEncoder { /// https://gpuweb.github.io/gpuweb/#render-pass-encoder). #[derive(Debug)] pub struct RenderPass<'encoder> { + /// The inner data of the render pass, separated out so it's easy to replace the lifetime with 'static if desired. + inner: RenderPassInner, + + /// This lifetime is used to protect the [`CommandEncoder`] from being used + /// while the pass is alive. + encoder_guard: PhantomData<&'encoder ()>, +} + +#[derive(Debug)] +struct RenderPassInner { id: ObjectId, data: Box, - parent: &'encoder mut CommandEncoder, + context: Arc, } /// In-progress recording of a compute pass. @@ -3883,12 +3893,13 @@ impl CommandEncoder { /// Begins recording of a render pass. /// /// This function returns a [`RenderPass`] object which records a single render pass. - // - // TODO(https://github.com/gfx-rs/wgpu/issues/1453): - // Just like with compute passes, we should have a way to opt out of the lifetime constraint. - // See https://github.com/gfx-rs/wgpu/pull/5768 for details - // Once this is done, the documentation for `begin_render_pass` and `begin_compute_pass` should - // be nearly identical. + /// + /// As long as the returned [`RenderPass`] has not ended, + /// any mutating operation on this command encoder causes an error and invalidates it. + /// Note that the `'encoder` lifetime relationship protects against this, + /// but it is possible to opt out of it by calling [`RenderPass::forget_lifetime`]. + /// This can be useful for runtime handling of the encoder->pass + /// dependency e.g. when pass and encoder are stored in the same data structure. pub fn begin_render_pass<'encoder>( &'encoder mut self, desc: &RenderPassDescriptor<'_>, @@ -3901,9 +3912,12 @@ impl CommandEncoder { desc, ); RenderPass { - id, - data, - parent: self, + inner: RenderPassInner { + id, + data, + context: self.context.clone(), + }, + encoder_guard: PhantomData, } } @@ -4175,6 +4189,25 @@ impl CommandEncoder { } impl<'encoder> RenderPass<'encoder> { + /// Drops the lifetime relationship to the parent command encoder, making usage of + /// the encoder while this pass is recorded a run-time error instead. + /// + /// Attention: As long as the render pass has not been ended, any mutating operation on the parent + /// command encoder will cause a run-time error and invalidate it! + /// By default, the lifetime constraint prevents this, but it can be useful + /// to handle this at run time, such as when storing the pass and encoder in the same + /// data structure. + /// + /// This operation has no effect on pass recording. + /// It's a safe operation, since [`CommandEncoder`] is in a locked state as long as the pass is active + /// regardless of the lifetime constraint or its absence. + pub fn forget_lifetime(self) -> RenderPass<'static> { + RenderPass { + inner: self.inner, + encoder_guard: PhantomData, + } + } + /// Sets the active bind group for a given bind group index. The bind group layout /// in the active pipeline when any `draw_*()` method is called must match the layout of /// this bind group. @@ -4191,9 +4224,9 @@ impl<'encoder> RenderPass<'encoder> { offsets: &[DynamicOffset], ) { DynContext::render_pass_set_bind_group( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), index, &bind_group.id, bind_group.data.as_ref(), @@ -4206,9 +4239,9 @@ impl<'encoder> RenderPass<'encoder> { /// Subsequent draw calls will exhibit the behavior defined by `pipeline`. pub fn set_pipeline(&mut self, pipeline: &RenderPipeline) { DynContext::render_pass_set_pipeline( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &pipeline.id, pipeline.data.as_ref(), ) @@ -4221,9 +4254,9 @@ impl<'encoder> RenderPass<'encoder> { /// (all components zero). pub fn set_blend_constant(&mut self, color: Color) { DynContext::render_pass_set_blend_constant( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), color, ) } @@ -4234,9 +4267,9 @@ impl<'encoder> RenderPass<'encoder> { /// use `buffer` as the source index buffer. pub fn set_index_buffer(&mut self, buffer_slice: BufferSlice<'_>, index_format: IndexFormat) { DynContext::render_pass_set_index_buffer( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &buffer_slice.buffer.id, buffer_slice.buffer.data.as_ref(), index_format, @@ -4257,9 +4290,9 @@ impl<'encoder> RenderPass<'encoder> { /// [`draw_indexed`]: RenderPass::draw_indexed pub fn set_vertex_buffer(&mut self, slot: u32, buffer_slice: BufferSlice<'_>) { DynContext::render_pass_set_vertex_buffer( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), slot, &buffer_slice.buffer.id, buffer_slice.buffer.data.as_ref(), @@ -4279,9 +4312,9 @@ impl<'encoder> RenderPass<'encoder> { /// but it does not affect the coordinate system, only which fragments are discarded. pub fn set_scissor_rect(&mut self, x: u32, y: u32, width: u32, height: u32) { DynContext::render_pass_set_scissor_rect( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), x, y, width, @@ -4297,9 +4330,9 @@ impl<'encoder> RenderPass<'encoder> { /// targets. pub fn set_viewport(&mut self, x: f32, y: f32, w: f32, h: f32, min_depth: f32, max_depth: f32) { DynContext::render_pass_set_viewport( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), x, y, w, @@ -4315,9 +4348,9 @@ impl<'encoder> RenderPass<'encoder> { /// If this method has not been called, the stencil reference value defaults to `0`. pub fn set_stencil_reference(&mut self, reference: u32) { DynContext::render_pass_set_stencil_reference( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), reference, ); } @@ -4325,9 +4358,9 @@ impl<'encoder> RenderPass<'encoder> { /// Inserts debug marker. pub fn insert_debug_marker(&mut self, label: &str) { DynContext::render_pass_insert_debug_marker( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), label, ); } @@ -4335,9 +4368,9 @@ impl<'encoder> RenderPass<'encoder> { /// Start record commands and group it into debug marker group. pub fn push_debug_group(&mut self, label: &str) { DynContext::render_pass_push_debug_group( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), label, ); } @@ -4345,9 +4378,9 @@ impl<'encoder> RenderPass<'encoder> { /// Stops command recording and creates debug group. pub fn pop_debug_group(&mut self) { DynContext::render_pass_pop_debug_group( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), ); } @@ -4374,9 +4407,9 @@ impl<'encoder> RenderPass<'encoder> { /// It is not affected by changes to the state that are performed after it is called. pub fn draw(&mut self, vertices: Range, instances: Range) { DynContext::render_pass_draw( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), vertices, instances, ) @@ -4408,9 +4441,9 @@ impl<'encoder> RenderPass<'encoder> { /// It is not affected by changes to the state that are performed after it is called. pub fn draw_indexed(&mut self, indices: Range, base_vertex: i32, instances: Range) { DynContext::render_pass_draw_indexed( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), indices, base_vertex, instances, @@ -4432,9 +4465,9 @@ impl<'encoder> RenderPass<'encoder> { /// See details on the individual flags for more information. pub fn draw_indirect(&mut self, indirect_buffer: &Buffer, indirect_offset: BufferAddress) { DynContext::render_pass_draw_indirect( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4461,9 +4494,9 @@ impl<'encoder> RenderPass<'encoder> { indirect_offset: BufferAddress, ) { DynContext::render_pass_draw_indexed_indirect( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4484,16 +4517,16 @@ impl<'encoder> RenderPass<'encoder> { .map(|rb| (&rb.id, rb.data.as_ref())); DynContext::render_pass_execute_bundles( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &mut render_bundles, ) } } /// [`Features::MULTI_DRAW_INDIRECT`] must be enabled on the device in order to call these functions. -impl<'a> RenderPass<'a> { +impl<'encoder> RenderPass<'encoder> { /// Dispatches multiple draw calls from the active vertex buffer(s) based on the contents of the `indirect_buffer`. /// `count` draw calls are issued. /// @@ -4511,9 +4544,9 @@ impl<'a> RenderPass<'a> { count: u32, ) { DynContext::render_pass_multi_draw_indirect( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4539,9 +4572,9 @@ impl<'a> RenderPass<'a> { count: u32, ) { DynContext::render_pass_multi_draw_indexed_indirect( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4583,9 +4616,9 @@ impl<'encoder> RenderPass<'encoder> { max_count: u32, ) { DynContext::render_pass_multi_draw_indirect_count( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4630,9 +4663,9 @@ impl<'encoder> RenderPass<'encoder> { max_count: u32, ) { DynContext::render_pass_multi_draw_indexed_indirect_count( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &indirect_buffer.id, indirect_buffer.data.as_ref(), indirect_offset, @@ -4688,9 +4721,9 @@ impl<'encoder> RenderPass<'encoder> { /// [`PushConstant`]: https://docs.rs/naga/latest/naga/enum.StorageClass.html#variant.PushConstant pub fn set_push_constants(&mut self, stages: ShaderStages, offset: u32, data: &[u8]) { DynContext::render_pass_set_push_constants( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), stages, offset, data, @@ -4709,9 +4742,9 @@ impl<'encoder> RenderPass<'encoder> { /// for a string of operations to complete. pub fn write_timestamp(&mut self, query_set: &QuerySet, query_index: u32) { DynContext::render_pass_write_timestamp( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &query_set.id, query_set.data.as_ref(), query_index, @@ -4719,14 +4752,14 @@ impl<'encoder> RenderPass<'encoder> { } } -impl<'a> RenderPass<'a> { +impl<'encoder> RenderPass<'encoder> { /// Start a occlusion query on this render pass. It can be ended with /// `end_occlusion_query`. Occlusion queries may not be nested. pub fn begin_occlusion_query(&mut self, query_index: u32) { DynContext::render_pass_begin_occlusion_query( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), query_index, ); } @@ -4735,22 +4768,22 @@ impl<'a> RenderPass<'a> { /// `begin_occlusion_query`. Occlusion queries may not be nested. pub fn end_occlusion_query(&mut self) { DynContext::render_pass_end_occlusion_query( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), ); } } /// [`Features::PIPELINE_STATISTICS_QUERY`] must be enabled on the device in order to call these functions. -impl<'a> RenderPass<'a> { +impl<'encoder> RenderPass<'encoder> { /// Start a pipeline statistics query on this render pass. It can be ended with /// `end_pipeline_statistics_query`. Pipeline statistics queries may not be nested. pub fn begin_pipeline_statistics_query(&mut self, query_set: &QuerySet, query_index: u32) { DynContext::render_pass_begin_pipeline_statistics_query( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), &query_set.id, query_set.data.as_ref(), query_index, @@ -4761,18 +4794,17 @@ impl<'a> RenderPass<'a> { /// `begin_pipeline_statistics_query`. Pipeline statistics queries may not be nested. pub fn end_pipeline_statistics_query(&mut self) { DynContext::render_pass_end_pipeline_statistics_query( - &*self.parent.context, - &mut self.id, - self.data.as_mut(), + &*self.inner.context, + &mut self.inner.id, + self.inner.data.as_mut(), ); } } -impl<'encoder> Drop for RenderPass<'encoder> { +impl Drop for RenderPassInner { fn drop(&mut self) { if !thread::panicking() { - self.parent - .context + self.context .render_pass_end(&mut self.id, self.data.as_mut()); } }