Skip to content

Commit 0a76c0f

Browse files
WumpfWumpf
authored
Renderpass take resource ownership (gfx-rs#5884)
* share timestamp write struct * Make name of set_push_constants methods consistently plural * remove lifetime bounds of resources passed into render pass * first render pass resource ownership test * introduce dynrenderpass & immediately create ArcCommands and take ownership of resources passed on pass creation * Use of dynrenderpass in deno * Separate active occlusion & pipeline statitics query * resolve render/compute command is now behind `replay` feature * add vertex & index buffer to ownership test * test for pipeline statistics query * add occlusion query set to pass resource test * add tests for resource ownership of render pass query timestamps * RenderPass can now be made 'static just like ComputePass. Add respective test * Extend encoder_operations_fail_while_pass_alive test to also check encoder locking errors with render passes * improve changelog entry on lifetime bounds
1 parent c9a2d97 commit 0a76c0f

21 files changed

Lines changed: 2135 additions & 786 deletions

‎CHANGELOG.md‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,38 @@ Bottom level categories:
4141

4242
### Major Changes
4343

44-
#### Remove lifetime bounds on `wgpu::ComputePass`
44+
#### Lifetime bounds on `wgpu::RenderPass` & `wgpu::ComputePass`
4545

46-
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.
46+
`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.).
47+
48+
This means the following pattern works now as expected:
49+
```rust
50+
let mut pipelines: Vec<wgpu::RenderPipeline> = ...;
51+
// ...
52+
let mut cpass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
53+
cpass.set_pipeline(&pipelines[123]);
54+
// Change pipeline container - this requires mutable access to `pipelines` while one of the pipelines is in use.
55+
pipelines.push(/* ... */);
56+
// Continue pass recording.
57+
cpass.set_bindgroup(...);
58+
```
59+
Previously, a set pipeline (or other resource) had to outlive pass recording which often affected wider systems,
60+
meaning that users needed to prove to the borrow checker that `Vec<wgpu::RenderPipeline>` (or similar constructs)
61+
aren't accessed mutably for the duration of pass recording.
4762

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

50-
Furthermore, you can now opt out of `wgpu::ComputePass`'s lifetime dependency on its parent `wgpu::CommandEncoder` using `wgpu::ComputePass::forget_lifetime`:
64+
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`:
5165
```rust
5266
fn independent_cpass<'enc>(encoder: &'enc mut wgpu::CommandEncoder) -> wgpu::ComputePass<'static> {
5367
let cpass: wgpu::ComputePass<'enc> = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
5468
cpass.forget_lifetime()
5569
}
5670
```
57-
⚠️ 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`.
58-
This is very useful for library authors, but opens up an easy way for incorrect use, so use with care.
59-
`forget_lifetime` is zero overhead and has no side effects on pass recording.
71+
⚠️ 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`.
72+
`forget_lifetime` can be very useful for library authors, but opens up an easy way for incorrect use, so use with care.
73+
This method doesn't add any additional overhead and has no side effects on pass recording.
6074

61-
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).
75+
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).
6276

6377
#### Querying shader compilation errors
6478

‎deno_webgpu/command_encoder.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
186186
.get::<WebGpuQuerySet>(timestamp_writes.query_set)?;
187187
let query_set = query_set_resource.1;
188188

189-
Some(wgpu_core::command::RenderPassTimestampWrites {
189+
Some(wgpu_core::command::PassTimestampWrites {
190190
query_set,
191191
beginning_of_pass_write_index: timestamp_writes.beginning_of_pass_write_index,
192192
end_of_pass_write_index: timestamp_writes.end_of_pass_write_index,
@@ -200,6 +200,8 @@ pub fn op_webgpu_command_encoder_begin_render_pass(
200200
.transpose()?
201201
.map(|query_set| query_set.1);
202202

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

211-
let render_pass = wgpu_core::command::RenderPass::new(command_encoder_resource.1, &descriptor);
212-
213+
let (render_pass, error) = gfx_select!(command_encoder => instance.command_encoder_create_render_pass_dyn(*command_encoder, &descriptor));
213214
let rid = state
214215
.resource_table
215216
.add(super::render_pass::WebGpuRenderPass(RefCell::new(
216217
render_pass,
217218
)));
218219

219-
Ok(WebGpuResult::rid(rid))
220+
Ok(WebGpuResult::rid_err(rid, error))
220221
}
221222

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

248-
Some(wgpu_core::command::ComputePassTimestampWrites {
249+
Some(wgpu_core::command::PassTimestampWrites {
249250
query_set,
250251
beginning_of_pass_write_index: timestamp_writes.beginning_of_pass_write_index,
251252
end_of_pass_write_index: timestamp_writes.end_of_pass_write_index,

‎deno_webgpu/render_pass.rs‎

Lines changed: 54 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@ use deno_core::ResourceId;
99
use serde::Deserialize;
1010
use std::borrow::Cow;
1111
use std::cell::RefCell;
12-
use wgpu_core::global::Global;
1312

1413
use super::error::WebGpuResult;
1514

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

45-
state.borrow::<Global>().render_pass_set_viewport(
46-
&mut render_pass_resource.0.borrow_mut(),
44+
render_pass_resource.0.borrow_mut().set_viewport(
45+
state.borrow(),
4746
args.x,
4847
args.y,
4948
args.width,
@@ -69,13 +68,10 @@ pub fn op_webgpu_render_pass_set_scissor_rect(
6968
.resource_table
7069
.get::<WebGpuRenderPass>(render_pass_rid)?;
7170

72-
state.borrow::<Global>().render_pass_set_scissor_rect(
73-
&mut render_pass_resource.0.borrow_mut(),
74-
x,
75-
y,
76-
width,
77-
height,
78-
)?;
71+
render_pass_resource
72+
.0
73+
.borrow_mut()
74+
.set_scissor_rect(state.borrow(), x, y, width, height)?;
7975

8076
Ok(WebGpuResult::empty())
8177
}
@@ -91,9 +87,10 @@ pub fn op_webgpu_render_pass_set_blend_constant(
9187
.resource_table
9288
.get::<WebGpuRenderPass>(render_pass_rid)?;
9389

94-
state
95-
.borrow::<Global>()
96-
.render_pass_set_blend_constant(&mut render_pass_resource.0.borrow_mut(), &color)?;
90+
render_pass_resource
91+
.0
92+
.borrow_mut()
93+
.set_blend_constant(state.borrow(), color)?;
9794

9895
Ok(WebGpuResult::empty())
9996
}
@@ -109,9 +106,10 @@ pub fn op_webgpu_render_pass_set_stencil_reference(
109106
.resource_table
110107
.get::<WebGpuRenderPass>(render_pass_rid)?;
111108

112-
state
113-
.borrow::<Global>()
114-
.render_pass_set_stencil_reference(&mut render_pass_resource.0.borrow_mut(), reference)?;
109+
render_pass_resource
110+
.0
111+
.borrow_mut()
112+
.set_stencil_reference(state.borrow(), reference)?;
115113

116114
Ok(WebGpuResult::empty())
117115
}
@@ -127,9 +125,10 @@ pub fn op_webgpu_render_pass_begin_occlusion_query(
127125
.resource_table
128126
.get::<WebGpuRenderPass>(render_pass_rid)?;
129127

130-
state
131-
.borrow::<Global>()
132-
.render_pass_begin_occlusion_query(&mut render_pass_resource.0.borrow_mut(), query_index)?;
128+
render_pass_resource
129+
.0
130+
.borrow_mut()
131+
.begin_occlusion_query(state.borrow(), query_index)?;
133132

134133
Ok(WebGpuResult::empty())
135134
}
@@ -144,9 +143,10 @@ pub fn op_webgpu_render_pass_end_occlusion_query(
144143
.resource_table
145144
.get::<WebGpuRenderPass>(render_pass_rid)?;
146145

147-
state
148-
.borrow::<Global>()
149-
.render_pass_end_occlusion_query(&mut render_pass_resource.0.borrow_mut())?;
146+
render_pass_resource
147+
.0
148+
.borrow_mut()
149+
.end_occlusion_query(state.borrow())?;
150150

151151
Ok(WebGpuResult::empty())
152152
}
@@ -172,9 +172,10 @@ pub fn op_webgpu_render_pass_execute_bundles(
172172
.resource_table
173173
.get::<WebGpuRenderPass>(render_pass_rid)?;
174174

175-
state
176-
.borrow::<Global>()
177-
.render_pass_execute_bundles(&mut render_pass_resource.0.borrow_mut(), &bundles)?;
175+
render_pass_resource
176+
.0
177+
.borrow_mut()
178+
.execute_bundles(state.borrow(), &bundles)?;
178179

179180
Ok(WebGpuResult::empty())
180181
}
@@ -189,12 +190,7 @@ pub fn op_webgpu_render_pass_end(
189190
.resource_table
190191
.take::<WebGpuRenderPass>(render_pass_rid)?;
191192

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

199195
Ok(WebGpuResult::empty())
200196
}
@@ -226,8 +222,8 @@ pub fn op_webgpu_render_pass_set_bind_group(
226222

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

229-
state.borrow::<Global>().render_pass_set_bind_group(
230-
&mut render_pass_resource.0.borrow_mut(),
225+
render_pass_resource.0.borrow_mut().set_bind_group(
226+
state.borrow(),
231227
index,
232228
bind_group_resource.1,
233229
dynamic_offsets_data,
@@ -247,8 +243,8 @@ pub fn op_webgpu_render_pass_push_debug_group(
247243
.resource_table
248244
.get::<WebGpuRenderPass>(render_pass_rid)?;
249245

250-
state.borrow::<Global>().render_pass_push_debug_group(
251-
&mut render_pass_resource.0.borrow_mut(),
246+
render_pass_resource.0.borrow_mut().push_debug_group(
247+
state.borrow(),
252248
group_label,
253249
0, // wgpu#975
254250
)?;
@@ -266,9 +262,10 @@ pub fn op_webgpu_render_pass_pop_debug_group(
266262
.resource_table
267263
.get::<WebGpuRenderPass>(render_pass_rid)?;
268264

269-
state
270-
.borrow::<Global>()
271-
.render_pass_pop_debug_group(&mut render_pass_resource.0.borrow_mut())?;
265+
render_pass_resource
266+
.0
267+
.borrow_mut()
268+
.pop_debug_group(state.borrow())?;
272269

273270
Ok(WebGpuResult::empty())
274271
}
@@ -284,8 +281,8 @@ pub fn op_webgpu_render_pass_insert_debug_marker(
284281
.resource_table
285282
.get::<WebGpuRenderPass>(render_pass_rid)?;
286283

287-
state.borrow::<Global>().render_pass_insert_debug_marker(
288-
&mut render_pass_resource.0.borrow_mut(),
284+
render_pass_resource.0.borrow_mut().insert_debug_marker(
285+
state.borrow(),
289286
marker_label,
290287
0, // wgpu#975
291288
)?;
@@ -307,10 +304,10 @@ pub fn op_webgpu_render_pass_set_pipeline(
307304
.resource_table
308305
.get::<WebGpuRenderPass>(render_pass_rid)?;
309306

310-
state.borrow::<Global>().render_pass_set_pipeline(
311-
&mut render_pass_resource.0.borrow_mut(),
312-
render_pipeline_resource.1,
313-
)?;
307+
render_pass_resource
308+
.0
309+
.borrow_mut()
310+
.set_pipeline(state.borrow(), render_pipeline_resource.1)?;
314311

315312
Ok(WebGpuResult::empty())
316313
}
@@ -341,8 +338,8 @@ pub fn op_webgpu_render_pass_set_index_buffer(
341338
None
342339
};
343340

344-
state.borrow::<Global>().render_pass_set_index_buffer(
345-
&mut render_pass_resource.0.borrow_mut(),
341+
render_pass_resource.0.borrow_mut().set_index_buffer(
342+
state.borrow(),
346343
buffer_resource.1,
347344
index_format,
348345
offset,
@@ -378,8 +375,8 @@ pub fn op_webgpu_render_pass_set_vertex_buffer(
378375
None
379376
};
380377

381-
state.borrow::<Global>().render_pass_set_vertex_buffer(
382-
&mut render_pass_resource.0.borrow_mut(),
378+
render_pass_resource.0.borrow_mut().set_vertex_buffer(
379+
state.borrow(),
383380
slot,
384381
buffer_resource.1,
385382
offset,
@@ -403,8 +400,8 @@ pub fn op_webgpu_render_pass_draw(
403400
.resource_table
404401
.get::<WebGpuRenderPass>(render_pass_rid)?;
405402

406-
state.borrow::<Global>().render_pass_draw(
407-
&mut render_pass_resource.0.borrow_mut(),
403+
render_pass_resource.0.borrow_mut().draw(
404+
state.borrow(),
408405
vertex_count,
409406
instance_count,
410407
first_vertex,
@@ -429,8 +426,8 @@ pub fn op_webgpu_render_pass_draw_indexed(
429426
.resource_table
430427
.get::<WebGpuRenderPass>(render_pass_rid)?;
431428

432-
state.borrow::<Global>().render_pass_draw_indexed(
433-
&mut render_pass_resource.0.borrow_mut(),
429+
render_pass_resource.0.borrow_mut().draw_indexed(
430+
state.borrow(),
434431
index_count,
435432
instance_count,
436433
first_index,
@@ -456,8 +453,8 @@ pub fn op_webgpu_render_pass_draw_indirect(
456453
.resource_table
457454
.get::<WebGpuRenderPass>(render_pass_rid)?;
458455

459-
state.borrow::<Global>().render_pass_draw_indirect(
460-
&mut render_pass_resource.0.borrow_mut(),
456+
render_pass_resource.0.borrow_mut().draw_indirect(
457+
state.borrow(),
461458
buffer_resource.1,
462459
indirect_offset,
463460
)?;
@@ -480,8 +477,8 @@ pub fn op_webgpu_render_pass_draw_indexed_indirect(
480477
.resource_table
481478
.get::<WebGpuRenderPass>(render_pass_rid)?;
482479

483-
state.borrow::<Global>().render_pass_draw_indexed_indirect(
484-
&mut render_pass_resource.0.borrow_mut(),
480+
render_pass_resource.0.borrow_mut().draw_indexed_indirect(
481+
state.borrow(),
485482
buffer_resource.1,
486483
indirect_offset,
487484
)?;

‎tests/tests/compute_pass_ownership.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ async fn compute_pass_query_set_ownership_pipeline_statistics(ctx: TestingContex
111111
}
112112

113113
#[gpu_test]
114-
static COMPUTE_PASS_QUERY_TIMESTAMPS: GpuTestConfiguration =
114+
static COMPUTE_PASS_QUERY_SET_OWNERSHIP_TIMESTAMPS: GpuTestConfiguration =
115115
GpuTestConfiguration::new()
116116
.parameters(TestParameters::default().test_features_limits().features(
117117
wgpu::Features::TIMESTAMP_QUERY | wgpu::Features::TIMESTAMP_QUERY_INSIDE_PASSES,
118118
))
119-
.run_async(compute_pass_query_timestamps);
119+
.run_async(compute_pass_query_set_ownership_timestamps);
120120

121-
async fn compute_pass_query_timestamps(ctx: TestingContext) {
121+
async fn compute_pass_query_set_ownership_timestamps(ctx: TestingContext) {
122122
let ResourceSetup {
123123
gpu_buffer,
124124
cpu_buffer,

0 commit comments

Comments
 (0)