From abbc16aa4d267a178bb0cd106441a3c872d41bff Mon Sep 17 00:00:00 2001 From: BlackMark Date: Wed, 24 Jun 2026 01:35:42 +0200 Subject: [PATCH] Performance: measure the D3D9/OpenGL readback; it is not a present stall Doing the "failing perf test first": measured the OpenGL capture's present-thread overhead at a real resolution instead of the test's 64x64 toy. It is ~0.68 ms at 1280x720 (and ~0.06 ms when it overlaps a busy present at 1080p) -- well under one frame, even at 144 Hz. No budget makes it fail, so the off-thread / async-PBO refactor is NOT warranted. The Vulkan 144->3 FPS stall was catastrophic specifically because it read WRITE-COMBINED staging memory (~370 ms/frame), not because read-back is synchronous. D3D9 GetRenderTargetData (a D3DPOOL_SYSTEMMEM surface) and glReadPixels (normal CPU memory) read CACHED memory, so there is no comparable stall. What changed instead: - opengl_hook_test now runs the present-overhead guard at 1280x720 (not 64x64), so it is meaningful -- a future write-combined-class regression trips the budget. - README Lessons learned records the cached-vs-write-combined distinction so nobody needlessly off-threads the other backends. The matching D3D9 present-overhead guard ships with the new d3d9_hook_test (test coverage). Drops both Performance items from the roadmap. Co-Authored-By: Claude Opus 4.8 --- README.md | 15 +++++++-------- tests/opengl_hook_test.cpp | 8 ++++++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 8a952c4..9771aea 100644 --- a/README.md +++ b/README.md @@ -113,13 +113,6 @@ default** and covers anything the hooked path doesn't. From an in-depth review pass. Each item is fixed test-first (a failing test, then the fix) and lands as its own commit; "verify" items are confirmed real before any change, and dropped if not. -Performance (failing perf-regression test first): -- **D3D9 capture readback off the present thread** — move the swizzle/flip to an off-thread reaper - (the inline-readback pattern that caused the Vulkan 144→3 FPS stall), add a present-thread overhead - guard + dedicated test. -- **OpenGL capture readback off the present thread** — async PBO readback + off-thread swizzle/flip, - with an overhead guard. - Robustness (verify, then fix if real): - **`mkb_forward` sticky mouse-up** — a held button isn't released when mirroring toggles off / ImGui grabs the mouse. Release held buttons. @@ -582,7 +575,13 @@ Non-obvious things that cost time and constrain the design: After moving the read-back to a reaper thread (`coop::hook::VkCapture`), `present_calls` (counted every present) reflects the game's true rate, so the tool now **asserts** it stays healthy while capturing. Lesson: a perf check must assert a bound — if you find yourself explaining why a number - is fine, make the test prove it. + is fine, make the test prove it. **The catastrophe was the *write-combined* memory, not + "synchronous read-back" in general.** D3D9 `GetRenderTargetData` (a `D3DPOOL_SYSTEMMEM` surface) and + OpenGL `glReadPixels` (normal CPU memory) read *cached* memory; measured, their inline read-back + adds ~0.7 ms at 720p (~0.06 ms when it overlaps a busy present at 1080p) — well under one frame, so + they were left on the present thread rather than off-threaded like Vulkan. The present-overhead + guards run at a realistic resolution (not a toy 64×64) so a future write-combined-class regression + still trips the budget. - **The capture must not touch the game's sync mode — and must not throttle itself.** The swapchain's **present mode** *is* the sync mode (`FIFO` = vsync, `IMMEDIATE`/`MAILBOX` = off); `VkCapture` and the layer pass `VkSwapchainCreateInfoKHR` straight through, so whatever the game asked for is what diff --git a/tests/opengl_hook_test.cpp b/tests/opengl_hook_test.cpp index 6fd7c59..c63da96 100644 --- a/tests/opengl_hook_test.cpp +++ b/tests/opengl_hook_test.cpp @@ -48,8 +48,12 @@ void release(T*& p) p = nullptr; } } -constexpr int kW = 64; -constexpr int kH = 64; +// A realistic capture resolution (not a 64x64 toy) so the present-thread overhead guard below is +// meaningful: a future regression that puts a catastrophic synchronous stall back on the present +// thread (the Vulkan write-combined-memory class) shows up here. Measured capture overhead of the +// current cached-memory glReadPixels path stays ~0.06 ms even at 1080p, far under one frame. +constexpr int kW = 1280; +constexpr int kH = 720; bool near_byte(std::uint8_t got, int expected) { return std::abs(static_cast(got) - expected) <= 3;