- Notifications
You must be signed in to change notification settings - Fork 1.1k
Multiview on DX12 #8495
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
base: trunk
Are you sure you want to change the base?
Multiview on DX12 #8495
Conversation
Wumpf left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unification in device.rs looks nice!
Definitely still need to improve helpfulness (and language!) of those comments.
Actual logic might actually be alright (minus open questions in the comments, in particular is the mask actually set? Oo) but I'm still brooding on how to make it safer & easier to maintain; I suggested pinning for safety, but tbh I haven't thought it through entirely yet.
wgpu-hal/src/dx12/pipeline_desc.rs Outdated
| /// Returned bytes contain pointers into this struct, for them to be valid, | ||
| /// this struct not move or be dropped. As if `as_bytes<'a>(&'a self) -> Vec<u8> + 'a` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we emit pointers to this struct we have to pin it. Otherwise, Rust may move it around thus invalidating those pointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the safety around that. But I'll leave this open since I'm not 100% sure I'm doing this right, even though both code paths work on my machine
| eh, guess I should have checked on the "depends on". Well, some feedback for the other PR in here as well I reckon |
| @Wumpf Damn, thanks for the quick and thorough review! Much appreciated! I definitely need to get better at writing comments lol. Will look at this over the next few days. |
Co-authored-by: Andreas Reich <r_andreas2@web.de>
…ggie70incorporated/wgpu into dx12-pipeline-stream-desc
Connections
Closes #8376
Followup to #8206
Depends on #8377
CC @Wumpf : will you review this, or someone else?
Description
Adds multiview support to DX12
Testing
Using standard multiview tests introduced in #8206
Squash or Rebase?
Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.