Skip to content
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

Merge MappedDeviceMemory into DeviceMemory, make MemoryAlloc reuse the logic #2300

Merged
merged 16 commits into from
Aug 24, 2023

Conversation

marc0246
Copy link
Contributor

@marc0246 marc0246 commented Aug 22, 2023

Changelog:

### Breaking changes
- `Subbuffer::mapped_ptr` was replaced by `Subbuffer::mapped_slice`.
- `MemoryAlloc::new` no longer returns a `Result`, and doesn't map the `DeviceMemory` automatically anymore.
- `MemoryAlloc::mapped_ptr` and `MemoryAlloc::mapped_slice[_mut]` were replaced by `MemoryAlloc::mapped_slice`, which returns a pointer.
- `MemoryAlloc::{invalidate, flush}_range` now take a `MappedMemoryRange` as argument.

### Additions
- Added `DeviceMemory::{map, unmap, mapping_state, invalidate_range, flush_range}`, `MappedDeviceMemory` has been deprecated.
- Added `MemoryMapInfo`, `MemoryUnmapInfo`, `MappingState` and `MappedMemoryRange`.

### Bugs fixed
- Fixed potential UB when using `MemoryAlloc::try_unwrap`, where the allocation was mapped on contruction of the `MemoryAlloc` but not unmapped on unwrapping, allowing double-mapping.
- Fixed a bug in `GenericMemoryAllocator::allocate`, where the root allocations weren't created with the configured `AllocationType`.

@Rua
Copy link
Contributor

Rua commented Aug 23, 2023

The VK_KHR_map_memory2 extension provides updated versions of some of these functions. It's not used by anything in Vulkan yet, but at least Vulkano would be ready for anything that does in the future.

@marc0246
Copy link
Contributor Author

The VK_KHR_map_memory2 extension provides updated versions of some of these functions. It's not used by anything in Vulkan yet, but at least Vulkano would be ready for anything that does in the future.

Yes, that's why I added a MemoryMapInfo struct.

@marc0246
Copy link
Contributor Author

marc0246 commented Aug 23, 2023

I missed, however, that VkMemoryUnmapInfoKHR and VkMemoryUnmapFlagsKHR existed as well. And I messed up the naming of the ones for mapping. Oops.

@Rua
Copy link
Contributor

Rua commented Aug 24, 2023

Everything is good now, thanks!

@Rua Rua merged commit de7b6a9 into vulkano-rs:master Aug 24, 2023
3 checks passed
Rua added a commit that referenced this pull request Aug 24, 2023
Rua added a commit that referenced this pull request Aug 24, 2023
@marc0246 marc0246 deleted the memory-mapping branch August 24, 2023 15:50
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
…use the logic (vulkano-rs#2300)

* Merge `MappedDeviceMemory` into `DeviceMemory`

* Fix soundness and utilize new `DeviceMemory` methods in `MemoryAlloc`

* Fix oopsies

* `Sync`ness

* `#[inline]`

* Big oopsie

* Language

* Sanity check for the deprecated stuff

* Full `khr_map_memory2`

* Missed trait impls

* `MemoryUnmapInfo::validate`

* Document mapping behavior of `GenericMemoryAllocator`

* Validate flags

* Update vulkano/src/memory/allocator/mod.rs

Co-authored-by: Rua <[email protected]>

* Remove flags

* Errors

---------

Co-authored-by: Rua <[email protected]>
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
hakolao pushed a commit to hakolao/vulkano that referenced this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants