diff --git a/README.md b/README.md index 86c1535..1c5b4ef 100644 --- a/README.md +++ b/README.md @@ -108,18 +108,6 @@ default** and covers anything the hooked path doesn't. ## Roadmap -### Current tasks - -- **DX12 capture is measurably slower than the other backends — find out why and improve it.** The - present-thread overhead guard shows DX12 ~0.34 ms vs DX11 ~0.05 ms / OpenGL ~0.09 ms (the D3D11On12 - bridge). Investigate and reduce it. -- **The mock game must implement and test *every* rendering backend.** Today `coop_mock_game` only - drives D3D11/D3D12, so the Vulkan/OpenGL/D3D9 capture paths have no game-driven coverage (which is - why the Vulkan present-thread stall slipped through). Add Vulkan, OpenGL, and D3D9 renderers to the - mock game and exercise each through the capture + perf tests, so it serves as a real-world test. - -### Done - The near-term tracked tasks are complete: injection hardening (the cross-backend safe-unhook drain), two-path audio-format correlation (rate + channels/bit-depth recovery), mouse + keyboard forwarding for DirectInput and Raw Input games, real-game Vulkan validation (`coop_vk_validate` against Sphere @@ -132,9 +120,16 @@ directory and `vk_hook` intercepts present/swapchain resolved via `vkGetInstance `vkGetDeviceProcAddr`). Both Vulkan paths are verified on the real game. The capture also **preserves the game's sync mode** — it passes the swapchain's present mode through untouched and no longer throttles the mirror (vsync paces it): Sphere Spectacle requests `FIFO` and holds a steady 144 Hz -with the layer attached. See **Lessons learned** and -the test suite for each. Open directions: per-game profiles, multi-guest virtual-pad mapping, and -continuous raw-mouse *movement* forwarding (the MKB event stream is position-based today). +with the layer attached. The **mock game now drives every backend uncapped** (DX9/10/11/12 + OpenGL + +Vulkan, thousands of fps — the slow vsync'd cadence was masking bugs), `mock_game_test` asserts each +keeps a healthy present rate while capturing, and that faster stress **exposed and fixed a +hook-removal use-after-free** (the inline-hook removal now disables → drains → destroys, and the +shared drain settles before concluding, so a backend presenting at thousands/s survives the +hook/unhook storm). DX12's higher capture cost was traced to the inherent D3D11On12 bridge (not +`CreateWrappedResource`) and documented; a native-D3D12 copy path is the remaining (deferred) +improvement. See **Lessons learned** and the test suite for each. Open directions: per-game profiles, +multi-guest virtual-pad mapping, continuous raw-mouse *movement* forwarding (the MKB event stream is +position-based today), and the native-D3D12 capture path. ## Building @@ -675,15 +670,23 @@ Non-obvious things that cost time and constrain the design: mid-flight on the game's render thread → use-after-free → the game crashed (Brotato, on its OpenGL path; reproduced across every backend by the `mock_game_test` storm). The generalised fix (`hook/src/hook_guard.hpp`, `DetourGate`): every detour wraps its body in an RAII active-count - `Guard`; `remove_*` (1) restores the hook so **no new detour can start** — reset the inline hook - (SafetyHook's mutex-guarded call wrappers make any in-flight trampoline call safe), or, for the - focus WNDPROC subclass, restore the window proc — then (2) `drain()`s the active count to zero, - and only **then** (3) frees the shared state. **Vulkan is the exception**: the game caches our - `hk_vkQueuePresentKHR` pointer at resolution time and keeps calling it even after the GPA hook is - reset, so a reset can't stop new detours — instead removal closes an atomic **capture gate** - first (the detour then passes straight through to the real present without touching the read-back - state), drains, and only then frees. The drain is bounded (~400 ms) so a wedged game thread can't - hang the worker; detours are micro- to milliseconds, so it returns almost immediately. + `Guard`; `remove_*` (1) **disables** the hook so **no new detour can start** — for a SafetyHook + inline hook that's `disable()` (restore the original bytes under thread suspension) **not** `= {}`, + because destroying frees the trampoline immediately and an in-flight detour about to call it + (`.stdcall()`) then uses freed memory; or, for the focus WNDPROC subclass, restore the window proc + — then (2) `drain()`s the active count to zero, and only **then** (3) destroys the hook (frees the + trampoline) and frees the shared state. Two subtleties the *uncapped* mock storm (thousands of + presents/s) exposed that the old vsync'd one (tens/s) masked: **(a)** the original code did `= {}` + before draining → trampoline UAF (now disable → drain → destroy, keeping the trampoline alive + across the drain); **(b)** `drain()` returned the instant the count read zero, but a thread can be + *inside* the detour yet not have reached its `Guard` constructor (the few-instruction prologue is + unguarded), so it `Sleep(1)`s **before** each zero-check to let such a thread register. **Vulkan is + the exception**: the game caches our `hk_vkQueuePresentKHR` pointer at resolution time and keeps + calling it even after the GPA hook is reset, so a reset can't stop new detours — instead removal + closes an atomic **capture gate** first (the detour then passes straight through to the real present + without touching the read-back state), drains, and only then frees. The drain is bounded (~400 ms) + so a wedged game thread can't hang the worker; detours are micro- to milliseconds, so it returns + almost immediately. - **Capturing at `Present` decouples the mirror from DWM composition.** The hook copies the backbuffer inside the game's `Present`, which the game issues at its true render rate regardless of how DWM composites that *window*. So an unfocused game window can diff --git a/tests/mock_game_test.cpp b/tests/mock_game_test.cpp index a3719fd..f439cd6 100644 --- a/tests/mock_game_test.cpp +++ b/tests/mock_game_test.cpp @@ -220,17 +220,38 @@ VideoShareView read_video_share(const SharedBlock* block) return v; } +// Real-world performance guard: with the hook (or layer) capturing, the game must keep rendering +// FAST. This is the dimension the frame-advance checks miss -- the Vulkan read-back stall (144->3 +// FPS) still ADVANCED frames, just ~3/s, so every "frames advance" assertion passed while the game +// was unplayable. The mock does trivial work and renders UNCAPPED (no vsync), so on any modern GPU it +// presents at thousands/s (measured no-hook: dx9 ~21000, dx10 ~2800, dx11 ~17000, dx12 ~12000, gl +// ~26000, vk ~24000); the capture costs some of that (e.g. OpenGL's glReadPixels is the heaviest), +// but a healthy backend stays in the hundreds-thousands. A rate that has collapsed below the display +// refresh is either a present-thread stall (the bug) or an accidental vsync -- both regressions we +// want to catch -- so the floor sits above any common refresh (240) and far below the healthy range. +inline constexpr std::uint64_t kMockMinCaptureFps = 300; + +void check_capture_present_rate(SharedBlock* block, const char* backend) +{ + const std::uint64_t p0 = block->video.present_calls; + Sleep(1000); + const std::uint64_t fps = block->video.present_calls - p0; + std::printf(" %s present rate while capturing = %llu /s\n", backend, static_cast(fps)); + check(fps >= kMockMinCaptureFps, + "game keeps rendering fast while capturing (no present-thread stall / accidental vsync)"); +} + // Capture from the mock game on `backend` and assert the decoded frame numbers form a // monotonic, advancing sequence. void test_video_capture(const char* backend, ID3D11Device* device) { std::printf("== video capture: %s ==\n", backend); - std::wstring args; + std::wstring wbackend; for (const char* p = backend; *p != '\0'; ++p) // backend names are ASCII (dx10/dx11/dx12/...) { - args.push_back(static_cast(*p)); + wbackend.push_back(static_cast(*p)); } - args += L" 30"; + std::wstring args = wbackend + L" 30"; MockGame game = MockGame::launch(args); if (!game.ok) { @@ -299,6 +320,10 @@ void test_video_capture(const char* backend, ID3D11Device* device) check(!all_same, "captured frames are not stuck on one number"); } + if (game.alive()) + { + check_capture_present_rate(block, backend); + } game.kill(); } @@ -402,6 +427,10 @@ void test_vk_capture(ID3D11Device* device) { check(seq.back() - seq.front() >= 10, "captured vk frame numbers advance"); } + if (alive()) + { + check_capture_present_rate(block, "vk (inject)"); + } cleanup(); } @@ -494,6 +523,10 @@ void test_vk_layer_capture(ID3D11Device* device) { check(seq.back() - seq.front() >= 10, "layer-captured frame numbers advance"); } + if (game.alive()) + { + check_capture_present_rate(block, "vk (layer)"); + } game.kill(); } diff --git a/tools/mock_game/render_dx09.cpp b/tools/mock_game/render_dx09.cpp index 2d8d847..ec3744e 100644 --- a/tools/mock_game/render_dx09.cpp +++ b/tools/mock_game/render_dx09.cpp @@ -39,7 +39,7 @@ public: pp.SwapEffect = D3DSWAPEFFECT_DISCARD; pp.hDeviceWindow = hwnd; pp.Windowed = TRUE; - pp.PresentationInterval = D3DPRESENT_INTERVAL_ONE; // vsync -> a game-like cadence + pp.PresentationInterval = D3DPRESENT_INTERVAL_IMMEDIATE; // uncapped: the mock must be fast if (ex_) { diff --git a/tools/mock_game/render_dx10.cpp b/tools/mock_game/render_dx10.cpp index 5df634e..46a5840 100644 --- a/tools/mock_game/render_dx10.cpp +++ b/tools/mock_game/render_dx10.cpp @@ -103,7 +103,7 @@ public: { device_->CopyResource(back.Get(), scratch_.Get()); } - swap_->Present(1, 0); // vsync -> a game-like cadence + swap_->Present(0, 0); // uncapped (BLT model): the mock does nothing -> it must be fast } [[nodiscard]] const char* name() const override diff --git a/tools/mock_game/render_dx11.cpp b/tools/mock_game/render_dx11.cpp index 1dd866f..762c84d 100644 --- a/tools/mock_game/render_dx11.cpp +++ b/tools/mock_game/render_dx11.cpp @@ -4,7 +4,7 @@ #include "render_backend.hpp" #include -#include +#include #include using Microsoft::WRL::ComPtr; @@ -43,6 +43,16 @@ public: return false; } + // The mock does trivial work, so it must render UNCAPPED (it's a perf fixture, not a real + // game) -- a flip-model swapchain only tears free of vsync with ALLOW_TEARING, so require it. + ComPtr factory5; + BOOL tearing = FALSE; + if (SUCCEEDED(factory.As(&factory5))) + { + factory5->CheckFeatureSupport(DXGI_FEATURE_PRESENT_ALLOW_TEARING, &tearing, sizeof(tearing)); + } + tearing_ = tearing != 0; + DXGI_SWAP_CHAIN_DESC1 desc = {}; desc.Width = width; desc.Height = height; @@ -51,6 +61,7 @@ public: desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT; desc.BufferCount = 2; desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + desc.Flags = tearing_ ? DXGI_SWAP_CHAIN_FLAG_ALLOW_TEARING : 0u; if (FAILED(factory->CreateSwapChainForHwnd(device_.Get(), hwnd, &desc, nullptr, nullptr, swap_.GetAddressOf()))) { @@ -90,7 +101,7 @@ public: const D3D11_RECT block = {0, 0, static_cast(kFrameBlock), static_cast(kFrameBlock)}; ctx_->ClearView(rtv_.Get(), code, &block, 1); - swap_->Present(1, 0); // vsync -> a game-like cadence + swap_->Present(0, tearing_ ? DXGI_PRESENT_ALLOW_TEARING : 0u); // uncapped: the mock must be fast } [[nodiscard]] const char* name() const override @@ -101,6 +112,7 @@ public: private: std::uint32_t width_ = 0; std::uint32_t height_ = 0; + bool tearing_ = false; ComPtr device_; ComPtr ctx_; ComPtr swap_; diff --git a/tools/mock_game/render_dx12.cpp b/tools/mock_game/render_dx12.cpp index 73589f0..53d1a28 100644 --- a/tools/mock_game/render_dx12.cpp +++ b/tools/mock_game/render_dx12.cpp @@ -4,7 +4,7 @@ #include "render_backend.hpp" #include -#include +#include #include using Microsoft::WRL::ComPtr; @@ -40,6 +40,16 @@ public: { return false; } + // Uncapped (the mock is a perf fixture): a flip-model swapchain needs ALLOW_TEARING to run + // free of vsync. + ComPtr factory5; + BOOL tearing = FALSE; + if (SUCCEEDED(factory.As(&factory5))) + { + factory5->CheckFeatureSupport(DXGI_FEATURE_PRESENT_ALLOW_TEARING, &tearing, sizeof(tearing)); + } + tearing_ = tearing != 0; + DXGI_SWAP_CHAIN_DESC1 desc = {}; desc.Width = width; desc.Height = height; @@ -48,6 +58,7 @@ public: desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT; desc.BufferCount = kBackBuffers; desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + desc.Flags = tearing_ ? DXGI_SWAP_CHAIN_FLAG_ALLOW_TEARING : 0u; ComPtr sc1; if (FAILED(factory->CreateSwapChainForHwnd(queue_.Get(), hwnd, &desc, nullptr, nullptr, sc1.GetAddressOf())) || @@ -129,7 +140,7 @@ public: ID3D12CommandList* lists[] = {list_.Get()}; queue_->ExecuteCommandLists(1, lists); - swap_->Present(1, 0); + swap_->Present(0, tearing_ ? DXGI_PRESENT_ALLOW_TEARING : 0u); // uncapped: the mock must be fast wait_for_gpu(); // simple per-frame sync (mock game: correctness over throughput) } @@ -175,6 +186,7 @@ private: std::uint32_t width_ = 0; std::uint32_t height_ = 0; + bool tearing_ = false; ComPtr device_; ComPtr queue_; ComPtr swap_; diff --git a/tools/mock_game/render_gl.cpp b/tools/mock_game/render_gl.cpp index 0c9c666..b53144d 100644 --- a/tools/mock_game/render_gl.cpp +++ b/tools/mock_game/render_gl.cpp @@ -47,12 +47,12 @@ public: { return false; } - // vsync if available (a game-like cadence; avoids spinning uncapped). Runtime extension - // lookup -- no loader/submodule needed. + // Uncapped: the mock is a perf fixture and must run as fast as it can (disable vsync), so a + // capture-induced slowdown is visible. Runtime extension lookup -- no loader/submodule needed. using PFN_wglSwapIntervalEXT = BOOL(WINAPI*)(int); if (auto swap_interval = reinterpret_cast(wglGetProcAddress("wglSwapIntervalEXT"))) { - swap_interval(1); + swap_interval(0); } return true; } diff --git a/tools/mock_game/render_vk.cpp b/tools/mock_game/render_vk.cpp index 1a24c97..a61fd67 100644 --- a/tools/mock_game/render_vk.cpp +++ b/tools/mock_game/render_vk.cpp @@ -237,7 +237,21 @@ private: sc.imageSharingMode = VK_SHARING_MODE_EXCLUSIVE; sc.preTransform = caps.currentTransform; sc.compositeAlpha = VK_COMPOSITE_ALPHA_OPAQUE_BIT_KHR; - sc.presentMode = VK_PRESENT_MODE_FIFO_KHR; // vsync, universally supported + // The mock is a perf fixture and must run UNCAPPED: prefer IMMEDIATE (no vsync) > MAILBOX > + // FIFO. FIFO (vsync) would cap it at the refresh, hiding capture-induced slowdowns. + std::uint32_t pmn = 0; + vkGetPhysicalDeviceSurfacePresentModesKHR(phys_, surface_, &pmn, nullptr); + std::vector pmodes(pmn); + vkGetPhysicalDeviceSurfacePresentModesKHR(phys_, surface_, &pmn, pmodes.data()); + auto has_mode = [&](VkPresentModeKHR m) { + for (VkPresentModeKHR p : pmodes) + if (p == m) + return true; + return false; + }; + sc.presentMode = has_mode(VK_PRESENT_MODE_IMMEDIATE_KHR) ? VK_PRESENT_MODE_IMMEDIATE_KHR + : has_mode(VK_PRESENT_MODE_MAILBOX_KHR) ? VK_PRESENT_MODE_MAILBOX_KHR + : VK_PRESENT_MODE_FIFO_KHR; sc.clipped = VK_TRUE; if (vkCreateSwapchainKHR(device_, &sc, nullptr, &swapchain_) != VK_SUCCESS) {