Skip to content

Conversation

@inner-daemons
Copy link
Collaborator

@inner-daemons inner-daemons commented Nov 8, 2025

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

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.
Copy link
Member

@Wumpf Wumpf left a 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.

Comment on lines 17 to 18
/// 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`
Copy link
Member

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

Copy link
Collaborator Author

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

@Wumpf
Copy link
Member

Wumpf commented Nov 9, 2025

eh, guess I should have checked on the "depends on". Well, some feedback for the other PR in here as well I reckon

@inner-daemons
Copy link
Collaborator Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants