Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/xrt/auxiliary/vk/vk_hud_blend.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ vk_hud_blend_init_ex(struct vk_hud_blend *blend,
};
VkDescriptorPoolCreateInfo dp_ci = {
.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO,
// Individual frees: vk_hud_blend_forget_image() returns a set (#1782).
.flags = VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT,
.maxSets = VK_HUD_BLEND_MAX_IMAGES,
.poolSizeCount = 1,
.pPoolSizes = &pool_size,
Expand Down Expand Up @@ -557,6 +559,27 @@ vk_hud_blend_draw_no_layout(struct vk_hud_blend *blend,
vk->vkCmdEndRenderPass(cmd);
}

void
vk_hud_blend_forget_image(struct vk_hud_blend *blend, struct vk_bundle *vk, VkImage hud_image)
{
if (!blend->initialized || hud_image == VK_NULL_HANDLE) {
return;
}

for (uint32_t i = 0; i < blend->image_count; i++) {
if (blend->cached_images[i].image != hud_image) {
continue;
}
vk->vkDestroyImageView(vk->device, blend->cached_images[i].view, NULL);
vk->vkFreeDescriptorSets(vk->device, blend->desc_pool, 1, &blend->cached_images[i].desc_set);

// Order does not matter: move the last entry into the hole.
blend->image_count--;
blend->cached_images[i] = blend->cached_images[blend->image_count];
return;
}
}

void
vk_hud_blend_fini(struct vk_hud_blend *blend, struct vk_bundle *vk)
{
Expand Down
24 changes: 22 additions & 2 deletions src/xrt/auxiliary/vk/vk_hud_blend.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
* the source image (HUD swapchain image), its dimensions, and the
* destination rect on the swapchain. Image views and descriptor sets
* are cached by VkImage so an OpenXR swapchain with rotating per-frame
* images doesn't pay setup cost beyond first sight of each image.
* images doesn't pay setup cost beyond first sight of each image. A
* destroyed source image must be removed with vk_hud_blend_forget_image().
*
* @author David Fattal
* @ingroup aux_vk
Expand All @@ -28,7 +29,8 @@ extern "C" {
#endif

// Each window-space layer is its own OpenXR swapchain, and swapchains rotate
// through ~3 images; the image cache is keyed by VkImage and never evicts. So
// through ~3 images; the image cache is keyed by VkImage and only evicts what
// vk_hud_blend_forget_image() is told about. So
// the bound must cover (max concurrent window-space layers) × (images/swapchain),
// not just the 1-2 layers an undersized cache happened to allow. 64 ≈ 21 layers ×
// 3 images — generous headroom for realistic per-depth-plane 2D UI (see issue #389
Expand Down Expand Up @@ -169,6 +171,24 @@ vk_hud_blend_draw_no_layout(struct vk_hud_blend *blend,
uint32_t dst_w,
uint32_t dst_h);

/*!
* Drop the cached view + descriptor set for a HUD source image (#1782).
*
* The cache is keyed by the raw VkImage handle, and drivers hand a destroyed
* image's handle value to the next image they create. An entry that outlives
* its image is then returned for an unrelated image, and the draw samples a
* view of freed memory (on NVIDIA: VK_ERROR_DEVICE_LOST, Xid 109). Whoever
* destroys a HUD source image must forget it here before the next draw that
* could see the reused handle.
*
* The caller must ensure no submitted command buffer still uses the entry.
* No-op if @p hud_image is not cached.
*
* @ingroup aux_vk
*/
void
vk_hud_blend_forget_image(struct vk_hud_blend *blend, struct vk_bundle *vk, VkImage hud_image);

/*!
* Destroy HUD blend resources.
* @ingroup aux_vk
Expand Down
71 changes: 71 additions & 0 deletions src/xrt/compositor/vk_native/comp_vk_native_compositor.c
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,26 @@ struct comp_vk_native_compositor
//! HUD per-eye disparity / parallax (#210).
struct vk_hud_blend window_space_blend;
bool window_space_blend_attempted;
/*!
* Swapchain images destroyed since the last window-space pass (#1782).
*
* window_space_blend caches a view per source VkImage, and the driver
* reuses a destroyed image's handle for the next image it creates: an app
* that resizes a HUD (destroy + create its swapchain) then gets the old
* entry back, and the pass samples freed memory (VK_ERROR_DEVICE_LOST).
* Swapchain destroy queues the images here and the pass forgets them
* before it looks anything up. A leaf lock of its own: destroy runs on
* the app thread, the pass can run on the weave thread, and c->mutex may
* be held by a wedged weave thread (#1394).
*/
struct
{
struct os_mutex mutex;
VkImage images[VK_HUD_BLEND_MAX_IMAGES];
uint32_t count;
//! More images died than fit: forget the whole cache instead.
bool overflow;
} ws_dead;
//! Cached framebuffer for atlas window-space pass (one per atlas view).
/*!
* Framebuffer for the window-space-into-atlas pass, keyed by the atlas
Expand Down Expand Up @@ -2443,6 +2463,31 @@ vk_compositor_layer_zone_3d(struct xrt_compositor *xc,
* @param tile_columns Atlas tile columns.
* @param tile_rows Atlas tile rows.
*/
/*!
* Drop window_space_blend's cache entries for swapchain images destroyed since
* the last call (#1782). See comp_vk_native_compositor::ws_dead.
*/
static void
vk_compositor_forget_dead_window_space_images(struct comp_vk_native_compositor *c)
{
struct vk_bundle *vk = &c->vk;

os_mutex_lock(&c->ws_dead.mutex);
if (c->ws_dead.overflow) {
while (c->window_space_blend.image_count > 0) {
vk_hud_blend_forget_image(&c->window_space_blend, vk,
c->window_space_blend.cached_images[0].image);
}
} else {
for (uint32_t i = 0; i < c->ws_dead.count; i++) {
vk_hud_blend_forget_image(&c->window_space_blend, vk, c->ws_dead.images[i]);
}
}
c->ws_dead.count = 0;
c->ws_dead.overflow = false;
os_mutex_unlock(&c->ws_dead.mutex);
}

static void
vk_compositor_render_window_space_into_atlas(struct comp_vk_native_compositor *c,
VkCommandBuffer cmd,
Expand All @@ -2457,6 +2502,11 @@ vk_compositor_render_window_space_into_atlas(struct comp_vk_native_compositor *c
{
struct vk_bundle *vk = &c->vk;

// #1782: before any lookup can hit a reused handle. Safe to free here for
// the same reason the framebuffer eviction below is: the previous frame's
// submit is waited on before this recording begins.
vk_compositor_forget_dead_window_space_images(c);

bool has_ws = false;
for (uint32_t i = 0; i < c->layer_accum.layer_count; i++) {
if (c->layer_accum.layers[i].data.type == XRT_LAYER_WINDOW_SPACE) {
Expand Down Expand Up @@ -8601,6 +8651,7 @@ vk_compositor_destroy(struct xrt_compositor *xc)
os_mutex_destroy(&c->mutex);
os_cond_destroy(&c->weave_hand.cond);
os_mutex_destroy(&c->weave_hand.mutex);
os_mutex_destroy(&c->ws_dead.mutex);

// XR_DXR_depth_budget: the runner owns a mutex.
comp_rear_budget_fini(&c->rear_budget);
Expand Down Expand Up @@ -9161,6 +9212,7 @@ comp_vk_native_compositor_create(struct xrt_device *xdev,
// #1394: the hand-off handshake runs on its own leaf lock — see weave_hand.mutex.
os_mutex_init(&c->weave_hand.mutex);
os_cond_init(&c->weave_hand.cond);
os_mutex_init(&c->ws_dead.mutex); // #1782
os_thread_helper_init(&c->repaint_thread);

// XR_DXR_depth_budget: the policy exists from the first frame so that a
Expand Down Expand Up @@ -12054,6 +12106,25 @@ comp_vk_native_compositor_get_vk(struct comp_vk_native_compositor *c)
return &c->vk;
}

void
comp_vk_native_compositor_swapchain_images_destroyed(struct comp_vk_native_compositor *c,
const VkImage *images,
uint32_t count)
{
os_mutex_lock(&c->ws_dead.mutex);
for (uint32_t i = 0; i < count; i++) {
if (images[i] == VK_NULL_HANDLE) {
continue;
}
if (c->ws_dead.count >= ARRAY_SIZE(c->ws_dead.images)) {
c->ws_dead.overflow = true;
break;
}
c->ws_dead.images[c->ws_dead.count++] = images[i];
}
os_mutex_unlock(&c->ws_dead.mutex);
}

uint32_t
comp_vk_native_compositor_get_queue_family(struct comp_vk_native_compositor *c)
{
Expand Down
12 changes: 12 additions & 0 deletions src/xrt/compositor/vk_native/comp_vk_native_compositor.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,18 @@ comp_vk_native_compositor_snap_window_rect(struct xrt_compositor *xc,
struct vk_bundle *
comp_vk_native_compositor_get_vk(struct comp_vk_native_compositor *c);

/*!
* A swapchain is about to destroy these images (#1782).
*
* Caches keyed by VkImage must drop them before a new image can reuse the
* handle value; the compositor forgets them at the start of its next
* window-space pass. Callable from any thread.
*/
void
comp_vk_native_compositor_swapchain_images_destroyed(struct comp_vk_native_compositor *c,
const VkImage *images,
uint32_t count);

/*!
* Get the queue family index from a VK native compositor (for sub-modules).
*
Expand Down
10 changes: 10 additions & 0 deletions src/xrt/compositor/vk_native/comp_vk_native_swapchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ struct comp_vk_native_swapchain
//! Vulkan bundle (borrowed from compositor).
struct vk_bundle *vk;

//! Owning compositor: told about the images before they are destroyed (#1782).
struct comp_vk_native_compositor *c;

//! VkImages.
VkImage images[MAX_SWAPCHAIN_IMAGES];

Expand Down Expand Up @@ -238,6 +241,12 @@ vk_swapchain_destroy(struct xrt_swapchain *xsc)
struct comp_vk_native_swapchain *sc = vk_sc(xsc);
struct vk_bundle *vk = sc->vk;

// #1782: the compositor's HUD cache is keyed by VkImage, and the driver
// hands these handle values to the next images it creates.
if (sc->c != NULL) {
comp_vk_native_compositor_swapchain_images_destroyed(sc->c, sc->images, sc->image_count);
}

for (uint32_t i = 0; i < sc->image_count; i++) {
if (sc->true_views[i] != VK_NULL_HANDLE) {
vk->vkDestroyImageView(vk->device, sc->true_views[i], NULL);
Expand Down Expand Up @@ -289,6 +298,7 @@ comp_vk_native_swapchain_create(struct comp_vk_native_compositor *c,
}

sc->vk = vk;
sc->c = c;
sc->info = *info;
sc->image_count = image_count;
comp_swapchain_ring_init(&sc->ring, image_count);
Expand Down
Loading