register_vk_layer writes an HKCU implicit-layer entry that makes the loader pull
our DLL into every Vulkan app; it's session-scoped (unregistered on clean exit).
If the host crashed or was killed while registered, the entry leaked and kept
loading our DLL into every Vulkan process until the next clean run.
Add cleanup_stale_vk_layer(), called once at the top of run(): since registration
is opt-in per session, anything registered at startup is a crash leftover, so it
removes it (delegates to unregister_vk_layer). No-op when nothing is registered.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The injected coop_hook.dll links the dynamic CRT, so it failed to load into games
on machines without the matching VC++ redistributable -- a real field failure of
the core feature. It was deferred because SafetyHook + Zydis (linked into the DLL)
default to /MD, so a per-target /MT would mismatch.
Set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreaded[Debug] project-wide (CMP0091 NEW,
available at our 3.21 minimum). Every target -- the DLL, the vendored deps, the
host, tools, and tests, on both x64 and the x86 sub-build -- now shares one static
CRT, so there's no mismatch and the whole tool ships redist-free.
Verified: x64 + x86 full builds are clean; dumpbin shows coop_hook.dll and
coop_hook_x86.dll import only system DLLs (USER32/ole32/d3d11/KERNEL32) -- no
VCRUNTIME/MSVCP -- and the /MT test binaries run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
focus_spoof_test plays game + host: it creates a top-level window (which
find_main_window picks up), installs the spoof, and checks that
GetForegroundWindow / GetActiveWindow / GetFocus now report that window, that the
status flag flips on install and off on removal, and that a ClipCursor request is
swallowed while cursor release is the default. Skips if the window can't be
enumerated.
With this and d3d9_hook_test, the dedicated-hook-tests item is done. vk_hook's
present path is intentionally not given a separate in-process test: it's exercised
end to end by mock_game_test's vk-inject path ("hook captured Vulkan present
calls"), and the off-thread VkCapture it drives has vk_capture_perf_test -- a
standalone in-process Vulkan swapchain harness would just duplicate that at high
cost.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The D3D9 capture path had no dedicated test. d3d9_hook_test plays game + host:
installs the IDirect3DDevice9::Present hook, drives a real D3D9 device (Clear +
Present), and verifies the full GetRenderTargetData -> BGRA->RGBA swizzle ->
shared keyed-mutex texture path by opening coop_video_<pid> from a D3D11 device
and reading the rendered color back ({51,102,153}).
It also carries the present-thread overhead guard the perf section deferred here,
at a realistic 1280x720: measured capture overhead is ~0.39 ms -- far under one
frame, confirming the cached-memory D3D9 read-back is not a stall (and tripping
the budget if a write-combined-class regression ever lands on the present thread).
Skips cleanly without a D3D9/D3D11 device.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fills small coverage gaps:
- shared_memory_test: SharedMemory create-or-open aliasing, move (steal + empty
the source, no double-free), reset, open-missing.
- wav_test: malformed input -- truncation, bad magic, missing data chunk,
over-long data size (clamps), odd-sized chunk (word-align skip), and a corrupt
~4 GB chunk_size. The reader gains an advance guard so that last case can't wrap
pos on a 32-bit size_t (x86) or spin the walk; it stops cleanly.
- tool_paths_test: deployed_artifact_path resolution -- next-to-exe, one-dir-up,
and the not-found fallback -- with real marker files.
- audio_ring_test: an overrun-at-the-seam case (write head near the end: a
wrapping push that fits vs. an over-capacity wrapping push dropped whole),
exercising the wrap split + overrun together, not just at offset 0.
The injector bitness check isn't added as a unit test: is_wow64_process is
file-local and the real WOW64 path needs a 32-bit target, so it stays
inspection-covered (and exercised by the x86 injection path).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New protocol_test covers two previously-untested cross-process contracts:
- publish_pads/read_pads seqlock: a concurrent writer + reader run for ~700 ms;
the reader (2.5M snapshots) never observes a torn snapshot, and read_pads
returns false (bounded, no hang) when the sequence is stuck odd mid-write.
- IpcClient::connect refuses a section with a mismatched version or a bad magic,
and accepts a matching header -- the ABI safety net that only had happy-path
coverage before.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The lossy MPSC log ring published each record by writing its text and THEN storing
the slot's sequence. A consumer that passed the seq==generation check could then
read text while a producer 'capacity' generations later overwrote that same slot
(it wrote text before bumping seq), yielding a torn line. Diagnostics-only and
practically unreachable (it needs the consumer a full ring behind -- ~60k lines/s
between two host drains), but a real data race.
Make it a proper seqlock: the producer stores seq 0 (in-progress) and fences
BEFORE touching the record, then publishes the generation after the text; the
consumer copies the record out and re-checks seq, dropping the line if it changed.
The ring stays lossy, never torn.
Adds log_ring_test (previously zero coverage): a deterministic wrap-drop case plus
a threaded torn-read guard (4 producers + a slow consumer on a 32-slot ring) that
emits 0 torn lines out of ~300k produced. Closes both the cross-process item and
the log_ring coverage gap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-game override store keyed and persisted image names through narrow(), which
masked each character with & 0x7F, and widen() used the full byte -- not a true
inverse. So a non-ASCII exe name was corrupted on reload, and two names differing
only in their high bits collapsed onto the same key (e.g. U+00E9 'é' masked to
'i', so "café.exe" collided with "cafi.exe").
Use real WideCharToMultiByte/MultiByteToWideChar(CP_UTF8) so the round-trip is
lossless for any Unicode name. ASCII names are byte-identical under UTF-8, so
existing override files stay compatible.
audio_overrides_test gains a high-bit-collision case (café vs cafi, built from a
code point to keep the source ASCII) plus a non-ASCII persist/reload check -- both
of which the old 7-bit mask failed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cross-process block's layout is a wire protocol shared by the x64 host and the
x86 hook, but only three front offsets were asserted, and hook_selftest's
dump_layout merely printed the rest. A field reordered/resized inside HookStatus
(which precedes control/video/mkb) would silently shift everything with no
compile-time tripwire and, if the developer forgot to bump kProtocolVersion, ship
a silent host<->DLL mismatch.
- protocol.hpp now static_asserts sizeof(SharedBlock) and every sub-channel offset
(status/control/video/mkb) plus each sub-struct size (HookStatus/HookControl/
VideoShare/AudioStreamInfo/HookEntry/MkbRing). protocol.hpp is compiled for both
arches, so a cross-bitness divergence fails to compile on the one that disagrees.
- hook_selftest's dump_layout now ASSERTS the same numbers instead of only
printing, so hook_selftest_x86 confirms the x86 layout at runtime too.
Verified: x64 and x86 builds both compile (identical layout) and both selftests
pass; sizeof(SharedBlock)=3936 on both.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- injector bitness gate: IsWow64Process2 failure was treated as "native", which
would send the x64 DLL into a 32-bit target. Fall back to the legacy
IsWow64Process before giving up to permissive.
- injector helper exit: GetExitCodeProcess's BOOL was ignored; on a failed query
surface GetLastError instead of a misleading exit code.
- window_capture: CreateShaderResourceView's HRESULT was ignored, and width_/
height_ were committed even on failure, so the (latest_ == nullptr) recreate
guard never retried -- a silently black mirror until the next resize. Only
commit the dims on success; otherwise drop latest_ so the next frame retries.
- xinput rumble: make the best-effort XInputSetState ignore explicit (a
disconnected pad re-syncs on the next refresh; the result isn't actionable).
The GetClientRect/ClientToScreen reads in mkb_forward are left as-is: a failure
there is a single self-correcting frame (the mapping is rejected and reused next
frame), so checking them adds no actionable behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
verify_stream_format ignored ProcessLoopbackCapture::start()'s bool. A failed
loopback activation then produced an empty ground-truth signal, so the result was
ok=false -- indistinguishable from "captured fine but the two paths didn't
correlate" -- after burning the whole measurement window capturing only the hook
side for nothing.
Now it checks start(): on failure it restores the ring tap, emits a clear
OutputDebugString diagnostic, and returns immediately (ok=false) instead of
wasting the window. The caller still falls back to the measured guess, but the
cause is now visible.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
forward_mkb_frame early-returned the whole mouse block when mirroring was off or
ImGui wanted the mouse, and the top-level gates returned when the subsystem was
off / focus was lost / the game died. A key or mouse button held at that moment
never got its KeyUp/MouseUp, so it stuck DOWN in the guest -- a held mouse button
fires continuously, a held key walks forever -- contradicting the "send the up so
nothing sticks" intent.
Track what we've forwarded as held (g_mouse_down / g_key_down) and release it
whenever we stop forwarding for any reason: the can't-forward gate, ImGui grabbing
the keyboard/mouse, or the mouse-not-mirroring path all now release held inputs
before returning. Normal down/up still flips the held state.
Fix by inspection: forward_mkb_frame needs a live ImGui context + injection panel,
so it isn't unit-tested; the logic is a straightforward held-state release.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 <noreply@anthropic.com>
present_calls, frames_dropped (VideoShare) and frames_rendered (AudioStreamInfo)
were plain `+= 1` / stores in the DLL, read by the host cross-process. On an x86
DLL a 64-bit store is two halves, so the x64 host could read a torn value during
a carry. Benign (display-only), but a real data race.
Use std::atomic_ref at the access sites rather than changing the field types:
the structs stay plain POD so the layout/offset asserts are unchanged and
AudioStreamInfo stays trivially copyable (it's published/read as a whole struct).
The DLL writers (note_present / note_video_dropped / note_audio_frames) and the
host readers (IpcServer::video_share / hook_status) now use relaxed atomic_ref;
hook_status reloads frames_rendered atomically after the wholesale struct copy.
The dev-tool readers (vk_validate, audio_probe) keep plain reads -- diagnostics of
diagnostics, and same-bitness in practice.
Validated by present_hook_test (present_calls via atomic_ref) and audio_hook_test
(frames_rendered) -- also confirms no atomic_ref alignment fault.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each forwarded raw-input event is written to g_raw_slots[head++ % kRawSlots] and
a WM_INPUT carrying that slot's ADDRESS is posted to the game, which reads it back
through hk_GetRawInputData. With only 64 slots, a burst that queues more than 64
WM_INPUTs before the game pumps could overwrite a slot before the game reads it,
so it would decode a newer event for a stale message. No memory unsafety (the
address stays in-bounds), but wrong event data under backlog.
Grow the ring to 512 (a few tens of KB) so realistic input rates can't lap it.
Deliberately not per-slot consume-tracking: that would permanently exhaust slots
and silently stop forwarding for a game that ignores WM_INPUT, whereas a large
ring always forwards and only risks a rare stale read under extreme backlog.
Also drops the "publish() synthetic-input timing" review item: verified it uses
GetTickCount64() (thread-safe), not ImGui state -- not a bug, no change needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
g_swaps (vk_hook.cpp and the Vulkan layer) is pushed from the create-swapchain
detour and iterated by the present detour, which can run on different game
threads (Vulkan external sync is per-object, not global), and cleared on removal
from another thread -- all with no mutex. A push_back realloc could dangle the
SwapInfo* a concurrent find_swap/present is using. It was also never pruned, so a
game that recreates its swapchain each resize grew it without bound and could
match a recycled handle's stale images.
Add g_swaps_mutex around every access; the present detour now copies the matched
swapchain's fields out under the lock and captures without holding it (no GPU
submit under the lock, no dangling pointer). Create de-dups by handle and an LRU
cap (8) bounds growth -- the active swapchain is the newest, so it's never
evicted. Deliberately NOT hooking vkDestroySwapchainKHR: forwarding a destroy
incorrectly could break the game, and the de-dup + cap already bound growth and
defeat handle recycling.
Verified real by inspection (a concurrent-create+present Vulkan race isn't
deterministically reproducible in a test); validated by the full mock_game_test
Vulkan paths (capture + layer + too-late) staying green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SharedTextureSource::update() acquired the shared-texture keyed mutex with
`AcquireSync(...) == S_OK`. But WAIT_ABANDONED -- a prior owner (e.g. a host that
crashed mid-acquire, then reconnected) died holding it -- actually GRANTS us
ownership. Treating it as failure skipped the copy AND never released, so the
next AcquireSync blocked forever and the mirror froze permanently after a crash
+ reconnect (directly relevant to the new reconnect path).
Factor the decision into keyed_mutex_acquired(HRESULT) (capture/keyed_mutex.hpp):
S_OK or WAIT_ABANDONED -> copy + release; timeout/hard errors -> skip the frame.
update() now uses it.
Test-first: keyed_mutex_test asserts WAIT_ABANDONED is treated as acquired while
the genuine "didn't get it" cases (timeout, E_FAIL, device-removed) are not. The
full cross-process abandonment is keyed-mutex OS semantics, not re-tested with a
child process here -- the predicate is the regression surface.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
D3D11Window ignored the HRESULTs from Present, ResizeBuffers, and
CreateRenderTargetView, so a host-side TDR / driver reset / GPU hang left the
render loop presenting to a dead device forever with no error.
Now note_device_loss() inspects those HRESULTs; on DXGI_ERROR_DEVICE_REMOVED/RESET
it captures GetDeviceRemovedReason() and sets device_lost(). The main loop checks
it after render_frame, shows a MessageBox with the reason, and stops cleanly.
Per the agreed scope this is detect-surface-halt, not full device re-creation
(which would have to re-init ImGui + the capture pipeline) -- that's future work.
Not unit-testable (TDR isn't deterministically reproducible); fixed by inspection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
register_ui_settings installs an ImGui settings handler whose UserData points at
the run()-local UiState. ~ImGuiLayer calls DestroyContext, which flushes the .ini
through that handler (ui_settings_write_all dereferences UserData). But UiState
was declared after ImGuiLayer in run(), so it (and the panels between them) were
destroyed first -- the shutdown save read freed/clobbered stack every clean exit
(UB; could corrupt coop_layout.ini / the persisted debug-details flag).
Declare UiState before ImGuiLayer so it outlives the context and is destroyed
last. Not unit-testable (shutdown lifetime ordering); fixed by inspection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
From an in-depth review pass: confirmed bugs, correctness/robustness items to
verify-then-fix, the D3D9/OpenGL present-thread readback perf fixes, cross-process
ABI hardening, test-coverage gaps, the static-CRT + Vulkan-layer-cleanup features,
three UX fixes, and stale-comment/VtableHook cleanup. Each lands test-first as its
own commit and is removed from this list when done.
Also drop the inaccurate completed-work-lives-in-Lessons-learned line -- done
tasks live in git history; Lessons learned is only for genuinely important
lessons.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Disconnect -> reconnect now reuses the DLL already in the game instead of
injecting again, including across a tool restart or crash: a connected DLL keeps
its per-pid shared section (and worker) alive after the host goes away, so a
fresh host can find it and re-attach to the same section.
- hook_dll_alive(pid) (host/src/inject/dll_probe.cpp): detect a live DLL by
opening the per-pid section and polling its heartbeat (returns as soon as a
beat lands; a missing section or stalled worker reads as not-alive). It does
not check magic -- a graceful disconnect zeroes magic but the DLL keeps
beating and the worker never re-checks magic post-connect.
- InjectionPanel: the Inject and Connect button branches to reconnect_selected()
when a live DLL is detected -- IpcServer::start() re-attaches to the SAME
section the DLL still holds and re-publishes the subsystem state; no
re-injection. Factored the shared post-connect setup (publish_subsystem_state
/ begin_liveness_tracking). The DLL needed no change -- it just resumes reading
the re-attached section.
- A false not-alive is benign: the inject path still re-attaches an
already-injected DLL (LoadLibrary no-ops), so the timeout only needs to clear
the worker's ~250ms beat period with margin.
Test (mock_game_test test_reconnect): inject -> hooked -> graceful disconnect ->
drop the host handle (simulating a restart while the DLL keeps the section alive)
-> detect via heartbeat -> re-attach to the same section -> hooks re-install
without re-injecting -> and hook_dll_alive goes false once the game is gone.
Roadmap: both current tasks (graceful disconnect, reconnect) done -> removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On an explicit Disconnect and on graceful tool exit, the host now asks the
injected DLL to remove every subsystem so the game runs exactly as if it was
never hooked (each hook restores its original bytes). The DLL stays injected but
dormant, ready for a later reconnect -- we never eject it.
Before, both paths just dropped the IPC channel (IpcServer::stop) without telling
the DLL, leaving the hooks active with frozen forwarded state until the game
exited.
- IpcServer::request_unhook_all() sets every subsystem_disabled flag (the DLL
reconciles to fully unhooked on its next tick); all_hooks_removed() reads the
hook registry back so the host can confirm the game is vanilla.
- InjectionPanel::disconnect_graceful() requests the unhook, waits (bounded) for
the registry to clear, then stops. Wired into the Disconnect button (700ms) and
the destructor (300ms). The flags persist in the section the DLL keeps alive, so
the unhook completes even if the host exits before confirming.
Tests (failing first):
- ipc_server_test: request_unhook_all() disables all subsystems; all_hooks_removed()
tracks the registry. Deterministic, no game.
- mock_game_test test_graceful_disconnect: inject -> hooks installed -> request
unhook-all -> every hook removed (game vanilla) while the DLL stays alive
(heartbeat advancing). Full suite still passes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Split the roadmap into Current tasks and Future work.
Current tasks (new, this session):
- Graceful disconnect asks the DLL to disable all subsystems (game runs vanilla,
DLL stays injected/dormant), on an explicit Disconnect and on graceful exit.
- Reconnect to an already-injected DLL, including across a tool restart/crash,
by detecting the live DLL via its IPC heartbeat and re-attaching to the same
per-pid section without re-injecting. DLL self-cleanup on host crash is out of
scope by design.
Future work: the pre-existing open directions (per-game profiles, multi-guest
pad mapping, raw-mouse movement forwarding, native-D3D12 capture path), moved
under their own subsection so they read as deferred rather than active.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Roadmap section had grown into a recap of finished tasks (Vulkan perf fix,
inline-hook path, uncapped mock, hook-race family, etc.). Per the rule that done
work leaves the roadmap, drop the completed narrative -- it already lives in
Lessons learned and the test suite -- and keep only the open directions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A multithreaded test that tight-looped InlineHook enable()/disable() while
other threads called the hooked function flaked ~1/10. Isolation proved this is
a SafetyHook limitation, not our code: with the hook created once (no install
race, no trampoline UAF), tight-loop toggling AVs ~1/3 of runs in Debug
(0xC0000005, faulting RIP in the target body), while a no-toggle control is
clean at ~60M calls. enable()/disable() re-patch the prologue in place under a
VEH page-trap that only relocates a thread parked ON the prologue; a thread in
the function body faults on the briefly-non-exec page and relies on instruction
retry, which under rapid toggling races a half-rewritten prologue.
Rather than silently drop the flaky test, preserve the finding:
- tools/sh_concurrency_repro/: minimal, committed, non-CI reproducer
(coop_sh_concurrency_repro; --callonly is the control). Surfaces 5/16 AVs.
- docs/safetyhook-concurrency.md: upstream-ready write-up (mechanism + fix
directions + why it does not affect us).
- README lessons-learned + memory updated; tests/CMakeLists cross-references it.
Our code stays in SafetyHook's safe envelope (install/remove reconciled from a
single tick-bounded worker thread, never a tight loop), so the mock_game_test
storm is reliably green; the persistent-trampoline contract is covered
deterministically by hook_install_test + detour_gate_test. Removes the temp
_sh_probe wiring.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hook-removal fix evolved from disable->drain->destroy to the persistent-hook
model (never free the trampoline during the session); update the roadmap summary
to match the Lessons-learned detail.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The uncapped, input-polling mock_game_test storm (thousands of presents/s, now
also driving the input/focus/MKB hooks) drove out a family of install/remove races
the slow vsync'd mock had masked. Fixes (hook/src/hook_install.hpp + hook_guard.hpp):
- Persistent hooks. The old model created a hook on install and DESTROYED it on
remove (= {}), freeing the trampoline; a detour about to call it (.stdcall) then
hit freed memory -> 0xC0000005. drain() can't fully close that window (a thread
can be inside the detour but not past its Guard ctor). So hooks are now created
ONCE and only enable()/disable()d across install/remove cycles -- never destroyed
during the session -- so a stale detour always calls a live trampoline (disabled,
it just runs the original). Reused, so no churn and no leak. remove_* therefore
disable()s + drain()s but does not destroy; install guards check .enabled().
- Install race. create_inline() enables the hook before the result is move-assigned
into the global the detour reads; a call landing in the detour mid-assign reads a
torn hook -> AV. install_inline() creates StartDisabled, assigns, then enable()s.
- drain() Sleep(1)s BEFORE each zero-check, so a thread that entered the detour but
hasn't reached its Guard registers before we conclude zero.
- Focus: publish g_orig_proc before SetWindowLongPtr activates the subclass (and
subclass_proc falls back to DefWindowProc if null); and disable the focus-query
hooks in reverse install order, because GetForegroundWindow shares user32 code
with GetActiveWindow (keep GFW hooked until GAW is unhooked).
- disable()/enable() [[nodiscard]] results are handled (logged), not (void)-discarded.
Storm now survives on every backend across repeated runs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mock backends presented with vsync ("a game-like cadence") -- wrong for a
perf/stress fixture: it does trivial work on an RTX 4090, so it must run as fast
as it can. Vsync capped them to tens of fps (dx9 30, dx10 23, dx11 63, dx12 126),
which hid both capture-induced slowdowns and the hook-removal race. Uncapped now:
dx9/dx10 INTERVAL_IMMEDIATE / Present(0,0) (BLT), dx11/dx12 ALLOW_TEARING +
Present(0, ALLOW_TEARING) (flip), gl wglSwapIntervalEXT(0), vk IMMEDIATE/MAILBOX.
Measured no-hook: dx9 ~21000, dx10 ~2800, dx11 ~17000, dx12 ~12000, gl ~26000, vk
~24000 fps.
mock_game_test now adds a present-rate floor per backend (>= 300/s while
capturing): with the hook live every backend stays in the hundreds-thousands
(vk 13500, dx11 9000+, gl 1800, dx9/10 ~1000-1600, dx12 2500). This is the
dimension the frame-advance checks missed -- the Vulkan 144->3 FPS stall still
advanced frames -- so it catches a present-thread stall OR an accidental vsync.
The faster storm exposed the hook-removal UAF fixed in the previous commit.
README roadmap + lessons-learned updated (incl. correcting the old "reset makes
in-flight trampoline calls safe" claim). Full suite 21/21.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The capture must never change vsync, and must not frame-limit itself.
- Removed the ~150 Hz capture throttle from VkCapture. It was wrong: vsync already
paces capture (a 144 Hz FIFO game presents 144x/s, so we mirror 144x/s). The only
limiter left is ring backpressure (skip a frame if the reaper is behind), which is
correctness, not a cap, and never touches the game's present thread or sync mode.
- The layer/hook already pass VkSwapchainCreateInfoKHR straight through, so the
present mode (= the sync mode) is untouched. Added a presentMode log to prove it.
Measured on Sphere Spectacle (direct launch, layer attached): presentMode=2 (FIFO),
steady 144.0 fps, and with the throttle gone the mirror now tracks it at 144/s
(was capped ~130). The earlier 400-600 fps reading was a direct-launch artifact --
a non-foreground windowed FIFO app isn't throttled by DWM -- not the layer, and not
the case through Steam (144). vk_validate now states the mirror follows the present
rate (no throttle) and still asserts a present-rate floor.
Full suite 21/21.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-ups from real-game testing:
1. The Vulkan capture must not change the swapchain sync mode, and must not
throttle/frame-limit capture (that's vsync's job). Remove the ~150 Hz throttle
and prove the present mode/rate is unchanged with vs without the layer.
2. DX12 capture overhead (~0.34 ms) is higher than DX11 (~0.05) / GL (~0.09) due
to the D3D11On12 bridge -- investigate and improve.
3. The mock game only drives D3D11/D3D12; add Vulkan/OpenGL/D3D9 renderers so every
capture path has game-driven coverage (this gap is why the Vulkan stall slipped
through). Make the mock game a real-world test across all backends.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The earlier validation concluded suspended-inject was "not applicable -- the
title requires launching through Steam." That was wrong; it was two bugs:
1. coop_vk_validate's inject mode launched the exe with CreateProcessW and a
null working directory, so the game couldn't load steam_api64.dll / resources/
(loaded relative to cwd) and never rendered -> no presents. Launch with the
game's own folder as cwd and it runs fine directly, no Steam needed.
2. The game resolves vkQueuePresentKHR / vkCreateSwapchainKHR via
vkGetInstanceProcAddr (volk's volkLoadInstance does this), but vk_hook only
substituted our detours when they were resolved via vkGetDeviceProcAddr -- so
the present bypassed the hook. Intercept those names in hk_vkGetInstanceProcAddr
too (our detours already gate on g_capture_enabled/g_device, so handing them out
before the device exists is safe).
With both fixed, inject mode captures Sphere Spectacle correctly: 1920x1080,
correct colors/orientation (screenshot), ~480 fps present while mirroring at the
~150 Hz throttle -- no present-thread impact (the VkCapture fix is shared).
Also makes the validator ASSERT a present-rate floor while capturing (it used to
report the rate and rationalize it, which is exactly what hid the 144->3 FPS
stall), and reports the true mirror rate from video.generation. Division of labor
is about who launches the game: layer for Steam-launched (can't suspend), inject
when we control the launch. README lessons-learned corrected accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Against Sphere Spectacle (144 FPS, runs without Steam) the implicit-layer
capture dropped the game to ~3 FPS. Measured cause (per-stage trace in the
layer): the read-back ran on the game's PRESENT THREAD and spent ~370 ms per
1080p frame -- not the GPU copy (~2 ms) but the CPU swizzle, because the staging
buffer was a plain HOST_VISIBLE|HOST_COHERENT type (write-combined / uncached on
a discrete GPU), where a scattered CPU read runs at PCIe latency. 3 captures/s =
the 3 FPS the user saw.
Test-first: tests/vk_capture_perf_test reproduces the stall as a deterministic
unit test (372 ms/present, ratio 1.0 -> FAIL via `--sync`), then proves the fix
(0.02 ms/present, byte-correct BGRA->RGBA, ratio ~0 -> PASS).
Fix: extract the near-identical read-back from vk_hook.cpp and coop_vk_layer.cpp
into one shared coop::hook::VkCapture that:
* has the present thread only record + submit the copy (sub-ms) and return;
* runs a dedicated reaper thread for the fence wait + swizzle + D3D upload, off
the critical path, with a ring of in-flight slots (game never waits);
* allocates HOST_CACHED staging (fast CPU read), invalidating when non-coherent;
* throttles capture to ~150 Hz (a guest stream is <= the host refresh; no point
mirroring an uncapped 400+ FPS game and burning reaper CPU).
Real-game A/B: present rate now matches the no-capture baseline (605->470 vs
593->405 over the same ramp) with the mirror at ~130 fps -- no measurable impact.
Also adds present-thread overhead guards to the other GPU backends' hook tests
(present_overhead.hpp): DX11 0.05 ms, DX12 0.34 ms, OpenGL 0.09 ms overhead, all
asserted < one 60 Hz frame, so any future synchronous-stall regression fails.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real-game testing of Sphere Spectacle (144 FPS, no focus throttle, runs without
Steam) surfaced two genuine defects the earlier "validation" missed:
1. The implicit-layer capture drops the game to ~3 FPS -- it stalls the present
thread on a full GPU readback every frame. Vulkan support is meaningless until
capture is (near-)free.
2. The inline-hook (suspended-inject) path must actually work on this title, or
be proven a true limitation -- Steam is not the reason it currently doesn't.
Final verification of both paths is to be done with the game launched through
Steam. Tracked as Current tasks; will move to Done as each lands.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds coop_vk_validate, a harness that drives the Vulkan capture path end-to-end
against a shipping title (default Sphere Spectacle, a pure-Vulkan game) and asserts
frames reach the shared texture and advance, the captured resolution/colors are
sane, saves a BMP screenshot for visual confirmation, and reports the present rate.
Findings:
- Layer method WORKS: coop_vk_layer mirrors the game correctly at 1920x1080 --
right colors/brightness, no BGRA/RGBA swizzle, no sRGB darkening (screenshot
confirmed). The layer captures every present (no drops).
- Suspended-inject "Auto-attach" is NOT applicable to a Steam title that must launch
through Steam: its exe renders nothing when launched directly, so there's no Vulkan
present to catch. The layer is the method for Steam Vulkan games (the early-inject
mechanism itself is covered by mock_game_test's suspended-launch path).
- The layer does video, but the game needs coop_hook.dll co-injected for focus-spoof
or an unfocused, event-driven game throttles itself to a few fps (looks like a
capture slowdown but isn't). A present-rate number alone can't prove "no FPS impact"
-- it's the game's own cadence; capture stays off the critical path (every present
copied, read-back on its own queue + present-semaphore re-chain).
Also adds SharedTextureSource::read_frame (bulk RGBA readback) for the screenshot.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The MKB subsystem covered message-loop (PostMessage) and polling
(GetAsyncKeyState/GetKeyboardState/GetCursorPos) games. Add the two remaining
read paths, both fed from the same synthesized state:
- DirectInput: vtable-swap IDirectInputDevice8::GetDeviceState (a COM method ->
vtable swap, not inline, per the x86 COM-prologue trap), reading the shared
vtable from a kept-alive probe device (the game made its devices before we
injected). Dispatch on cbData: 256 = keyboard BYTE[256] indexed by DIK
scan-code (map VK->DIK via MapVirtualKey VK_TO_VSC); DIMOUSESTATE = mouse
buttons + relative deltas from the forwarded cursor.
- Raw Input: games get no WM_INPUT while unfocused, so mkb_pump synthesizes it
(PostMessage WM_INPUT with lParam = one of our RAWINPUT slots) and the hooked
GetRawInputData serves that slot back (RID_HEADER + RID_INPUT). Covers keys +
buttons; relative raw-mouse movement isn't in the position-based MKB stream.
Both detours use the DetourGate safe-unhook guard, and the mock_game_test storm
exercises their install/remove. dinput_hook_test drives the real path end-to-end:
forward a key + mouse button through the ring, then a real DI keyboard/mouse
device's GetDeviceState returns them. The Raw Input round-trip is operator-
validated against a real game (too brittle to assert in-process). vtable_hook.hpp
factors the COM vtable-swap helper out for reuse.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the two-path correlation from rate-only to the full layout, removing the
"channels/bit-depth assumed = device" limitation. correlate_format tries each
candidate de-interleaving (float32 / int16; mono..7.1) of the hook capture, runs
the rate correlation per layout, and keeps whichever aligns with the loopback;
a wrong de-interleaving is noise and won't.
The catch: the hook can't know a guessed stream's real frame size, so its verify
tap pads each render buffer to the device block -- which over-reads stale staging
bytes for a stream with fewer channels/bits, scrambling the audio. So the tap is
now self-describing: it prefixes each buffer with its frame count
([count][count*device_block bytes]), and the host strips the padding per candidate
layout (take the real count*real_block of each chunk) before de-interleaving.
- audio_correlate.hpp: ChunkedCapture + chunk-aware correlate_format + candidate
layouts; absolute-margin confidence gate (the true layout scores ~1.0, a truly
ambiguous alternative within ~0.001 -- 2ch@R == 1ch@2R for identical channels --
is correctly left unconfident).
- audio_hook.cpp: chunked verify tap (free-space-checked so framing can't tear).
- audio_format_verifier: parse chunks; recover_layout path. AudioMirror now corrects
the full format.
- audio_correlation_test: layout recovery from padded chunks (stereo float, 16-bit
PCM, 5.1, mono). audio_verify_test gains scenario (b): 2ch on a multichannel
endpoint with distinct per-channel content (new env-gated ToneSource mode) ->
recovers ch=2/32-bit float end-to-end.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When the host attaches to an already-running game it never saw the stream's
Initialize, so the render-hook assumes the device mix format and measures only
the sample rate from the render cadence -- which a jittery game can make wrong
(intermittent pitch shift). But during the measurement window the game is still
audible, so we have the same audio twice: the hook (pre-mix, unknown format) and
a process-loopback (post-mix, the known device format). Cross-correlating them
pins the true rate from ground truth.
- common/include/coop/audio_correlate.hpp: the pure correlator. Resample the hook
by each candidate standard rate up to the device rate and score how well it
aligns with the loopback across the window (drift-detecting). audio_correlation_test
recovers every rate (score ~1.0 vs ~0.01 for wrong ones), incl. 44100-vs-48000,
and rejects unrelated signals.
- Hook measurement tap: a host-set verify_capture ring flag makes the hook push a
still-being-measured (guessed) stream's raw pre-mix bytes WITHOUT silencing, so
the host can co-capture both signals (a silenced game's loopback is silent).
Inert by default -- the shipping no-echo path is untouched.
- host/src/audio/audio_format_verifier: co-captures hook + loopback and correlates,
feeding a correction into the existing override channel. Wired into AudioMirror's
measurement window (hidden in the gap loopback already covers, so exact streams
pay nothing). audio_verify_test drives it end-to-end against coop_mock_game.
Rate vs layout are coupled (correlating the waveform needs the right channel
de-interleaving), so this step assumes the hook layout matches the device (the
common stereo-on-stereo case); recovering a different channel count / bit depth
is step b.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spamming a subsystem toggle (the "Mirror video" button) could crash the game:
remove_*_hooks freed a hook's shared D3D / Vulkan / IPC state immediately, while a
capture detour was still mid-flight on the game's render thread -> use-after-free.
Only the audio hooks had the safe-unhook drain; the video (Present/D3D9/D3D10/GL/
Vulkan) and XInput/focus/MKB hooks did not.
Test-first: mock_game_test now runs an aggressive hook/unhook storm -- a separate
thread thrashes every subsystem on/off while the game presents, across all backends.
It crashed gl + vk (0xC0000005) and failed dx9 capture-resume before the fix.
Fix (hook/src/hook_guard.hpp, DetourGate): each detour wraps its body in an RAII
active-count Guard; remove_* restores the hook first (so no new detour starts),
drains the in-flight detours to zero, and only then frees the shared state. Vulkan
is special-cased -- the game caches hk_vkQueuePresentKHR, so removal closes an
atomic capture gate (detours then pass through to the real present), drains, then
frees the read-back resources. Storm now passes on every backend.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a fourth Current Task: exercise the Vulkan capture path end-to-end on
Sphere Spectacle (Steam appid 1123040) via both early-presence methods
(Auto-attach and the implicit coop_vk_layer), verifying capture, image
correctness, and no performance regression.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the Future-work section with a single "Current Tasks" list:
- Injection hardening: toggling "Mirror video" has crashed a real game
(Brotato) via a hook install/remove race. Test-first task -- make
mock_game_test's hook/unhook stress aggressive enough to reproduce the
crash (tight video-subsystem toggles from a separate thread while the game
presents, across all backends), then apply the audio hooks' safe-unhook
guard (epoch bump + restore-then-drain in-flight detours) to the Present /
D3D9 / D3D10 / OpenGL / Vulkan hooks.
- Audio format by correlating the loopback (known device format) and
render-hook (unknown format) captures instead of guessing: (a) rate
verification/correction, (b) channel + bit-depth recovery.
- Mouse + keyboard forwarding (Raw Input / DirectInput) folded in.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The hooked path must capture the game's frames AND mute its local playback
("no echo"). The mute was implemented as zero-the-buffer (memset) + release
with AUDCLNT_BUFFERFLAGS_SILENT. Zeroing num_frames*block is only safe when
block is the real frame size; for a guessed format (late attach -- the
Brotato case, where we never saw Initialize) the guessed block can exceed
the real buffer, so the conservative code skipped the whole mute for guessed
streams. That left the game audible: it played locally AND the mirror
re-rendered the same audio a few ms later = a metallic, out-of-sync double.
Fix: AUDCLNT_BUFFERFLAGS_SILENT already makes WASAPI ignore the buffer
contents and play silence -- it mutes with no write at all, so it's safe for
any format. Decouple the two: always mute via the flag; keep the memset only
for an exact/override format (belt-and-suspenders). One-line behavior change;
the byte-incompatible cases confirm the flag-mute never over-writes.
Test-first (now a documented rule, README "Tests"): added an
audio_frames_silenced() counter and a mute assertion to audio_hook_test for
both the exact and guessed paths. The guessed assertion FAILS on the unfixed
code (3 rates) and passes after the fix -- the regression guard for this bug.
README also updates the now-correct no-echo limitation and adds a
lessons-learned writeup.
ctest 17/17. Brotato confirmed fixed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All of M1 (UI-fit + live inspection) and M2 (every backend mock + capture, incl.
the Vulkan implicit layer + checkbox) are done, so the "Current work" roadmap
section is removed -- only Future work (Raw Input / DirectInput MKB) remains.
Architecture lists the Vulkan layer as the implemented early-presence path;
lessons-learned add the chain-dispatch + loader sType 47/48 gotcha; build/test
docs note the layer artifact + coverage; submodule list updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The opt-in implicit-layer piece requires the loader/layer interface header
vk_layer.h, which Vulkan-Headers doesn't ship (it's in Vulkan-Loader / the SDK)
-- so it needs a new submodule or hand-declared link structs, confirming it's a
separate, higher-risk component rather than a quick add.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Architecture now lists Vulkan as a hooked producer (GPA interception + read-back,
early-presence required). Adds a lessons-learned bullet (can't late-hook Vulkan;
the present-semaphore re-chaining trap; the too-late heuristic; early-load
testing). mock_game_test docs note the suspended-launch capture + too-late check.
The Vulkan milestone is trimmed to its one remaining piece -- the opt-in implicit
layer for immediate-init games -- with the capture/banner/best-effort marked done.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reflect that the Vulkan mock backend + submodules are done, and scope the
remaining Vulkan capture honestly: hook vkGetInstance/DeviceProcAddr +
intercept device/swapchain creation, and read the presented image back with
vkCmdCopyImageToBuffer (the same read-back pattern as D3D10/D3D9/OpenGL --
simpler/robuster than a VK_KHR_external_memory keyed-mutex blit). Note testing
needs the early-load path (late-inject can't catch a Vulkan present), and Vulkan
games mirror via WGC meanwhile.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The headless ui_fit_test proves panels *fit* their windows but not that the
overlay *looks* right. Document that closing M1 requires running the actual
coop_host.exe (-DCOOP_TEST_HARNESS=ON), driving it to maximum info, taking an
F10 screenshot, and eyeballing the real overlay -- a green unit test is not a
substitute for looking at the product. Verified live: every panel fits and reads
correctly at the monitor resolution, and the live `uifit` reports "fit".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add render_gl.cpp (selectable as `gl`): renders the animated pattern with
scissored clears (glClear + glScissor -- GL 1.1, exported straight from
opengl32) and presents with SwapBuffers. No glad/submodule needed: a legacy
wglCreateContext + <GL/gl.h> suffices, so the planned loader dependency was
dropped. GL is bottom-left origin and the capture flips top-down, so the
frame-counter block is drawn at the GL top to land top-left in the captured
image. Window class gains CS_OWNDC for a stable GL DC; best-effort vsync via a
runtime wglSwapIntervalEXT lookup.
The GL SwapBuffers/wglSwapBuffers hook now bumps the shared present counter
(it's the GL present), so present_calls works for GL games too. mock_game_test
decodes GL frames through the existing glReadPixels capture path; 15/15 ctest.
Roadmap: OpenGL milestone done and removed (Vulkan renumbered to M2);
architecture/lessons/test docs updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New d3d9_hook.cpp (own file like opengl_hook): inline-hooks
IDirect3DDevice9::Present (vtable index 17, discovered from a throwaway device;
clean prologue so inline is safe, stdcall() on x86) and read-backs the backbuffer
with GetRenderTargetData into a D3DPOOL_SYSTEMMEM surface, swizzles BGRA->RGBA,
and uploads it into the shared keyed-mutex texture on a hook-owned D3D11 device
(a D3D9 surface isn't D3D11-shareable). Both plain D3D9 and D3D9Ex call this same
Present, so one read-back path serves both -- the GPU shared-surface fast path
the plan sketched for D3D9Ex wasn't worth it. Wired into the video subsystem
(install/remove in dllmain); hook links d3d9.
mock_game_test decodes DX9 + DX9Ex frames through the hook; 15/15 ctest (incl.
the x86 sub-build). Roadmap: DX9 milestone done and removed (remaining
renumbered); architecture + lessons updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A pure-D3D10 game's backbuffer QIs to ID3D11Texture2D but that view reads back
empty (content lives on the game's D3D10 device), and a D3D11 backbuffer also
QIs to ID3D10Texture2D -- so GetBuffer can't discriminate. The reliable signal
is that a feature-level-10 device rejects CreateTexture2D with the NT-handle
keyed-mutex share flags (E_INVALIDARG). The present hook now tries the D3D11
fast path and, on that failure, switches (sticky) to reading the backbuffer
through the game's own D3D10 device into a staging texture and uploading it into
the shared texture on a hook-owned D3D11 device (Map blocks until the GPU copy
completes -> no cross-device race). The host reader is unchanged.
mock_game_test now decodes DX10 frames through the hook (monotonic/advancing)
alongside DX11/DX12; 15/15 ctest. Roadmap: DX10 milestone done and removed
(remaining renumbered); architecture + lessons updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add panel overflow instrumentation (record_panel_fit reading ImGui ScrollMax),
a forced layout-reference + debug-aware center split, and host harness commands
(uisize/uifit). New headless ui_fit_test drives the real Controllers + Audio
panels at reference resolutions with Debug details on and asserts no panel
overflows its assigned size; the Audio panel gains a demo mode so its richest
content renders without a live mirror.
Fixes from the measured overflow: widen the center column (was too narrow ->
horizontal overflow), merge the Controllers poll/round-trip tables and fold the
trigger line into the slot line, and make the center height split
Debug-details-aware (Video's height is mirroring-driven, not debug-driven, so a
static split can't serve both modes). Controllers + Audio now fit at 1920x1080
with max info. Video/Injection/Log fit is finalized at the end via the live
uifit harness (M1 stays open until then).
15/15 ctest.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
