From 0e57863d1feaf7f05723e057bf72db03c4c4b2cf Mon Sep 17 00:00:00 2001 From: BlackMark Date: Sun, 12 Jul 2026 12:17:30 +0200 Subject: [PATCH] Share the Vulkan swapchain tracking between the hook and the layer vk_hook.cpp and coop_vk_layer.cpp each carried a verbatim copy of the swap-chain registry -- the SwapInfo struct, the vector+mutex, find_swap, the create-time de-dup + LRU cap, and the present-time lookup -- because they are two independent early-presence paths (inline hook vs implicit layer). The tracking logic is identical, so lift it into one VkSwapchainRegistry (hook/src/vk_swapchain_registry.hpp); each module owns an instance. add() de-dups + LRU-caps, lookup() copies the frame out under the lock, clear() resets -- same behavior, one definition. Net -45 lines. mock_game_test exercises both paths (the inline-hook vk storm and the implicit-layer capture) and passes. --- hook/src/vk_hook.cpp | 85 ++++-------------------------- hook/src/vk_swapchain_registry.hpp | 79 +++++++++++++++++++++++++++ vk_layer/coop_vk_layer.cpp | 77 ++++----------------------- 3 files changed, 98 insertions(+), 143 deletions(-) create mode 100644 hook/src/vk_swapchain_registry.hpp diff --git a/hook/src/vk_hook.cpp b/hook/src/vk_hook.cpp index 706a1ee..e191617 100644 --- a/hook/src/vk_hook.cpp +++ b/hook/src/vk_hook.cpp @@ -1,9 +1,7 @@ #include "vk_hook.hpp" -#include #include #include -#include #include #include @@ -23,6 +21,7 @@ #include "hook_install.hpp" #include "hook_registry.hpp" #include "vk_capture.hpp" +#include "vk_swapchain_registry.hpp" namespace coop::hook { @@ -62,25 +61,7 @@ std::uint32_t g_qfam = 0; VkCapture g_cap; // the shared, off-present-thread read-back (same component the layer uses) -// Tracked swap chains (small; engines have one or two). -struct SwapInfo { - VkSwapchainKHR sc; - VkFormat fmt; - std::uint32_t w; - std::uint32_t h; - std::vector images; -}; -std::vector g_swaps; -// g_swaps is pushed from the create-swapchain detour and read from the present detour, which can run -// on different game threads (Vulkan external-sync is per-object, not global), and cleared from the -// worker thread on removal. This mutex serializes all of that -- a push_back realloc must not race a -// concurrent find_swap iteration. We don't hook vkDestroySwapchainKHR (failing to forward a destroy -// could break the game); instead create de-dups by handle and an LRU cap bounds growth, so a game -// that recreates its swapchain on every resize can't grow g_swaps without bound or match a recycled -// handle's stale images. The active swapchain is always the most-recently created, so it's never -// evicted. -std::mutex g_swaps_mutex; -constexpr std::size_t kMaxTrackedSwaps = 8; // engines use 1-3; headroom for transient resize overlap +VkSwapchainRegistry g_swaps; // create-swapchain records images here; present maps them back for capture // --- the real vkGetInstanceProcAddr, via the inline hook's trampoline -------- PFN_vkVoidFunction real_gipa(VkInstance inst, const char* name) @@ -88,18 +69,6 @@ PFN_vkVoidFunction real_gipa(VkInstance inst, const char* name) return g_hk_gipa.stdcall(inst, name); } -// Caller must hold g_swaps_mutex; the returned pointer is only valid until the lock is released -// (copy out what you need before unlocking, since another thread can push_back and reallocate). -const SwapInfo* find_swap(VkSwapchainKHR sc) -{ - for (const SwapInfo& s : g_swaps) { - if (s.sc == sc) { - return &s; - } - } - return nullptr; -} - VKAPI_ATTR VkResult VKAPI_CALL hk_vkQueuePresentKHR(VkQueue queue, const VkPresentInfoKHR* pPresentInfo) { DetourGate::Guard guard(g_gate); // keep the read-back resources alive for this whole detour @@ -114,27 +83,10 @@ VKAPI_ATTR VkResult VKAPI_CALL hk_vkQueuePresentKHR(VkQueue queue, const VkPrese // read-back state, even though the game keeps calling this cached detour pointer. if (g_capture_enabled.load(std::memory_order_acquire) && g_device != VK_NULL_HANDLE && pPresentInfo != nullptr && pPresentInfo->swapchainCount == 1) { - // Copy the matched swapchain's fields out under the lock, then capture without holding it (so - // the GPU submit can't block a concurrent create, and the SwapInfo* can't dangle on a realloc). - VkImage image = VK_NULL_HANDLE; - VkFormat fmt = VK_FORMAT_UNDEFINED; - std::uint32_t w = 0, h = 0; - bool matched = false; - { - std::scoped_lock lock(g_swaps_mutex); - const SwapInfo* s = find_swap(pPresentInfo->pSwapchains[0]); - const std::uint32_t idx = pPresentInfo->pImageIndices[0]; - if (s != nullptr && idx < s->images.size()) { - image = s->images[idx]; - fmt = s->fmt; - w = s->w; - h = s->h; - matched = true; - } - } - if (matched) { + VkSwapchainRegistry::Frame f; + if (g_swaps.lookup(pPresentInfo->pSwapchains[0], pPresentInfo->pImageIndices[0], f)) { VkSemaphore chained = VK_NULL_HANDLE; - if (g_cap.present(image, fmt, w, h, pPresentInfo->pWaitSemaphores, pPresentInfo->waitSemaphoreCount, + if (g_cap.present(f.image, f.fmt, f.w, f.h, pPresentInfo->pWaitSemaphores, pPresentInfo->waitSemaphoreCount, chained)) { // Replace the present's wait with our chained semaphore (our submit consumed the // originals and signals this one), so the present still orders after rendering. @@ -154,27 +106,11 @@ VKAPI_ATTR VkResult VKAPI_CALL hk_vkCreateSwapchainKHR(VkDevice device, const Vk DetourGate::Guard guard(g_gate); // keep g_swaps stable while remove may be clearing it const VkResult r = g_real_create_swapchain(device, ci, alloc, out); if (r == VK_SUCCESS && out != nullptr && g_get_swapchain_images != nullptr) { - SwapInfo info{}; - info.sc = *out; - info.fmt = ci->imageFormat; - info.w = ci->imageExtent.width; - info.h = ci->imageExtent.height; std::uint32_t n = 0; g_get_swapchain_images(device, *out, &n, nullptr); - info.images.resize(n); - g_get_swapchain_images(device, *out, &n, info.images.data()); - { - std::scoped_lock lock(g_swaps_mutex); - // De-dup a recycled handle value, then bound growth (drop the oldest; the just-created - // active swapchain is newest and stays). - g_swaps.erase( - std::remove_if(g_swaps.begin(), g_swaps.end(), [&](const SwapInfo& e) { return e.sc == info.sc; }), - g_swaps.end()); - g_swaps.push_back(std::move(info)); - if (g_swaps.size() > kMaxTrackedSwaps) { - g_swaps.erase(g_swaps.begin()); - } - } + std::vector images(n); + g_get_swapchain_images(device, *out, &n, images.data()); + g_swaps.add(*out, ci->imageFormat, ci->imageExtent.width, ci->imageExtent.height, std::move(images)); // (Don't log the VkSwapchainKHR handle: it's a uint64_t on x86, not a pointer.) logf("vk: swapchain %ux%u fmt=%d images=%u", ci->imageExtent.width, ci->imageExtent.height, static_cast(ci->imageFormat), n); @@ -359,10 +295,7 @@ void remove_vk_hooks() g_cap.shutdown(); // joins the reaper, drains the device, frees the read-back resources hook_set_installed(g_id_present, false); - { - std::scoped_lock lock(g_swaps_mutex); // drained above, but keep all g_swaps access serialized - g_swaps.clear(); - } + g_swaps.clear(); // registry locks internally g_get_swapchain_images = nullptr; g_device = VK_NULL_HANDLE; g_instance = VK_NULL_HANDLE; diff --git a/hook/src/vk_swapchain_registry.hpp b/hook/src/vk_swapchain_registry.hpp new file mode 100644 index 0000000..ebe7c88 --- /dev/null +++ b/hook/src/vk_swapchain_registry.hpp @@ -0,0 +1,79 @@ +// Tracks the swap chains a Vulkan game creates so the present path can map a (swapchain, image +// index) back to the VkImage being presented (and its format/size) for capture. Shared by BOTH +// early-presence paths -- the inline hook (vk_hook.cpp) and the implicit layer +// (vk_layer/coop_vk_layer.cpp) -- which each own one instance. +// +// Thread-safety: add() runs on the create-swapchain path and lookup() on the present path, which can +// be different game threads (Vulkan external-sync is per-object, not global); clear() runs on the +// worker/teardown thread. One mutex serializes all of it -- a push_back realloc must not race a +// concurrent lookup. lookup() copies the fields out under the lock, so the caller never holds a +// pointer that a concurrent add() could dangle. +// +// We deliberately do NOT track swap-chain destruction (forwarding a vkDestroySwapchainKHR wrong could +// break the game): add() de-dups a recycled handle value and an LRU cap bounds growth, so a game that +// recreates its swapchain every resize can neither grow the table without bound nor match a recycled +// handle's stale images. The active swapchain is always the most-recently added, so it is never evicted. +#pragma once + +#include +#include +#include + +#include + +namespace coop::hook { + +class VkSwapchainRegistry { + public: + // Record (or refresh) a swapchain's images. Takes ownership of `images`. + void add(VkSwapchainKHR sc, VkFormat fmt, std::uint32_t w, std::uint32_t h, std::vector images) + { + std::scoped_lock lock(mutex_); + std::erase_if(swaps_, [&](const Entry& e) { return e.sc == sc; }); // drop a recycled handle + swaps_.push_back({sc, fmt, w, h, std::move(images)}); + if (swaps_.size() > kMax) { + swaps_.erase(swaps_.begin()); // bound growth; the just-added active swapchain stays + } + } + + // The presented frame for (sc, image_index). False if the swapchain isn't tracked or the index is + // out of range. + struct Frame { + VkImage image = VK_NULL_HANDLE; + VkFormat fmt = VK_FORMAT_UNDEFINED; + std::uint32_t w = 0; + std::uint32_t h = 0; + }; + bool lookup(VkSwapchainKHR sc, std::uint32_t image_index, Frame& out) const + { + std::scoped_lock lock(mutex_); + for (const Entry& e : swaps_) { + if (e.sc == sc && image_index < e.images.size()) { + out = {e.images[image_index], e.fmt, e.w, e.h}; + return true; + } + } + return false; + } + + void clear() + { + std::scoped_lock lock(mutex_); + swaps_.clear(); + } + + private: + struct Entry { + VkSwapchainKHR sc; + VkFormat fmt; + std::uint32_t w; + std::uint32_t h; + std::vector images; + }; + + static constexpr std::size_t kMax = 8; // engines use 1-3; headroom for transient resize overlap + mutable std::mutex mutex_; + std::vector swaps_; +}; + +} // namespace coop::hook diff --git a/vk_layer/coop_vk_layer.cpp b/vk_layer/coop_vk_layer.cpp index 2fd6556..5fd00e8 100644 --- a/vk_layer/coop_vk_layer.cpp +++ b/vk_layer/coop_vk_layer.cpp @@ -15,12 +15,10 @@ // The loader/layer interface structs (VkLayer*CreateInfo, VkNegotiateLayerInterface) live in // vk_layer.h, which Vulkan-Headers doesn't ship, so they're declared here to the stable // loader-interface-version-2 ABI. -#include #include #include #include #include -#include #include #include @@ -36,6 +34,7 @@ #include "coop/shared_memory.hpp" #include "ipc_client.hpp" #include "vk_capture.hpp" +#include "vk_swapchain_registry.hpp" // --- Loader/layer interface (interface version 2) --------------------------- extern "C" { @@ -99,6 +98,7 @@ constexpr VkStructureType kLoaderDeviceCreateInfo = static_cast using coop::hook::IpcClient; using coop::hook::VkCapture; +using coop::hook::VkSwapchainRegistry; IpcClient g_ipc; std::atomic g_ipc_tried{false}; // reaper-thread side: connect once on the first published frame @@ -118,19 +118,7 @@ std::uint32_t g_qfam = 0; VkCapture g_cap; // the shared, off-present-thread read-back -struct SwapInfo { - VkSwapchainKHR sc; - VkFormat fmt; - std::uint32_t w, h; - std::vector images; -}; -std::vector g_swaps; -// Serialize all g_swaps access: create (push) and present (find) can run on different game threads, -// and DestroyDevice clears it. We don't track swapchain destruction (forwarding a destroy wrong could -// break the game); create de-dups by handle and an LRU cap bounds growth so repeated resize/recreate -// can't grow g_swaps unbounded or match a recycled handle's stale images. -std::mutex g_swaps_mutex; -constexpr std::size_t kMaxTrackedSwaps = 8; +VkSwapchainRegistry g_swaps; // create-swapchain records images here; present maps them back for capture bool eq(const char* a, const char* b) { @@ -199,18 +187,6 @@ bool decide_active() return _stricmp(self8, want) == 0; } -// Caller must hold g_swaps_mutex; copy out what you need before unlocking (a concurrent push_back -// can reallocate and dangle the returned pointer). -const SwapInfo* find_swap(VkSwapchainKHR sc) -{ - for (const SwapInfo& s : g_swaps) { - if (s.sc == sc) { - return &s; - } - } - return nullptr; -} - // Per-second present-rate trace (enable with COOP_VK_LAYER_LOG): confirms the game keeps its frame // rate while capturing (the whole point of the off-present-thread read-back) and how many of those // presents the reaper actually mirrored. @@ -252,26 +228,10 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_QueuePresentKHR(VkQueue queue, const VkPres g_ipc.note_present(); } if (g_active && pi != nullptr && pi->swapchainCount == 1) { - // Copy the matched swapchain out under the lock, then capture without holding it. - VkImage image = VK_NULL_HANDLE; - VkFormat fmt = VK_FORMAT_UNDEFINED; - std::uint32_t w = 0, h = 0; - bool matched = false; - { - std::scoped_lock lock(g_swaps_mutex); - const SwapInfo* s = find_swap(pi->pSwapchains[0]); - const std::uint32_t idx = pi->pImageIndices[0]; - if (s != nullptr && idx < s->images.size()) { - image = s->images[idx]; - fmt = s->fmt; - w = s->w; - h = s->h; - matched = true; - } - } - if (matched) { + VkSwapchainRegistry::Frame f; + if (g_swaps.lookup(pi->pSwapchains[0], pi->pImageIndices[0], f)) { VkSemaphore chained = VK_NULL_HANDLE; - if (g_cap.present(image, fmt, w, h, pi->pWaitSemaphores, pi->waitSemaphoreCount, chained)) { + if (g_cap.present(f.image, f.fmt, f.w, f.h, pi->pWaitSemaphores, pi->waitSemaphoreCount, chained)) { VkPresentInfoKHR p = *pi; p.waitSemaphoreCount = 1; p.pWaitSemaphores = &chained; @@ -292,25 +252,11 @@ VKAPI_ATTR VkResult VKAPI_CALL layer_CreateSwapchainKHR(VkDevice device, const V static_cast(ci->presentMode), ci->imageExtent.width, ci->imageExtent.height, ci->minImageCount); const VkResult r = g_real_create_swapchain(device, ci, a, out); if (g_active && r == VK_SUCCESS && out && g_get_swapchain_images) { - SwapInfo info{}; - info.sc = *out; - info.fmt = ci->imageFormat; - info.w = ci->imageExtent.width; - info.h = ci->imageExtent.height; std::uint32_t n = 0; g_get_swapchain_images(device, *out, &n, nullptr); - info.images.resize(n); - g_get_swapchain_images(device, *out, &n, info.images.data()); - { - std::scoped_lock lock(g_swaps_mutex); - g_swaps.erase( - std::remove_if(g_swaps.begin(), g_swaps.end(), [&](const SwapInfo& e) { return e.sc == info.sc; }), - g_swaps.end()); - g_swaps.push_back(std::move(info)); - if (g_swaps.size() > kMaxTrackedSwaps) { - g_swaps.erase(g_swaps.begin()); - } - } + std::vector images(n); + g_get_swapchain_images(device, *out, &n, images.data()); + g_swaps.add(*out, ci->imageFormat, ci->imageExtent.width, ci->imageExtent.height, std::move(images)); } return r; } @@ -428,10 +374,7 @@ VKAPI_ATTR void VKAPI_CALL layer_DestroyDevice(VkDevice device, const VkAllocati auto destroy = reinterpret_cast(g_next_gdpa(device, "vkDestroyDevice")); if (g_active && device == g_device) { g_cap.shutdown(); // joins the reaper, drains the device, frees the read-back resources - { - std::scoped_lock lock(g_swaps_mutex); - g_swaps.clear(); - } + g_swaps.clear(); // registry locks internally g_device = VK_NULL_HANDLE; } destroy(device, a);