Transition resources #6678
Transition resources #6678
Conversation
There was a problem hiding this comment.
love it, I long wanted this loophole. Obviously wgpu tracking should get better, but having this "expert level" fallback is imho an important asset regardless.
Exposing this directly via hal types at first felt a bit too much like exposing the guts, but that's what this feature is about, so I actually appreciate that it's not usable without dropping into a different namespace.
However, speaking of interfaces: I really don't want us to add a new method to the wgpu api without ever calling it in a test that ensures that it doesn't randomly break down completely. Can you please add a short test that users the new method? Naturally, for the happy case there's not much more to test than "didn't fail".
Ideally it would also come with failure case checking as well (ensuring the right errors are raised), but I'm happy to skip that.
| /// * Use [`CommandEncoder::transition_resources`] to transition resources X and Y from TextureUses::COLOR_TARGET to TextureUses::RESOURCE | ||
| /// * Use resources X and Y in a bind group | ||
| /// | ||
| /// At submission time, wgpu will record and insert some new command buffers, resulting in a submission that looks like `queue.submit(&[0, a, b, 1, c])`: | ||
| /// * CommandBuffer 0: Barrier to transition resources X and Y from TextureUses::RESOURCE (from last frame) to TextureUses::COLOR_TARGET | ||
| /// * CommandBuffer A: Use resource X as a render pass attachment | ||
| /// * CommandBuffer B: Use resource Y as a render pass attachment | ||
| /// * CommandBuffer 1: Barrier to transition resources X and Y from TextureUses::COLOR_TARGET to TextureUses::RESOURCE | ||
| /// * CommandBuffer C: Use resources X and Y in a bind group |
There was a problem hiding this comment.
Why is CommandBuffer 1 still separated out? This was described as adding the resource transition to RESOURCE manually as part of CommandBuffer C, therefore wgpu should see COLOR_TARGET as expected start state of C.
If this is another limitation we have today it would be good to capture this here. Also, the transition_resources call in C is not doing anything then.
There was a problem hiding this comment.
Oh whoops, I think that's a mistake. I don't actually know what will happen with the second transition_resources() call.
Tbh how wgpu's barrier tracking system works still confuses me. Like the fact that it's still inserting CommandBuffer 0 to begin with. I was hoping it could just be a part of A directly.
Can you adjust the docs for me to match how this is supposed to work?
There was a problem hiding this comment.
Hmmm yeah actually CommandBuffer 0 shouldn't be there either 🤔
Let's wait for @cwfitzgerald to have a look - my knowledge of the tracker is quite shaky as well.
|
I would like to review this before it lands |
looks like you found it (: |
There was a problem hiding this comment.
thanks for adding the test! 👍
letting @cwfitzgerald take over as requested
|
Discussing this on a call on Saturday |
|
CI mad |
Connections
Description
Wgpu often needs to generate barriers when submitting multiple command buffers.
This PR provides a new API that advanced users can use to generate the barriers manually, so that wgpu won't have to patch them in later (suboptimally).
See the API doc comment.
Testing
Ran Bevy with wgpu main and calling transition_resources().
Before:
After:
No more barriers (vertical blue lines) between the shadow passes.
I also checked the event list, and wgpu correctly batches all 6 of the resources (I submitted 6 resources to transition) into one barrier.
Checklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.