Use pointers instead of IDs for tracing #8289
Conversation
Use it to deduplicate some `Foo` / `ArcFoo` / `TraceFoo` definitions.
This updates pass operations. The trace player is temporarily broken.
This updates non-pass command encoder operations and device operations.
|
This does have all my latest local changes, but there's still a little work to do to fix the CI failures, and I want to revisit the way that polling and |
| // this check can't go in the body of `create_bind_group_layout` since the closure might not get called | ||
| if let Err(e) = device.check_is_valid() { | ||
| break 'error e.into(); | ||
| } |
There was a problem hiding this comment.
I think we can remove this check since create_bind_group_layout now contains it as well.
| Ok(submit_index) | ||
| } | ||
|
|
||
| pub fn get_mapped_range( |
There was a problem hiding this comment.
get_mapped_range on the global is not calling this method.
| hashbrown.workspace = true | ||
| indexmap.workspace = true | ||
| log.workspace = true | ||
| macro_rules_attribute.workspace = true |
There was a problem hiding this comment.
We can gate this dependency behind the serde feature.
| // Unfortunately, because `Arc::as_ptr` returns a pointer to the | ||
| // contained data, and `Arc` stores reference counts before the data, | ||
| // we are adding an offset to the pointer here. | ||
| Self(Arc::as_ptr(arc) as usize, PhantomData) |
| #[cfg_attr(not(any(feature = "serde", feature = "replay")), allow(dead_code))] | ||
| //#[cfg_attr(not(any(feature = "serde", feature = "replay")), allow(dead_code))] | ||
| color: u32, | ||
| len: usize, | ||
| }, | ||
|
|
||
| PopDebugGroup, | ||
|
|
||
| InsertDebugMarker { | ||
| #[cfg_attr(not(any(feature = "serde", feature = "replay")), allow(dead_code))] |
There was a problem hiding this comment.
It seems like we kept those guards for the render pass, but not here.
| fn resolve_buffer_id( | ||
| &self, | ||
| id: wgc::id::PointerId<wgc::id::markers::Buffer>, | ||
| ) -> Arc<wgc::resource::Buffer> { | ||
| self.buffers.get(&id).expect("invalid buffer").clone() | ||
| } | ||
|
|
||
| pub fn get_buffer( | ||
| &self, | ||
| id: wgc::id::PointerId<wgc::id::markers::Buffer>, | ||
| ) -> Arc<wgc::resource::Buffer> { | ||
| self.resolve_buffer_id(id) | ||
| } |
| let external_texture = self | ||
| .external_textures | ||
| .remove(&id) | ||
| .expect("invalid external texture"); | ||
| external_texture.destroy(); |
There was a problem hiding this comment.
I don't think the free functions should remove the resource from the map since the spec says it's ok to call destroy multiple times.
|
I wanted to mention that the main difference I noticed between this approach and the previous is that we now effectively only record commands after successful validation. I find that this somewhat limits the usefulness of recording and replaying traces since they won't record invalid commands. I've mostly used the record/replay functionality to debug overly restrictive or wrong validation in Firefox which will no longer be possible with this PR. We should probably talk about this at the maintainers meeting. |
2 participants
On top of #8288.
This resolves the duplicate storage of
IdandArcversions of commands when tracing that was introduced by encode-on-finish. It now stores theArcversion only, and uses the pointers from theArcs as object IDs when tracing. It also overhauls the trace player to play back traces in this format.This fixes tracing of compute and render passes when encoded using the standard APIs. Tracing of render bundles is still broken.
Testing
TBD. Existing player tests, manual tests, and it would be nice to have an automated test of recording a trace but I don't think we do right now.
Squash or Rebase? TBD, probably rebase.
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.