From e116851b90410742bd185e513a3c65f94f2a0954 Mon Sep 17 00:00:00 2001 From: janis Date: Mon, 26 Oct 2026 13:44:17 +0100 Subject: [PATCH] feat: migrate GPU memory allocator from gpu-allocator to VMA (vk-mem) - Replace gpu-allocator with vk-mem 0.5.0 (VMA 3.3.0) in renderer_device - Add public GpuAllocation wrapper, MemoryLocation, AllocationDesc, and allocate_for_buffer/image + bind_buffer/image_memory helpers - Restrict CPU-mapped allocations to HOST_VISIBLE|HOST_COHERENT memory types - Handle guards: raw buffer/image handles owned before allocation so failure paths never leak - Use renderer_device::device::MemoryLocation across renderer, renderer_resources, and renderer_rendergraph; remove all gpu-allocator dependencies - Error on dedicated allocation below Vulkan 1.1 instead of silent downgrade - Add Send/Sync asserts, coherent-mapping and dedicated-allocation validation tests; update AGENTS.md docs --- Cargo.toml | 3 +- crates/renderer/AGENTS.md | 6 +- crates/renderer/Cargo.toml | 1 - crates/renderer/src/egui.rs | 3 +- .../src/render_graph/graph_builder.rs | 11 +- crates/renderer/src/rendering/mod.rs | 7 +- crates/renderer_device/AGENTS.md | 10 +- crates/renderer_device/Cargo.toml | 2 +- crates/renderer_device/src/device.rs | 247 +++++++++++-- crates/renderer_device/src/instance.rs | 12 +- crates/renderer_device/src/lib.rs | 6 +- crates/renderer_device/src/testing.rs | 75 ++-- crates/renderer_rendergraph/Cargo.toml | 1 - crates/renderer_rendergraph/src/resources.rs | 9 +- .../tests/graph_validation.rs | 7 +- crates/renderer_resources/AGENTS.md | 6 +- crates/renderer_resources/Cargo.toml | 1 - crates/renderer_resources/src/buffers.rs | 179 +++++++--- crates/renderer_resources/src/images.rs | 72 ++-- .../tests/managed_set_validation.rs | 7 +- .../gpu-allocator-to-vma-migration.md | 326 ++++++++++++++++++ 21 files changed, 802 insertions(+), 189 deletions(-) create mode 100644 design-plans/gpu-allocator-to-vma-migration.md diff --git a/Cargo.toml b/Cargo.toml index 5a09373..a260040 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,8 +41,7 @@ thread_local = "1.1.8" ash = "0.38.0" ash-window = "0.13.0" -# vk-mem = "0.5.0" -gpu-allocator = { git = "https://github.com/janis-bhm/gpu-allocator", branch = "main" } +vk-mem = "0.5.0" rectangle-pack = "0.4.2" # vk-sync = "0.1.6" diff --git a/crates/renderer/AGENTS.md b/crates/renderer/AGENTS.md index 068c37d..194f067 100644 --- a/crates/renderer/AGENTS.md +++ b/crates/renderer/AGENTS.md @@ -35,7 +35,7 @@ - `RenderGraph::add_resource` / `import_image` / `import_buffer` / `import_framebuffer` register graph resources; `add_pass` records read/write declarations and a callback; `resolve` creates internal resources and returns a command list with graph transitions. - `rendering::Wireframe` is an example higher-level renderer; egui pass functions are re-exported from `rendering` rather than the private `egui_pass` module. - Resource types are defined in `renderer_resources`: `Buffer`/`BufferDesc`; `Image`/`ImageDesc`, `ImageView`/`ImageViewDesc`, `MipRange`, `Extent`, and `Offset`; and pipeline, descriptor, shader, and sampler types. -- Buffer/image descriptors carry Vulkan usage, memory location, queue-sharing flags, and allocation strategy; `Buffer::map`/`map_mut` expose mapped memory only when the allocator returns a mapped allocation. +- Buffer/image descriptors carry Vulkan usage, `renderer_device::device::MemoryLocation`, queue-sharing flags, and allocation strategy; `Buffer::map`/`map_mut` expose mapped memory for VMA-mapped allocations (CPU-mapped locations require host-visible, host-coherent memory). - The egui integration uploads texture deltas to GPU images and records passes through the legacy `RenderGraph`; shader binaries remain under `crates/renderer/shaders`, and renderer's hardcoded shader paths are unchanged. - `ImageDesc` includes image shape, format, layout, usage, queue family selection, and allocation options. The resource crate's `Image::new_with_allocation` checks allocation size and memory-type compatibility before binding. - `ImageViewDesc` selects view type, format, aspects, mip range, and array layers; the resource crate's `Image::create_view` normalizes `REMAINING_MIP_LEVELS` / `REMAINING_ARRAY_LAYERS` and checks image/view compatibility. @@ -60,7 +60,7 @@ - Device ownership is delegated to `renderer_device`: wrappers here strongly retain `Device` through its `DeviceObject` or explicit fields. Do not place a device-owning wrapper back inside `DeviceInner`. - `Image::from_swapchain_raw(image, device, format, extent, usage)` wraps a swapchain-owned raw image without taking image ownership. `Swapchain::acquire_image` passes the swapchain device, configured format/usage, and configured extent with depth 1. Its descriptor remains TYPE_2D, one mip/layer, TYPE_1 samples, OPTIMAL tiling, PRESENT queue flags, UNDEFINED layout, and GpuOnly memory. Do not destroy that image as an independently allocated image. - `ImageView` retains an `Arc` so its source image outlives the Vulkan view. `Image::create_view` in `renderer_resources` returns `ManuallyDrop`; renderer call sites transfer it deliberately (the swapchain path uses `ManuallyDrop::into_inner`). -- For allocated images, `Allocation::Owned` frees allocation memory with its `DeviceObject`; `Allocation::Shared` permits shared ownership and only exposes mutable access when uniquely held; `Unmanaged` carries no allocator allocation. +- For allocated images, `Allocation::Owned` frees VMA allocation memory with its `DeviceObject`; `Allocation::Shared` permits shared ownership and only exposes mutable access when uniquely held; `Unmanaged` carries no allocation. - `Swapchain` owns raw swapchain images and manually created acquire/release semaphores. Its drop path waits idle and destroys those semaphores; the `DeviceObject` destroys the swapchain handle. - `SwapchainImage` / `SwapchainFrame` are `#[must_use]` acquired-frame values. Presentation consumes the frame; preserve its `Arc` so image ownership stays valid until presentation. - `Surface` uses a `DropGuard` for surface destruction and keeps its `Instance`; field order drops the swapchain lock first, then the guard, then the instance. @@ -89,7 +89,7 @@ - The crate root also exposes `Extent2D` and `utils` alongside renderer modules and device re-exports. - `SamplerDesc` implements float-aware equality and hashing with utility helpers; keep those implementations consistent if adding sampler fields. - `Pipeline::new_graphics` supports dynamic rendering state through `RenderingState`; its render-pass and rendering choices are supplied in `GraphicsPipelineDesc`. -- `Buffer` and `Image` creation in `renderer_resources` allocates/binds via `DeviceInner.alloc2`; device ownership and final allocation release are handled by the device-layer wrappers. +- `Buffer` and `Image` creation in `renderer_resources` allocates/binds through `DeviceInner::allocate_for_buffer` / `allocate_for_image` and `bind_buffer_memory` / `bind_image_memory`; device ownership and final VMA allocation release are handled by the device-layer wrappers. - Resource types live in `renderer_resources`; the `renderer` aliases are crate-private and are not externally reachable as `renderer::buffers::...` etc. - `Renderer2::new` selects an adapter with `None` for the surface, then creates the logical device using its retained display handle. `choose_adapter_default` does not evaluate raw surface compatibility. - `render_graph/legacy.rs` implements the active public graph resolver. Its resources are ordered by `GraphResourceId`, and passes declare reads/writes through `PassDesc` so the resolver can derive dependencies and transitions. diff --git a/crates/renderer/Cargo.toml b/crates/renderer/Cargo.toml index e74de5e..0986184 100644 --- a/crates/renderer/Cargo.toml +++ b/crates/renderer/Cargo.toml @@ -24,7 +24,6 @@ tracing = { workspace = true } ash = { workspace = true } ash-window = { workspace = true } # vk-mem = { workspace = true } -gpu-allocator = { workspace = true } rectangle-pack = { workspace = true } raw-window-handle = { workspace = true } diff --git a/crates/renderer/src/egui.rs b/crates/renderer/src/egui.rs index df1614b..d431339 100644 --- a/crates/renderer/src/egui.rs +++ b/crates/renderer/src/egui.rs @@ -5,14 +5,13 @@ use std::{ }; use ash::vk; -use gpu_allocator::MemoryLocation; use indexmap::IndexMap; use crate::{ EguiState, buffers::{Buffer, BufferDesc}, commands::traits::CommandBufferExt, - device::{self, AllocationStrategy}, + device::{self, AllocationStrategy, MemoryLocation}, images::{Image, ImageDesc, ImageViewDesc}, render_graph::{ Access, Barrier, GraphResourceDesc, GraphResourceId, PassDesc, RecordFn, RenderContext, diff --git a/crates/renderer/src/render_graph/graph_builder.rs b/crates/renderer/src/render_graph/graph_builder.rs index 3116c55..350d3a5 100644 --- a/crates/renderer/src/render_graph/graph_builder.rs +++ b/crates/renderer/src/render_graph/graph_builder.rs @@ -4,7 +4,9 @@ use ash::vk; use derive_more::derive::{Deref, DerefMut}; use crate::{ - buffers, images, + buffers, + device::MemoryLocation, + images, render_graph::{ recorder::CommandList, resources::{self, ResourceId}, @@ -48,7 +50,7 @@ impl<'r> GraphBuilder<'r> { &self, _size: u64, _usage: vk::BufferUsageFlags, - _location: gpu_allocator::MemoryLocation, + _location: MemoryLocation, ) -> Buffer<'r> { todo!() } @@ -57,7 +59,7 @@ impl<'r> GraphBuilder<'r> { &self, _size: u64, _usage: vk::BufferUsageFlags, - _location: gpu_allocator::MemoryLocation, + _location: MemoryLocation, ) -> Buffer<'r> { todo!() } @@ -66,7 +68,7 @@ impl<'r> GraphBuilder<'r> { &self, _size: u64, _usage: vk::BufferUsageFlags, - _location: gpu_allocator::MemoryLocation, + _location: MemoryLocation, ) -> Buffer<'r> { todo!() } @@ -84,7 +86,6 @@ mod api_demo { use super::*; use ash::vk::{self, BufferUsageFlags}; - use gpu_allocator::MemoryLocation; use indexmap::IndexMap; fn egui_graph<'a>( diff --git a/crates/renderer/src/rendering/mod.rs b/crates/renderer/src/rendering/mod.rs index 11baddb..68974f7 100644 --- a/crates/renderer/src/rendering/mod.rs +++ b/crates/renderer/src/rendering/mod.rs @@ -1,19 +1,18 @@ use std::sync::Arc; +pub use crate::egui_pass::{egui_pass, egui_pre_pass}; + use ash::vk; use glam::{ camera::rh::{proj, view}, vec3, }; -use gpu_allocator::MemoryLocation; - -pub use crate::egui_pass::{egui_pass, egui_pre_pass}; use crate::{ Result, buffers::{Buffer, BufferDesc}, commands::{self, traits::CommandBufferExt}, - device::Device, + device::{Device, MemoryLocation}, images::ImageViewDesc, pipeline, render_graph::{ diff --git a/crates/renderer_device/AGENTS.md b/crates/renderer_device/AGENTS.md index 3b4016b..2a8475a 100644 --- a/crates/renderer_device/AGENTS.md +++ b/crates/renderer_device/AGENTS.md @@ -16,7 +16,7 @@ ## Key types and ownership - `Device(Arc)` is the cloneable owning handle. It dereferences to `DeviceInner`; equality is `Arc::ptr_eq`. -- `DeviceInner` keeps the raw `ash::Device`, allocator mutex, adapter and instance, queues, synchronization threadpool, extension loaders, and inline `DevicePools`. +- `DeviceInner` keeps the raw `ash::Device`, VMA (`vk_mem`) allocator mutex, adapter and instance, queues, synchronization threadpool, extension loaders, and inline `DevicePools`. - `Device::enabled_features()` returns the feature snapshot requested for logical-device creation; it is distinct from `Device::features()`, which reports adapter-supported features. Descriptor capability validation must use the enabled snapshot, not support data. - `PhysicalDeviceInfo::create_logical_device` selects queues, enables requested/supported extensions and features, initializes the allocator/cache, and constructs `DeviceInner` with `Arc::new_cyclic`. - `DeviceInner::dev`, `instance`, `queues`, `physical_device`, `main_queue`, `compute_queue`, and `transfer_queue` are the main convenience accessors for downstream code. @@ -27,14 +27,16 @@ - The `DeviceRef` proc macro is implemented in the separate `renderer_macros` package and emits paths rooted at `::renderer_device::`. - `sync::Fence` and `sync::Semaphore` distinguish `Dedicated` (`DeviceObject`) from `Pooled` (`PoolObject`) ownership. `SemaphoreInner`, `BinarySemaphore`, and `TimelineSemaphore` represent semaphore-kind handles. - `PhysicalDeviceInfo`, `PhysicalDeviceFeatures`, `PhysicalDeviceProperties`, and `SurfaceCapabilities` carry adapter/capability data. `Instance::choose_adapter_default` accepts `Option` (a raw handle), not a swapchain/surface wrapper. -- `Error` and `Result` are the crate-wide error types; `Error` covers Vulkan, loading, allocator, I/O, and renderer-facing resource/capability failures. +- `Error` and `Result` are the crate-wide error types; `Error` covers Vulkan/VMA (`vk_mem`), loading, I/O, and renderer-facing resource/capability failures. - `InstanceDesc` configures borrowed instance extensions/layers, optional app metadata, and a raw display handle; its default requests surface and validation layers, with validation settings in debug builds. `Instance::new` creates a shared `Instance` and requests debug-utils plus optional capability-query extensions alongside requested/platform extensions. - `Instance::expose_adapter` queries one adapter. `choose_adapter_default` ranks enumerated adapters against requested extensions/features and prefers discrete devices, then newer APIs, device-local memory, and texture size. - `PhysicalDeviceFeatures` stores core, Vulkan 1.1/1.2/1.3, and optional mesh-shader feature structures; `push_to_device_create_info` chains enabled feature structs. - `PhysicalDeviceProperties` stores supported extensions, core/versioned properties, optional mesh-shader properties, memory properties, and API version; `supports_extension` checks name and minimum version. - `get_physical_device_features` and `get_physical_device_properties` query the feature/property chains. Extension query results are consumed by the instance and logical-device setup helpers. - `DeviceExtensions` holds the debug-utils device loader and optional swapchain/mesh-shader loaders. `DeviceInner.enabled_extensions` records enabled extension names. -- `AllocationStrategy` selects allocator-managed blocks or a dedicated allocation. `Allocation` represents `Owned`, `Shared`, or `Unmanaged` GPU allocations. +- `MemoryLocation` is the public allocator-independent memory preference; `AllocationDesc` combines it with a resource name and `AllocationStrategy` (`AllocatorManaged` or `Dedicated`). `GpuAllocation` wraps the private `vk_mem::Allocation` and retains memory properties needed by callers. +- `DeviceInner::allocate_for_buffer` / `allocate_for_image` obtain VMA allocations, and `bind_buffer_memory` / `bind_image_memory` bind them to Vulkan resources without exposing `vk_mem` types to upper layers. CPU-mapped locations require host-visible, host-coherent memory. +- `Allocation` represents `Owned`, `Shared`, or `Unmanaged` GPU allocations. - `get_extensions` returns supported/enabled and unsupported requested extension sets; `get_layers` returns available requested layers or the supported/unsupported split. - `Queue` shares a `QueueInner` through `Arc`; `with_locked` serializes operations on the Vulkan queue. `QueueFamily` records index, count, flags, granularity, and timestamp bits. - `DeviceQueues` provides graphics, compute, and transfer queues; absent compute/transfer queues alias the graphics queue. Queue selection requires presentation support only when a display handle is provided. @@ -94,7 +96,7 @@ - `Device::PartialEq` compares `Arc` identity, not raw Vulkan handles or physical-device identity. - `Instance::get_adapter_surface_capabilities` receives the surface loader separately from the raw handle and returns format/mode data alongside generic capabilities. - In debug builds `DebugName` stores its string; in non-debug builds its dereference returns a placeholder and does not retain the supplied name. `debug_name_object` assigns names through debug utils. -- GPU allocation free is routed through `DeviceObject` into the allocator mutex. Do not call allocator free on an allocation after ownership has moved or dropped. +- GPU allocation free is routed through `DeviceObject` into the VMA allocator mutex with `free_memory`. Do not call allocator free on an allocation after ownership has moved or dropped. - `ExternallyManagedObject` must destroy each owned handle once. The `vk::CommandBuffer` implementation is intentionally a no-op: its outer command-buffer wrapper frees it through its command pool. `vk::CommandPool` itself is really destroyed. - Device handle types are hand-written around `DeviceObject` with `DeviceOwned` implementations where needed; the old `define_device_owned_handle!` macro no longer exists. diff --git a/crates/renderer_device/Cargo.toml b/crates/renderer_device/Cargo.toml index b27768e..6f752b4 100644 --- a/crates/renderer_device/Cargo.toml +++ b/crates/renderer_device/Cargo.toml @@ -9,7 +9,7 @@ testing = [] [dependencies] ash = { workspace = true } ash-window = { workspace = true } -gpu-allocator = { workspace = true } +vk-mem = { workspace = true } parking_lot = { workspace = true } tracing = { workspace = true } raw-window-handle = { workspace = true } diff --git a/crates/renderer_device/src/device.rs b/crates/renderer_device/src/device.rs index 21f6b85..b1b98ff 100644 --- a/crates/renderer_device/src/device.rs +++ b/crates/renderer_device/src/device.rs @@ -3,6 +3,7 @@ use std::{ collections::{BTreeSet, HashMap, HashSet}, ffi::CStr, ops::{Deref, DerefMut}, + ptr::NonNull, sync::{Arc, Weak}, }; @@ -66,7 +67,7 @@ impl From for DebugName { } } use crate::{ - Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, Result, + Error, Instance, PhysicalDeviceFeatures, PhysicalDeviceInfo, Result, device::asdf::traits::ExternallyManagedObject, pipeline_cache::PipelineCache, queue::{DeviceQueueInfos, DeviceQueues, Queue}, @@ -108,12 +109,89 @@ pub struct DeviceExtensions { pub mesh_shader: Option, } -type GpuAllocation = gpu_allocator::vulkan::Allocation; +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] +pub enum MemoryLocation { + #[default] + Unknown, + GpuOnly, + CpuToGpu, + GpuToCpu, +} + +#[derive(Clone, Copy)] +struct SendSyncPtr(NonNull); + +unsafe impl Send for SendSyncPtr {} +unsafe impl Sync for SendSyncPtr {} + +impl core::fmt::Debug for SendSyncPtr { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_tuple("SendSyncPtr").field(&self.0).finish() + } +} + +pub struct GpuAllocation { + raw: vk_mem::Allocation, + memory: vk::DeviceMemory, + offset: u64, + size: u64, + memory_type_index: u32, + mapped: Option, + dedicated_memory: bool, +} + +unsafe impl Send for GpuAllocation {} +unsafe impl Sync for GpuAllocation {} + +impl core::fmt::Debug for GpuAllocation { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("GpuAllocation") + .field("memory", &self.memory) + .field("offset", &self.offset) + .field("size", &self.size) + .field("memory_type_index", &self.memory_type_index) + .field("mapped", &self.mapped) + .field("dedicated_memory", &self.dedicated_memory) + .finish_non_exhaustive() + } +} + +impl GpuAllocation { + pub fn memory(&self) -> vk::DeviceMemory { + self.memory + } + + pub fn offset(&self) -> u64 { + self.offset + } + + pub fn size(&self) -> u64 { + self.size + } + + pub fn memory_type_index(&self) -> u32 { + self.memory_type_index + } + + pub fn is_dedicated(&self) -> bool { + self.dedicated_memory + } + + pub fn mapped_slice(&self) -> Option<&[u8]> { + let mapped = self.mapped?; + Some(unsafe { core::slice::from_raw_parts(mapped.0.as_ptr(), self.size as usize) }) + } + + pub fn mapped_slice_mut(&mut self) -> Option<&mut [u8]> { + let mapped = self.mapped?; + Some(unsafe { core::slice::from_raw_parts_mut(mapped.0.as_ptr(), self.size as usize) }) + } +} #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] pub enum AllocationStrategy { #[default] - /// Let gpu_allocator manage the memory for this allocation, sub-allocating + /// Let the allocator manage the memory for this allocation, sub-allocating /// from larger blocks as needed. AllocatorManaged, /// Allocate a dedicated block of memory for this allocation. This is @@ -122,6 +200,12 @@ pub enum AllocationStrategy { Dedicated, } +pub struct AllocationDesc<'a> { + pub name: &'a str, + pub location: MemoryLocation, + pub strategy: AllocationStrategy, +} + #[derive(Debug)] pub enum Allocation { Owned(DeviceObject), @@ -148,7 +232,7 @@ impl Allocation { } pub struct DeviceInner { - pub alloc2: Mutex, + pub allocator: Mutex, pub raw: ash::Device, pub adapter: PhysicalDeviceInfo, pub instance: Instance, @@ -438,25 +522,19 @@ impl PhysicalDeviceInfo { }, }; - let alloc2 = - gpu_allocator::vulkan::Allocator::new(&gpu_allocator::vulkan::AllocatorCreateDesc { - instance: instance.inner.raw.clone(), - device: device.clone(), - physical_device: self.pdev, - debug_settings: Default::default(), - buffer_device_address: false, - allocation_sizes: { - const MB: u64 = 1024 * 1024; - gpu_allocator::AllocationSizes::new(8 * MB, 64 * MB) - .with_max_host_memblock_size(256 * MB) - .with_max_device_memblock_size(256 * MB) - }, - })?; + let mut allocator_info = + vk_mem::AllocatorCreateInfo::new(&instance.inner.raw, &device, self.pdev); + allocator_info.preferred_large_heap_block_size = 64 * 1024 * 1024; + allocator_info.vulkan_api_version = instance + .inner + .api_version + .min(self.properties.device_api_version); + let allocator = unsafe { vk_mem::Allocator::new(allocator_info) }?; let pipeline_cache = PipelineCache::new(&device, &self).unwrap(); let shared = Arc::new_cyclic(|inner: &Weak| DeviceInner { raw: device.clone(), - alloc2: Mutex::new(alloc2), + allocator: Mutex::new(allocator), instance: instance.clone(), adapter: self, queues: device_queues, @@ -579,6 +657,126 @@ where } impl DeviceInner { + fn allocation_create_info( + &self, + desc: &AllocationDesc<'_>, + ) -> Result { + use vk_mem::{AllocationCreateFlags, AllocationCreateInfo, MemoryUsage}; + + #[allow(deprecated)] + let mut info = AllocationCreateInfo { + usage: match desc.location { + MemoryLocation::Unknown => MemoryUsage::Unknown, + MemoryLocation::GpuOnly => MemoryUsage::GpuOnly, + MemoryLocation::CpuToGpu => MemoryUsage::CpuToGpu, + MemoryLocation::GpuToCpu => MemoryUsage::GpuToCpu, + }, + ..Default::default() + }; + + if matches!( + desc.location, + MemoryLocation::CpuToGpu | MemoryLocation::GpuToCpu + ) { + let memory_properties = &self.adapter.properties.memory_properties; + let coherent_host_types = memory_properties + .memory_types + .iter() + .take(memory_properties.memory_type_count as usize) + .enumerate() + .filter(|(_, memory_type)| { + memory_type.property_flags.contains( + vk::MemoryPropertyFlags::HOST_VISIBLE + | vk::MemoryPropertyFlags::HOST_COHERENT, + ) + }) + .fold(0_u32, |bits, (index, _)| bits | (1 << index)); + if coherent_host_types == 0 { + return Err(vk::Result::ERROR_FEATURE_NOT_PRESENT.into()); + } + info.memory_type_bits = coherent_host_types; + info.flags |= AllocationCreateFlags::MAPPED; + } + + let api_version = self + .instance + .inner + .api_version + .min(self.adapter.properties.device_api_version); + if desc.strategy == AllocationStrategy::Dedicated && api_version < vk::API_VERSION_1_1 { + return Err(Error::DedicatedAllocationRequiresApi11); + } + if desc.strategy == AllocationStrategy::Dedicated { + info.flags |= AllocationCreateFlags::DEDICATED_MEMORY; + } + + Ok(info) + } + + pub fn allocate_for_buffer( + &self, + buffer: vk::Buffer, + desc: AllocationDesc<'_>, + ) -> Result { + use vk_mem::Alloc as _; + + let create_info = self.allocation_create_info(&desc)?; + let allocator = self.allocator.lock(); + let raw = unsafe { allocator.allocate_memory_for_buffer(buffer, &create_info) }?; + let allocation_info = allocator.get_allocation_info2(&raw); + let dedicated_memory = allocation_info.dedicated_memory; + let allocation_info = allocation_info.allocation_info; + let mapped = NonNull::new(allocation_info.mapped_data.cast::()).map(SendSyncPtr); + Ok(GpuAllocation { + raw, + memory: allocation_info.device_memory, + offset: allocation_info.offset, + size: allocation_info.size, + memory_type_index: allocation_info.memory_type, + mapped, + dedicated_memory, + }) + } + + pub fn allocate_for_image( + &self, + image: vk::Image, + desc: AllocationDesc<'_>, + ) -> Result { + use vk_mem::Alloc as _; + + let create_info = self.allocation_create_info(&desc)?; + let allocator = self.allocator.lock(); + let raw = unsafe { allocator.allocate_memory_for_image(image, &create_info) }?; + let allocation_info = allocator.get_allocation_info2(&raw); + let dedicated_memory = allocation_info.dedicated_memory; + let allocation_info = allocation_info.allocation_info; + let mapped = NonNull::new(allocation_info.mapped_data.cast::()).map(SendSyncPtr); + Ok(GpuAllocation { + raw, + memory: allocation_info.device_memory, + offset: allocation_info.offset, + size: allocation_info.size, + memory_type_index: allocation_info.memory_type, + mapped, + dedicated_memory, + }) + } + + pub fn bind_buffer_memory(&self, alloc: &GpuAllocation, buffer: vk::Buffer) -> Result<()> { + unsafe { + self.allocator + .lock() + .bind_buffer_memory(&alloc.raw, buffer)? + }; + Ok(()) + } + + pub fn bind_image_memory(&self, alloc: &GpuAllocation, image: vk::Image) -> Result<()> { + unsafe { self.allocator.lock().bind_image_memory(&alloc.raw, image)? }; + Ok(()) + } + pub fn sync_threadpool(&self) -> &sync::SyncThreadpool { &self.sync_threadpool } @@ -1072,8 +1270,8 @@ pub mod asdf { } impl traits::ExternallyManagedObject for GpuAllocation { - unsafe fn destroy(self, device: &Device) { - _ = device.alloc2.lock().free(self); + unsafe fn destroy(mut self, device: &Device) { + unsafe { device.allocator.lock().free_memory(&mut self.raw) }; } } @@ -1155,4 +1353,11 @@ pub mod asdf { vk::FALSE ); } + + #[test] + fn gpu_allocation_is_send_sync() { + fn assert_send_sync() {} + + assert_send_sync::(); + } } diff --git a/crates/renderer_device/src/instance.rs b/crates/renderer_device/src/instance.rs index 10c7cae..6c7b63b 100644 --- a/crates/renderer_device/src/instance.rs +++ b/crates/renderer_device/src/instance.rs @@ -30,6 +30,7 @@ pub struct InstanceInner { // it drops before `raw` during instance teardown. pub _debug_utils: DebugUtils, pub raw: ash::Instance, + pub api_version: u32, pub extensions: InstanceExtensions, pub entry: Entry, } @@ -123,16 +124,14 @@ impl Instance { unsafe { entry.try_enumerate_instance_version()? }.unwrap_or(vk::API_VERSION_1_0); let app_info = vk::ApplicationInfo::default() - .api_version(if version < vk::API_VERSION_1_1 { - vk::API_VERSION_1_0 - } else { - // ash doesn't support 1.4 yet - vk::API_VERSION_1_3 - }) + // ash/VMA support Vulkan 1.3, and the application must not request a + // version newer than the loader supports. + .api_version(version.min(vk::API_VERSION_1_3)) .application_name(&app_name) .application_version(desc.app_version) .engine_name(c"Bevy Engine") .engine_version(vk::make_api_version(0, 0, 1, 0)); + let api_version = app_info.api_version; let mut validation_info = vk::LayerSettingsCreateInfoEXT::default().settings(desc.layer_settings); @@ -228,6 +227,7 @@ impl Instance { let instance = Arc::new(InstanceInner { _debug_utils: debug_utils, raw: instance, + api_version, extensions, entry, }); diff --git a/crates/renderer_device/src/lib.rs b/crates/renderer_device/src/lib.rs index e611491..8aed0d6 100644 --- a/crates/renderer_device/src/lib.rs +++ b/crates/renderer_device/src/lib.rs @@ -77,10 +77,12 @@ pub enum Error { Utf8Error(#[from] core::str::Utf8Error), #[error(transparent)] NulError(#[from] std::ffi::NulError), - #[error(transparent)] - GpuAllocatorError(#[from] gpu_allocator::AllocationError), #[error("No Physical Device found.")] NoPhysicalDevice, + #[error( + "Dedicated allocation requires Vulkan 1.1 or both VK_KHR_dedicated_allocation and VK_KHR_get_memory_requirements2 extensions." + )] + DedicatedAllocationRequiresApi11, #[error(transparent)] Io(#[from] std::io::Error), #[error( diff --git a/crates/renderer_device/src/testing.rs b/crates/renderer_device/src/testing.rs index 418e345..bbf901d 100644 --- a/crates/renderer_device/src/testing.rs +++ b/crates/renderer_device/src/testing.rs @@ -282,6 +282,7 @@ unsafe extern "system" fn capture_callback( #[cfg(test)] mod tests { use super::*; + use crate::device::{AllocationDesc, AllocationStrategy, DeviceObject, MemoryLocation}; #[test] fn instance_and_device_creation() { @@ -305,30 +306,23 @@ mod tests { .usage(vk::BufferUsageFlags::TRANSFER_SRC | vk::BufferUsageFlags::TRANSFER_DST) .sharing_mode(vk::SharingMode::EXCLUSIVE); let buffer = unsafe { device.dev().create_buffer(&info, None) }.expect("create buffer"); - let requirements = unsafe { device.dev().get_buffer_memory_requirements(buffer) }; + let buffer = DeviceObject::new(device.clone(), buffer); let allocation = device - .alloc2 - .lock() - .allocate(&gpu_allocator::vulkan::AllocationCreateDesc { - name: "renderer_device validation smoke buffer", - requirements, - location: gpu_allocator::MemoryLocation::CpuToGpu, - linear: true, - allocation_scheme: gpu_allocator::vulkan::AllocationScheme::GpuAllocatorManaged, - }) + .allocate_for_buffer( + *buffer, + AllocationDesc { + name: "renderer_device validation smoke buffer", + location: MemoryLocation::CpuToGpu, + strategy: AllocationStrategy::AllocatorManaged, + }, + ) .expect("allocate buffer memory"); - unsafe { - device - .dev() - .bind_buffer_memory(buffer, allocation.memory(), allocation.offset()) - .expect("bind buffer memory"); - device.dev().destroy_buffer(buffer, None); - } + let allocation = DeviceObject::new(device.clone(), allocation); device - .alloc2 - .lock() - .free(allocation) - .expect("free buffer memory"); + .bind_buffer_memory(&allocation, *buffer) + .expect("bind buffer memory"); + drop(buffer); + drop(allocation); test.assert_no_validation_errors(); } @@ -344,24 +338,21 @@ mod tests { .sharing_mode(vk::SharingMode::EXCLUSIVE); let buffer = unsafe { device.dev().create_buffer(&buffer_info, None) } .expect("create validation probe buffer"); - let requirements = unsafe { device.dev().get_buffer_memory_requirements(buffer) }; + let buffer = DeviceObject::new(device.clone(), buffer); let allocation = device - .alloc2 - .lock() - .allocate(&gpu_allocator::vulkan::AllocationCreateDesc { - name: "renderer_device validation probe buffer", - requirements, - location: gpu_allocator::MemoryLocation::CpuToGpu, - linear: true, - allocation_scheme: gpu_allocator::vulkan::AllocationScheme::GpuAllocatorManaged, - }) + .allocate_for_buffer( + *buffer, + AllocationDesc { + name: "renderer_device validation probe buffer", + location: MemoryLocation::CpuToGpu, + strategy: AllocationStrategy::AllocatorManaged, + }, + ) .expect("allocate validation probe buffer"); - unsafe { - device - .dev() - .bind_buffer_memory(buffer, allocation.memory(), allocation.offset()) - .expect("bind validation probe buffer"); - } + let allocation = DeviceObject::new(device.clone(), allocation); + device + .bind_buffer_memory(&allocation, *buffer) + .expect("bind validation probe buffer"); let pool_info = vk::CommandPoolCreateInfo::default() .queue_family_index(device.queues.graphics().family.index.index()); let pool = unsafe { device.dev().create_command_pool(&pool_info, None) } @@ -380,19 +371,15 @@ mod tests { // The size must be a multiple of four. The invalid command is never submitted. device .dev() - .cmd_fill_buffer(command_buffer, buffer, 0, 3, 0); + .cmd_fill_buffer(command_buffer, *buffer, 0, 3, 0); device .dev() .end_command_buffer(command_buffer) .expect("end command buffer"); device.dev().destroy_command_pool(pool, None); - device.dev().destroy_buffer(buffer, None); } - device - .alloc2 - .lock() - .free(allocation) - .expect("free probe allocation"); + drop(buffer); + drop(allocation); let messages = test.take_messages(); assert!( diff --git a/crates/renderer_rendergraph/Cargo.toml b/crates/renderer_rendergraph/Cargo.toml index 54dd1b7..1436fcf 100644 --- a/crates/renderer_rendergraph/Cargo.toml +++ b/crates/renderer_rendergraph/Cargo.toml @@ -5,7 +5,6 @@ edition = "2024" [dependencies] ash.workspace = true -gpu-allocator.workspace = true renderer_device = { path = "../renderer_device" } renderer_resources = { path = "../renderer_resources" } ash-formats = { git = "https://github.com/janis-bhm/ash-formats", branch = "main" } diff --git a/crates/renderer_rendergraph/src/resources.rs b/crates/renderer_rendergraph/src/resources.rs index d3f16a6..677f56e 100644 --- a/crates/renderer_rendergraph/src/resources.rs +++ b/crates/renderer_rendergraph/src/resources.rs @@ -1,6 +1,7 @@ use std::{borrow::Cow, sync::Arc}; use ash::vk; +use renderer_device::device::MemoryLocation; use renderer_resources::{ buffers::Buffer as BufferResource, images::{Image as ImageResource, ImageSubresourceExt}, @@ -13,7 +14,7 @@ use renderer_resources::{ pub struct GraphBufferDesc { pub size: u64, pub name: Option>, - pub mem_location: gpu_allocator::MemoryLocation, + pub mem_location: MemoryLocation, /// Additional Vulkan usages not visible in the graph's recorded commands. pub extra_usage: vk::BufferUsageFlags, } @@ -24,7 +25,7 @@ impl Default for GraphBufferDesc { size: 0, name: None, // Transient graph resources default to device-local memory. - mem_location: gpu_allocator::MemoryLocation::GpuOnly, + mem_location: MemoryLocation::GpuOnly, extra_usage: vk::BufferUsageFlags::empty(), } } @@ -43,7 +44,7 @@ pub struct GraphTextureDesc { pub tiling: vk::ImageTiling, pub layout: vk::ImageLayout, pub name: Option>, - pub mem_location: gpu_allocator::MemoryLocation, + pub mem_location: MemoryLocation, /// Additional Vulkan usages not visible in the graph's recorded commands. pub extra_usage: vk::ImageUsageFlags, } @@ -66,7 +67,7 @@ impl Default for GraphTextureDesc { tiling: vk::ImageTiling::OPTIMAL, layout: vk::ImageLayout::UNDEFINED, name: None, - mem_location: gpu_allocator::MemoryLocation::GpuOnly, + mem_location: MemoryLocation::GpuOnly, extra_usage: vk::ImageUsageFlags::empty(), } } diff --git a/crates/renderer_rendergraph/tests/graph_validation.rs b/crates/renderer_rendergraph/tests/graph_validation.rs index a10294d..7835451 100644 --- a/crates/renderer_rendergraph/tests/graph_validation.rs +++ b/crates/renderer_rendergraph/tests/graph_validation.rs @@ -1,7 +1,10 @@ use std::sync::Arc; use ash::vk; -use renderer_device::{device::QueueFlags, testing::TestVulkan}; +use renderer_device::{ + device::{MemoryLocation, QueueFlags}, + testing::TestVulkan, +}; use renderer_rendergraph::{ RenderGraph, commands::CopyBuffer, @@ -28,7 +31,7 @@ fn imported_buffer_copy_records_under_validation() { size: 256, usage: vk::BufferUsageFlags::TRANSFER_SRC | vk::BufferUsageFlags::TRANSFER_DST, queue_families: QueueFlags::empty(), - mem_location: gpu_allocator::MemoryLocation::CpuToGpu, + mem_location: MemoryLocation::CpuToGpu, ..Default::default() }, ) diff --git a/crates/renderer_resources/AGENTS.md b/crates/renderer_resources/AGENTS.md index 05a16b4..7b10b3a 100644 --- a/crates/renderer_resources/AGENTS.md +++ b/crates/renderer_resources/AGENTS.md @@ -13,9 +13,9 @@ ## Key types and API - These modules are public from `renderer_resources`; the crate root re-exports the core device types and `device` module from `renderer_device` for resource construction. -- `BufferDesc` configures Vulkan buffer flags, size, usage, queue families, memory location, allocation strategy, and optional debug name. `Buffer::new` allocates/binds memory; `map` and `map_mut` return mapped slices when available. -- `Buffer::raw` returns the Vulkan handle and `len` returns the configured size. Mapping is bounded to the descriptor size, not the allocator block size. -- `ImageDesc` configures image type, format, extent, layers/mips, samples, tiling, usage, initial layout, queue sharing, and allocation. +- `BufferDesc` configures Vulkan buffer flags, size, usage, queue families, `renderer_device::device::MemoryLocation`, allocation strategy, and optional debug name. `Buffer::new` requests a VMA allocation through `DeviceInner::allocate_for_buffer` and binds it through `bind_buffer_memory`; `map` and `map_mut` return mapped slices when available. +- `Buffer::raw` returns the Vulkan handle and `len` returns the configured size. Mapping is bounded to the descriptor size, not the VMA allocation block size. CPU-mapped locations select host-visible, host-coherent memory. +- `ImageDesc` configures image type, format, extent, layers/mips, samples, tiling, usage, initial layout, queue sharing, and allocation strategy/location. `Image::new` uses the device's VMA allocation and image-binding helpers. - `Image::new` creates an allocated image. `Image::new_with_allocation` accepts an explicit `renderer_device::device::Allocation` and validates size and memory-type compatibility. - `Image::from_swapchain_raw(image, device, format, extent, usage)` builds an image wrapper for an image owned by a swapchain; renderer calls it from `Swapchain::acquire_image`. - `ImageViewDesc` describes a view's type, format, component mapping, aspect, mip range, and layer range. `Image::create_view` validates compatibility and returns `ManuallyDrop`. diff --git a/crates/renderer_resources/Cargo.toml b/crates/renderer_resources/Cargo.toml index f2d5648..c38e382 100644 --- a/crates/renderer_resources/Cargo.toml +++ b/crates/renderer_resources/Cargo.toml @@ -9,7 +9,6 @@ testing = [] [dependencies] renderer_device = { path = "../renderer_device" } ash = { workspace = true } -gpu-allocator = { workspace = true } parking_lot = { workspace = true } bytemuck = { workspace = true } tracing = { workspace = true } diff --git a/crates/renderer_resources/src/buffers.rs b/crates/renderer_resources/src/buffers.rs index 4babd82..ffe6efd 100644 --- a/crates/renderer_resources/src/buffers.rs +++ b/crates/renderer_resources/src/buffers.rs @@ -1,12 +1,13 @@ use std::{borrow::Cow, sync::Arc}; use ash::vk; -use gpu_allocator::vulkan::AllocationScheme; use renderer_device::queue::QueueFamilyIndices; use crate::{ Device, - device::{Allocation, AllocationStrategy, DeviceObject, QueueFlags}, + device::{ + Allocation, AllocationDesc, AllocationStrategy, DeviceObject, MemoryLocation, QueueFlags, + }, }; #[derive(Clone)] @@ -17,7 +18,7 @@ pub struct BufferDesc { pub usage: vk::BufferUsageFlags, pub queue_families: QueueFlags, - pub mem_location: gpu_allocator::MemoryLocation, + pub mem_location: MemoryLocation, pub alloc_scheme: AllocationStrategy, } @@ -65,7 +66,7 @@ impl Default for BufferDesc { size: Default::default(), usage: Default::default(), queue_families: QueueFlags::empty(), - mem_location: gpu_allocator::MemoryLocation::Unknown, + mem_location: MemoryLocation::Unknown, alloc_scheme: AllocationStrategy::default(), } } @@ -102,46 +103,30 @@ impl Buffer { } pub fn new(device: Device, desc: BufferDesc) -> crate::Result { - let (buffer, requirements) = Self::new_raw(device.clone(), &desc)?; + let buffer = Self::new_raw(device.clone(), &desc)?; + let alloc = device.allocate_for_buffer( + *buffer, + AllocationDesc { + name: desc.name.as_deref().unwrap_or_default(), + location: desc.mem_location, + strategy: desc.alloc_scheme, + }, + )?; - let alloc = - device - .alloc2 - .lock() - .allocate(&gpu_allocator::vulkan::AllocationCreateDesc { - name: desc.name.as_deref().unwrap_or_default(), - requirements, - location: desc.mem_location, - linear: true, - allocation_scheme: match desc.alloc_scheme { - AllocationStrategy::AllocatorManaged => { - AllocationScheme::GpuAllocatorManaged - } - AllocationStrategy::Dedicated => AllocationScheme::DedicatedBuffer(buffer), - }, - })?; - - unsafe { - device - .raw - .bind_buffer_memory(buffer, alloc.memory(), alloc.offset())?; - } - - Ok(Self { - buffer: BufferHandle::Owned(DeviceObject::new_debug_named( - device.clone(), - buffer, - desc.name.clone(), - )), + let buffer = Self { + buffer: BufferHandle::Owned(buffer), desc, - alloc: Allocation::Owned(DeviceObject::new(device, alloc)), - }) + alloc: Allocation::Owned(DeviceObject::new(device.clone(), alloc)), + }; + device.bind_buffer_memory( + buffer.alloc.allocation().expect("owned buffer allocation"), + buffer.raw(), + )?; + + Ok(buffer) } - fn new_raw( - device: Device, - desc: &BufferDesc, - ) -> crate::Result<(vk::Buffer, vk::MemoryRequirements)> { + fn new_raw(device: Device, desc: &BufferDesc) -> crate::Result> { let queue_families = device.queues.family_indices(desc.queue_families); let sharing_mode = if queue_families.len() > 1 { @@ -156,9 +141,11 @@ impl Buffer { .sharing_mode(sharing_mode); let buffer = unsafe { device.dev().create_buffer(&create_info, None)? }; - let mem_reqs = unsafe { device.dev().get_buffer_memory_requirements(buffer) }; - - Ok((buffer, mem_reqs)) + Ok(DeviceObject::new_debug_named( + device, + buffer, + desc.name.clone(), + )) } pub fn map(&mut self) -> Option<&[u8]> { @@ -210,6 +197,112 @@ impl Buffer { } } +#[cfg(test)] +mod allocation_validation_tests { + use ash::vk; + use renderer_device::testing::TestVulkan; + + use super::{Buffer, BufferDesc}; + use crate::{ + device::MemoryLocation, + images::{Image, ImageDesc}, + }; + use renderer_device::device::AllocationStrategy; + + #[test] + fn vma_mapping_and_dedicated_resources_under_validation() { + let test = TestVulkan::expect(); + let device = test.device().clone(); + + fn assert_send_sync() {} + assert_send_sync::(); + assert_send_sync::(); + + let mut mapped_buffer = Buffer::new( + device.clone(), + BufferDesc { + size: 64, + usage: vk::BufferUsageFlags::TRANSFER_SRC, + mem_location: MemoryLocation::CpuToGpu, + ..Default::default() + }, + ) + .expect("create CpuToGpu buffer"); + let allocation = mapped_buffer + .alloc + .allocation() + .expect("CpuToGpu buffer allocation"); + let memory_type = test + .physical_device() + .properties + .memory_properties + .memory_types[allocation.memory_type_index() as usize]; + assert!(memory_type.property_flags.contains( + vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT + )); + + let expected = (0..64).map(|byte| byte as u8).collect::>(); + mapped_buffer + .map_mut() + .expect("CpuToGpu buffer is mapped") + .copy_from_slice(&expected); + assert_eq!( + mapped_buffer.map().expect("CpuToGpu buffer remains mapped"), + expected + ); + + let dedicated_buffer = Buffer::new( + device.clone(), + BufferDesc { + size: 256, + usage: vk::BufferUsageFlags::TRANSFER_DST, + alloc_scheme: AllocationStrategy::Dedicated, + ..Default::default() + }, + ) + .expect("create dedicated buffer"); + assert!( + dedicated_buffer + .alloc + .allocation() + .expect("dedicated buffer allocation") + .is_dedicated(), + "VMA should allocate the dedicated buffer from its own memory block" + ); + + let dedicated_image = Image::new( + device, + ImageDesc { + format: vk::Format::R8G8B8A8_UNORM, + extent: vk::Extent3D { + width: 16, + height: 16, + depth: 1, + }, + usage: vk::ImageUsageFlags::SAMPLED, + alloc_scheme: AllocationStrategy::Dedicated, + ..Default::default() + }, + ) + .expect("create dedicated image"); + assert!( + dedicated_image + .allocation() + .expect("dedicated image allocation") + .allocation() + .expect("dedicated image VMA allocation") + .is_dedicated(), + "VMA should allocate the dedicated image from its own memory block" + ); + + drop(dedicated_image); + drop(dedicated_buffer); + drop(mapped_buffer); + test.device().wait_idle().expect("wait for device idle"); + test.assert_no_validation_errors(); + } +} + /// Byte range selected by a buffer view. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct BufferRange { diff --git a/crates/renderer_resources/src/images.rs b/crates/renderer_resources/src/images.rs index 2c6bb69..e677182 100644 --- a/crates/renderer_resources/src/images.rs +++ b/crates/renderer_resources/src/images.rs @@ -6,14 +6,15 @@ use std::{ }; use crate::{ - device::{Allocation, AllocationStrategy, DeviceObject, QueueFlags}, + device::{ + Allocation, AllocationDesc, AllocationStrategy, DeviceObject, MemoryLocation, QueueFlags, + }, util::WeakVec, }; use super::Device; use ash::vk::{self, Handle}; use ash_formats::FormatExt; -use gpu_allocator::vulkan::{AllocationCreateDesc, AllocationScheme}; use parking_lot::Mutex; use renderer_device::queue::QueueFamilyIndices; @@ -32,7 +33,7 @@ pub struct ImageDesc { pub queue_families: QueueFlags, pub layout: vk::ImageLayout, - pub mem_location: gpu_allocator::MemoryLocation, + pub mem_location: MemoryLocation, pub alloc_scheme: AllocationStrategy, } @@ -123,7 +124,7 @@ impl Default for ImageDesc { usage: Default::default(), queue_families: QueueFlags::empty(), layout: vk::ImageLayout::UNDEFINED, - mem_location: gpu_allocator::MemoryLocation::Unknown, + mem_location: MemoryLocation::Unknown, alloc_scheme: AllocationStrategy::AllocatorManaged, } } @@ -187,17 +188,15 @@ pub struct Image { impl Image { pub fn new(device: Device, desc: ImageDesc) -> crate::Result { - let (image, requirements) = Self::new_raw(device.clone(), &desc)?; - let alloc = device.alloc2.lock().allocate(&AllocationCreateDesc { - name: desc.name.as_deref().unwrap_or(""), - requirements, - location: desc.mem_location, - linear: desc.tiling == vk::ImageTiling::LINEAR, - allocation_scheme: match desc.alloc_scheme { - AllocationStrategy::AllocatorManaged => AllocationScheme::GpuAllocatorManaged, - AllocationStrategy::Dedicated => AllocationScheme::DedicatedImage(image), + let image = Self::new_raw(device.clone(), &desc)?; + let alloc = device.allocate_for_image( + *image, + AllocationDesc { + name: desc.name.as_deref().unwrap_or(""), + location: desc.mem_location, + strategy: desc.alloc_scheme, }, - })?; + )?; Self::new_with_allocation_unchecked( device.clone(), @@ -209,27 +208,24 @@ impl Image { fn new_with_allocation_unchecked( device: Device, - image: vk::Image, + image: DeviceObject, allocation: Allocation, desc: ImageDesc, ) -> crate::Result { - // bind memory - if let Some(alloc) = allocation.allocation() { - unsafe { - device - .raw - .bind_image_memory(image, alloc.memory(), alloc.offset())?; + let image = Self { + image: ImageInner::Allocated(image, allocation), + desc, + views: Default::default(), + }; + + // Bind after ownership is established so failures destroy the image before freeing memory. + if let ImageInner::Allocated(raw_image, allocation) = &image.image { + if let Some(alloc) = allocation.allocation() { + device.bind_image_memory(alloc, **raw_image)?; } } - Ok(Self { - image: ImageInner::Allocated( - DeviceObject::new_debug_named(device, image, desc.name.clone()), - allocation, - ), - desc, - views: Default::default(), - }) + Ok(image) } pub fn new_with_allocation( @@ -237,7 +233,8 @@ impl Image { allocation: Allocation, desc: ImageDesc, ) -> crate::Result { - let (image, requirements) = Self::new_raw(device.clone(), &desc)?; + let image = Self::new_raw(device.clone(), &desc)?; + let requirements = unsafe { device.dev().get_image_memory_requirements(*image) }; // validate allocation let alloc_size = allocation @@ -286,7 +283,7 @@ impl Image { usage, queue_families: QueueFlags::PRESENT, layout: vk::ImageLayout::UNDEFINED, - mem_location: gpu_allocator::MemoryLocation::GpuOnly, + mem_location: MemoryLocation::GpuOnly, ..Default::default() }, views: Default::default(), @@ -307,10 +304,7 @@ impl Image { } } - fn new_raw( - device: Device, - desc: &ImageDesc, - ) -> crate::Result<(vk::Image, vk::MemoryRequirements)> { + fn new_raw(device: Device, desc: &ImageDesc) -> crate::Result> { tracing::trace!("new image with desc={desc:?}"); let ImageDesc { flags, @@ -372,9 +366,11 @@ impl Image { let image = unsafe { device.raw.create_image(&info, None)? }; - let requirements = unsafe { device.raw.get_image_memory_requirements(image) }; - - Ok((image, requirements)) + Ok(DeviceObject::new_debug_named( + device, + image, + desc.name.clone(), + )) } pub fn raw(&self) -> vk::Image { diff --git a/crates/renderer_resources/tests/managed_set_validation.rs b/crates/renderer_resources/tests/managed_set_validation.rs index ca5b04f..07a929a 100644 --- a/crates/renderer_resources/tests/managed_set_validation.rs +++ b/crates/renderer_resources/tests/managed_set_validation.rs @@ -1,7 +1,10 @@ use std::sync::Arc; use ash::vk; -use renderer_device::{device::QueueFlags, testing::TestVulkan}; +use renderer_device::{ + device::{MemoryLocation, QueueFlags}, + testing::TestVulkan, +}; use renderer_resources::{ buffers::{Buffer, BufferDesc}, pipeline::{ @@ -23,7 +26,7 @@ fn managed_set_write_and_pool_reset_under_validation() { size: 256, usage: vk::BufferUsageFlags::UNIFORM_BUFFER, queue_families: QueueFlags::empty(), - mem_location: gpu_allocator::MemoryLocation::CpuToGpu, + mem_location: MemoryLocation::CpuToGpu, ..Default::default() }, ) diff --git a/design-plans/gpu-allocator-to-vma-migration.md b/design-plans/gpu-allocator-to-vma-migration.md new file mode 100644 index 0000000..3d1f738 --- /dev/null +++ b/design-plans/gpu-allocator-to-vma-migration.md @@ -0,0 +1,326 @@ +# Migration Plan: gpu-allocator → VMA via vk-mem + +Status: proposed — rev 2 (Luna review fixes applied) +Date: 2026-10-26 +Scope: workspace-wide memory-allocator swap + +## Goal + +Replace the `gpu-allocator` crate with VMA (Vulkan Memory Allocator) via the +`vk-mem` bindings crate, preserving current allocation behavior (buffer/image +create/bind/destroy, mapping, dedicated allocations) and the existing real-Vulkan +tests. + +## Architecture decision + +### Requirements + +- Hard: remove `gpu-allocator`; use `vk-mem`/VMA; preserve the dependency + direction `bevy_vulkan_render` → `renderer` → `renderer_resources` → + `renderer_device`; keep buffer/image create/bind/destroy and mapping behavior; + keep real-Vulkan tests working. +- Nice: dedicated allocations, allocation naming/stats, defragmentation (not + required now). + +### Constraints + +- Nightly toolchain, edition 2024. +- `renderer_resources` must not depend on `renderer`. +- `renderer_device` owns device/allocator state. +- `renderer_resources`/`renderer` should not expose `vk_mem` types. +- The `testing` feature gates real-device tests. +- No per-frame allocation is acceptable on current paths. +- `renderer_rendergraph` is also a workspace member and directly uses + `gpu_allocator::MemoryLocation`, so it must be included (it is not mentioned in + the workspace guide). + +### Current architecture + +- Deps: root `Cargo.toml:45` git `gpu-allocator` (line 44 has a commented + `# vk-mem = "0.5.0"`); `renderer_device/Cargo.toml:12`, + `renderer_resources/Cargo.toml:12`, `renderer/Cargo.toml:27`, + `renderer_rendergraph/Cargo.toml:8`. `Cargo.lock:3423-3435` is the git package; + `Cargo.lock:3409-3420` is the registry copy pulled by `wgpu-hal` via the Bevy + dev-dependency. +- Allocator state: `crates/renderer_device/src/device.rs:151` + `pub alloc2: Mutex`, initialized at + `:441-456` (`AllocatorCreateDesc`, `AllocationSizes::new(8MB,64MB)` with 256MB + maxima). `GpuAllocation` is the private alias at `:111`; `AllocationStrategy` + at `:114-124`; `Allocation::{Owned,Shared,Unmanaged}` at `:126-147`. Free path + is `ExternallyManagedObject for GpuAllocation` at `:1074-1077`, which + calls `alloc2.lock().free(...)`. +- Errors: `crates/renderer_device/src/lib.rs:81` + `GpuAllocatorError(#[from] gpu_allocator::AllocationError)`. +- Buffer flow: `crates/renderer_resources/src/buffers.rs:4` imports + `AllocationScheme`; `:20,68` `MemoryLocation`; `:108-122` allocates from + `alloc2`; `:128-130` binds with raw `vkBindBufferMemory`; `:137` wraps in + `Allocation::Owned(DeviceObject)`; `:164-181` maps via + `mapped_slice`/`mapped_slice_mut`. +- Image flow: `crates/renderer_resources/src/images.rs:16` imports VMA-named + `AllocationCreateDesc/Scheme`; `:35,126,289` `MemoryLocation`; `:191-205` + allocates and wraps; `:217-223` binds; `:244-260` validates size/memory-type + for `new_with_allocation`; `:418-428` exposes allocation. +- Dedicated logic: `buffers.rs:116-121` uses + `AllocationScheme::DedicatedBuffer(buffer)`; `images.rs:196-199` uses + `DedicatedImage(image)`; `renderer_rendergraph/src/graph.rs:1352-1357` chooses + dedicated at 64 MiB. +- Direct test allocations: `crates/renderer_device/src/testing.rs:312-319` and + `:351-357` allocate/free directly through `device.alloc2`. +- Other type consumers: `renderer/src/rendering/mod.rs:8,56,77,87,97`; + `renderer/src/egui.rs:8,74-75,140,157,510,531,538,545,555`; + `renderer/src/render_graph/graph_builder.rs:51,60,69,87,168-183`; + `renderer_rendergraph/src/resources.rs:16,27,46,69`; tests + `renderer_resources/tests/managed_set_validation.rs:26` and + `renderer_rendergraph/tests/graph_validation.rs:31`. +- No stats/reporting or defragmentation call sites exist today. `Buffer`/`Image` + drop field order: buffer/image handle first, then allocation + (`buffers.rs:74-78`, `images.rs:144-148`). + +### Alternatives considered + +1. **Swap allocator inside `renderer_device`, add a private wrapper and a + workspace-owned `MemoryLocation` (chosen)**: least invasive, upper crates stay + VMA-free. +2. Re-export `vk_mem` types as public resource API: least code, but leaks VMA + into `renderer_resources`/`renderer` and locks the workspace API to `vk-mem`. +3. Use `vk_mem::create_buffer/create_image` directly in `renderer_resources`: + fewer Vulkan calls and idiomatic VMA, but larger ownership refactor and + couples the resource crate to `vk_mem`. +4. Add a memory-backend trait: easy A/B testing, but unnecessary indirection and + lifetime complexity for a one-time swap. + +### Decision rationale (feasibility verified) + +- `vk-mem 0.5.0` bundles **VMA 3.3.0** and depends on **ash 0.38** (workspace ash + is `0.38.0+1.3.281`), so there is one ash and no duplicate-ash problem. +- `vk_mem::Allocator` is `Send + Sync` and internally synchronized unless + `EXTERNALLY_SYNCHRONIZED` is set; keep the `Mutex`. +- `vk_mem::Allocation` is `Copy, Send, Sync` and has no inherent + `size/memory/offset` accessors, so the wrapper must retain `AllocationInfo`. + +## Target design + +- `renderer_device::device::MemoryLocation { Unknown, GpuOnly, CpuToGpu, + GpuToCpu }` with `Default = Unknown`, `Copy/Eq/Hash`; use it as the public + `mem_location` type. +- `pub struct GpuAllocation { raw: vk_mem::Allocation, memory: vk::DeviceMemory, + offset: u64, size: u64, memory_type_index: u32, mapped: Option }`. + `SendSyncPtr` is a private `NonNull` wrapper with unsafe `Send/Sync`, + mirroring `gpu_allocator`'s pointer wrapper. Implement `Debug`, + `memory()/offset()/size()/memory_type_index()/mapped_slice()/mapped_slice_mut()`. +- `pub struct AllocationDesc<'a> { name: &'a str, location: MemoryLocation, + strategy: AllocationStrategy }` and + `DeviceInner::{allocate_for_buffer, allocate_for_image, bind_buffer_memory, + bind_image_memory}`. Note the call shapes: `allocate_memory_for_buffer/image` + live on the `vk_mem::Alloc` trait (import `vk_mem::Alloc as _`) and are + `unsafe` in `vk-mem 0.5.0`; `bind_buffer_memory`/`bind_image_memory` are + inherent (also `unsafe`) `Allocator` methods, not trait methods. +- `DeviceInner.alloc2: Mutex` remains the single allocator + owner. Keep `Allocation::{Owned(DeviceObject), + Shared(Arc<...>), Unmanaged}` unchanged in shape. +- `renderer_resources`/`renderer`/`renderer_rendergraph` use only + `renderer_device::device::MemoryLocation`, `Allocation`, and the new `Device` + helpers; no `vk_mem` imports outside `renderer_device`. + +Suggested interface: + +```rust +impl DeviceInner { + pub fn allocate_for_buffer(&self, buffer: vk::Buffer, desc: AllocationDesc<'_>) -> Result; + pub fn allocate_for_image(&self, image: vk::Image, desc: AllocationDesc<'_>) -> Result; + pub fn bind_buffer_memory(&self, alloc: &GpuAllocation, buffer: vk::Buffer) -> Result<()>; + pub fn bind_image_memory(&self, alloc: &GpuAllocation, image: vk::Image) -> Result<()>; +} +``` + +## Dependency changes + +- Root `Cargo.toml`: add `vk-mem = "0.5.0"` to `[workspace.dependencies]` (the + commented line at `:44` anticipates this); remove the git `gpu-allocator` entry + in phase 3. +- `renderer_device/Cargo.toml`: add `vk-mem = { workspace = true }`; remove + `gpu-allocator` in phase 2 (after the swap compiles). +- `renderer_resources/Cargo.toml`, `renderer/Cargo.toml`, + `renderer_rendergraph/Cargo.toml`: remove `gpu-allocator` in phase 3. +- Ash alignment: `vk-mem 0.5.0` pins ash 0.38; an ash upgrade requires a + matching `vk-mem` release or fork. + +## API mapping + +| gpu-allocator | VMA / `vk-mem` | +|---|---| +| `AllocatorCreateDesc` instance/device/physical_device | `AllocatorCreateInfo::new(&instance, &device, physical_device)` | +| `debug_settings`, allocation sizes | VMA compile-time debug macros; `preferred_large_heap_block_size` only (loss of min/max block tuning) | +| `buffer_device_address` | `AllocatorCreateFlags::BUFFER_DEVICE_ADDRESS` (off here) | +| `MemoryLocation::{Unknown,GpuOnly,CpuToGpu,GpuToCpu}` | `MemoryUsage::{Unknown,GpuOnly,CpuToGpu,GpuToCpu}` | +| `AllocationScheme::GpuAllocatorManaged` | default allocation (`AllocationCreateFlags::empty()`) | +| `DedicatedBuffer(buffer)` / `DedicatedImage(image)` | `DEDICATED_MEMORY` + `allocate_memory_for_buffer/image` (passes `VkMemoryDedicatedAllocateInfo` when API version ≥ 1.1) | +| `AllocationCreateDesc.linear` | implicit: use `allocate_memory_for_buffer/image`, not generic allocation | +| `Allocation.memory()/offset()/size()/memory_type_index()` | `AllocationInfo.device_memory/offset/size/memory_type` captured at allocation | +| `mapped_slice()/mapped_slice_mut()` | `AllocationInfo.mapped_data` from `MAPPED` allocation, or `Allocator::map_memory` | +| `Allocator::free(allocation)` | `Allocator::free_memory(&mut allocation)` | +| `bind_buffer_memory`/`bind_image_memory` | `Allocator::{bind_buffer_memory, bind_image_memory}` | +| stats report | `calculate_statistics()` / `get_heap_budgets()`; no allocation names in `vk-mem 0.5.0` | +| debug names | unchanged: `DeviceObject::new_debug_named` sets Vulkan object names; VMA allocation names are not exposed | + +## Migration phases + +### Phase 0 — baseline + +Run `cargo check --workspace`, `cargo test -p renderer_device --features +testing`, inspect `git status`. No edits. + +### Phase 1 — dependency/type scaffolding (non-breaking) + +Add `vk-mem = "0.5.0"` to root `[workspace.dependencies]`; add +`vk-mem = { workspace = true }` to `renderer_device/Cargo.toml`; add +`MemoryLocation`, `GpuAllocation` skeleton, and `AllocationDesc` in +`crates/renderer_device/src/device.rs` while leaving the old allocator +untouched. + +Verify: `cargo check -p renderer_device`. + +### Phase 2 — atomic allocator swap (one compile unit) + +In `crates/renderer_device/src/device.rs` replace `alloc2`/creation/ +`GpuAllocation` alias/free impl with VMA; remove `GpuAllocatorError` from +`crates/renderer_device/src/lib.rs:81`; rewrite `testing.rs:312-319,351-357`; +in `crates/renderer_resources/src/buffers.rs` and `images.rs` replace +`alloc2.allocate` + raw bind with `allocate_for_buffer/image` + `bind_*`, adding +handle guards in the same pass (raw handle wrapped in `DeviceObject` immediately +after creation, allocation ownership established before binding — see +implementation notes), but temporarily keep the public fields as +`gpu_allocator::MemoryLocation` and convert at the call site. + +Verify: `cargo check --workspace` and `cargo test -p renderer_device --features +testing`. + +### Phase 3 — public type/dependency cleanup + +Change `BufferDesc.mem_location`/`ImageDesc.mem_location` to +`renderer_device::device::MemoryLocation`; update `renderer/src/rendering/mod.rs`, +`renderer/src/egui.rs`, `renderer/src/render_graph/graph_builder.rs`, +`renderer_rendergraph/src/resources.rs`, and both tests +(`renderer_resources/tests/managed_set_validation.rs`, +`renderer_rendergraph/tests/graph_validation.rs`); remove `gpu-allocator` from +`renderer_resources`, `renderer`, `renderer_rendergraph`, and root manifests. + +Verify: `cargo check --workspace`; `rg -n 'gpu_allocator|gpu-allocator' --glob +'!Cargo.lock'` should be empty. + +### Phase 4 — hardening/docs + +- Ensure `GpuAllocation` is `Send + Sync`. +- Add handle guards: wrap raw buffer/image handles in `DeviceObject` right after + creation and establish allocation ownership before binding, so allocation and + bind failures never leak (see implementation notes). +- Add a validation test for `CpuToGpu` mapping that asserts the allocated memory + type is `HOST_VISIBLE | HOST_COHERENT` and round-trips a CPU write through + `map_mut`, plus dedicated buffers/images. +- Update `crates/renderer_device/AGENTS.md`, + `crates/renderer_resources/AGENTS.md`, `crates/renderer/AGENTS.md`, and the + root workspace guide to say VMA/`vk-mem`. +- Optionally rename `alloc2` to `allocator`. + +Verify: full checklist below. + +## Implementation notes + +- Allocator creation: `unsafe { vk_mem::Allocator::new(create_info) }?`; set + `preferred_large_heap_block_size` to ~64 MiB (VMA's default is 256 MiB). + `vulkan_api_version` must not exceed the instance's requested version or the + selected physical device's supported version: record the instance request + (add `pub api_version: u32` to `InstanceInner` when building `ApplicationInfo` + in `instance.rs:126-135`) and clamp it with the physical device's + `limits.api_version` at device selection, passing the effective minimum (a + safe floor is 1.1 when both sides support it, else 1.0). Dedicated-allocation + structs require API ≥ 1.1; on Vulkan 1.0 devices require + `VK_KHR_dedicated_allocation` (+ `VK_KHR_get_memory_requirements2`) at device + creation or skip `DEDICATED_MEMORY` there. +- Do not enable `EXTERNALLY_SYNCHRONIZED`; keep the `Mutex`. +- Allocation flags: `MAPPED` for `CpuToGpu`/`GpuToCpu`, plus `DEDICATED_MEMORY` + for `AllocationStrategy::Dedicated`. +- **Coherence policy (required)**: VMA's `CpuToGpu`/`GpuToCpu` usages guarantee + host-visible memory, not host-coherent memory, but the current mapping API + (`mapped_slice`/`mapped_slice_mut`) has no flush/invalidate step. Require + coherent memory: compute the memory-type bitmask of types with + `HOST_VISIBLE | HOST_COHERENT` from the physical device's memory properties + and set `AllocationCreateInfo.memory_type_bits` for CPU-mapped allocations; + fail the allocation if no such type exists (stricter than gpu-allocator, + which silently falls back to non-coherent types). If a non-coherent-only + device must be supported later, switch to explicit + `Allocator::flush_allocation`/`invalidate_allocation` around map windows and + change the API contract accordingly — do not ship a silent fallback. +- **Handle guards before allocation**: the buffer/image flows create raw Vulkan + handles before allocating (`buffers.rs:108-130`, `images.rs:191-223`); wrap + each raw handle in `DeviceObject` immediately after creation and establish + `Allocation` ownership (`DeviceObject`) before binding, so + every failure path — allocation failure or bind failure — releases the handle + and the allocation in the correct order (destroy handle first, then + `free_memory`). No raw handle may escape a fallible span unowned. +- Free with `free_memory` after the buffer/image handle is destroyed; do not + also call `destroy_buffer/destroy_image` for the same allocation. + +## Validation checklist + +- `cargo check -p renderer_device`, `-p renderer_resources`, `-p renderer`, + `-p renderer_rendergraph`, then `cargo check --workspace`. +- `cargo test -p renderer_device` and `cargo test -p renderer_device --features + testing` (real Vulkan/validation; skips if unavailable). +- `cargo test -p renderer_resources` and `cargo test -p renderer_rendergraph` + (both exercise `TestVulkan` through their dev-dependency features). +- `cargo tree -d | rg '^ash'` should show one ash. The lockfile has two + `gpu-allocator 0.28.0` packages (registry + git), so `cargo tree -i + gpu-allocator` is ambiguous; use source-qualified package IDs: + `cargo tree -i 'registry+https://github.com/rust-lang/crates.io-index#gpu-allocator@0.28.0'` + should list only `wgpu-hal` via the Bevy dev-dependency, and + `cargo tree -i 'https://github.com/janis-bhm/gpu-allocator#gpu-allocator@0.28.0'` + should be empty after phase 3. + `rg -n 'gpu_allocator|gpu-allocator' --glob '!Cargo.lock'` should return + nothing. +- Re-run the baseline Vulkan smoke tests and, if a display is available, + `cargo build --examples`. + +## Risks & gotchas + +- **FFI/lifetimes**: `vk_mem::Allocator` is a raw-pointer owner; it must outlive + every `Allocation`. The existing `DeviceInner` field order (`alloc2` before + `_drop`) is correct; do not move the allocator after `_drop`. `GpuAllocation` + must not be `Copy` and must free exactly once through + `DeviceObject`/`ExternallyManagedObject`. +- **Thread safety**: `AllocationInfo.mapped_data` is `*mut c_void`, which is not + `Send/Sync`; storing it directly breaks `Arc` thread-safety. Use a + private `SendSyncPtr` or unsafe `Send/Sync` wrapper and add a compile-time + `assert_send_sync::()`/`Image` test. +- **Mapping differs**: `gpu-allocator` mapped host-visible blocks + opportunistically; VMA maps only with `MAPPED`. `CpuToGpu`/`GpuToCpu` keep + working; `GpuOnly` mapping on integrated GPUs may now return `None`. + **Coherence**: VMA only guarantees host-visible (not host-coherent) memory + for CPU usages; this plan requires coherent types via `memory_type_bits` + (see implementation notes) so the existing flush-free `map`/`map_mut` usage + stays correct. Non-coherent support would need explicit + `flush_allocation`/`invalidate_allocation`; current code does not + flush/invalidate either. +- **Dedicated allocation**: use `allocate_memory_for_buffer/image` with + `DEDICATED_MEMORY`; generic `allocate_memory` would create a separate block + without dedicated buffer/image info. `VkMemoryDedicatedAllocateInfo` is only + emitted when the runtime API version is ≥ 1.1 (or the KHR dedicated + extensions are enabled); pass the clamped effective version per the + implementation notes and require the extensions on Vulkan 1.0 devices. +- **OOM handling**: `gpu_allocator::AllocationError` variants disappear; VMA + returns `vk::Result` (`ERROR_OUT_OF_DEVICE_MEMORY`, + `ERROR_FEATURE_NOT_PRESENT`, etc.), already handled by `Error::Result`. No + call site pattern-matches the old variant. +- **Build environment**: `vk-mem 0.5.0` compiles VMA C++ (`wrapper.cpp`, C++17, + links stdc++ on Linux); ensure CI has a C++ compiler. It pins ash 0.38; an ash + upgrade requires a matching `vk-mem` release/fork. The crate is less actively + maintained than `gpu-allocator`. +- **Lost features**: no `vk_mem` allocation naming setter + (`vmaSetAllocationName`) and no `build_stats_string` in 0.5.0; VMA + stats/heap budgets remain available. Defragmentation is intentionally out of + scope; if enabled later, cached `memory/offset/mapped_data` in the wrapper + must be re-queried. +- **Pre-existing hazard**: `Image::new_with_allocation` still creates the raw + image before validation (`images.rs:235-260`); the handle-guard requirement + (implementation notes) covers this by owning the raw image in `DeviceObject` + from creation onward.