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

Renderpass take resource ownership #5884

Merged
merged 15 commits into from
Jul 1, 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
30 changes: 22 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,38 @@ Bottom level categories:

### Major Changes

#### Remove lifetime bounds on `wgpu::ComputePass`
#### Lifetime bounds on `wgpu::RenderPass` & `wgpu::ComputePass`

TODO(wumpf): This is still work in progress. Should write a bit more about it. Also will very likely extend to `wgpu::RenderPass` before release.
`wgpu::RenderPass` & `wgpu::ComputePass` recording methods (e.g. `wgpu::RenderPass:set_render_pipeline`) no longer impose a lifetime constraint to objects passed to a pass (like pipelines/buffers/bindgroups/query-sets etc.).

This means the following pattern works now as expected:
```rust
let mut pipelines: Vec<wgpu::RenderPipeline> = ...;
// ...
let mut cpass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
cpass.set_pipeline(&pipelines[123]);
// Change pipeline container - this requires mutable access to `pipelines` while one of the pipelines is in use.
pipelines.push(/* ... */);
// Continue pass recording.
cpass.set_bindgroup(...);
```
Previously, a set pipeline (or other resource) had to outlive pass recording which often affected wider systems,
meaning that users needed to prove to the borrow checker that `Vec<wgpu::RenderPipeline>` (or similar constructs)
aren't accessed mutably for the duration of pass recording.

`wgpu::ComputePass` recording methods (e.g. `wgpu::ComputePass:set_render_pipeline`) no longer impose a lifetime constraint passed in resources.

Furthermore, you can now opt out of `wgpu::ComputePass`'s lifetime dependency on its parent `wgpu::CommandEncoder` using `wgpu::ComputePass::forget_lifetime`:
Furthermore, you can now opt out of `wgpu::RenderPass`/`wgpu::ComputePass`'s lifetime dependency on its parent `wgpu::CommandEncoder` using `wgpu::RenderPass::forget_lifetime`/`wgpu::ComputePass::forget_lifetime`:
```rust
fn independent_cpass<'enc>(encoder: &'enc mut wgpu::CommandEncoder) -> wgpu::ComputePass<'static> {
let cpass: wgpu::ComputePass<'enc> = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
cpass.forget_lifetime()
}
```
⚠️ As long as a `wgpu::ComputePass` is pending for a given `wgpu::CommandEncoder`, creation of a compute or render pass is an error and invalidates the `wgpu::CommandEncoder`.
This is very useful for library authors, but opens up an easy way for incorrect use, so use with care.
`forget_lifetime` is zero overhead and has no side effects on pass recording.
⚠️ As long as a `wgpu::RenderPass`/`wgpu::ComputePass` is pending for a given `wgpu::CommandEncoder`, creation of a compute or render pass is an error and invalidates the `wgpu::CommandEncoder`.
`forget_lifetime` can be very useful for library authors, but opens up an easy way for incorrect use, so use with care.
This method doesn't add any additional overhead and has no side effects on pass recording.

By @wumpf in [#5569](https://github.com/gfx-rs/wgpu/pull/5569), [#5575](https://github.com/gfx-rs/wgpu/pull/5575), [#5620](https://github.com/gfx-rs/wgpu/pull/5620), [#5768](https://github.com/gfx-rs/wgpu/pull/5768) (together with @kpreid), [#5671](https://github.com/gfx-rs/wgpu/pull/5671).
By @wumpf in [#5569](https://github.com/gfx-rs/wgpu/pull/5569), [#5575](https://github.com/gfx-rs/wgpu/pull/5575), [#5620](https://github.com/gfx-rs/wgpu/pull/5620), [#5768](https://github.com/gfx-rs/wgpu/pull/5768) (together with @kpreid), [#5671](https://github.com/gfx-rs/wgpu/pull/5671), [#5794](https://github.com/gfx-rs/wgpu/pull/5794), [#5884](https://github.com/gfx-rs/wgpu/pull/5884).

#### Querying shader compilation errors

Expand Down
11 changes: 6 additions & 5 deletions deno_webgpu/command_encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
.get::<WebGpuQuerySet>(timestamp_writes.query_set)?;
let query_set = query_set_resource.1;

Some(wgpu_core::command::RenderPassTimestampWrites {
Some(wgpu_core::command::PassTimestampWrites {
query_set,
beginning_of_pass_write_index: timestamp_writes.beginning_of_pass_write_index,
end_of_pass_write_index: timestamp_writes.end_of_pass_write_index,
Expand All @@ -200,6 +200,8 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
.transpose()?
.map(|query_set| query_set.1);

let instance = state.borrow::<super::Instance>();
let command_encoder = &command_encoder_resource.1;
let descriptor = wgpu_core::command::RenderPassDescriptor {
label: Some(label),
color_attachments: Cow::from(color_attachments),
Expand All @@ -208,15 +210,14 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
occlusion_query_set: occlusion_query_set_resource,
};

let render_pass = wgpu_core::command::RenderPass::new(command_encoder_resource.1, &descriptor);

let (render_pass, error) = gfx_select!(command_encoder => instance.command_encoder_create_render_pass_dyn(*command_encoder, &descriptor));
let rid = state
.resource_table
.add(super::render_pass::WebGpuRenderPass(RefCell::new(
render_pass,
)));

Ok(WebGpuResult::rid(rid))
Ok(WebGpuResult::rid_err(rid, error))
}

#[derive(Deserialize)]
Expand Down Expand Up @@ -245,7 +246,7 @@ pub fn op_webgpu_command_encoder_begin_compute_pass(
.get::<WebGpuQuerySet>(timestamp_writes.query_set)?;
let query_set = query_set_resource.1;

Some(wgpu_core::command::ComputePassTimestampWrites {
Some(wgpu_core::command::PassTimestampWrites {
query_set,
beginning_of_pass_write_index: timestamp_writes.beginning_of_pass_write_index,
end_of_pass_write_index: timestamp_writes.end_of_pass_write_index,
Expand Down
111 changes: 54 additions & 57 deletions deno_webgpu/render_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ use deno_core::ResourceId;
use serde::Deserialize;
use std::borrow::Cow;
use std::cell::RefCell;
use wgpu_core::global::Global;

use super::error::WebGpuResult;

pub(crate) struct WebGpuRenderPass(pub(crate) RefCell<wgpu_core::command::RenderPass>);
pub(crate) struct WebGpuRenderPass(pub(crate) RefCell<Box<dyn wgpu_core::command::DynRenderPass>>);
impl Resource for WebGpuRenderPass {
fn name(&self) -> Cow<str> {
"webGPURenderPass".into()
Expand Down Expand Up @@ -42,8 +41,8 @@ pub fn op_webgpu_render_pass_set_viewport(
.resource_table
.get::<WebGpuRenderPass>(args.render_pass_rid)?;

state.borrow::<Global>().render_pass_set_viewport(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().set_viewport(
state.borrow(),
args.x,
args.y,
args.width,
Expand All @@ -69,13 +68,10 @@ pub fn op_webgpu_render_pass_set_scissor_rect(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_set_scissor_rect(
&mut render_pass_resource.0.borrow_mut(),
x,
y,
width,
height,
)?;
render_pass_resource
.0
.borrow_mut()
.set_scissor_rect(state.borrow(), x, y, width, height)?;

Ok(WebGpuResult::empty())
}
Expand All @@ -91,9 +87,10 @@ pub fn op_webgpu_render_pass_set_blend_constant(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_set_blend_constant(&mut render_pass_resource.0.borrow_mut(), &color)?;
render_pass_resource
.0
.borrow_mut()
.set_blend_constant(state.borrow(), color)?;

Ok(WebGpuResult::empty())
}
Expand All @@ -109,9 +106,10 @@ pub fn op_webgpu_render_pass_set_stencil_reference(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_set_stencil_reference(&mut render_pass_resource.0.borrow_mut(), reference)?;
render_pass_resource
.0
.borrow_mut()
.set_stencil_reference(state.borrow(), reference)?;

Ok(WebGpuResult::empty())
}
Expand All @@ -127,9 +125,10 @@ pub fn op_webgpu_render_pass_begin_occlusion_query(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_begin_occlusion_query(&mut render_pass_resource.0.borrow_mut(), query_index)?;
render_pass_resource
.0
.borrow_mut()
.begin_occlusion_query(state.borrow(), query_index)?;

Ok(WebGpuResult::empty())
}
Expand All @@ -144,9 +143,10 @@ pub fn op_webgpu_render_pass_end_occlusion_query(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_end_occlusion_query(&mut render_pass_resource.0.borrow_mut())?;
render_pass_resource
.0
.borrow_mut()
.end_occlusion_query(state.borrow())?;

Ok(WebGpuResult::empty())
}
Expand All @@ -172,9 +172,10 @@ pub fn op_webgpu_render_pass_execute_bundles(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_execute_bundles(&mut render_pass_resource.0.borrow_mut(), &bundles)?;
render_pass_resource
.0
.borrow_mut()
.execute_bundles(state.borrow(), &bundles)?;

Ok(WebGpuResult::empty())
}
Expand All @@ -189,12 +190,7 @@ pub fn op_webgpu_render_pass_end(
.resource_table
.take::<WebGpuRenderPass>(render_pass_rid)?;

// TODO: Just like parent_id ComputePass, there's going to be DynComputePass soon which will eliminate the need of doing gfx_select here.
let instance = state.borrow::<Global>();
let parent_id = render_pass_resource.0.borrow().parent_id();
gfx_select!(parent_id => instance.render_pass_end(
&mut render_pass_resource.0.borrow_mut()
))?;
render_pass_resource.0.borrow_mut().end(state.borrow())?;

Ok(WebGpuResult::empty())
}
Expand Down Expand Up @@ -226,8 +222,8 @@ pub fn op_webgpu_render_pass_set_bind_group(

let dynamic_offsets_data: &[u32] = &dynamic_offsets_data[start..start + len];

state.borrow::<Global>().render_pass_set_bind_group(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().set_bind_group(
state.borrow(),
index,
bind_group_resource.1,
dynamic_offsets_data,
Expand All @@ -247,8 +243,8 @@ pub fn op_webgpu_render_pass_push_debug_group(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_push_debug_group(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().push_debug_group(
state.borrow(),
group_label,
0, // wgpu#975
)?;
Expand All @@ -266,9 +262,10 @@ pub fn op_webgpu_render_pass_pop_debug_group(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state
.borrow::<Global>()
.render_pass_pop_debug_group(&mut render_pass_resource.0.borrow_mut())?;
render_pass_resource
.0
.borrow_mut()
.pop_debug_group(state.borrow())?;

Ok(WebGpuResult::empty())
}
Expand All @@ -284,8 +281,8 @@ pub fn op_webgpu_render_pass_insert_debug_marker(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_insert_debug_marker(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().insert_debug_marker(
state.borrow(),
marker_label,
0, // wgpu#975
)?;
Expand All @@ -307,10 +304,10 @@ pub fn op_webgpu_render_pass_set_pipeline(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_set_pipeline(
&mut render_pass_resource.0.borrow_mut(),
render_pipeline_resource.1,
)?;
render_pass_resource
.0
.borrow_mut()
.set_pipeline(state.borrow(), render_pipeline_resource.1)?;

Ok(WebGpuResult::empty())
}
Expand Down Expand Up @@ -341,8 +338,8 @@ pub fn op_webgpu_render_pass_set_index_buffer(
None
};

state.borrow::<Global>().render_pass_set_index_buffer(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().set_index_buffer(
state.borrow(),
buffer_resource.1,
index_format,
offset,
Expand Down Expand Up @@ -378,8 +375,8 @@ pub fn op_webgpu_render_pass_set_vertex_buffer(
None
};

state.borrow::<Global>().render_pass_set_vertex_buffer(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().set_vertex_buffer(
state.borrow(),
slot,
buffer_resource.1,
offset,
Expand All @@ -403,8 +400,8 @@ pub fn op_webgpu_render_pass_draw(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_draw(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().draw(
state.borrow(),
vertex_count,
instance_count,
first_vertex,
Expand All @@ -429,8 +426,8 @@ pub fn op_webgpu_render_pass_draw_indexed(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_draw_indexed(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().draw_indexed(
state.borrow(),
index_count,
instance_count,
first_index,
Expand All @@ -456,8 +453,8 @@ pub fn op_webgpu_render_pass_draw_indirect(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_draw_indirect(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().draw_indirect(
state.borrow(),
buffer_resource.1,
indirect_offset,
)?;
Expand All @@ -480,8 +477,8 @@ pub fn op_webgpu_render_pass_draw_indexed_indirect(
.resource_table
.get::<WebGpuRenderPass>(render_pass_rid)?;

state.borrow::<Global>().render_pass_draw_indexed_indirect(
&mut render_pass_resource.0.borrow_mut(),
render_pass_resource.0.borrow_mut().draw_indexed_indirect(
state.borrow(),
buffer_resource.1,
indirect_offset,
)?;
Expand Down
6 changes: 3 additions & 3 deletions tests/tests/compute_pass_ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,14 @@ async fn compute_pass_query_set_ownership_pipeline_statistics(ctx: TestingContex
}

#[gpu_test]
static COMPUTE_PASS_QUERY_TIMESTAMPS: GpuTestConfiguration =
static COMPUTE_PASS_QUERY_SET_OWNERSHIP_TIMESTAMPS: GpuTestConfiguration =
GpuTestConfiguration::new()
.parameters(TestParameters::default().test_features_limits().features(
wgpu::Features::TIMESTAMP_QUERY | wgpu::Features::TIMESTAMP_QUERY_INSIDE_PASSES,
))
.run_async(compute_pass_query_timestamps);
.run_async(compute_pass_query_set_ownership_timestamps);

async fn compute_pass_query_timestamps(ctx: TestingContext) {
async fn compute_pass_query_set_ownership_timestamps(ctx: TestingContext) {
let ResourceSetup {
gpu_buffer,
cpu_buffer,
Expand Down
Loading
Loading