From a70b77453523adddd5e229f97fd85323b543a3ff Mon Sep 17 00:00:00 2001 From: Raphael Amorim Date: Fri, 1 May 2026 21:13:16 +0200 Subject: [PATCH] fix losing device close #1568 --- sugarloaf/examples/draw_order.rs | 19 +-- sugarloaf/examples/layer.rs | 9 +- sugarloaf/examples/line_height.rs | 11 +- sugarloaf/examples/text.rs | 9 +- sugarloaf/src/context/vulkan.rs | 270 +++++++++++++++++++++--------- sugarloaf/src/grid/vulkan.rs | 158 ++++++++--------- sugarloaf/src/renderer/vulkan.rs | 135 +++++++-------- sugarloaf/src/text.rs | 70 ++++---- 8 files changed, 386 insertions(+), 295 deletions(-) diff --git a/sugarloaf/examples/draw_order.rs b/sugarloaf/examples/draw_order.rs index ead809fb..d7b66bb9 100644 --- a/sugarloaf/examples/draw_order.rs +++ b/sugarloaf/examples/draw_order.rs @@ -17,7 +17,9 @@ use rio_window::{ dpi::LogicalSize, event::WindowEvent, event_loop::EventLoop, window::WindowAttributes, }; use std::error::Error; -use sugarloaf::{layout::RootStyle, Sugarloaf, SugarloafWindow, SugarloafWindowSize}; +use sugarloaf::{ + layout::RootStyle, Color, Sugarloaf, SugarloafWindow, SugarloafWindowSize, +}; fn main() { let width = 400.0; @@ -84,15 +86,12 @@ impl ApplicationHandler for Application { ) .expect("Sugarloaf instance should be created"); - sugarloaf.set_background_color(Some( - wgpu::Color { - r: 0.1, - g: 0.1, - b: 0.1, - a: 1.0, - } - .into(), - )); + sugarloaf.set_background_color(Some(Color { + r: 0.1, + g: 0.1, + b: 0.1, + a: 1.0, + })); window.request_redraw(); self.sugarloaf = Some(sugarloaf); diff --git a/sugarloaf/examples/layer.rs b/sugarloaf/examples/layer.rs index 3be443e5..e74933cd 100644 --- a/sugarloaf/examples/layer.rs +++ b/sugarloaf/examples/layer.rs @@ -8,7 +8,7 @@ use rio_window::{ }; use std::error::Error; use sugarloaf::{ - layout::RootStyle, SpanStyle, Sugarloaf, SugarloafWindow, SugarloafWindowSize, + layout::RootStyle, Color, SpanStyle, Sugarloaf, SugarloafWindow, SugarloafWindowSize, }; fn main() { @@ -76,7 +76,12 @@ impl ApplicationHandler for Application { ) .expect("Sugarloaf instance should be created"); - sugarloaf.set_background_color(Some(wgpu::Color::RED.into())); + sugarloaf.set_background_color(Some(Color { + r: 1.0, + g: 0.0, + b: 0.0, + a: 1.0, + })); window.request_redraw(); // we will add three layers diff --git a/sugarloaf/examples/line_height.rs b/sugarloaf/examples/line_height.rs index 71b0ba51..656eb47c 100644 --- a/sugarloaf/examples/line_height.rs +++ b/sugarloaf/examples/line_height.rs @@ -11,8 +11,8 @@ use rio_window::{ }; use std::error::Error; use sugarloaf::{ - layout::RootStyle, CursorKind, SpanStyle, SugarCursor, Sugarloaf, SugarloafWindow, - SugarloafWindowSize, + layout::RootStyle, Color, CursorKind, SpanStyle, SugarCursor, Sugarloaf, + SugarloafWindow, SugarloafWindowSize, }; fn main() { @@ -83,7 +83,12 @@ impl ApplicationHandler for Application { ) .expect("Sugarloaf instance should be created"); - sugarloaf.set_background_color(Some(wgpu::Color::BLUE.into())); + sugarloaf.set_background_color(Some(Color { + r: 0.0, + g: 0.0, + b: 1.0, + a: 1.0, + })); window.request_redraw(); self.sugarloaf = Some(sugarloaf); diff --git a/sugarloaf/examples/text.rs b/sugarloaf/examples/text.rs index ba405bc5..9f2cf38f 100644 --- a/sugarloaf/examples/text.rs +++ b/sugarloaf/examples/text.rs @@ -8,7 +8,7 @@ use rio_window::{ }; use std::error::Error; use sugarloaf::{ - layout::RootStyle, SpanStyle, SpanStyleDecoration, Sugarloaf, SugarloafWindow, + layout::RootStyle, Color, SpanStyle, SpanStyleDecoration, Sugarloaf, SugarloafWindow, SugarloafWindowSize, UnderlineInfo, UnderlineShape, }; @@ -77,7 +77,12 @@ impl ApplicationHandler for Application { ) .expect("Sugarloaf instance should be created"); - sugarloaf.set_background_color(Some(wgpu::Color::RED.into())); + sugarloaf.set_background_color(Some(Color { + r: 1.0, + g: 0.0, + b: 0.0, + a: 1.0, + })); window.request_redraw(); self.sugarloaf = Some(sugarloaf); diff --git a/sugarloaf/src/context/vulkan.rs b/sugarloaf/src/context/vulkan.rs index 5f671357..6d94eeea 100644 --- a/sugarloaf/src/context/vulkan.rs +++ b/sugarloaf/src/context/vulkan.rs @@ -23,6 +23,72 @@ use raw_window_handle::{ HasDisplayHandle, HasWindowHandle, RawDisplayHandle, RawWindowHandle, }; use std::ffi::{c_char, CStr}; +use std::sync::Arc; + +/// Reference-counted owner of the raw Vulkan handles whose destruction +/// must be sequenced last. Held as `Arc` by every struct +/// (VulkanContext, VulkanGridRenderer, VulkanRenderer, VulkanBuffer, +/// VulkanImage, VulkanImageTexture, the text overlay's Vulkan state) +/// that would otherwise call into the device from its own `Drop`. +/// +/// `vkDestroyDevice` runs only when the last `Arc` clone is dropped, +/// so per-resource Drop order across `Sugarloaf` and `Screen.grids` +/// stops being load-bearing — the field-declaration trick that worked +/// for `VulkanRenderer`-inside-`Sugarloaf` (single parent) cannot +/// extend to `VulkanGridRenderer`-inside-`Screen.grids` (a sibling +/// of the parent that owns the device), which is the bug behind +/// raphamorim/rio#1568. +/// +/// Mirrors `wgpu_hal::vulkan::DeviceShared`. `raw` is named for the +/// underlying `ash::Device` to match wgpu-hal's convention; `Deref` +/// dispatches `shared.method(...)` calls straight to the device's +/// dispatch table, so consumers don't have to write `shared.raw.method()`. +pub struct VkShared { + // Declaration order = drop order. Vulkan rules: + // * `vkDestroyDevice` requires the parent `Instance` to still be + // alive (we look up the destroy entry point through it). + // * `vkDestroyInstance` requires the loaded `libvulkan` symbols + // (the `Entry`) to still be loaded. + pub raw: Device, + pub instance: Instance, + pub physical_device: vk::PhysicalDevice, + _entry: Entry, +} + +// `ash::Device` / `ash::Instance` / `ash::Entry` are dispatch tables +// with no interior mutability; the underlying Vulkan handles are +// thread-safe per spec (external synchronisation is per-object, not +// per-device). `vk::PhysicalDevice` is a plain handle. So `VkShared` +// is safe to share across threads via `Arc`. +unsafe impl Send for VkShared {} +unsafe impl Sync for VkShared {} + +impl Drop for VkShared { + fn drop(&mut self) { + unsafe { + // Defensive: the last clone of `VkShared` should normally + // be `VulkanContext`, which already idled in its own + // `Drop`. If a leaf resource (buffer, image, grid + // renderer) outlives the context — possible because the + // grid renderers live in `Screen.grids` while + // `VulkanContext` lives in `Screen.sugarloaf` — this is + // the only `device_wait_idle` we get. Cheap on an idle + // queue. + let _ = self.raw.device_wait_idle(); + self.raw.destroy_device(None); + self.instance.destroy_instance(None); + // `_entry` drops here, unloading `libvulkan`. + } + } +} + +impl std::ops::Deref for VkShared { + type Target = Device; + #[inline] + fn deref(&self) -> &Device { + &self.raw + } +} /// How many frames the CPU is allowed to pipeline ahead of the GPU. /// Three matches the Metal backend (`MetalLayer::set_maximum_drawable_count(3)` @@ -75,8 +141,17 @@ pub struct VulkanContext { // pool, pipeline creation) don't have to re-probe the family. #[allow(dead_code)] queue_family_index: u32, - device: Device, - physical_device: vk::PhysicalDevice, + + /// Reference-counted owner of `device`, `instance`, `physical_device`, + /// and the loader (`Entry`). Cloned into every per-resource struct + /// (`VulkanBuffer`, `VulkanImage`, `VulkanGridRenderer`, + /// `VulkanRenderer`, `VulkanImageTexture`, the text overlay's + /// Vulkan state) so `vkDestroyDevice` runs only after the last + /// dependent resource is dropped. Replaces the previous bare + /// `device.clone()` cloning, which was unsafe across struct + /// boundaries (raphamorim/rio#1568). `Deref` to `ash::Device` + /// keeps call sites unchanged: `self.shared.cmd_bind_pipeline(...)`. + shared: Arc, /// Pipeline cache shared by every `create_graphics_pipelines` /// call. Loaded from `~/.cache/rio/sugarloaf-vulkan.cache` (best @@ -93,8 +168,6 @@ pub struct VulkanContext { /// `instance` (declaration order) so the messenger handle is /// destroyed while the instance is still alive. _debug_messenger: Option, - instance: Instance, - _entry: Entry, } /// Owns one `vk::DebugUtilsMessengerEXT` and its loader. Destroyed @@ -202,6 +275,13 @@ impl VulkanContext { ); log_memory_heap_choice(&instance, physical_device); + let shared = Arc::new(VkShared { + raw: device, + instance, + physical_device, + _entry: entry, + }); + VulkanContext { size, scale, @@ -219,14 +299,11 @@ impl VulkanContext { swapchain_loader, queue, queue_family_index, - device, - physical_device, + shared, pipeline_cache, surface, surface_loader, _debug_messenger, - instance, - _entry: entry, } } @@ -255,21 +332,21 @@ impl VulkanContext { // This is a resize, not a per-frame operation, so the stall is // acceptable (wgpu does the same thing). unsafe { - let _ = self.device.device_wait_idle(); + let _ = self.shared.device_wait_idle(); } for &view in &self.swapchain_views { - unsafe { self.device.destroy_image_view(view, None) }; + unsafe { self.shared.destroy_image_view(view, None) }; } self.swapchain_views.clear(); self.swapchain_images.clear(); let old = self.swapchain; let (swapchain, format, color_space, extent, images, views) = create_swapchain( - &self.device, + &self.shared.raw, &self.surface_loader, &self.swapchain_loader, - self.physical_device, + self.shared.physical_device, self.surface, width, height, @@ -295,7 +372,7 @@ impl VulkanContext { let sync = &self.frames[slot]; unsafe { - self.device + self.shared .wait_for_fences(&[sync.in_flight], true, u64::MAX) .expect("wait_for_fences"); } @@ -326,13 +403,13 @@ impl VulkanContext { // before acquire_next_image would leave us deadlocked if the // acquire returned OUT_OF_DATE and we bailed out. unsafe { - self.device + self.shared .reset_fences(&[sync.in_flight]) .expect("reset_fences"); - self.device + self.shared .reset_command_pool(sync.cmd_pool, vk::CommandPoolResetFlags::empty()) .expect("reset_command_pool"); - self.device + self.shared .begin_command_buffer( sync.cmd_buffer, &vk::CommandBufferBeginInfo::default() @@ -356,7 +433,7 @@ impl VulkanContext { pub fn present_frame(&mut self, frame: VulkanFrame) { let sync = &self.frames[frame.slot]; unsafe { - self.device + self.shared .end_command_buffer(sync.cmd_buffer) .expect("end_command_buffer"); @@ -369,7 +446,7 @@ impl VulkanContext { .wait_dst_stage_mask(&wait_stages) .command_buffers(&cmd_buffers) .signal_semaphores(&signal_semaphores); - self.device + self.shared .queue_submit(self.queue, &[submit], sync.in_flight) .expect("queue_submit"); @@ -401,7 +478,22 @@ impl VulkanContext { /// Expose the underlying device so the renderer can record commands. #[inline] pub fn device(&self) -> &Device { - &self.device + &self.shared.raw + } + + /// Reference-counted handle to the device + instance + entry. Each + /// per-resource type (`VulkanBuffer`, `VulkanImage`, + /// `VulkanGridRenderer`, `VulkanRenderer`, `VulkanImageTexture`, + /// `TextVulkanState`) clones this and stores it directly so its + /// own `Drop` can call `destroy_*` on a device that is guaranteed + /// to still be alive — `vkDestroyDevice` runs only when the last + /// `Arc` is dropped. The previous design (each leaf cloning the + /// raw `ash::Device` dispatch table) crashed when the parent + /// `VulkanContext` happened to drop first; see + /// raphamorim/rio#1568. + #[inline] + pub fn shared(&self) -> &Arc { + &self.shared } /// Color attachment format the swapchain was created with. Real @@ -421,12 +513,12 @@ impl VulkanContext { /// `resize` which only has `&mut self`). #[inline] pub fn instance(&self) -> &Instance { - &self.instance + &self.shared.instance } #[inline] pub fn physical_device(&self) -> vk::PhysicalDevice { - self.physical_device + self.shared.physical_device } /// Pipeline cache shared by every renderer's @@ -453,7 +545,7 @@ impl VulkanContext { .queue_family_index(self.queue_family_index) .flags(vk::CommandPoolCreateFlags::TRANSIENT); let pool = self - .device + .shared .create_command_pool(&pool_info, None) .expect("create_command_pool(oneshot)"); @@ -462,37 +554,37 @@ impl VulkanContext { .level(vk::CommandBufferLevel::PRIMARY) .command_buffer_count(1); let cmd = self - .device + .shared .allocate_command_buffers(&alloc) .expect("allocate_command_buffers(oneshot)")[0]; let begin = vk::CommandBufferBeginInfo::default() .flags(vk::CommandBufferUsageFlags::ONE_TIME_SUBMIT); - self.device + self.shared .begin_command_buffer(cmd, &begin) .expect("begin_command_buffer(oneshot)"); record(cmd); - self.device + self.shared .end_command_buffer(cmd) .expect("end_command_buffer(oneshot)"); let fence = self - .device + .shared .create_fence(&vk::FenceCreateInfo::default(), None) .expect("create_fence(oneshot)"); let cmds = [cmd]; let submit = vk::SubmitInfo::default().command_buffers(&cmds); - self.device + self.shared .queue_submit(self.queue, &[submit], fence) .expect("queue_submit(oneshot)"); - self.device + self.shared .wait_for_fences(&[fence], true, u64::MAX) .expect("wait_for_fences(oneshot)"); - self.device.destroy_fence(fence, None); - self.device.destroy_command_pool(pool, None); + self.shared.destroy_fence(fence, None); + self.shared.destroy_command_pool(pool, None); } } @@ -533,15 +625,15 @@ impl VulkanContext { .usage(usage) .sharing_mode(vk::SharingMode::EXCLUSIVE); let buffer = unsafe { - self.device + self.shared .create_buffer(&buffer_info, None) .expect("create_buffer") }; - let req = unsafe { self.device.get_buffer_memory_requirements(buffer) }; + let req = unsafe { self.shared.get_buffer_memory_requirements(buffer) }; let mem_type = find_memory_type( - &self.instance, - self.physical_device, + &self.shared.instance, + self.shared.physical_device, req.memory_type_bits, vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT, @@ -552,12 +644,12 @@ impl VulkanContext { .allocation_size(req.size) .memory_type_index(mem_type); let memory = unsafe { - self.device + self.shared .allocate_memory(&alloc_info, None) .expect("allocate_memory") }; unsafe { - self.device + self.shared .bind_buffer_memory(buffer, memory, 0) .expect("bind_buffer_memory"); } @@ -565,13 +657,13 @@ impl VulkanContext { // HOST_COHERENT means we never have to flush; mapping stays // valid until `vkUnmapMemory`, which we only do at Drop. let mapped = unsafe { - self.device + self.shared .map_memory(memory, 0, vk::WHOLE_SIZE, vk::MemoryMapFlags::empty()) .expect("map_memory") as *mut u8 }; VulkanBuffer { - device: self.device.clone(), + shared: self.shared.clone(), buffer, memory, mapped, @@ -584,7 +676,11 @@ impl VulkanContext { /// (raw pointer; caller owns the layout / bounds checks). The buffer /// destroys itself + frees its backing memory on drop. pub struct VulkanBuffer { - device: Device, + /// Shared device handle. The Arc keeps the underlying + /// `vkDestroyDevice` from running until *every* `VulkanBuffer` + /// (and other resource) is dropped, regardless of the order in + /// which their parents drop. See `VkShared`. + shared: Arc, buffer: vk::Buffer, memory: vk::DeviceMemory, mapped: *mut u8, @@ -592,9 +688,10 @@ pub struct VulkanBuffer { } // `vk::Buffer`, `vk::DeviceMemory`, and the mapped pointer are all -// values the driver hands out per-allocation; ash::Device is itself -// Send+Sync. Buffers are never shared across threads in sugarloaf, but -// `Send` lets them sit inside `Sugarloaf` (which is not `!Send`). +// values the driver hands out per-allocation; `Arc` is +// Send+Sync (see the `unsafe impl` on `VkShared`). Buffers are never +// shared across threads in sugarloaf, but `Send` lets them sit inside +// `Sugarloaf` (which is not `!Send`). unsafe impl Send for VulkanBuffer {} unsafe impl Sync for VulkanBuffer {} @@ -626,23 +723,21 @@ impl Drop for VulkanBuffer { // `vkFreeMemory` on a non-mapped allocation is safe; we // unmap first only because some validation layers warn // about freeing memory that's still mapped. - self.device.unmap_memory(self.memory); - self.device.destroy_buffer(self.buffer, None); - self.device.free_memory(self.memory, None); + self.shared.unmap_memory(self.memory); + self.shared.destroy_buffer(self.buffer, None); + self.shared.free_memory(self.memory, None); } } } /// Free-function variant of [`VulkanContext::allocate_host_visible_buffer`] -/// for callers that hold cached `(device, instance, physical_device)` -/// rather than a live `&VulkanContext` borrow. The grid / text / -/// image renderers stash those handles at construction time so they -/// can allocate from inside their own `resize` paths (which only have -/// `&mut self`, not the parent context). +/// for callers that hold a cached `Arc` rather than a live +/// `&VulkanContext` borrow. The grid / text / image renderers stash +/// the shared handle at construction time so they can allocate from +/// inside their own `resize` paths (which only have `&mut self`, not +/// the parent context). pub fn allocate_host_visible_buffer_raw( - device: &Device, - instance: &Instance, - physical_device: vk::PhysicalDevice, + shared: &Arc, size: u64, usage: vk::BufferUsageFlags, ) -> VulkanBuffer { @@ -652,14 +747,14 @@ pub fn allocate_host_visible_buffer_raw( .usage(usage) .sharing_mode(vk::SharingMode::EXCLUSIVE); let buffer = unsafe { - device + shared .create_buffer(&buffer_info, None) .expect("create_buffer") }; - let req = unsafe { device.get_buffer_memory_requirements(buffer) }; + let req = unsafe { shared.get_buffer_memory_requirements(buffer) }; let mem_type = find_memory_type( - instance, - physical_device, + &shared.instance, + shared.physical_device, req.memory_type_bits, vk::MemoryPropertyFlags::HOST_VISIBLE | vk::MemoryPropertyFlags::HOST_COHERENT, ) @@ -668,22 +763,22 @@ pub fn allocate_host_visible_buffer_raw( .allocation_size(req.size) .memory_type_index(mem_type); let memory = unsafe { - device + shared .allocate_memory(&alloc_info, None) .expect("allocate_memory") }; unsafe { - device + shared .bind_buffer_memory(buffer, memory, 0) .expect("bind_buffer_memory"); } let mapped = unsafe { - device + shared .map_memory(memory, 0, vk::WHOLE_SIZE, vk::MemoryMapFlags::empty()) .expect("map_memory") as *mut u8 }; VulkanBuffer { - device: device.clone(), + shared: shared.clone(), buffer, memory, mapped, @@ -800,15 +895,15 @@ impl VulkanContext { .sharing_mode(vk::SharingMode::EXCLUSIVE) .initial_layout(vk::ImageLayout::UNDEFINED); let image = unsafe { - self.device + self.shared .create_image(&image_info, None) .expect("create_image") }; - let req = unsafe { self.device.get_image_memory_requirements(image) }; + let req = unsafe { self.shared.get_image_memory_requirements(image) }; let mem_type = find_memory_type( - &self.instance, - self.physical_device, + &self.shared.instance, + self.shared.physical_device, req.memory_type_bits, vk::MemoryPropertyFlags::DEVICE_LOCAL, ) @@ -818,12 +913,12 @@ impl VulkanContext { .allocation_size(req.size) .memory_type_index(mem_type); let memory = unsafe { - self.device + self.shared .allocate_memory(&alloc_info, None) .expect("allocate_memory(image)") }; unsafe { - self.device + self.shared .bind_image_memory(image, memory, 0) .expect("bind_image_memory"); } @@ -842,13 +937,13 @@ impl VulkanContext { .layer_count(1), ); let view = unsafe { - self.device + self.shared .create_image_view(&view_info, None) .expect("create_image_view") }; VulkanImage { - device: self.device.clone(), + shared: self.shared.clone(), image, view, memory, @@ -864,7 +959,8 @@ impl VulkanContext { /// — the first command that uses it must barrier-transition to a /// usable layout (`TRANSFER_DST_OPTIMAL` for the initial upload). pub struct VulkanImage { - device: Device, + /// Shared device handle. See `VkShared`. + shared: Arc, image: vk::Image, view: vk::ImageView, memory: vk::DeviceMemory, @@ -891,9 +987,9 @@ impl VulkanImage { impl Drop for VulkanImage { fn drop(&mut self) { unsafe { - self.device.destroy_image_view(self.view, None); - self.device.destroy_image(self.image, None); - self.device.free_memory(self.memory, None); + self.shared.destroy_image_view(self.view, None); + self.shared.destroy_image(self.image, None); + self.shared.free_memory(self.memory, None); } } } @@ -901,30 +997,38 @@ impl Drop for VulkanImage { impl Drop for VulkanContext { fn drop(&mut self) { unsafe { - let _ = self.device.device_wait_idle(); + // Idle the queue before tearing down anything that might + // still be in flight (swapchain image views, sync prims). + // `vkDestroyDevice` itself happens later, when the last + // `Arc` clone drops — see `VkShared::drop`. + let _ = self.shared.device_wait_idle(); // Best-effort: serialize the pipeline cache to disk // before destroying it. Failure (no XDG_CACHE_HOME, no // write perms, etc) is logged but not fatal. - save_pipeline_cache(&self.device, self.pipeline_cache); - self.device + save_pipeline_cache(&self.shared.raw, self.pipeline_cache); + self.shared .destroy_pipeline_cache(self.pipeline_cache, None); for frame in &self.frames { - self.device.destroy_semaphore(frame.image_available, None); - self.device.destroy_semaphore(frame.render_finished, None); - self.device.destroy_fence(frame.in_flight, None); - self.device.destroy_command_pool(frame.cmd_pool, None); + self.shared.destroy_semaphore(frame.image_available, None); + self.shared.destroy_semaphore(frame.render_finished, None); + self.shared.destroy_fence(frame.in_flight, None); + self.shared.destroy_command_pool(frame.cmd_pool, None); } for &view in &self.swapchain_views { - self.device.destroy_image_view(view, None); + self.shared.destroy_image_view(view, None); } self.swapchain_loader .destroy_swapchain(self.swapchain, None); - self.device.destroy_device(None); self.surface_loader.destroy_surface(self.surface, None); - self.instance.destroy_instance(None); + // `_debug_messenger` (declared after this Drop's body + // unwind path completes) drops before `shared` (declared + // before it), so the messenger handle is destroyed while + // the instance is still alive. `vkDestroyDevice` and + // `vkDestroyInstance` run in `VkShared::drop` once the + // last `Arc` clone is gone. } } } diff --git a/sugarloaf/src/grid/vulkan.rs b/sugarloaf/src/grid/vulkan.rs index 5bd7077a..319106f8 100644 --- a/sugarloaf/src/grid/vulkan.rs +++ b/sugarloaf/src/grid/vulkan.rs @@ -23,11 +23,12 @@ use ash::vk; use rustc_hash::FxHashMap; +use std::sync::Arc; use super::atlas::{AtlasSlot, GlyphKey, RasterizedGlyph}; use super::cell::{CellBg, CellText, GridUniforms}; use crate::context::vulkan::{ - allocate_host_visible_buffer_raw, VulkanBuffer, VulkanContext, VulkanImage, + allocate_host_visible_buffer_raw, VkShared, VulkanBuffer, VulkanContext, VulkanImage, FRAMES_IN_FLIGHT, }; use crate::renderer::image_cache::atlas::AtlasAllocator; @@ -127,16 +128,14 @@ impl VulkanGlyphAtlas { /// `VUID-vkCmdCopyBufferToImage-renderpass` forbids transfer /// commands inside one. No-op when there are no pending uploads. /// - /// We take `(device, instance, physical_device)` rather than - /// `&VulkanContext` so the text overlay path can call this - /// without holding an immutable borrow on the context - /// (`Sugarloaf::render_vulkan` keeps `ctx: &mut VulkanContext` - /// for the swapchain acquire/present cycle). + /// We take `&Arc` rather than `&VulkanContext` so the + /// text overlay path can call this without holding an immutable + /// borrow on the context (`Sugarloaf::render_vulkan` keeps + /// `ctx: &mut VulkanContext` for the swapchain acquire/present + /// cycle). pub fn flush_uploads( &mut self, - device: &ash::Device, - instance: &ash::Instance, - physical_device: vk::PhysicalDevice, + shared: &Arc, cmd: vk::CommandBuffer, slot: usize, ) { @@ -153,14 +152,11 @@ impl VulkanGlyphAtlas { // us from churning allocations during the first-frame burst. if total_bytes > self.staging_capacity[slot] { let new_cap = total_bytes.next_power_of_two().max(256 * 1024); - self.staging[slot] = - Some(crate::context::vulkan::allocate_host_visible_buffer_raw( - device, - instance, - physical_device, - new_cap as u64, - vk::BufferUsageFlags::TRANSFER_SRC, - )); + self.staging[slot] = Some(allocate_host_visible_buffer_raw( + shared, + new_cap as u64, + vk::BufferUsageFlags::TRANSFER_SRC, + )); self.staging_capacity[slot] = new_cap; } let staging = self.staging[slot].as_ref().unwrap(); @@ -185,7 +181,7 @@ impl VulkanGlyphAtlas { } } - upload_to_atlas(device, cmd, staging_handle, self, &copies); + upload_to_atlas(&shared.raw, cmd, staging_handle, self, &copies); } #[inline] @@ -250,12 +246,16 @@ impl VulkanGlyphAtlas { // ======================================================================= pub struct VulkanGridRenderer { - device: ash::Device, - /// Cached so `resize` (which only has `&mut self`) can allocate - /// new bg buffers via `allocate_host_visible_buffer_raw` without - /// needing a `&VulkanContext` borrow. - instance: ash::Instance, - physical_device: vk::PhysicalDevice, + /// Shared device handle. Cloned from `VulkanContext` at + /// construction so this renderer's `Drop` can call `destroy_*` on + /// pipelines/descriptor pools/etc. without depending on + /// `VulkanContext` still being alive — `vkDestroyDevice` runs + /// only when the last `Arc` clone (across all + /// renderers, buffers, images, atlases) is dropped. See + /// `VkShared`. Also lets `resize` (which only has `&mut self`) + /// allocate bg buffers via `allocate_host_visible_buffer_raw` + /// without needing a `&VulkanContext` borrow. + shared: Arc, cols: u32, rows: u32, @@ -302,9 +302,8 @@ pub struct VulkanGridRenderer { impl VulkanGridRenderer { pub fn new(ctx: &VulkanContext, cols: u32, rows: u32) -> Self { - let device = ctx.device().clone(); - let instance = ctx.instance().clone(); - let physical_device = ctx.physical_device(); + let shared = ctx.shared().clone(); + let device = &shared.raw; // ----- bg + uniforms ----- let bg_buffers = std::array::from_fn(|_| alloc_bg_buffer(ctx, cols, rows)); @@ -315,26 +314,26 @@ impl VulkanGridRenderer { ) }); - let bg_descriptor_set_layout = create_bg_descriptor_set_layout(&device); - let bg_descriptor_pool = create_bg_descriptor_pool(&device); + let bg_descriptor_set_layout = create_bg_descriptor_set_layout(device); + let bg_descriptor_pool = create_bg_descriptor_pool(device); let bg_descriptor_sets = allocate_descriptor_sets( - &device, + device, bg_descriptor_pool, bg_descriptor_set_layout, ); for slot in 0..FRAMES_IN_FLIGHT { update_bg_descriptor_set( - &device, + device, bg_descriptor_sets[slot], &uniform_buffers[slot], &bg_buffers[slot], ); } let bg_pipeline_layout = - create_pipeline_layout(&device, &[bg_descriptor_set_layout]); + create_pipeline_layout(device, &[bg_descriptor_set_layout]); let pipeline_cache = ctx.pipeline_cache(); let bg_pipeline = create_bg_pipeline( - &device, + device, pipeline_cache, bg_pipeline_layout, ctx.swapchain_format(), @@ -343,34 +342,34 @@ impl VulkanGridRenderer { // ----- text ----- let atlas_grayscale = VulkanGlyphAtlas::new_grayscale(ctx); let atlas_color = VulkanGlyphAtlas::new_color(ctx); - let sampler = create_sampler(&device); + let sampler = create_sampler(device); let text_uniform_descriptor_set_layout = - create_text_uniform_descriptor_set_layout(&device); + create_text_uniform_descriptor_set_layout(device); let text_atlas_descriptor_set_layout = - create_text_atlas_descriptor_set_layout(&device); + create_text_atlas_descriptor_set_layout(device); // One pool that holds (FRAMES_IN_FLIGHT uniform sets) + (1 atlas set). - let text_descriptor_pool = create_text_descriptor_pool(&device); + let text_descriptor_pool = create_text_descriptor_pool(device); let text_uniform_descriptor_sets = allocate_descriptor_sets( - &device, + device, text_descriptor_pool, text_uniform_descriptor_set_layout, ); for slot in 0..FRAMES_IN_FLIGHT { update_text_uniform_descriptor_set( - &device, + device, text_uniform_descriptor_sets[slot], &uniform_buffers[slot], ); } let text_atlas_descriptor_set = allocate_one_descriptor_set( - &device, + device, text_descriptor_pool, text_atlas_descriptor_set_layout, ); update_text_atlas_descriptor_set( - &device, + device, text_atlas_descriptor_set, &atlas_grayscale.image, &atlas_color.image, @@ -378,14 +377,14 @@ impl VulkanGridRenderer { ); let text_pipeline_layout = create_pipeline_layout( - &device, + device, &[ text_uniform_descriptor_set_layout, text_atlas_descriptor_set_layout, ], ); let text_pipeline = create_text_pipeline( - &device, + device, pipeline_cache, text_pipeline_layout, ctx.swapchain_format(), @@ -393,9 +392,7 @@ impl VulkanGridRenderer { let bg_len = (cols as usize) * (rows as usize); Self { - device, - instance, - physical_device, + shared, cols, rows, bg_buffers, @@ -442,7 +439,7 @@ impl VulkanGridRenderer { return; } unsafe { - let _ = self.device.device_wait_idle(); + let _ = self.shared.device_wait_idle(); } self.cols = cols; @@ -450,23 +447,20 @@ impl VulkanGridRenderer { let bg_len = (cols as usize) * (rows as usize); self.bg_cpu = vec![CellBg::TRANSPARENT; bg_len]; - // Reallocate bg buffers via the cached (instance, - // physical_device) pair and re-wire descriptor sets to the - // new buffer handles. + // Reallocate bg buffers via the cached `Arc` and + // re-wire descriptor sets to the new buffer handles. let bg_byte_size = (bg_len * std::mem::size_of::()) .max(std::mem::size_of::()) as u64; self.bg_buffers = std::array::from_fn(|_| { allocate_host_visible_buffer_raw( - &self.device, - &self.instance, - self.physical_device, + &self.shared, bg_byte_size, vk::BufferUsageFlags::STORAGE_BUFFER, ) }); for slot in 0..FRAMES_IN_FLIGHT { update_bg_descriptor_set( - &self.device, + &self.shared.raw, self.bg_descriptor_sets[slot], &self.uniform_buffers[slot], &self.bg_buffers[slot], @@ -645,12 +639,12 @@ impl VulkanGridRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.bg_pipeline, ); - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.bg_pipeline_layout, @@ -658,7 +652,7 @@ impl VulkanGridRenderer { &[self.bg_descriptor_sets[slot]], &[], ); - self.device.cmd_draw(cmd, 3, 1, 0, 0); + self.shared.cmd_draw(cmd, 3, 1, 0, 0); } } @@ -709,12 +703,12 @@ impl VulkanGridRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.text_pipeline, ); - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.text_pipeline_layout, @@ -726,9 +720,9 @@ impl VulkanGridRenderer { &[], ); let buf = self.fg_buffers[slot].as_ref().unwrap(); - self.device + self.shared .cmd_bind_vertex_buffers(cmd, 0, &[buf.handle()], &[0]); - self.device.cmd_draw(cmd, 4, instance_count, 0, 0); + self.shared.cmd_draw(cmd, 4, instance_count, 0, 0); } } @@ -741,20 +735,8 @@ impl VulkanGridRenderer { cmd: vk::CommandBuffer, slot: usize, ) { - self.atlas_grayscale.flush_uploads( - &self.device, - &self.instance, - self.physical_device, - cmd, - slot, - ); - self.atlas_color.flush_uploads( - &self.device, - &self.instance, - self.physical_device, - cmd, - slot, - ); + self.atlas_grayscale.flush_uploads(&self.shared, cmd, slot); + self.atlas_color.flush_uploads(&self.shared, cmd, slot); } } @@ -844,28 +826,32 @@ fn upload_to_atlas( impl Drop for VulkanGridRenderer { fn drop(&mut self) { unsafe { - let _ = self.device.device_wait_idle(); - self.device.destroy_pipeline(self.text_pipeline, None); - self.device + // Idle the queue before destroying pipelines / descriptor + // resources. The shared `Arc` keeps the + // underlying device alive across this whole Drop — + // `vkDestroyDevice` runs only after every clone is gone. + let _ = self.shared.device_wait_idle(); + self.shared.destroy_pipeline(self.text_pipeline, None); + self.shared .destroy_pipeline_layout(self.text_pipeline_layout, None); - self.device + self.shared .destroy_descriptor_pool(self.text_descriptor_pool, None); - self.device.destroy_descriptor_set_layout( + self.shared.destroy_descriptor_set_layout( self.text_atlas_descriptor_set_layout, None, ); - self.device.destroy_descriptor_set_layout( + self.shared.destroy_descriptor_set_layout( self.text_uniform_descriptor_set_layout, None, ); - self.device.destroy_sampler(self.sampler, None); + self.shared.destroy_sampler(self.sampler, None); - self.device.destroy_pipeline(self.bg_pipeline, None); - self.device + self.shared.destroy_pipeline(self.bg_pipeline, None); + self.shared .destroy_pipeline_layout(self.bg_pipeline_layout, None); - self.device + self.shared .destroy_descriptor_pool(self.bg_descriptor_pool, None); - self.device + self.shared .destroy_descriptor_set_layout(self.bg_descriptor_set_layout, None); // Buffers + atlas images drop themselves. } diff --git a/sugarloaf/src/renderer/vulkan.rs b/sugarloaf/src/renderer/vulkan.rs index 6ae7fd7d..c20d7912 100644 --- a/sugarloaf/src/renderer/vulkan.rs +++ b/sugarloaf/src/renderer/vulkan.rs @@ -20,9 +20,10 @@ //! flag enabled. use ash::vk; +use std::sync::Arc; use crate::context::vulkan::{ - allocate_host_visible_buffer_raw, VulkanBuffer, VulkanContext, VulkanFrame, + allocate_host_visible_buffer_raw, VkShared, VulkanBuffer, VulkanContext, VulkanFrame, VulkanImage, FRAMES_IN_FLIGHT, }; use crate::renderer::batch::{QuadInstance, Vertex}; @@ -62,16 +63,16 @@ pub struct VulkanRenderer { bootstrap_pipeline: vk::Pipeline, bootstrap_layout: vk::PipelineLayout, bootstrap_visible: bool, - /// Cloned from `VulkanContext::device()` so `Drop` can destroy our - /// pipelines even after the parent context has dropped its borrow. - /// `ash::Device` is `Clone` (just a wrapper around fn pointers + a - /// `vk::Device` handle); cloning does not create a new logical - /// device. - device: ash::Device, - /// Cached for buffer (re)allocation in `render_quads` (which only - /// has `&mut self`, no `&VulkanContext`). - instance: ash::Instance, - physical_device: vk::PhysicalDevice, + /// Shared device handle. The Arc keeps `vkDestroyDevice` from + /// running until every per-resource holder (buffers, images, + /// other renderers, atlases, the per-panel grid renderers in + /// `Screen.grids`) is dropped, so this renderer's `Drop` can + /// safely tear down pipelines regardless of struct field-order + /// elsewhere — fixes the cross-struct ordering trap behind + /// raphamorim/rio#1568. Also lets `render_quads` allocate from + /// `&mut self` without needing a live `&VulkanContext` borrow. + /// See `VkShared`. + shared: Arc, /// Set once at construction from the configured `Colorspace`. /// Mirrors `MetalRenderer::input_colorspace`. Value is `0 = sRGB`, /// `1 = DisplayP3`, `2 = Rec.2020`. @@ -134,9 +135,8 @@ pub struct VulkanRenderer { impl VulkanRenderer { pub fn new(ctx: &VulkanContext, colorspace: crate::sugarloaf::Colorspace) -> Self { - let device = ctx.device().clone(); - let instance = ctx.instance().clone(); - let physical_device = ctx.physical_device(); + let shared = ctx.shared().clone(); + let device = &shared.raw; let color_format = ctx.swapchain_format(); let input_colorspace = match colorspace { crate::sugarloaf::Colorspace::Srgb => 0u32, @@ -276,9 +276,7 @@ impl VulkanRenderer { bootstrap_pipeline, bootstrap_layout, bootstrap_visible, - device, - instance, - physical_device, + shared, input_colorspace, quad_pipeline, quad_pipeline_layout, @@ -347,9 +345,7 @@ impl VulkanRenderer { if vertex_count > self.geometry_vertex_capacity[slot] { let new_cap = vertex_count.next_power_of_two().max(256); self.geometry_vertex_buffers[slot] = Some(allocate_host_visible_buffer_raw( - &self.device, - &self.instance, - self.physical_device, + &self.shared, (new_cap * std::mem::size_of::()) as u64, vk::BufferUsageFlags::VERTEX_BUFFER, )); @@ -365,12 +361,12 @@ impl VulkanRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.geometry_pipeline, ); - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.quad_pipeline_layout, @@ -378,12 +374,12 @@ impl VulkanRenderer { &[self.quad_descriptor_sets[slot]], &[], ); - self.device + self.shared .cmd_bind_vertex_buffers(cmd, 0, &[vertex_buf.handle()], &[0]); // Caller-provided vertices are TRIANGLE_LIST — the emit // path tessellates polygons / arcs / lines into // triangles before pushing. - self.device.cmd_draw(cmd, vertex_count as u32, 1, 0, 0); + self.shared.cmd_draw(cmd, vertex_count as u32, 1, 0, 0); } } @@ -431,9 +427,7 @@ impl VulkanRenderer { if count > self.image_instance_capacity[slot] { let new_cap = count.next_power_of_two().max(16); self.image_instance_buffers[slot] = Some(allocate_host_visible_buffer_raw( - &self.device, - &self.instance, - self.physical_device, + &self.shared, (new_cap * stride) as u64, vk::BufferUsageFlags::VERTEX_BUFFER, )); @@ -449,13 +443,13 @@ impl VulkanRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.image_pipeline, ); // Set 0 (uniform) is constant across draws — bind once. - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.image_pipeline_layout, @@ -465,7 +459,7 @@ impl VulkanRenderer { ); for (i, (texture_set, _inst)) in draws.iter().enumerate() { // Set 1 (texture) changes per-draw — rebind. - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.image_pipeline_layout, @@ -474,14 +468,14 @@ impl VulkanRenderer { &[], ); let byte_offset = (i * stride) as u64; - self.device.cmd_bind_vertex_buffers( + self.shared.cmd_bind_vertex_buffers( cmd, 0, &[buf.handle()], &[byte_offset], ); let _ = needed_bytes; // shut up unused warning - self.device.cmd_draw(cmd, 4, 1, 0, 0); + self.shared.cmd_draw(cmd, 4, 1, 0, 0); } } } @@ -526,12 +520,12 @@ impl VulkanRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.image_pipeline, ); - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.image_pipeline_layout, @@ -542,13 +536,13 @@ impl VulkanRenderer { ], &[], ); - self.device.cmd_bind_vertex_buffers( + self.shared.cmd_bind_vertex_buffers( cmd, 0, &[self.image_bg_vertex_buffers[slot].handle()], &[0], ); - self.device.cmd_draw(cmd, 4, 1, 0, 0); + self.shared.cmd_draw(cmd, 4, 1, 0, 0); } } @@ -586,9 +580,7 @@ impl VulkanRenderer { if instance_count > self.quad_instance_capacity[slot] { let new_cap = instance_count.next_power_of_two().max(256); self.quad_instance_buffers[slot] = Some(allocate_host_visible_buffer_raw( - &self.device, - &self.instance, - self.physical_device, + &self.shared, (new_cap * std::mem::size_of::()) as u64, vk::BufferUsageFlags::VERTEX_BUFFER, )); @@ -604,12 +596,12 @@ impl VulkanRenderer { } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.quad_pipeline, ); - self.device.cmd_bind_descriptor_sets( + self.shared.cmd_bind_descriptor_sets( cmd, vk::PipelineBindPoint::GRAPHICS, self.quad_pipeline_layout, @@ -617,10 +609,10 @@ impl VulkanRenderer { &[self.quad_descriptor_sets[slot]], &[], ); - self.device + self.shared .cmd_bind_vertex_buffers(cmd, 0, &[instance_buf.handle()], &[0]); // 4 vertices per instance (TRIANGLE_STRIP quad). - self.device.cmd_draw(cmd, 4, instance_count as u32, 0, 0); + self.shared.cmd_draw(cmd, 4, instance_count as u32, 0, 0); } } @@ -642,13 +634,13 @@ impl VulkanRenderer { return; } unsafe { - self.device.cmd_bind_pipeline( + self.shared.cmd_bind_pipeline( cmd, vk::PipelineBindPoint::GRAPHICS, self.bootstrap_pipeline, ); let color: [f32; 4] = [1.0, 0.0, 1.0, 1.0]; - self.device.cmd_push_constants( + self.shared.cmd_push_constants( cmd, self.bootstrap_layout, vk::ShaderStageFlags::FRAGMENT, @@ -657,7 +649,7 @@ impl VulkanRenderer { ); // Triangle strip, 4 vertices — `clear.vert.glsl` // generates a centered rect in NDC. - self.device.cmd_draw(cmd, 4, 1, 0, 0); + self.shared.cmd_draw(cmd, 4, 1, 0, 0); } } } @@ -755,38 +747,39 @@ pub fn build_color_attachment( impl Drop for VulkanRenderer { fn drop(&mut self) { unsafe { - // Idle the device before tearing down pipelines. The parent - // `VulkanContext::Drop` also waits, but `Sugarloaf`'s field - // declaration order has us dropping first — and an outstanding - // submit using this pipeline would crash the driver if we - // destroyed it under the GPU's nose. - let _ = self.device.device_wait_idle(); - - self.device.destroy_pipeline(self.image_pipeline, None); - self.device + // Idle the queue before tearing down pipelines. The + // shared `Arc` keeps the underlying device + // alive across this Drop — `vkDestroyDevice` runs only + // when the last clone goes (in `VkShared::drop`), so + // ordering relative to `VulkanContext::drop` is no + // longer load-bearing. + let _ = self.shared.device_wait_idle(); + + self.shared.destroy_pipeline(self.image_pipeline, None); + self.shared .destroy_pipeline_layout(self.image_pipeline_layout, None); - self.device.destroy_sampler(self.image_sampler, None); - self.device + self.shared.destroy_sampler(self.image_sampler, None); + self.shared .destroy_descriptor_pool(self.image_uniform_descriptor_pool, None); - self.device.destroy_descriptor_set_layout( + self.shared.destroy_descriptor_set_layout( self.image_uniform_descriptor_set_layout, None, ); - self.device.destroy_descriptor_set_layout( + self.shared.destroy_descriptor_set_layout( self.image_texture_descriptor_set_layout, None, ); - self.device.destroy_pipeline(self.geometry_pipeline, None); - self.device.destroy_pipeline(self.quad_pipeline, None); - self.device + self.shared.destroy_pipeline(self.geometry_pipeline, None); + self.shared.destroy_pipeline(self.quad_pipeline, None); + self.shared .destroy_pipeline_layout(self.quad_pipeline_layout, None); - self.device + self.shared .destroy_descriptor_pool(self.quad_descriptor_pool, None); - self.device + self.shared .destroy_descriptor_set_layout(self.quad_descriptor_set_layout, None); - self.device.destroy_pipeline(self.bootstrap_pipeline, None); - self.device + self.shared.destroy_pipeline(self.bootstrap_pipeline, None); + self.shared .destroy_pipeline_layout(self.bootstrap_layout, None); // Buffers (uniform, instance) drop themselves via VulkanBuffer. } @@ -1345,7 +1338,8 @@ pub struct VulkanImageTexture { pub image: VulkanImage, descriptor_pool: vk::DescriptorPool, pub descriptor_set: vk::DescriptorSet, - device: ash::Device, + /// Shared device handle. See `VkShared`. + shared: Arc, } impl VulkanImageTexture { @@ -1365,7 +1359,8 @@ impl VulkanImageTexture { descriptor_set_layout: vk::DescriptorSetLayout, sampler: vk::Sampler, ) -> Self { - let device = ctx.device().clone(); + let shared = ctx.shared().clone(); + let device = &shared.raw; // `R8G8B8A8_SRGB` (vs `R8G8B8A8_UNORM`) tells the GPU to // sRGB-decode bytes at sample time. With bilinear filtering @@ -1506,7 +1501,7 @@ impl VulkanImageTexture { image, descriptor_pool, descriptor_set, - device, + shared, } } } @@ -1516,7 +1511,7 @@ impl Drop for VulkanImageTexture { unsafe { // Pool destruction frees the descriptor set; image drops // itself. - self.device + self.shared .destroy_descriptor_pool(self.descriptor_pool, None); } } diff --git a/sugarloaf/src/text.rs b/sugarloaf/src/text.rs index 37dca8a7..204d7557 100644 --- a/sugarloaf/src/text.rs +++ b/sugarloaf/src/text.rs @@ -155,9 +155,11 @@ struct TextCpuState { #[cfg(target_os = "linux")] struct TextVulkanState { - device: ash::Device, - instance: ash::Instance, - physical_device: ash::vk::PhysicalDevice, + /// Shared device handle. See `crate::context::vulkan::VkShared` + /// — the Arc keeps `vkDestroyDevice` from running until every + /// per-resource holder (this state, atlases, buffers, the + /// per-panel grid renderers in `Screen.grids`) is dropped. + shared: std::sync::Arc, /// Independent atlases owned by the UI text overlay — separate /// from each `VulkanGridRenderer`'s atlases so an overlay glyph /// doesn't have to compete for grid atlas space (and vice versa). @@ -1102,20 +1104,10 @@ impl Text { let Some(state) = self.vulkan.as_mut() else { return; }; - state.atlas_grayscale.flush_uploads( - &state.device, - &state.instance, - state.physical_device, - cmd, - slot, - ); - state.atlas_color.flush_uploads( - &state.device, - &state.instance, - state.physical_device, - cmd, - slot, - ); + state + .atlas_grayscale + .flush_uploads(&state.shared, cmd, slot); + state.atlas_color.flush_uploads(&state.shared, cmd, slot); } /// Record the UI text pass into `cmd`. Caller has already opened @@ -1150,9 +1142,7 @@ impl Text { let new_cap = instance_count.next_power_of_two().max(256); state.instance_buffers[slot] = Some(crate::context::vulkan::allocate_host_visible_buffer_raw( - &state.device, - &state.instance, - state.physical_device, + &state.shared, (new_cap * std::mem::size_of::()) as u64, ash::vk::BufferUsageFlags::VERTEX_BUFFER, )); @@ -1168,12 +1158,12 @@ impl Text { } unsafe { - state.device.cmd_bind_pipeline( + state.shared.cmd_bind_pipeline( cmd, ash::vk::PipelineBindPoint::GRAPHICS, state.pipeline, ); - state.device.cmd_bind_descriptor_sets( + state.shared.cmd_bind_descriptor_sets( cmd, ash::vk::PipelineBindPoint::GRAPHICS, state.pipeline_layout, @@ -1185,9 +1175,9 @@ impl Text { &[], ); state - .device + .shared .cmd_bind_vertex_buffers(cmd, 0, &[instance_buf.handle()], &[0]); - state.device.cmd_draw(cmd, 4, instance_count as u32, 0, 0); + state.shared.cmd_draw(cmd, 4, instance_count as u32, 0, 0); } } } @@ -1570,13 +1560,12 @@ fn build_text_vulkan_state( use crate::context::vulkan::FRAMES_IN_FLIGHT; use ash::vk; - let device = ctx.device().clone(); - let instance = ctx.instance().clone(); - let physical_device = ctx.physical_device(); + let shared = ctx.shared().clone(); + let device = &shared.raw; let atlas_grayscale = crate::grid::vulkan::VulkanGlyphAtlas::new_grayscale(ctx); let atlas_color = crate::grid::vulkan::VulkanGlyphAtlas::new_color(ctx); - let sampler = create_text_sampler(&device); + let sampler = create_text_sampler(device); let uniform_buffers = std::array::from_fn(|_| { ctx.allocate_host_visible_buffer(16, vk::BufferUsageFlags::UNIFORM_BUFFER) @@ -1707,16 +1696,14 @@ fn build_text_vulkan_state( .expect("create_pipeline_layout(ui_text)") }; let pipeline = build_ui_text_pipeline_vulkan( - &device, + device, ctx.pipeline_cache(), pipeline_layout, ctx.swapchain_format(), ); TextVulkanState { - device, - instance, - physical_device, + shared, atlas_grayscale, atlas_color, sampler, @@ -1900,17 +1887,22 @@ fn load_shader_module_vulkan( impl Drop for TextVulkanState { fn drop(&mut self) { unsafe { - let _ = self.device.device_wait_idle(); - self.device.destroy_pipeline(self.pipeline, None); - self.device + // Idle before tearing down. The shared `Arc` + // keeps the underlying device alive across this Drop — + // `vkDestroyDevice` runs only when the last clone goes, + // so ordering relative to other holders (`VulkanContext`, + // grid renderers, etc.) is no longer load-bearing. + let _ = self.shared.device_wait_idle(); + self.shared.destroy_pipeline(self.pipeline, None); + self.shared .destroy_pipeline_layout(self.pipeline_layout, None); - self.device + self.shared .destroy_descriptor_pool(self.descriptor_pool, None); - self.device + self.shared .destroy_descriptor_set_layout(self.atlas_descriptor_set_layout, None); - self.device + self.shared .destroy_descriptor_set_layout(self.uniform_descriptor_set_layout, None); - self.device.destroy_sampler(self.sampler, None); + self.shared.destroy_sampler(self.sampler, None); // Buffers + atlas images drop themselves. } } -- 2.51.2