Two small vulkan changes #8001
Two small vulkan changes #8001
Conversation
| i32::MAX as u64 | ||
| } else { | ||
| u64::MAX | ||
| 1u64 << 52 |
There was a problem hiding this comment.
4 PiB should be enough for somebody driving Vulkan, eh? 🤡
|
Just needs a Wish we had a way to test this…🤔 |
|
Do you foresee a way that this change is user-facing? My thought was that 4 PiB and 2**64 are functionally indistinguishable on any existing hardware. I'm not sure if you want a changelog entry because that's not your assessment, or if you want a changelog entry on the principle of noting even changes that we don't think have any impact, just so users may be aware of a change in that area if we happen to break something. A different question, that relates to this code but not the change in this PR, is whether we really want to disallow any buffer over 2**31 bytes on desktop Linux systems without NVIDIA hardware. Still unlikely to matter for WebGPU given limits set elsewhere in the system, but possibly something that some wgpu use case might care about. |
I'm definitely erring on the side of Hyrum's law here, nothing more intelligent than that. If you feel strongly about not including a
Let's raise this in an issue or in community chat; I'm concerned about that question getting lost here, and I unfortunately don't feel like I can speculate without more time after the EOD I'm currently at. 😅 |
| buffer: &'a super::Buffer, | ||
| ranges: I, | ||
| ) -> Option<impl 'a + Iterator<Item = vk::MappedMemoryRange>> { | ||
| ) -> Option<impl 'a + Iterator<Item = vk::MappedMemoryRange<'a>>> { |
There was a problem hiding this comment.
(The compiler suggests the
'_lifetime but'aseemed more appropriate, however, I don't claim to fully understand what's going on here.)
What is going on is that vk::MappedMemoryRange contains a single CPU-side pointer, p_next, whose lifetime is tracked here.
Since nothing assigns anything borrowed to p_next, realistically this should have been 'static 1. However, since it holds a vk::DeviceMemory handle in there from buffer: &'a Buffer it might make sense to "cheat" and pretend it borrows something from buffer :)
Footnotes
-
And
ash/ the Rust compiler allows that, because::default()creates a'staticwhich will only be specialized when using a builder function that takes a borrow. ↩
3 participants
Fix one more nightly compile warning like the ones in #7964 that I didn't see before because I wasn't building Vulkan. (The compiler suggests the
'_lifetime but'aseemed more appropriate, however, I don't claim to fully understand what's going on here.)Reduce the value for an "unlimited" maximum buffer size from
u64::MAXto1 << 52to avoid problems when the limit values are round-tripped through JS.Although these changes both involve buffers, they are unrelated.
Testing
For the first change, just that it compiles. The second change resolves a problem running the CTS on Mac with the
vulkan-portabilityfeature enabled. (The problem might also be reproducible on Linux systems with NVIDIA GPUs.)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.