From 6bc5944c3efe5b8f7be62c1e87ab8ed93c51f8b5 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:58:02 +0200 Subject: [PATCH 1/6] fix(render): pay crop debt, pace duplication retries, bound the fresh drain by time (#497) - A cropped near-full repaint left the cache outside the view stale once the screen settled: the next uncropped frame now copies in full, and a screen quiet for 250 ms with debt owed gets a fresh grab (review #5). - A duplication that cannot be created is no longer fatal at initialize (only a missing output is), so a start during UAC or with the duplication slots taken keeps the render engine (review #20). - capture() retries a failed duplication 250 ms doubling to 2 s and logs only power-of-two failures, instead of every frame (review #21). - The fresh-grab drain stops 12 ms after the first copy, not only after 40 attempts, which ran ~110 ms on 360-500 Hz panels (review #36). Pure policies in src/render/capture_policy.h, tested. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- docs/architecture/04-render-engine.md | 14 +++- src/render/capture_policy.h | 55 +++++++++++++++ src/render/render_engine.cpp | 99 +++++++++++++++++++++++---- tests/test_capture_policy.cpp | 60 ++++++++++++++++ 4 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 src/render/capture_policy.h create mode 100644 tests/test_capture_policy.cpp diff --git a/docs/architecture/04-render-engine.md b/docs/architecture/04-render-engine.md index 8d576301..e126ccc6 100644 --- a/docs/architecture/04-render-engine.md +++ b/docs/architecture/04-render-engine.md @@ -120,14 +120,24 @@ flowchart TD because the cursor comes from `GetCursorInfo`. - **Fresh grabs** (zoom-in, `invalidateCapture()`) drain to the latest frame, not the first: the first frame after recreating a duplication can be a transitional composite, which flashed the - window underneath. Bounded at ~3 ms per attempt and 100 ms in total. + window underneath. Bounded at 100 ms until the first frame is copied, then ~3 ms per attempt and + 12 ms of draining (`FreshDrainContinue`, `src/render/capture_policy.h`): a 360-500 Hz panel + delivers a frame inside every wait, which once ran all 40 attempts. +- **A duplication that cannot be created** (secure desktop, recorders holding the output's + duplication slots) is retried lazily by `capture()`, 250 ms doubling to 2 s (`DuplRetry`), logging + only the 1st, 2nd, 4th... failure; the cached frame stays on screen. A zoom-in, retarget or device + rebuild resets the pace. It is not fatal at `initialize` either: only a target output missing from + the adapter is. - **Dirty rects.** `copyChangedRegions` patches only the rects DDA reports and falls back to a full `CopyResource` whenever a partial update is not provably safe (no previous frame, move rects, missing metadata, out-of-range rect). - **Crop to the view.** On a near-full repaint the copy can be cropped to the magnified view. `cropCapture=0` by default (a desktop window switch would leave stale pixels outside the view); `gameCrop=1` (default) crops while the foreground covers the monitor, where every pixel is dirty - again next frame. + again next frame. That stops holding once the screen settles (a page turn in a fullscreen + viewer, Win+D), so a crop leaves a debt (`CropDebt`): the next uncropped frame is copied in full, + and a screen quiet for 250 ms with the debt owed gets a fresh grab (the duplication is re-created; + its first frame is the whole desktop). - Rotated outputs are not supported: `recreateDupl` logs them, the engine pick sends them to Transform, and a captured surface whose size differs from the desktop is never copied (the last good frame stays, logged once). diff --git a/src/render/capture_policy.h b/src/render/capture_policy.h new file mode 100644 index 00000000..9c1ea593 --- /dev/null +++ b/src/render/capture_policy.h @@ -0,0 +1,55 @@ +#pragma once +// Pure Desktop Duplication capture policies (no ), used by RenderEngine::State::capture. +// Tests: tests/test_capture_policy.cpp. +namespace wind { + +// Retry pacing for a duplication that cannot be (re)created (review 2026-10-16 #21): while the +// secure desktop is up or another tool holds the output's duplication slots, DuplicateOutput fails +// every call. Retrying per frame redid the output and HDR queries and logged 2-3 lines each frame. +// The first retry waits `firstMs`, each further failure doubles the wait up to `capMs`; a success +// or a deliberate fresh start (zoom-in, retarget, device rebuild) resets it. +struct DuplRetry { + unsigned long long nextMs = 0; + unsigned long long delayMs = 0; + unsigned failures = 0; + bool due(unsigned long long nowMs) const { return failures == 0 || nowMs >= nextMs; } + void fail(unsigned long long nowMs, unsigned long long firstMs = 250, unsigned long long capMs = 2000) { + delayMs = delayMs == 0 ? firstMs : (delayMs * 2 > capMs ? capMs : delayMs * 2); + nextMs = nowMs + delayMs; + ++failures; + } + void reset() { nextMs = 0; delayMs = 0; failures = 0; } + // Log the 1st, 2nd, 4th, 8th... failure only, so a long lock screen stays a few lines. + bool logThisFailure() const { return failures != 0 && (failures & (failures - 1)) == 0; } +}; + +// Crop debt (review 2026-10-16 #5). A cropped frame (near-full repaint, dirty rects clipped to the +// magnified view) leaves the cached desktop outside the view as it was. That only stays invisible +// while the screen keeps repainting; once it settles (a page turn in a fullscreen viewer, Win+D), +// panning showed the old pixels. So a crop leaves a debt that the next full copy pays: the next +// uncropped frame copies in full, and a screen that goes quiet for `quietMs` with debt owed gets a +// fresh grab (a re-created duplication hands back the whole current desktop). +struct CropDebt { + bool owed = false; + unsigned long long lastMs = 0; // last crop, or the last refresh asked for + void noteCropped(unsigned long long nowMs) { owed = true; lastMs = nowMs; } + void paid() { owed = false; } + // An uncropped frame arrived: copy it whole instead of patching its dirty rects. + bool fullCopyNow(bool croppingThisFrame) const { return owed && !croppingThisFrame; } + // The screen has been quiet since the last crop: ask for a fresh grab, at most once per quietMs. + bool refreshDue(unsigned long long nowMs, unsigned long long quietMs) const { + return owed && nowMs - lastMs >= quietMs; + } + void refreshAsked(unsigned long long nowMs) { lastMs = nowMs; } +}; + +// Whether a fresh grab keeps draining toward the newest frame (review 2026-10-16 #36). Before any +// frame was copied the whole grab is bounded by `firstBudgetMs`; after the first copy only +// `drainBudgetMs` more is spent, however fast new frames arrive (a 360-500 Hz panel delivers one +// inside every short wait, which ran all 40 attempts: ~110 ms on the tick thread per zoom-in). +inline bool FreshDrainContinue(bool copied, double msSinceStart, double msSinceFirstCopy, + double firstBudgetMs = 100.0, double drainBudgetMs = 12.0) { + return copied ? msSinceFirstCopy < drainBudgetMs : msSinceStart < firstBudgetMs; +} + +} // namespace wind diff --git a/src/render/render_engine.cpp b/src/render/render_engine.cpp index 5aa877ff..b5fdc0a4 100644 --- a/src/render/render_engine.cpp +++ b/src/render/render_engine.cpp @@ -12,6 +12,7 @@ #include "upscale_pass.h" // the xBR / NIS scaling filters (issue #456) #include "dwm_watch.h" // DwmGeneration: re-apply the overlay's exclusions after a DWM restart (#458) #include "hlsl_compile.h" // CompileHlsl: the one D3DCompile wrapper +#include "capture_policy.h" // DuplRetry, CropDebt, FreshDrainContinue (pure, tested) #include #include #include @@ -73,6 +74,9 @@ struct RenderEngine::State { ComPtr desktopSRV; bool haveDesktop = false; bool freshCapture = false; // next capture() must drain to the latest desktop frame (zoom-in) + DuplRetry duplRetry; // paces re-creating a duplication that keeps failing (review 2026-10-16 #21) + bool duplNoOutput = false; // the last recreateDupl found no output for targetDevice on our adapter + CropDebt cropDebt; // a cropped copy left the cache outside the view stale (review 2026-10-16 #5) LONGLONG primeQpc = 0; // QPC at the last primeReveal() (evidence gate, issue #140) LONGLONG lastCopiedPresentQpc = 0; // LastPresentTime of the newest desktop frame we copied ComPtr revealFence; // event query: has the reveal frame's Present executed on the GPU? @@ -183,8 +187,9 @@ struct RenderEngine::State { // Patch only DDA's changed (dirty) rects into desktopCopy from `src` (steady-state fast path). // Returns false when a partial update isn't safe, so the caller does a full CopyResource. When // `crop` and the dirty area is a near-full repaint, copies only the parts within `view`. + // `cropped` is set when a rect was clipped (the cache outside `view` was left as it was). bool copyChangedRegions(ID3D11Texture2D* src, const DXGI_OUTDUPL_FRAME_INFO& fi, - const RECT& view, bool crop); + const RECT& view, bool crop, bool& cropped); void render(const RenderFrameParams& p); // draw into the back-buffer (no Present) UpscalePass upscale; // renderScaling xBR / NIS (issue #456) }; @@ -259,13 +264,16 @@ bool RenderEngine::State::recreateDupl() { RLog("dwm restarted: overlay exclusions re-applied (affinity=0x%lx)", (unsigned long)aff); } dupl.Reset(); + // A retry after a failure stays quiet until it succeeds (capture() logs the failures, paced). + const bool quiet = duplRetry.failures > 0; // Capture the target monitor's output (matched by device name). Only the legacy // single-monitor path (empty targetDevice) falls back to the first output: a NAMED monitor // that is not on our adapter (multi-GPU) fails, as retarget() does, rather than silently // capturing another monitor's pixels. IDXGIOutput* output = selectOutput(targetDevice, /*fallbackToFirst=*/targetDevice[0] == 0); - if (!output) { RLog("recreateDupl: no output for targetDevice=%ls on our adapter", targetDevice); return false; } - RLog("recreateDupl: targetDevice=%ls", targetDevice[0] ? targetDevice : L"(first)"); + duplNoOutput = output == nullptr; + if (!output) { if (!quiet) RLog("recreateDupl: no output for targetDevice=%ls on our adapter", targetDevice); return false; } + if (!quiet) RLog("recreateDupl: targetDevice=%ls", targetDevice[0] ? targetDevice : L"(first)"); // Diagnostics: the output's color space + bit depth (HDR detection). IDXGIOutput6* output6 = nullptr; if (SUCCEEDED(output->QueryInterface(__uuidof(IDXGIOutput6), (void**)&output6)) && output6) { @@ -292,14 +300,14 @@ bool RenderEngine::State::recreateDupl() { hr = output5->DuplicateOutput1(device.Get(), 0, ARRAYSIZE(fmts), fmts, dupl.ReleaseAndGetAddressOf()); output5->Release(); if (SUCCEEDED(hr) && dupl) { capFp16 = true; RLog("recreateDupl: DuplicateOutput1 FP16 OK"); } - else RLog("recreateDupl: DuplicateOutput1 FP16 failed hr=0x%08lX", (unsigned long)hr); + else if (!quiet) RLog("recreateDupl: DuplicateOutput1 FP16 failed hr=0x%08lX", (unsigned long)hr); } } if (FAILED(hr)) { // SDR, not opted in, or FP16 unavailable -> plain duplication IDXGIOutput1* output1 = nullptr; output->QueryInterface(__uuidof(IDXGIOutput1), (void**)&output1); if (output1) { hr = output1->DuplicateOutput(device.Get(), dupl.ReleaseAndGetAddressOf()); output1->Release(); } - RLog("recreateDupl: DuplicateOutput hr=0x%08lX capFp16=0", (unsigned long)hr); + if (!quiet || SUCCEEDED(hr)) RLog("recreateDupl: DuplicateOutput hr=0x%08lX capFp16=0", (unsigned long)hr); } waitOutput = output; // keep a ref for waitVBlank (released with the device set) SafeRelease(output); @@ -352,7 +360,8 @@ bool RenderEngine::State::ensureDesktopCopy(DXGI_FORMAT fmt) { // run before ReleaseFrame() while `src` is still valid. A bail-out after some sub-copies is safe: // the caller's full copy overwrites everything anyway. bool RenderEngine::State::copyChangedRegions(ID3D11Texture2D* src, const DXGI_OUTDUPL_FRAME_INFO& fi, - const RECT& view, bool crop) { + const RECT& view, bool crop, bool& cropped) { + cropped = false; if (!haveDesktop) return false; // no valid previous frame to patch onto const UINT meta = fi.TotalMetadataBufferSize; if (meta == 0) return false; // no dirty/move metadata -> full copy @@ -375,7 +384,10 @@ bool RenderEngine::State::copyChangedRegions(ID3D11Texture2D* src, const DXGI_OU // screen). Then copying just the view saves the most, and because the screen keeps fully // repainting, regions panned-to are dirty again next frame -> no visible staleness. Small // desktop changes (notifications, clocks) keep being copied in full, so panning to them later is - // never stale. Threshold: dirty area covers > 50% of the screen. + // never stale. Threshold: dirty area covers > 50% of the screen. The "keeps repainting" part + // does not hold once the screen settles (a page turn in a fullscreen viewer, Win+D), so a crop + // leaves a debt (cropDebt): the next uncropped frame is copied in full, and capture() asks for + // a fresh grab when the screen goes quiet first (review 2026-10-16 #5). long long dirtyArea = 0; for (UINT i = 0; i < n; ++i) { const RECT& r = rects[i]; @@ -383,13 +395,16 @@ bool RenderEngine::State::copyChangedRegions(ID3D11Texture2D* src, const DXGI_OU dirtyArea += (long long)(r.right - r.left) * (r.bottom - r.top); } const bool cropNow = crop && dirtyArea * 2 > (long long)sw * sh; + if (cropDebt.fullCopyNow(cropNow)) return false; // pay the debt: this frame is copied whole for (UINT i = 0; i < n; ++i) { RECT r = rects[i]; if (cropNow) { // clamp to the magnified view (skip if fully outside) + const RECT raw = r; if (r.left < view.left) r.left = view.left; if (r.top < view.top) r.top = view.top; if (r.right > view.right) r.right = view.right; if (r.bottom > view.bottom) r.bottom = view.bottom; + if (raw.right > raw.left && raw.bottom > raw.top && !EqualRect(&raw, &r)) cropped = true; } if (r.right <= r.left || r.bottom <= r.top) continue; // empty (or fully outside view) if (r.left < 0 || r.top < 0 || r.right > sw || r.bottom > sh) // out of range -> bail to full copy @@ -401,7 +416,21 @@ bool RenderEngine::State::copyChangedRegions(ID3D11Texture2D* src, const DXGI_OU } bool RenderEngine::State::capture(const RECT& view, bool crop) { - if (!dupl && !recreateDupl()) return false; + if (!dupl) { + // Paced (review 2026-10-16 #21): while the secure desktop is up or the output's duplication + // slots are taken, every attempt fails. The cached frame stays on screen meanwhile. + const unsigned long long nowMs = GetTickCount64(); + if (!duplRetry.due(nowMs)) return false; + if (!recreateDupl()) { + duplRetry.fail(nowMs); + if (duplRetry.logThisFailure()) + RLog("capture: duplication unavailable (%s), failure %u, next try in %llu ms", + duplNoOutput ? "no output" : "DuplicateOutput failed", duplRetry.failures, duplRetry.delayMs); + return false; + } + if (duplRetry.failures) RLog("capture: duplication back after %u failed tries", duplRetry.failures); + duplRetry.reset(); + } // A settled desktop polls non-blocking (0 ms, 1 attempt): a static screen returns // WAIT_TIMEOUT immediately and we re-pan the cached copy, so panning is never gated on a @@ -422,16 +451,42 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { // tick + input sampling on zoom-in. Normally the first AcquireNextFrame returns immediately, so // this only caps the pathological case; if we give up frameless, fresh stays true (haveDesktop // unchanged) and the next tick retries. - const unsigned long long freshDeadlineMs = fresh ? GetTickCount64() + 100 : 0; + // Once a frame is copied, the drain toward a newer one is bounded by time too, not only by the + // attempt count: on a 360-500 Hz panel a new frame lands inside every 3 ms wait, which ran all + // 40 attempts (~110 ms on the tick thread, review 2026-10-16 #36). QPC: GetTickCount64 ticks at + // ~15.6 ms, coarser than the drain budget. + LARGE_INTEGER qf{}, q0{}, qFirst{}; + if (fresh) { QueryPerformanceFrequency(&qf); QueryPerformanceCounter(&q0); } + auto msSince = [&](const LARGE_INTEGER& from) { + LARGE_INTEGER qn; QueryPerformanceCounter(&qn); + return (double)(qn.QuadPart - from.QuadPart) * 1000.0 / (double)qf.QuadPart; + }; bool gotThisCall = false; for (int a = 0; a < firstAttempts; ++a) { - if (fresh && !gotThisCall && GetTickCount64() >= freshDeadlineMs) break; // budget spent, no frame yet + if (fresh && !FreshDrainContinue(gotThisCall, msSince(q0), gotThisCall ? msSince(qFirst) : 0.0)) + break; // budget spent (no frame yet, or the drain after the first copy) IDXGIResource* res = nullptr; DXGI_OUTDUPL_FRAME_INFO fi{}; const DWORD to = gotThisCall ? 3 : firstTimeout; // after a copy, only briefly seek a newer frame HRESULT hr = dupl->AcquireNextFrame(to, &fi, &res); - if (hr == DXGI_ERROR_WAIT_TIMEOUT) { SafeRelease(res); if (gotThisCall) break; continue; } + if (hr == DXGI_ERROR_WAIT_TIMEOUT) { + SafeRelease(res); + // A settled screen with crop debt owed: ask for a fresh grab (the next capture re-creates + // the duplication, whose first frame is the whole current desktop). The cached copy + // stays on screen meanwhile; haveDesktop is kept so nothing goes black. + if (!fresh) { + const unsigned long long nowMs = GetTickCount64(); + if (cropDebt.refreshDue(nowMs, 250)) { + cropDebt.refreshAsked(nowMs); + dupl.Reset(); + freshCapture = true; + return haveDesktop; + } + } + if (gotThisCall) break; + continue; + } if (hr == DXGI_ERROR_ACCESS_LOST) { SafeRelease(res); dupl.Reset(); return gotThisCall || haveDesktop; } if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET) { SafeRelease(res); deviceLost = true; RLog("capture: device lost hr=0x%08lX", (unsigned long)hr); @@ -468,12 +523,18 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { dupl->ReleaseFrame(); return haveDesktop; } - if (fresh || !copyChangedRegions(tex, fi, view, crop)) + bool cropped = false; + if (fresh || !copyChangedRegions(tex, fi, view, crop, cropped)) { ctx->CopyResource(desktopCopy.Get(), tex); + cropDebt.paid(); + } else if (cropped) { + cropDebt.noteCropped(GetTickCount64()); + } if (!haveDesktop) RLog("capture: first frame acquiredFormat=%u copyFormat=%u capFp16=%d", (unsigned)td.Format, (unsigned)copyFormat, (int)capFp16); haveDesktop = true; + if (!gotThisCall) QueryPerformanceCounter(&qFirst); gotThisCall = true; lastCopiedPresentQpc = fi.LastPresentTime.QuadPart; // when DWM composited this frame } @@ -754,7 +815,17 @@ bool RenderEngine::State::buildDeviceResources() { } // --- Desktop Duplication --- - if (!recreateDupl()) { RLog("buildDeviceResources: recreateDupl failed"); return false; } + // A duplication that cannot be created right now is not fatal (review 2026-10-16 #20): the secure + // desktop (UAC, lock screen) or recorders holding the output's duplication slots refuse it for a + // while, and capture() re-creates it lazily, paced by duplRetry. Failing here made a start during + // a UAC prompt transform-only until Wind restarted, and a device rebuild loop every 0.5 s. Only + // a target output missing from our adapter stays fatal: no retry can capture it. + duplRetry.reset(); + if (!recreateDupl()) { + if (duplNoOutput) { RLog("buildDeviceResources: recreateDupl failed (no output)"); return false; } + duplRetry.fail(GetTickCount64()); + RLog("buildDeviceResources: recreateDupl failed (non-fatal, capture retries)"); + } RLog("buildDeviceResources done: capFp16=%d sdrWhiteNits=%.1f", (int)capFp16, sdrWhiteNits); deviceLost = false; return true; @@ -913,6 +984,7 @@ bool RenderEngine::revealFrameDone(double spinBudgetMs) { void RenderEngine::invalidateCapture() { if (!s_) return; s_->dupl.Reset(); + s_->duplRetry.reset(); // a deliberate fresh start (zoom-in/out) tries at once s_->haveDesktop = false; s_->freshCapture = true; } @@ -1036,6 +1108,7 @@ bool RenderEngine::retarget(const MonitorTarget& m) { SetWindowPos(s_->hwnd, HWND_TOPMOST, px, m.y, m.w, m.h, SWP_NOACTIVATE); lstrcpynW(s_->targetDevice, m.device, 32); s_->dupl.Reset(); + s_->duplRetry.reset(); s_->haveDesktop = false; s_->freshCapture = true; s_->lastClickX = s_->lastClickY = INT_MIN; // don't skip the first SetCursorPos on the new monitor diff --git a/tests/test_capture_policy.cpp b/tests/test_capture_policy.cpp new file mode 100644 index 00000000..603dc5af --- /dev/null +++ b/tests/test_capture_policy.cpp @@ -0,0 +1,60 @@ +#include "doctest.h" +#include "capture_policy.h" +using namespace wind; + +TEST_CASE("duplication retry: immediate first try, then 250 ms doubling to a 2 s cap") { + DuplRetry r; + CHECK(r.due(0)); + r.fail(1000); + CHECK(r.delayMs == 250); + CHECK_FALSE(r.due(1249)); + CHECK(r.due(1250)); + r.fail(1250); CHECK(r.delayMs == 500); + r.fail(1750); CHECK(r.delayMs == 1000); + r.fail(2750); CHECK(r.delayMs == 2000); + r.fail(4750); CHECK(r.delayMs == 2000); // capped + CHECK(r.failures == 5); + r.reset(); + CHECK(r.due(0)); + CHECK(r.failures == 0); +} + +TEST_CASE("duplication retry logs only power-of-two failures") { + DuplRetry r; + CHECK_FALSE(r.logThisFailure()); + int logged = 0; + for (int i = 0; i < 100; ++i) { r.fail((unsigned long long)i * 5000); if (r.logThisFailure()) ++logged; } + CHECK(logged == 7); // 1 2 4 8 16 32 64 +} + +TEST_CASE("crop debt: a crop is paid by the next uncropped frame") { + CropDebt d; + CHECK_FALSE(d.fullCopyNow(false)); + d.noteCropped(100); + CHECK_FALSE(d.fullCopyNow(true)); // still a full repaint: keep cropping + CHECK(d.fullCopyNow(false)); // the screen settled into small changes: copy whole + d.paid(); + CHECK_FALSE(d.fullCopyNow(false)); +} + +TEST_CASE("crop debt: a quiet screen asks for one fresh grab per quiet window") { + CropDebt d; + CHECK_FALSE(d.refreshDue(10000, 250)); // nothing owed + d.noteCropped(1000); + CHECK_FALSE(d.refreshDue(1249, 250)); + CHECK(d.refreshDue(1250, 250)); + d.refreshAsked(1250); + CHECK_FALSE(d.refreshDue(1400, 250)); // not re-asked every tick + CHECK(d.refreshDue(1500, 250)); // the grab did not land: ask again later + d.paid(); + CHECK_FALSE(d.refreshDue(5000, 250)); +} + +TEST_CASE("fresh drain: 100 ms for the first frame, then 12 ms of draining") { + CHECK(FreshDrainContinue(false, 0.0, 0.0)); + CHECK(FreshDrainContinue(false, 99.0, 0.0)); + CHECK_FALSE(FreshDrainContinue(false, 100.0, 0.0)); + CHECK(FreshDrainContinue(true, 5.0, 0.0)); + CHECK(FreshDrainContinue(true, 50.0, 11.9)); // the first-frame budget no longer applies + CHECK_FALSE(FreshDrainContinue(true, 20.0, 12.0)); // fast panel: frames keep coming, stop anyway +} From c0161df9507dd65f2f23d76ee33d69f5d42ea677 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:58:02 +0200 Subject: [PATCH 2/6] fix(engine): keep transform on the primary monitor, end the render session on device-lost failover (#497) - Transform's visual offsets are monitor-local while its input mapping adds the origin, so a non-primary pick (protected, excluded, rotated or pinned) magnified the wrong region. Every pick off the primary is now render (except while the render device is lost), and a session only moves to a non-primary monitor when the render engine took the retarget (MayRetargetSession). No origin offset was added: nothing in the repo proves the DWM coordinate convention there (review #11). - The device-lost failover now runs TeardownSession for any active render session (a 1x Inspect one included) and clears prevActive/prevInspect, so the overlay is hidden and the next active tick enters a fresh transform session (review #8). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- docs/architecture/02-tick-loop.md | 5 ++-- docs/architecture/03-engines.md | 22 +++++++++++++---- src/core/main.cpp | 23 +++++++++++++---- src/engine/engine_pick.h | 41 ++++++++++++++++++++++++------- tests/test_engine_pick.cpp | 36 +++++++++++++++++++++++++-- 5 files changed, 104 insertions(+), 23 deletions(-) diff --git a/docs/architecture/02-tick-loop.md b/docs/architecture/02-tick-loop.md index 3b135788..fbdcc368 100644 --- a/docs/architecture/02-tick-loop.md +++ b/docs/architecture/02-tick-loop.md @@ -199,8 +199,9 @@ Device-lost recovery lives in the main loop: restore the cursor, clean Inspect s churny backstop if a transform game session was live in the last 30 s, rebuild on a 500 ms backoff. While a transform session runs the idle render engine cannot notice a TDR itself, so the loop polls `GetDeviceRemovedReason` (`RenderEngine::pollDeviceRemoved`). A render session that loses its device -ends at 1x and hands over to the transform engine instead of freezing the loop; picks go to -Transform while the device stays lost (`EnginePickInputs::renderLost`). +is torn down (`TeardownSession`, a 1x Inspect session included) and the view goes to the transform +engine at 1x instead of freezing the loop; the next active tick enters a fresh transform session. +Picks go to Transform while the device stays lost (`EnginePickInputs::renderLost`). ## Idle: the loop sleeps at 1x diff --git a/docs/architecture/03-engines.md b/docs/architecture/03-engines.md index 497735ea..b92f393c 100644 --- a/docs/architecture/03-engines.md +++ b/docs/architecture/03-engines.md @@ -50,12 +50,20 @@ and only then does an engine change relaunch Wind (Settings' Restart Wind button If the transform half fails to initialize, Auto logs a warning and runs render only; every pick site checks `t.mTransform`. +**Transform is primary-only.** Its visual offsets are monitor-local while its input transform, edge +clip and sprite add the monitor origin, so on another monitor the picture and the input mapping +would describe different rects (review 2026-10-16 #11). Every pick off the primary returns Render, +except while the render device is lost. A session moves to a non-primary monitor only when the +render engine took the retarget (`MayRetargetSession`): a render engine on another adapter, or a +transform-only process, keeps the session where it was. + ## The pick `wind::ShouldPickTransform` (`src/engine/engine_pick.h`) is header-only, has no `` and is doctested. It runs at the zoom-in edge and every zoomed tick, with the same `EnginePickInputs`. In order: +0. **A lost render device forces Transform; otherwise a non-primary monitor forces Render.** 1. **Protected content forces Transform.** `captureProtected` (the foreground or a child window has display-affinity capture protection: Netflix, Apple TV, PlayReady) or an exe on `renderExclude`. On Render such a window is a black rectangle, so this beats every other rule. @@ -64,8 +72,8 @@ In order: 2. **The per-category preference applies next.** `ClassifyWindow` sorts the foreground into Game, Acrylic, Desktop or Other, and `engineGame`, `engineAcrylic`, `engineDesktop` and `engineOther` (each Auto, Transform or Render) choose for it. Render is honoured outright; Transform still - needs the primary monitor and an exe not on `transformExclude`. -3. **Auto rule.** Transform when the session is on the primary monitor, the exe is not on + needs an exe not on `transformExclude`. +3. **Auto rule.** Transform when the exe is not on `transformExclude` or in `churny_apps.txt` (unless `tdrTest` is on), and either: - **game path**: the foreground covers the monitor, is borderless and is not the shell desktop (Win+D reads as a borderless cover, issue #172); or @@ -78,16 +86,20 @@ In order: ```mermaid flowchart TD - A[Zoom-in edge or foreground change while zoomed] --> D{capture-protected, renderExclude or rotated output?} + A[Zoom-in edge or foreground change while zoomed] --> L{render device lost?} + L -- yes --> T + L -- no --> M{primary monitor?} + M -- no --> R + M -- yes --> D{capture-protected, renderExclude or rotated output?} D -- yes --> T[TRANSFORM] D -- no --> PR{category preference} PR -- Render --> R[RENDER] - PR -- Transform --> PX{primary and not transformExclude?} + PR -- Transform --> PX{not transformExclude?} PX -- yes --> T PX -- no --> R PR -- Auto --> B{game path or desktop path?} B -- no --> R - B -- yes --> P{primary, not excluded, not churny?} + B -- yes --> P{not excluded, not churny?} P -- yes --> T P -- no --> R ``` diff --git a/src/core/main.cpp b/src/core/main.cpp index 0cdcd8e2..415e5e1d 100644 --- a/src/core/main.cpp +++ b/src/core/main.cpp @@ -1486,13 +1486,20 @@ static void RetargetSessionMonitor(TickState& t, bool zoomed, bool inspect) { if (zoomed || inspect) { MonitorTarget nt = t.cfg.multiMonitor ? MonitorUnderCursor() : PrimaryMonitor(); IMagnifierModel* rt = t.mRender ? t.mRender : t.model; - bool ok = SameMonitor(nt, t.mon) ? false : rt->retarget(nt); - const bool renderOk = ok; + // The transform engine magnifies the primary only (engine_pick.h): a transform-only + // process (rt is the transform) never moves a session to another monitor. + const bool ntPrimary = nt.x == 0 && nt.y == 0; + const bool rtIsRender = dynamic_cast(rt) != nullptr; + bool ok = SameMonitor(nt, t.mon) ? false + : (wind::MayRetargetSession(ntPrimary, rtIsRender) && rt->retarget(nt)); + const bool renderOk = ok && rtIsRender; const bool hadTransformHalf = t.mTransform && t.mTransform != rt; // The transform half owns its own bounds and must follow even when the render // half refuses (retarget returns false across adapters, where the overlay cannot - // move but the transform is still valid on the new geometry). - if (!SameMonitor(nt, t.mon) && t.mTransform && t.mTransform != rt) { + // move but the transform is still valid on the new geometry), but only onto the + // primary: on any other monitor it would magnify the wrong region (review 2026-10-16 #11). + if (!SameMonitor(nt, t.mon) && t.mTransform && t.mTransform != rt && + wind::MayRetargetSession(ntPrimary, renderOk)) { if (t.mTransform->retarget(nt)) ok = true; } // Hybrid with a render half that refused while the transform half took it: the @@ -3709,7 +3716,13 @@ int WINAPI wWinMain(HINSTANCE hInst, HINSTANCE, PWSTR, int) { // transform engine all ride RunTick): end the dead render session, hand the view // to the transform engine at a clean 1x and keep ticking. Picks stay on transform // while the device is lost (EnginePickInputs::renderLost); recovery below retries. - if (ts.prevLvl > 1.0) ts.model->setActive(false); // parks the overlay; safe when lost + // Any active render session ends here, a 1x Inspect one included (gating on the + // level left its overlay up, frozen), and the next active tick starts a fresh + // transform session through EnterSession (a held zoom bind otherwise ramped on + // without the transform's enter: review 2026-10-16 #8). Teardown hides the overlay + // by layer alpha, which is safe while the device is lost. + if (ts.prevActive) TeardownSession(ts); + ts.prevActive = false; ts.prevInspect = false; ts.model = ts.mTransform; ts.prevLvl = 1.0; ts.zoom = ZoomController(1.0, ts.cfg.maxLevel); } diff --git a/src/engine/engine_pick.h b/src/engine/engine_pick.h index 896eb8d5..91314da3 100644 --- a/src/engine/engine_pick.h +++ b/src/engine/engine_pick.h @@ -3,8 +3,18 @@ // predicate in the app is unit-tested (issues #148 exclusion, #172 shell desktop) instead of // living inline in two RunTick sites that had to stay identical by hand. // -// Order of the rules (see ShouldPickTransform): capture-protected, renderExclude or rotated output -// -> transform always; then the per-window-category preference; then the Auto rule. +// Order of the rules (see ShouldPickTransform): a lost render device -> transform; then a non-primary +// monitor -> render; then capture-protected, renderExclude or rotated output -> transform; then the +// per-window-category preference; then the Auto rule. +// +// The transform engine magnifies the PRIMARY monitor only. Its visual offsets are monitor-local +// (ComputeMagTransform gets the mapper's source rect without the monitor origin) while its input +// transform, edge clip and sprite add the origin, so on another monitor the picture and the input +// mapping describe two different rects (review 2026-10-16 #11). Nothing in the repo pins down DWM's +// coordinate convention for MagSetFullscreenTransform off the primary, so no offset is added there: +// a non-primary session gets render (a rotated or protected secondary shows its known render limits +// instead of a wrong region), and a session only moves to a non-primary monitor when the render +// engine took the retarget (MayRetargetSession). // // Auto picks transform on the PRIMARY target monitor for (a) a borderless foreground that covers // the monitor (games, F11 video: compositor-internal magnification survives a heavy game's present @@ -75,7 +85,11 @@ inline bool ShouldPickTransform(const EnginePickInputs& in) { // already mitigate. Netflix inside a browser is both at once, and this is the case that // ordering resolves: black video every time beats an occasional crash risk. // A lost render device is the same hard limit: it cannot draw anything, so transform it is. - if (in.renderLost || in.captureProtected || in.renderExcluded || in.rotatedOutput) return true; + // The one exception to the primary-only rule: a lost device cannot draw at all, and the + // device-lost failover hands every session to transform until recovery. + if (in.renderLost) return true; + if (!in.primaryMonitor) return false; // transform is primary-only (header note) + if (in.captureProtected || in.renderExcluded || in.rotatedOutput) return true; // 2. An explicit user preference for this window category. Transform is still refused off the // primary monitor (no cross-adapter transform chase) and on an excluded exe, because those // are correctness limits rather than taste. The churny list and the input-transform check @@ -83,15 +97,14 @@ inline bool ShouldPickTransform(const EnginePickInputs& in) { // explicit per-category Transform choice is the user accepting that trade (review // 2026-10-09 #44). if (in.pref == EnginePref::Render) return false; - if (in.pref == EnginePref::Transform) return in.primaryMonitor && !in.excluded; + if (in.pref == EnginePref::Transform) return !in.excluded; // 3. Auto: the historical behaviour, unchanged. // The GAME path: a borderless cover that is not the shell desktop (issue #172 - Win+D is // not a game). The DESKTOP path: explicit opt-in + verified input transform; the shell // desktop is fine there (that IS the desktop). Exclusions and the churny list veto both. const bool game = in.coversMonitor && in.borderless && !in.shellDesktop; const bool desktop = in.desktopTransformOptIn && in.inputTransformOk; - return (game || desktop) && in.primaryMonitor && - !in.excluded && (in.tdrHarness || !in.churny); + return (game || desktop) && !in.excluded && (in.tdrHarness || !in.churny); } // Which category a foreground falls into, from signals the caller has already read. Order is @@ -107,12 +120,22 @@ inline WindowCategory ClassifyWindow(bool coversMonitor, bool borderless, bool s // THE ENGINE SETTING (`model`), applied live. Both engines are built at startup whatever `model` // says, so a change takes effect on the next pick without a restart. Auto: the automatic rule above. -// A pinned engine: Transform always; Render unless the render engine cannot draw at all (device -// lost) or its overlay sits on the wrong monitor (renderStale), where showing nothing is worse. +// A pinned engine: Transform on the primary monitor (render elsewhere, see the header note); Render +// unless the render engine cannot draw at all (device lost) or its overlay sits on the wrong monitor +// (renderStale), where showing nothing is worse. renderStale only happens on the primary +// (MayRetargetSession keeps a session off a non-primary monitor the render engine refused). inline bool PickTransformFor(const std::string& model, const EnginePickInputs& in, bool renderStale) { - if (model == "transform") return true; + if (model == "transform") return in.primaryMonitor || in.renderLost; if (model == "render") return in.renderLost || renderStale; return ShouldPickTransform(in) || renderStale; } +// Whether a zoom session may move to another monitor (multiMonitor=1). The primary is always fine: +// either engine can magnify it. A non-primary monitor needs the render engine to have taken the +// retarget; a render engine on another adapter refuses it, and a transform-only process (the render +// engine failed to start) has none, so the session stays where it is. +inline bool MayRetargetSession(bool targetPrimary, bool renderTookIt) { + return targetPrimary || renderTookIt; +} + } // namespace wind diff --git a/tests/test_engine_pick.cpp b/tests/test_engine_pick.cpp index 72c2b030..2cbae457 100644 --- a/tests/test_engine_pick.cpp +++ b/tests/test_engine_pick.cpp @@ -7,6 +7,7 @@ using wind::ClassifyWindow; using wind::WindowCategory; using wind::EnginePref; using wind::ParseEnginePref; +using wind::MayRetargetSession; static EnginePickInputs game() { EnginePickInputs in; @@ -152,10 +153,12 @@ TEST_CASE("an explicit transform preference still respects the correctness limit TEST_CASE("a rotated output never gets render: duplication cannot capture it (review M3)") { EnginePickInputs in; + in.primaryMonitor = true; in.rotatedOutput = true; in.pref = EnginePref::Render; CHECK(ShouldPickTransform(in)); EnginePickInputs flat; // the same inputs on a landscape output: render + flat.primaryMonitor = true; flat.pref = EnginePref::Render; CHECK_FALSE(ShouldPickTransform(flat)); } @@ -163,12 +166,12 @@ TEST_CASE("a rotated output never gets render: duplication cannot capture it (re TEST_CASE("protected content never gets render, whatever anything else says") { // Desktop Duplication captures DRM surfaces as black, so render shows nothing at all. EnginePickInputs in; + in.primaryMonitor = true; in.captureProtected = true; in.pref = EnginePref::Render; // the user explicitly asked for render CHECK(ShouldPickTransform(in)); // and still does not get it - in.primaryMonitor = false; // not even off the primary monitor - CHECK(ShouldPickTransform(in)); EnginePickInputs man; + man.primaryMonitor = true; man.renderExcluded = true; // the manual escape hatch behaves identically man.pref = EnginePref::Render; CHECK(ShouldPickTransform(man)); @@ -198,6 +201,35 @@ TEST_CASE("a lost render device sends every pick to the transform engine (review CHECK(ShouldPickTransform(in)); } +TEST_CASE("transform is primary-only: off the primary every non-lost pick is render (review 2026-10-16 #11)") { + // Transform's visual offsets are monitor-local while its input mapping adds the origin, so on + // a secondary it would show the primary's region. Protected, excluded and rotated secondaries + // get render with its known limits instead. + EnginePickInputs in; + in.primaryMonitor = false; + in.coversMonitor = true; in.borderless = true; // a game + CHECK_FALSE(ShouldPickTransform(in)); + in.captureProtected = true; // DRM on the secondary + CHECK_FALSE(ShouldPickTransform(in)); + in.captureProtected = false; in.rotatedOutput = true; // a portrait secondary + CHECK_FALSE(ShouldPickTransform(in)); + in.rotatedOutput = false; in.renderExcluded = true; + CHECK_FALSE(ShouldPickTransform(in)); + CHECK_FALSE(PickTransformFor("transform", in, false)); // a pinned transform too + CHECK_FALSE(PickTransformFor("hybrid", in, false)); + // A lost render device is the exception: render cannot draw anything until it recovers. + in.renderLost = true; + CHECK(ShouldPickTransform(in)); + CHECK(PickTransformFor("transform", in, false)); +} + +TEST_CASE("a session moves to a non-primary monitor only when the render engine took it") { + CHECK(MayRetargetSession(true, false)); // the primary: either engine can magnify it + CHECK(MayRetargetSession(true, true)); + CHECK(MayRetargetSession(false, true)); // a secondary the render overlay moved to + CHECK_FALSE(MayRetargetSession(false, false)); // render refused (other adapter) or transform-only +} + TEST_CASE("the engine setting applies live: a pinned engine wins, Auto keeps its rule") { EnginePickInputs desktop; // the plain desktop: Auto picks render desktop.primaryMonitor = true; From 3c5b5c205a31b7fc6ac5329682ea313d0b14f55d Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:58:02 +0200 Subject: [PATCH 3/6] fix(track): check every folder-supplied bridge dependency, map Java carets off (0,0), retry failed folders (#497) - LoadVerified reads the bridge DLL's import and delay-import tables (pure PeImportNames) and requires a valid signature on every DLL the loader would take from its folder, transitively; a fixed list of four CRT names missed e.g. MSVCR100.dll (review #15). - Java carets are converted both around the monitor origin (JDK 17+ and JBR per-monitor user space) and around (0,0); the one inside the Java window wins (JavaPickCaretPx), fixing scaled monitors away from (0,0) (review #16). - A folder whose bridge did not load is retried after a 5 s to 5 min backoff instead of being written off for the session (review #31). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- docs/architecture/07-cursor.md | 12 ++- src/track/java_bridge.cpp | 120 ++++++++++++++++++++---- src/track/java_bridge.h | 6 +- src/track/java_bridge_util.h | 35 +++++-- src/track/pe_imports.h | 158 ++++++++++++++++++++++++++++++++ tests/test_java_bridge_util.cpp | 31 +++++++ tests/test_pe_imports.cpp | 103 +++++++++++++++++++++ 7 files changed, 432 insertions(+), 33 deletions(-) create mode 100644 src/track/pe_imports.h create mode 100644 tests/test_pe_imports.cpp diff --git a/docs/architecture/07-cursor.md b/docs/architecture/07-cursor.md index 708c5d06..6e0eade1 100644 --- a/docs/architecture/07-cursor.md +++ b/docs/architecture/07-cursor.md @@ -362,11 +362,17 @@ leaves the caret to `followCaret` (sources in `findCaret`: xterm input, Win32, U (`src/track/java_bridge.*`). UIPI drops the JVM's handshake to a UIAccess process, so the bridge's hidden windows get a narrow `ChangeWindowMessageFilterEx` allowance. Never poll the bridge (each read runs on the Java app's UI thread); read only after bridge callbacks. Load only - Authenticode-signed bridge DLLs. Read the caret with `getCaretLocation`, scaled by the monitor - DPI (Java answers in its user space, device px / scale). The older character bounds + Authenticode-signed bridge DLLs, and only when every DLL the loader would take from the bridge's + folder is signed too: its import and delay-import tables are read (`PeImportNames`, + `src/track/pe_imports.h`) and walked through the folder (API sets and names with no file there + resolve from the system). Read the caret with `getCaretLocation`, scaled by the monitor + DPI (Java answers in its user space, device px / scale). Modern JDKs anchor each monitor's user + space at its device origin; older ones scale around (0,0). The two agree only on a monitor at + (0,0), so both are computed and the one inside the Java window wins (`JavaPickCaretPx`). The older character bounds (`getAccessibleTextRect`) gave x=2 for every Swing caret and stay only as a fallback, scaled the same way (`JavaSpanRectPx`). A bridge probe that fails backs off per process (2 s doubling to - 60 s), and the retry after a failed read backs off 250 ms doubling to 4 s; a bridge event or a + 60 s) and per app folder (5 s doubling to 5 min; a load failure can be transient, so a folder is + never written off), and the retry after a failed read backs off 250 ms doubling to 4 s; a bridge event or a window switch resets it. The MSAA caret (`MsaaCaret`) skips windows Windows reports as hung and sits out 2 s after a call over 250 ms. - **Mouse edge mode** (`mouseAlign=1`, free-pointer sessions only): the pointer is unwelded and diff --git a/src/track/java_bridge.cpp b/src/track/java_bridge.cpp index 02ff170f..e62df370 100644 --- a/src/track/java_bridge.cpp +++ b/src/track/java_bridge.cpp @@ -1,11 +1,14 @@ #include "java_bridge.h" #include "java_bridge_util.h" +#include "pe_imports.h" // PeImportNames: the bridge DLL's folder-supplied dependencies #include "logging.h" #include // GetDpiForMonitor: Java caret user space to device px #pragma comment(lib, "shcore.lib") #include #include #pragma comment(lib, "wintrust.lib") +#include +#include #include #include namespace wind { @@ -126,28 +129,85 @@ static bool SignedFile(const std::wstring& path) { return r == ERROR_SUCCESS; } +// The whole file behind an open handle (a bridge DLL and its runtime are well under 64 MB). +static bool ReadHeldFile(HANDLE h, std::vector& out) { + LARGE_INTEGER sz{}; + if (!GetFileSizeEx(h, &sz) || sz.QuadPart <= 0 || sz.QuadPart > (64ll << 20)) return false; + out.resize((size_t)sz.QuadPart); + LARGE_INTEGER zero{}; + if (!SetFilePointerEx(h, zero, nullptr, FILE_BEGIN)) return false; + DWORD got = 0; + return ReadFile(h, out.data(), (DWORD)out.size(), &got, nullptr) && got == out.size(); +} + +static std::string LowerAscii(const std::string& a) { + std::string o = a; + for (char& c : o) if (c >= 'A' && c <= 'Z') c = (char)(c - 'A' + 'a'); + return o; +} + +// System32 is writable by administrators only, and its DLLs are mostly catalog-signed (an embedded +// signature check would refuse them), so a bridge there needs no dependency walk. +static bool IsSystemDir(const std::wstring& dir) { + wchar_t sys[MAX_PATH]; + const UINT n = GetSystemDirectoryW(sys, MAX_PATH); + return n > 0 && n < MAX_PATH && CompareStringOrdinal(dir.c_str(), -1, sys, -1, TRUE) == CSTR_EQUAL; +} + // Loads one candidate bridge DLL, safely. Wind is a UIAccess process, and the candidate's folder is // chosen by whatever process owns a window with a Java class name, which any program can register // (review #281). So: the DLL must carry a valid Authenticode signature (every real one does: JetBrains, -// Oracle, Microsoft, Amazon, ...), its only non-system dependency (the MSVC runtime a bundled JRE ships -// beside it) must be signed too, both files are held open against replacement while verified and -// loaded, and dependencies resolve only from that folder and System32. +// Oracle, Microsoft, Amazon, ...), and so must every DLL the loader would take from that folder: the +// candidate is loaded with its folder on the search path, so each of its imports (and delay imports) +// that also sits in the folder is checked, and that file's own imports in turn. A fixed list of the +// MSVC runtime names missed a JDK 8 bridge's MSVCR100.dll (review 2026-10-16 #15). API sets resolve +// from the system; a name with no file in the folder resolves from System32 or KnownDLLs. Every +// checked file is held open against replacement while verified and loaded. static HMODULE LoadVerified(const std::wstring& path, bool log) { std::vector held; auto hold = [&](const std::wstring& p) { HANDLE h = CreateFileW(p.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, 0, nullptr); if (h != INVALID_HANDLE_VALUE) held.push_back(h); - return h != INVALID_HANDLE_VALUE; + return h; }; auto release = [&] { for (HANDLE h : held) CloseHandle(h); }; - if (!hold(path)) { release(); return nullptr; } - bool ok = SignedFile(path); const size_t cut = path.find_last_of(L"\\/"); const std::wstring dir = cut == std::wstring::npos ? std::wstring() : path.substr(0, cut); - for (const wchar_t* dep : { L"vcruntime140.dll", L"vcruntime140_1.dll", L"msvcp140.dll", L"ucrtbase.dll" }) { - const std::wstring d = dir + L"\\" + dep; - if (ok && GetFileAttributesW(d.c_str()) != INVALID_FILE_ATTRIBUTES) - ok = hold(d) && SignedFile(d); + std::vector pending; // dependency names still to look for in the folder + std::set seen; // lower-case names already handled + seen.insert("windowsaccessbridge-64.dll"); + // Signed, and its import table read: the names go on the pending list. + auto verify = [&](const std::wstring& p, HANDLE h) { + std::vector bytes; + std::vector names; + if (!SignedFile(p)) return false; + if (!ReadHeldFile(h, bytes) || !PeImportNames(bytes, names)) { + wind::Log(wind::LogLevel::Warn, "track", "java: cannot read the import table of %ls", p.c_str()); + return false; + } + for (const std::string& n : names) pending.push_back(n); + return true; + }; + HANDLE root = hold(path); + if (root == INVALID_HANDLE_VALUE) { release(); return nullptr; } + bool ok = verify(path, root); + // The runtime a bundled JRE ships beside the bridge stays checked even when it is reached only + // through an API set (ucrtbase) rather than imported by name. + for (const char* dep : { "vcruntime140.dll", "vcruntime140_1.dll", "msvcp140.dll", "ucrtbase.dll" }) + pending.push_back(dep); + if (IsSystemDir(dir)) pending.clear(); + while (ok && !pending.empty()) { + const std::string name = pending.back(); + pending.pop_back(); + if (!seen.insert(LowerAscii(name)).second) continue; + if (seen.size() > 64) { ok = false; break; } // a dependency tree no bridge has + if (IsApiSetName(name)) continue; + const std::wstring d = dir + L"\\" + std::wstring(name.begin(), name.end()); + if (GetFileAttributesW(d.c_str()) == INVALID_FILE_ATTRIBUTES) continue; // not from the folder + HANDLE h = hold(d); + ok = h != INVALID_HANDLE_VALUE && verify(d, h); + if (!ok) wind::Log(wind::LogLevel::Warn, "track", "java: %ls needs %ls from its folder, which is unsigned or unreadable", + path.c_str(), d.c_str()); } // Any validly signed DLL passes the check above, and loading runs its DllMain. So look for the // bridge's entry points in a copy mapped WITHOUT running DllMain or resolving imports first; only @@ -190,9 +250,17 @@ bool JavaBridge::ensure(HWND javaWindow, DWORD wakeTid, UINT wakeMsg, bool log) }; const std::wstring dir = ProcessDir(javaWindow); if (dir.empty()) { noteFail(); return false; } // retry after the backoff - // A miss is remembered per app folder, never for the whole session: one Java app without a - // bridge DLL must not switch Java tracking off for IntelliJ later (review #281). - if (failedDirs_.count(dir)) { noteFail(); return false; } + // A miss is remembered per app folder, never for the whole process: one Java app without a + // bridge DLL must not switch Java tracking off for IntelliJ later (review #281), and the folder + // itself is tried again after its backoff (failedDirs_). + auto dirFail = failedDirs_.find(dir); + if (dirFail != failedDirs_.end() && now < dirFail->second.until) { noteFail(); return false; } + auto noteDirFail = [&] { + if (failedDirs_.size() > 64) failedDirs_.clear(); + ProbeFail& f = failedDirs_[dir]; + f.backoffMs = BackoffMs(f.backoffMs, 5000, 300000); + f.until = now + f.backoffMs; + }; for (const std::wstring& c : JavaBridgeDllCandidates(dir)) { if (GetFileAttributesW(c.c_str()) == INVALID_FILE_ATTRIBUTES) continue; mod_ = LoadVerified(c, log); @@ -207,7 +275,7 @@ bool JavaBridge::ensure(HWND javaWindow, DWORD wakeTid, UINT wakeMsg, bool log) } } if (!mod_) { - failedDirs_.insert(dir); + noteDirFail(); noteFail(); wind::Log(wind::LogLevel::Info, "track", "java: no usable Access Bridge DLL near %ls", dir.c_str()); return false; @@ -225,7 +293,7 @@ bool JavaBridge::ensure(HWND javaWindow, DWORD wakeTid, UINT wakeMsg, bool log) wind::Log(wind::LogLevel::Warn, "track", "java: bridge DLL is missing exports; skipped"); FreeLibrary(mod_); mod_ = nullptr; s_isJava = nullptr; s_withFocus = nullptr; s_textInfo = nullptr; s_textRect = nullptr; s_release = nullptr; - failedDirs_.insert(dir); + noteDirFail(); noteFail(); return false; } @@ -273,20 +341,32 @@ bool JavaBridge::caret(HWND javaWindow, RECT& out) { // Java answers in its user space (see JavaUserToPx). A DPI-unaware Java app is stretched by // the same monitor scale, so the monitor DPI converts every answer, the character-bounds // fallbacks included (they used to come back unscaled, off by the scale at 150-225%). + // On a monitor away from (0,0) the JDK's mapping matters (JavaUserToPx): both are computed + // and the one inside the Java window is kept. UINT dx = 96, dy = 96; - GetDpiForMonitor(MonitorFromWindow(javaWindow, MONITOR_DEFAULTTONEAREST), MDT_EFFECTIVE_DPI, &dx, &dy); + const HMONITOR mon = MonitorFromWindow(javaWindow, MONITOR_DEFAULTTONEAREST); + GetDpiForMonitor(mon, MDT_EFFECTIVE_DPI, &dx, &dy); const double s = dx / 96.0; - auto set = [&](const JavaRectPx& p) { out = { p.left, p.top, p.right, p.bottom }; ok = true; }; + MONITORINFO mi{ sizeof(mi) }; + const bool haveMon = GetMonitorInfoW(mon, &mi) != 0; + const long ox = haveMon ? mi.rcMonitor.left : 0, oy = haveMon ? mi.rcMonitor.top : 0; + RECT wr{}; + const bool haveWin = GetWindowRect(javaWindow, &wr) != 0; + auto set = [&](const JavaRectPx& anchored, const JavaRectPx& legacy) { + const JavaRectPx p = haveWin ? JavaPickCaretPx(anchored, legacy, wr.left, wr.top, wr.right, wr.bottom) + : anchored; + out = { p.left, p.top, p.right, p.bottom }; ok = true; + }; if (s_caretLoc && s_caretLoc(vm, ac, &r, ti.caretIndex) && good()) { - set(JavaSpanRectPx(r.x, r.y, r.width, r.height, s)); + set(JavaSpanRectPx(r.x, r.y, r.width, r.height, s, ox, oy), JavaSpanRectPx(r.x, r.y, r.width, r.height, s)); lastSrc_ = "caretloc"; } else if (s_textRect(vm, ac, &r, ti.caretIndex) && good()) { lastSrc_ = "charrect"; - set(JavaSpanRectPx(r.x, r.y, r.width, r.height, s)); + set(JavaSpanRectPx(r.x, r.y, r.width, r.height, s, ox, oy), JavaSpanRectPx(r.x, r.y, r.width, r.height, s)); } else if (ti.caretIndex > 0 && s_textRect(vm, ac, &r, ti.caretIndex - 1) && good()) { // At the very end of the text there is no character under the caret: use the right edge // of the one before it. - set(JavaAfterRectPx(r.x, r.y, r.width, r.height, s)); + set(JavaAfterRectPx(r.x, r.y, r.width, r.height, s, ox, oy), JavaAfterRectPx(r.x, r.y, r.width, r.height, s)); lastSrc_ = "charrect-prev"; } lastIndex_ = ti.caretIndex; diff --git a/src/track/java_bridge.h b/src/track/java_bridge.h index c2669dd6..2cf962f7 100644 --- a/src/track/java_bridge.h +++ b/src/track/java_bridge.h @@ -9,7 +9,6 @@ // Authenticode-signed (Wind is UIAccess; see LoadVerified). #include #include -#include #include namespace wind { class JavaBridge { @@ -29,10 +28,13 @@ class JavaBridge { const char* lastSrc_ = ""; int lastIndex_ = -1; HMODULE mod_ = nullptr; - std::set failedDirs_; // app folders with no usable bridge DLL (retried per folder) // Per-process probe backoff: ensure() is asked on every poll of a Java foreground window. struct ProbeFail { unsigned long long until = 0, backoffMs = 0; }; std::map probeFail_; + // App folders whose bridge DLL did not load, retried after a backoff (5 s doubling to 5 min): a + // failure can be transient (an antivirus scan or an IDE update holding the file, a certificate + // not cached yet), so a folder is never written off for the session (review 2026-10-16 #31). + std::map failedDirs_; bool enabledDone_ = false; unsigned long long enableRetryAt_ = 0; }; diff --git a/src/track/java_bridge_util.h b/src/track/java_bridge_util.h index 00c9901c..8f5b3ba9 100644 --- a/src/track/java_bridge_util.h +++ b/src/track/java_bridge_util.h @@ -73,18 +73,37 @@ inline std::string EnableAccessBridgeText(const std::string& text, bool& changed // Java answers in its own user space: device px divided by the monitor scale (at 225%, x=59 for a // caret at device x=134, field 2026-10-04). Every bridge answer, the caret location AND the character // bounds the fallbacks use, goes through these so they land in the same (device px) space. +// The scale is applied around an origin (ox, oy). JDKs with per-monitor user spaces (JDK-8211999: +// JDK 17+ and the JetBrains runtime) anchor each monitor's user space at its DEVICE origin, +// user = origin + (device - origin) / scale, so the monitor's top-left is the origin there; older and +// DPI-unaware ones scale around (0,0). Both agree on a monitor at (0,0), which is where the field +// numbers above come from; JavaPickCaretPx chooses between them elsewhere (review 2026-10-16 #16). struct JavaRectPx { long left, top, right, bottom; }; -inline long JavaUserToPx(int v, double scale) { return (long)(v * scale + (v >= 0 ? 0.5 : -0.5)); } +inline long JavaUserToPx(int v, double scale, long origin = 0) { + const double d = origin + ((double)v - origin) * scale; + return (long)(d + (d >= 0 ? 0.5 : -0.5)); +} // A span (the caret, or the character at it): at least 2 px wide. -inline JavaRectPx JavaSpanRectPx(int x, int y, int w, int h, double scale) { - const long pw = JavaUserToPx(w, scale); - return { JavaUserToPx(x, scale), JavaUserToPx(y, scale), - JavaUserToPx(x, scale) + (pw > 2 ? pw : 2), JavaUserToPx(y + h, scale) }; +inline JavaRectPx JavaSpanRectPx(int x, int y, int w, int h, double scale, long ox = 0, long oy = 0) { + const long pw = JavaUserToPx(w, scale); // a length: no origin + const long l = JavaUserToPx(x, scale, ox); + return { l, JavaUserToPx(y, scale, oy), l + (pw > 2 ? pw : 2), JavaUserToPx(y + h, scale, oy) }; } // The right edge of a character (the caret after the last one): a 2 px sliver. -inline JavaRectPx JavaAfterRectPx(int x, int y, int w, int h, double scale) { - const long r = JavaUserToPx(x + w, scale); - return { r, JavaUserToPx(y, scale), r + 2, JavaUserToPx(y + h, scale) }; +inline JavaRectPx JavaAfterRectPx(int x, int y, int w, int h, double scale, long ox = 0, long oy = 0) { + const long r = JavaUserToPx(x + w, scale, ox); + return { r, JavaUserToPx(y, scale, oy), r + 2, JavaUserToPx(y + h, scale, oy) }; +} +// The bridge does not say which user space its JDK uses, so the caret is converted both ways +// (anchored at the monitor origin, and around (0,0)) and the one whose centre lies in the Java +// window (device px) wins. The anchored one, the modern JDKs' mapping, wins a tie either way. +inline JavaRectPx JavaPickCaretPx(const JavaRectPx& anchored, const JavaRectPx& legacy, + long wl, long wt, long wr, long wb) { + auto inside = [&](const JavaRectPx& r) { + const long cx = (r.left + r.right) / 2, cy = (r.top + r.bottom) / 2; + return cx >= wl && cx < wr && cy >= wt && cy < wb; + }; + return (!inside(anchored) && inside(legacy)) ? legacy : anchored; } // Retry delay that doubles per consecutive failure (`first` on the first, capped at `cap`): the Java diff --git a/src/track/pe_imports.h b/src/track/pe_imports.h new file mode 100644 index 00000000..6af2674b --- /dev/null +++ b/src/track/pe_imports.h @@ -0,0 +1,158 @@ +#pragma once +// Pure PE import-table reader (no ) for the Java bridge DLL check (review 2026-10-16 #15): +// LoadVerified loads a signed bridge DLL with its folder on the search path, so every DLL it imports +// that also sits in that folder must be checked before the loader resolves it. Reads the file bytes +// only (no mapping, no loader). Tests: tests/test_pe_imports.cpp. +#include +#include +#include +#include +#include +namespace wind { + +namespace pe_detail { +inline bool Rd16(const std::vector& f, size_t o, uint32_t& v) { + if (o > f.size() || f.size() - o < 2) return false; + v = (uint32_t)f[o] | ((uint32_t)f[o + 1] << 8); + return true; +} +inline bool Rd32(const std::vector& f, size_t o, uint32_t& v) { + if (o > f.size() || f.size() - o < 4) return false; + v = (uint32_t)f[o] | ((uint32_t)f[o + 1] << 8) | ((uint32_t)f[o + 2] << 16) | ((uint32_t)f[o + 3] << 24); + return true; +} +inline bool Rd64(const std::vector& f, size_t o, uint64_t& v) { + uint32_t lo = 0, hi = 0; + if (!Rd32(f, o, lo) || !Rd32(f, o + 4, hi)) return false; + v = ((uint64_t)hi << 32) | lo; + return true; +} +struct Section { uint32_t va, vsize, raw, rawSize; }; +// File offset of an RVA, through the section table (headers below the first section map 1:1). +inline bool RvaToOffset(const std::vector
& secs, uint32_t headersSize, uint32_t rva, size_t& off) { + if (rva < headersSize) { off = rva; return true; } + for (const Section& s : secs) { + const uint32_t span = s.vsize > s.rawSize ? s.vsize : s.rawSize; + if (rva >= s.va && rva - s.va < span) { + if (rva - s.va >= s.rawSize) return false; // in the zero-filled tail: no name there + off = (size_t)s.raw + (rva - s.va); + return true; + } + } + return false; +} +// A DLL name as the loader will search for it: printable ASCII, no path, bounded. +inline bool ReadName(const std::vector& f, size_t off, std::string& out) { + out.clear(); + for (size_t i = off; i < f.size() && out.size() <= 255; ++i) { + const uint8_t c = f[i]; + if (c == 0) return !out.empty(); + if (c < 0x20 || c > 0x7e || c == '\\' || c == '/' || c == ':') return false; + out.push_back((char)c); + } + return false; // unterminated or too long +} +} // namespace pe_detail + +// Every DLL name in the import and delay-import tables of a PE image given as file bytes. False +// when the file is not a PE image the reader can account for fully (truncated, a table outside the +// file, a name with a path in it, too many entries): the caller then refuses the DLL. +inline bool PeImportNames(const std::vector& f, std::vector& names) { + using namespace pe_detail; + names.clear(); + uint32_t mz = 0, lfanew = 0, sig = 0; + if (!Rd16(f, 0, mz) || mz != 0x5A4D || !Rd32(f, 0x3C, lfanew)) return false; + if (!Rd32(f, lfanew, sig) || sig != 0x00004550) return false; // "PE\0\0" + const size_t fh = (size_t)lfanew + 4; + uint32_t nsec = 0, optSize = 0, magic = 0; + if (!Rd16(f, fh + 2, nsec) || !Rd16(f, fh + 16, optSize)) return false; + const size_t opt = fh + 20; + if (!Rd16(f, opt, magic)) return false; + const bool pe64 = magic == 0x20B; + if (!pe64 && magic != 0x10B) return false; + uint64_t imageBase = 0; + if (pe64) { if (!Rd64(f, opt + 24, imageBase)) return false; } + else { uint32_t b = 0; if (!Rd32(f, opt + 28, b)) return false; imageBase = b; } + uint32_t headersSize = 0, nDirs = 0; + if (!Rd32(f, opt + 60, headersSize)) return false; + const size_t dirCountOff = opt + (pe64 ? 108 : 92); + if (!Rd32(f, dirCountOff, nDirs)) return false; + const size_t dirs = dirCountOff + 4; + std::vector
secs; + const size_t secTable = opt + optSize; + for (uint32_t i = 0; i < nsec; ++i) { + Section s{}; + const size_t o = secTable + (size_t)i * 40; + if (!Rd32(f, o + 8, s.vsize) || !Rd32(f, o + 12, s.va) || !Rd32(f, o + 16, s.rawSize) || + !Rd32(f, o + 20, s.raw)) + return false; + secs.push_back(s); + } + auto dir = [&](uint32_t idx, uint32_t& rva) { + rva = 0; + if (idx >= nDirs) return true; // the table is absent + if (dirs + (size_t)idx * 8 + 8 > secTable) return false; // directories overlap the sections + return Rd32(f, dirs + (size_t)idx * 8, rva); + }; + const size_t kMaxEntries = 4096; + // Import directory (index 1): 20-byte descriptors, Name RVA at +12, an all-zero one ends it. + uint32_t impRva = 0; + if (!dir(1, impRva)) return false; + if (impRva) { + size_t off = 0; + if (!RvaToOffset(secs, headersSize, impRva, off)) return false; + for (size_t i = 0;; ++i) { + if (i >= kMaxEntries) return false; + uint32_t oft = 0, nameRva = 0, ft = 0; + if (!Rd32(f, off + i * 20, oft) || !Rd32(f, off + i * 20 + 12, nameRva) || + !Rd32(f, off + i * 20 + 16, ft)) + return false; + if (nameRva == 0 && oft == 0 && ft == 0) break; + size_t no = 0; std::string n; + if (!RvaToOffset(secs, headersSize, nameRva, no) || !ReadName(f, no, n)) return false; + names.push_back(n); + } + } + // Delay-import directory (index 13): 32-byte descriptors, name at +4, a zero name ends it. + // Attribute bit 0 clear is the old VC6 form, whose fields are VAs rather than RVAs. + uint32_t dlyRva = 0; + if (!dir(13, dlyRva)) return false; + if (dlyRva) { + size_t off = 0; + if (!RvaToOffset(secs, headersSize, dlyRva, off)) return false; + for (size_t i = 0;; ++i) { + if (i >= kMaxEntries) return false; + uint32_t attrs = 0, name = 0; + if (!Rd32(f, off + i * 32, attrs) || !Rd32(f, off + i * 32 + 4, name)) return false; + if (name == 0) break; + uint64_t rva = name; + if (!(attrs & 1)) { + if (name < imageBase) return false; + rva = name - imageBase; + if (rva > 0xFFFFFFFFull) return false; + } + size_t no = 0; std::string n; + if (!RvaToOffset(secs, headersSize, (uint32_t)rva, no) || !ReadName(f, no, n)) return false; + names.push_back(n); + } + } + return true; +} + +// An API set contract name (api-ms-win-*, ext-ms-*): resolved by the loader's schema to a system +// DLL, never searched for in a folder. +inline bool IsApiSetName(const std::string& n) { + auto starts = [&](const char* p) { + const size_t k = std::strlen(p); + if (n.size() < k) return false; + for (size_t i = 0; i < k; ++i) { + char c = n[i]; + if (c >= 'A' && c <= 'Z') c = (char)(c - 'A' + 'a'); + if (c != p[i]) return false; + } + return true; + }; + return starts("api-ms-") || starts("ext-ms-"); +} + +} // namespace wind diff --git a/tests/test_java_bridge_util.cpp b/tests/test_java_bridge_util.cpp index 5b29c646..d532cc64 100644 --- a/tests/test_java_bridge_util.cpp +++ b/tests/test_java_bridge_util.cpp @@ -70,3 +70,34 @@ TEST_CASE("bridge retries back off: first delay, doubling, capped, never polling CHECK(BackoffMs(0, 2000, 60000) == 2000); // the per-process probe uses its own curve CHECK(BackoffMs(40000, 2000, 60000) == 60000); } +TEST_CASE("a failed bridge folder is retried on a 5 s to 5 min curve, never written off (review 2026-10-16 #31)") { + unsigned long long b = 0; + b = BackoffMs(b, 5000, 300000); CHECK(b == 5000); + b = BackoffMs(b, 5000, 300000); CHECK(b == 10000); + for (int i = 0; i < 20; ++i) b = BackoffMs(b, 5000, 300000); + CHECK(b == 300000); +} +TEST_CASE("a caret on a scaled monitor away from (0,0): anchored at the monitor origin (review 2026-10-16 #16)") { + // Primary 3840 px at 100%; IntelliJ on a 150% monitor at device x=3840. A caret at device + // x=4000 is user x = 3840 + 160 / 1.5 = 3946.67, reported as 3947. + CHECK(JavaUserToPx(3947, 1.5, 3840) == 4001); + CHECK(JavaUserToPx(3947, 1.5) == 5921); // the old mapping: ~1920 px too far right + CHECK(JavaUserToPx(59, 2.25, 0) == 133); // a monitor at (0,0): both agree + CHECK(JavaUserToPx(-1000, 1.5, -1920) == -540); // a monitor left of the primary + const JavaRectPx a = JavaSpanRectPx(3947, 120, 1, 16, 1.5, 3840, 0); + CHECK(a.left == 4001); CHECK(a.top == 180); CHECK(a.bottom == 204); + CHECK(a.right == a.left + 2); + const JavaRectPx after = JavaAfterRectPx(3940, 120, 4, 16, 1.5, 3840, 0); + CHECK(after.left == 3996); CHECK(after.right == 3998); +} +TEST_CASE("the caret mapping whose centre is inside the Java window wins; anchored on a tie") { + const JavaRectPx anchored{ 4001, 180, 4003, 204 }, legacy{ 5921, 180, 5923, 204 }; + // The window on the secondary (3840..7680): the anchored mapping is inside it. + CHECK(JavaPickCaretPx(anchored, legacy, 3840, 0, 7680, 2160).left == 4001); + // A JDK that scales around (0,0): only its answer lands in the window. + CHECK(JavaPickCaretPx({ 9000, 180, 9002, 204 }, legacy, 3840, 0, 7680, 2160).left == 5921); + // Neither inside (a caret scrolled out of view): the modern mapping. + CHECK(JavaPickCaretPx({ 9000, 180, 9002, 204 }, { 9500, 180, 9502, 204 }, 3840, 0, 7680, 2160).left == 9000); + // Both inside (the monitor at (0,0) gives the same rect anyway): anchored. + CHECK(JavaPickCaretPx(anchored, { 4100, 180, 4102, 204 }, 3840, 0, 7680, 2160).left == 4001); +} diff --git a/tests/test_pe_imports.cpp b/tests/test_pe_imports.cpp new file mode 100644 index 00000000..b4109f05 --- /dev/null +++ b/tests/test_pe_imports.cpp @@ -0,0 +1,103 @@ +#include "doctest.h" +#include "pe_imports.h" +#include +using namespace wind; + +// A minimal PE32+ image: one section (RVA 0x1000 at file 0x200), an import table with two DLLs and +// a delay-import table with one. +namespace { +struct PeBuilder { + std::vector f = std::vector(0x400, 0); + void p16(size_t o, uint32_t v) { f[o] = (uint8_t)v; f[o + 1] = (uint8_t)(v >> 8); } + void p32(size_t o, uint32_t v) { for (int i = 0; i < 4; ++i) f[o + i] = (uint8_t)(v >> (8 * i)); } + void p64(size_t o, uint64_t v) { p32(o, (uint32_t)v); p32(o + 4, (uint32_t)(v >> 32)); } + void str(size_t o, const char* s) { std::memcpy(&f[o], s, std::strlen(s) + 1); } + static constexpr size_t kOpt = 0x98; + static constexpr uint64_t kBase = 0x180000000ull; + PeBuilder() { + f[0] = 'M'; f[1] = 'Z'; f[2] = 0x90; + p32(0x3C, 0x80); + str(0x80, "PE"); // "PE\0\0" + p16(0x84, 0x8664); p16(0x86, 1); // machine, one section + p16(0x94, 0xF0); // SizeOfOptionalHeader + p16(kOpt, 0x20B); // PE32+ + p64(kOpt + 24, kBase); + p32(kOpt + 60, 0x200); // SizeOfHeaders + p32(kOpt + 108, 16); // NumberOfRvaAndSizes + p32(kOpt + 112 + 8 * 1, 0x1000); // import directory + p32(kOpt + 112 + 8 * 13, 0x1040); // delay-import directory + const size_t sec = kOpt + 0xF0; + p32(sec + 8, 0x200); p32(sec + 12, 0x1000); p32(sec + 16, 0x200); p32(sec + 20, 0x200); + // Imports at file 0x200: two descriptors, then the zero one. + p32(0x200 + 0, 0x1080); p32(0x200 + 12, 0x1100); p32(0x200 + 16, 0x1090); + p32(0x214 + 0, 0x1080); p32(0x214 + 12, 0x1110); p32(0x214 + 16, 0x1090); + // Delay imports at file 0x240: one RVA-form descriptor, then a zero name. + p32(0x240 + 0, 1); p32(0x240 + 4, 0x1120); + str(0x300, "KERNEL32.dll"); + str(0x310, "MSVCR100.dll"); + str(0x320, "jawt.dll"); + } +}; +} // namespace + +TEST_CASE("PE import reader lists the import and delay-import DLL names") { + PeBuilder b; + std::vector n; + REQUIRE(PeImportNames(b.f, n)); + REQUIRE(n.size() == 3); + CHECK(n[0] == "KERNEL32.dll"); + CHECK(n[1] == "MSVCR100.dll"); // the JDK 8 runtime a fixed CRT list missed (review 2026-10-16 #15) + CHECK(n[2] == "jawt.dll"); +} + +TEST_CASE("PE import reader: an old-form delay descriptor uses a VA") { + PeBuilder b; + b.p32(0x240 + 0, 0); + b.p32(0x240 + 4, (uint32_t)((PeBuilder::kBase + 0x1120) & 0xFFFFFFFFull)); // the low 32 bits: below the base + std::vector n; + CHECK_FALSE(PeImportNames(b.f, n)); // VA below the image base: refused + // A PE32 image with a 32-bit base takes the VA form properly. + PeBuilder c; + c.p16(PeBuilder::kOpt, 0x10B); + c.p32(PeBuilder::kOpt + 28, 0x10000000); + // PE32 directories start 16 bytes earlier: move the count and the two entries. + c.p32(PeBuilder::kOpt + 108, 0); c.p32(PeBuilder::kOpt + 112 + 8, 0); c.p32(PeBuilder::kOpt + 112 + 104, 0); + c.p32(PeBuilder::kOpt + 92, 16); + c.p32(PeBuilder::kOpt + 96 + 8 * 1, 0x1000); + c.p32(PeBuilder::kOpt + 96 + 8 * 13, 0x1040); + c.p32(0x240 + 0, 0); + c.p32(0x240 + 4, 0x10000000 + 0x1120); + REQUIRE(PeImportNames(c.f, n)); + REQUIRE(n.size() == 3); + CHECK(n[2] == "jawt.dll"); +} + +TEST_CASE("PE import reader refuses what it cannot account for") { + std::vector n; + { PeBuilder b; b.f[0] = 'X'; CHECK_FALSE(PeImportNames(b.f, n)); } // not MZ + { PeBuilder b; b.f[0x80] = 'X'; CHECK_FALSE(PeImportNames(b.f, n)); } // not PE + { PeBuilder b; b.f.resize(0x210); CHECK_FALSE(PeImportNames(b.f, n)); } // truncated table + { PeBuilder b; b.str(0x310, "..\\evil.dll"); CHECK_FALSE(PeImportNames(b.f, n)); } // a path + { PeBuilder b; b.p32(0x200 + 12, 0x5000); CHECK_FALSE(PeImportNames(b.f, n)); } // name outside the image + { PeBuilder b; b.f[0x31C] = 'x'; b.f[0x31D] = 'x'; b.f[0x31E] = 'x'; // unterminated last name + for (size_t i = 0x320; i < b.f.size(); ++i) b.f[i] = 'a'; + CHECK_FALSE(PeImportNames(b.f, n)); } + CHECK_FALSE(PeImportNames({}, n)); +} + +TEST_CASE("PE import reader: no import tables is an empty, valid list") { + PeBuilder b; + b.p32(PeBuilder::kOpt + 112 + 8, 0); + b.p32(PeBuilder::kOpt + 112 + 104, 0); + std::vector n{ "stale" }; + REQUIRE(PeImportNames(b.f, n)); + CHECK(n.empty()); +} + +TEST_CASE("API set names are recognised case-insensitively") { + CHECK(IsApiSetName("api-ms-win-crt-runtime-l1-1-0.dll")); + CHECK(IsApiSetName("API-MS-Win-Core-Synch-l1-2-0.dll")); + CHECK(IsApiSetName("ext-ms-win-ntuser-window-l1-1-0.dll")); + CHECK_FALSE(IsApiSetName("MSVCR100.dll")); + CHECK_FALSE(IsApiSetName("api.dll")); +} From abc48e042cfcc59f5e624f24813b96073c0d6002 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:58:02 +0200 Subject: [PATCH 4/6] fix(track): publish a same-rect focus or caret again after a focus change (#497) publish() deduped against the last snapshot across focus generations, so a keyboard return to the same control after an unpublished focus (a skipped container) never reached StepViewOwner. The dedupe now holds within one focus generation only (TrackPublishIsRepeat, tested) (review #30). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- src/track/focus_track.cpp | 7 ++++--- src/track/focus_track.h | 3 ++- src/track/view_target.h | 9 +++++++++ tests/test_view_target.cpp | 10 ++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/track/focus_track.cpp b/src/track/focus_track.cpp index 71a9f93b..2b694f70 100644 --- a/src/track/focus_track.cpp +++ b/src/track/focus_track.cpp @@ -340,11 +340,12 @@ void FocusTracker::setActive(bool on, bool wantCaret, bool wantFocus, bool log) if (t && on != was) PostThreadMessageW(t, kActiveMsg, on ? 1 : 0, 0); // hooks + poll rate if (on && !was && t) PostThreadMessageW(t, kWakeMsg, 0, 0); } -void FocusTracker::publish(TrackKind k, double l, double t, double r, double b, const char* src) { +void FocusTracker::publish(TrackKind k, double l, double t, double r, double b, const char* src, unsigned gen) { { std::lock_guard g(mu_); - if (snap_.kind == k && snap_.l == l && snap_.t == t && snap_.r == r && snap_.b == b) return; // unchanged + if (TrackPublishIsRepeat(snap_, pubGen_, k, l, t, r, b, gen)) return; // unchanged, same focus snap_ = { k, ++seq_, l, t, r, b }; + pubGen_ = gen; } if (log_) wind::Log(wind::LogLevel::Info, "track", "%s via %s: %.0f,%.0f %.0fx%.0f", k == TrackKind::Caret ? "caret" : "focus", src, l, t, r - l, b - t); @@ -362,7 +363,7 @@ struct FocusRun { std::atomic& wantCaret_; std::atomic& wantFocus_; std::atomic& log_; - void publish(TrackKind k, double l, double t, double r, double b, const char* src) { tracker.publish(k, l, t, r, b, src); } + void publish(TrackKind k, double l, double t, double r, double b, const char* src) { tracker.publish(k, l, t, r, b, src, focusGen); } void setUp(); // COM, UIA and its timeouts, the focus lookup, the event hooks void messageLoop(MSG& m); // wakes, coalescing and the backstop poll until WM_QUIT diff --git a/src/track/focus_track.h b/src/track/focus_track.h index 244dd1f9..9f4afbba 100644 --- a/src/track/focus_track.h +++ b/src/track/focus_track.h @@ -31,6 +31,7 @@ class FocusTracker { mutable std::mutex mu_; TrackSnapshot snap_; unsigned seq_ = 0; - void publish(TrackKind k, double l, double t, double r, double b, const char* src); + unsigned pubGen_ = ~0u; // focus generation of snap_ (TrackPublishIsRepeat) + void publish(TrackKind k, double l, double t, double r, double b, const char* src, unsigned gen); }; } // namespace wind diff --git a/src/track/view_target.h b/src/track/view_target.h index 2515eb8b..a69f2098 100644 --- a/src/track/view_target.h +++ b/src/track/view_target.h @@ -21,6 +21,15 @@ namespace wind { enum class ViewOwner { Mouse, Caret, Focus, Keys }; // Keys: keyboard panning (#287) enum class TrackKind { None = 0, Caret = 1, Focus = 2 }; struct TrackSnapshot { TrackKind kind = TrackKind::None; unsigned seq = 0; double l = 0, t = 0, r = 0, b = 0; }; +// The tracker publishes a report only when it differs from the last one, but only WITHIN one focus +// generation (gen: the tracker's focusGen when the report was made). A report from a new focus is +// always published, even at the same rect: returning to the same text box after a focus that was +// never published (a skipped container, a timed-out lookup) must still reach StepViewOwner, which +// acts on a seq change only (review 2026-10-16 #30). +inline bool TrackPublishIsRepeat(const TrackSnapshot& last, unsigned lastGen, TrackKind k, + double l, double t, double r, double b, unsigned gen) { + return lastGen == gen && last.kind == k && last.l == l && last.t == t && last.r == r && last.b == b; +} struct ViewOwnerState { ViewOwner owner = ViewOwner::Mouse; unsigned lastSeq = 0; diff --git a/tests/test_view_target.cpp b/tests/test_view_target.cpp index 34f538a5..efe2bc11 100644 --- a/tests/test_view_target.cpp +++ b/tests/test_view_target.cpp @@ -10,6 +10,16 @@ static TrackSnapshot Snap(TrackKind k, unsigned seq) { TrackSnapshot s; s.kind = k; s.seq = seq; s.l = 100; s.t = 100; s.r = 102; s.b = 120; return s; } +TEST_CASE("publish dedupe holds within one focus generation only (review 2026-10-16 #30)") { + TrackSnapshot last; last.kind = TrackKind::Focus; last.seq = 4; last.l = 10; last.t = 20; last.r = 110; last.b = 40; + CHECK(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 10, 20, 110, 40, 7)); // same focus, same rect + CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 10, 20, 110, 40, 8)); // focus came back: publish + CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Caret, 10, 20, 110, 40, 7)); // another kind + CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 11, 20, 110, 40, 7)); // moved + TrackSnapshot none; // nothing published yet + CHECK_FALSE(TrackPublishIsRepeat(none, ~0u, TrackKind::Focus, 0, 0, 0, 0, 0)); +} + TEST_CASE("a new caret event takes the view; a repeat of the same seq does not re-trigger") { ViewOwnerState s; auto in = Base(); in.snap = Snap(TrackKind::Caret, 1); CHECK(StepViewOwner(s, in) == ViewOwner::Caret); From a07b2613a556001fba86a6efaa5158f561e39837 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:58:02 +0200 Subject: [PATCH 5/6] fix(transform): crash filter puts an enabled Wind input transform back to identity (#497) CursorCrashFilter restored cursor, clip and scheme but left an enabled MagSetInputTransform in the system-wide slot. It now disables it when the last Wind publish was enabled and the crash is on the thread that owns the Magnification runtime (calls are thread-affine); an off slot is left alone, as it may belong to native Magnifier. Startup still makes no Magnification calls (review #37). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- docs/architecture/04-render-engine.md | 3 ++- src/transform/mag_host.cpp | 21 ++++++++++++++++++++- src/transform/mag_host.h | 4 +++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs/architecture/04-render-engine.md b/docs/architecture/04-render-engine.md index e126ccc6..d826e4f3 100644 --- a/docs/architecture/04-render-engine.md +++ b/docs/architecture/04-render-engine.md @@ -255,7 +255,8 @@ engage by default on the desktop. - `DXGI_ERROR_DEVICE_REMOVED/RESET` latches `deviceLost()`. `recoverDeviceLost()` rebuilds through the same `buildDeviceResources` as `initialize`, so the paths cannot drift. - **Always restore the OS cursor.** `shutdown` and the shared crash filter (`wind::CursorCrashFilter`, `mag_host.cpp`: report first, then restore) restore - the pointer and release any `ClipCursor`. Cursor hiding goes through + the pointer and release any `ClipCursor`; on the tick thread the filter also puts an input + transform Wind enabled back to identity. Cursor hiding goes through `MagApiAcquire`/`MagApiRelease`; see [05](05-transform-engine.md) for why the pairing must be shared. diff --git a/src/transform/mag_host.cpp b/src/transform/mag_host.cpp index a67b6b85..9d2fb5b0 100644 --- a/src/transform/mag_host.cpp +++ b/src/transform/mag_host.cpp @@ -10,6 +10,11 @@ namespace wind { // Process-wide Magnification runtime refcount (see mag_host.h). Single-threaded by contract: // only the tick thread touches the Magnification API (it is thread-affine anyway). static int g_magRefs = 0; +static DWORD g_magThread = 0; // the thread that ran MagInitialize (the only one whose calls count) +// The input transform Wind last published, for the crash filter: an enabled one left in the single +// system-wide slot keeps hit-testing through a zoomed rect after Wind is gone (review 2026-10-16 #37). +static bool g_ixOn = false; +static RECT g_ixDst{}; // The Magnification API is thread-affine: a call from any thread but the one that ran MagInitialize // returns FALSE and changes nothing (measured, issue #206). That thread is the tick thread; nothing @@ -25,6 +30,7 @@ bool MagApiAcquire() { return false; } g_magRefs = 1; + g_magThread = GetCurrentThreadId(); wind::Log(wind::LogLevel::Info, "magapi", "MagInitialize -> refs=1 (DWM now magnification-aware)"); return true; } @@ -33,6 +39,7 @@ void MagApiRelease() { if (g_magRefs <= 0) return; if (--g_magRefs == 0) { MagUninitialize(); + g_ixOn = false; wind::Log(wind::LogLevel::Info, "magapi", "MagUninitialize -> refs=0 (runtime released)"); } else { wind::Log(wind::LogLevel::Info, "magapi", "release -> refs=%d (still held)", g_magRefs); @@ -46,6 +53,13 @@ LONG WINAPI CursorCrashFilter(EXCEPTION_POINTERS* ep) { if (InterlockedExchange(&s_inHandler, 1)) return EXCEPTION_CONTINUE_SEARCH; wind::WriteCrashReport(ep); // minidump + text summary into the log dir MagShowSystemCursor(TRUE); // no-op if the Magnification API was never initialized this run + // An enabled input transform Wind published goes back to identity. Thread-affine like every + // Magnification call, so only a crash on the tick thread can do it; an off slot is left alone + // (it may be native Magnifier's). + if (g_magRefs > 0 && g_ixOn && GetCurrentThreadId() == g_magThread) { + RECT r = g_ixDst; + MagSetInputTransform(FALSE, &r, &r); + } ClipCursor(nullptr); // never leave the cursor clipped if we crash while Inspect-locked SystemParametersInfoW(SPI_SETCURSORS, 0, nullptr, 0); // heals a blanked cursor scheme, no broadcast return EXCEPTION_CONTINUE_SEARCH; @@ -154,7 +168,12 @@ bool MagHost::setInputTransform(bool active, const RECT& src, const RECT& dst) { if (!initialized_) return false; SpanScope span(kSpanIx); RECT s = src, d = dst; // API takes non-const LPRECT - return MagSetInputTransform(active ? TRUE : FALSE, &s, &d) != FALSE; + const bool ok = MagSetInputTransform(active ? TRUE : FALSE, &s, &d) != FALSE; + // An enable is remembered even when the call says FALSE (a publish can land anyway, see + // getInputTransform); a disable clears it only when it took. + if (active) { g_ixOn = true; g_ixDst = dst; } + else if (ok) g_ixOn = false; + return ok; } bool MagHost::getInputTransform(bool& active, RECT& src, RECT& dst) { diff --git a/src/transform/mag_host.h b/src/transform/mag_host.h index d90842cc..bc70633e 100644 --- a/src/transform/mag_host.h +++ b/src/transform/mag_host.h @@ -13,7 +13,9 @@ bool MagApiAcquire(); void MagApiRelease(); bool MagApiAlive(); // Crash safety net shared by main.cpp (installed at startup) and RenderEngine (on its first cursor -// hide): if we go down while the cursor is hidden or confined, make it visible and free again. +// hide): if we go down while the cursor is hidden or confined, make it visible and free again, and +// (on the tick thread, where Magnification calls count) put an input transform Wind enabled back +// to identity. // Writes the crash report FIRST (the restore can otherwise delay the dump), restores without the // SPI broadcast (SENDCHANGE would run other windows' handlers, our own tint capture included, // inside a faulted process), and returns EXCEPTION_CONTINUE_SEARCH so the default handler still runs. From cc8540e2d8526cc03deafc71a4ba498928e6c975 Mon Sep 17 00:00:00 2001 From: Max <112043822+Maxaubert@users.noreply.github.com> Date: Fri, 16 Oct 2026 04:07:08 +0200 Subject: [PATCH 6/6] fix(render,track): crop-debt refresh on pointer-only frames, quiet settle recreate, folder-only bridge cap, focus dedupe bypass only after a miss (#497) - render: a pointer-only Desktop Duplication frame (LastPresentTime 0, or no texture) counts as a quiet screen, so a crop-debt refresh also fires while panning; the refresh's duplication re-create logs no settle diagnostics (refreshSdrWhite still runs). - java bridge: the dependency cap counts only DLLs opened from the bridge folder (refused above 32); API-set and system names no longer count. - focus tracking: the publish dedupe is bypassed only when a focus change since the last publish resolved to nothing published (skipped container, unanswered lookup) or after activation, so repeat focus events on the same control no longer take the view from the mouse. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TdjopXZfsUiesYxYLYHEdH --- src/render/render_engine.cpp | 54 +++++++++++++++++++++++------------- src/track/focus_track.cpp | 20 ++++++++++--- src/track/focus_track.h | 4 +-- src/track/java_bridge.cpp | 9 +++++- src/track/view_target.h | 17 ++++++------ tests/test_view_target.cpp | 14 +++++----- 6 files changed, 77 insertions(+), 41 deletions(-) diff --git a/src/render/render_engine.cpp b/src/render/render_engine.cpp index b5fdc0a4..e176b44e 100644 --- a/src/render/render_engine.cpp +++ b/src/render/render_engine.cpp @@ -77,6 +77,7 @@ struct RenderEngine::State { DuplRetry duplRetry; // paces re-creating a duplication that keeps failing (review 2026-10-16 #21) bool duplNoOutput = false; // the last recreateDupl found no output for targetDevice on our adapter CropDebt cropDebt; // a cropped copy left the cache outside the view stale (review 2026-10-16 #5) + bool quietRecreate = false; // the next recreateDupl is a crop-debt refresh: no settle diagnostics LONGLONG primeQpc = 0; // QPC at the last primeReveal() (evidence gate, issue #140) LONGLONG lastCopiedPresentQpc = 0; // LastPresentTime of the newest desktop frame we copied ComPtr revealFence; // event query: has the reveal frame's Present executed on the GPU? @@ -264,8 +265,12 @@ bool RenderEngine::State::recreateDupl() { RLog("dwm restarted: overlay exclusions re-applied (affinity=0x%lx)", (unsigned long)aff); } dupl.Reset(); + // A crop-debt refresh re-creates the duplication on every settle; its success diagnostics would + // repeat the session's first ones each time, so it logs nothing (refreshSdrWhite still runs). + const bool settle = quietRecreate; + quietRecreate = false; // A retry after a failure stays quiet until it succeeds (capture() logs the failures, paced). - const bool quiet = duplRetry.failures > 0; + const bool quiet = duplRetry.failures > 0 || settle; // Capture the target monitor's output (matched by device name). Only the legacy // single-monitor path (empty targetDevice) falls back to the first output: a NAMED monitor // that is not on our adapter (multi-GPU) fails, as retarget() does, rather than silently @@ -299,7 +304,7 @@ bool RenderEngine::State::recreateDupl() { DXGI_FORMAT fmts[] = { DXGI_FORMAT_R16G16B16A16_FLOAT }; hr = output5->DuplicateOutput1(device.Get(), 0, ARRAYSIZE(fmts), fmts, dupl.ReleaseAndGetAddressOf()); output5->Release(); - if (SUCCEEDED(hr) && dupl) { capFp16 = true; RLog("recreateDupl: DuplicateOutput1 FP16 OK"); } + if (SUCCEEDED(hr) && dupl) { capFp16 = true; if (!settle) RLog("recreateDupl: DuplicateOutput1 FP16 OK"); } else if (!quiet) RLog("recreateDupl: DuplicateOutput1 FP16 failed hr=0x%08lX", (unsigned long)hr); } } @@ -307,7 +312,8 @@ bool RenderEngine::State::recreateDupl() { IDXGIOutput1* output1 = nullptr; output->QueryInterface(__uuidof(IDXGIOutput1), (void**)&output1); if (output1) { hr = output1->DuplicateOutput(device.Get(), dupl.ReleaseAndGetAddressOf()); output1->Release(); } - if (!quiet || SUCCEEDED(hr)) RLog("recreateDupl: DuplicateOutput hr=0x%08lX capFp16=0", (unsigned long)hr); + if (!quiet || (SUCCEEDED(hr) && !settle)) + RLog("recreateDupl: DuplicateOutput hr=0x%08lX capFp16=0", (unsigned long)hr); } waitOutput = output; // keep a ref for waitVBlank (released with the device set) SafeRelease(output); @@ -324,7 +330,7 @@ bool RenderEngine::State::recreateDupl() { // The duplication is rebuilt on every zoom-in (invalidateCapture) and on ACCESS_LOST, so // this is where a session picks up the current slider position before its first frame. if (capFp16) refreshSdrWhite(); - RLog("recreateDupl: ddaModeFormat=%u colorSpace=%d isHdr=%d wantTonemap=%d rotation=%d%s", + if (!settle) RLog("recreateDupl: ddaModeFormat=%u colorSpace=%d isHdr=%d wantTonemap=%d rotation=%d%s", ddaFormat, outColorSpace, (int)isHdr, (int)wantHdrTonemap, (int)dd.Rotation, rotated ? " (ROTATED - capture unsupported, image may be wrong)" : ""); } @@ -462,6 +468,20 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { return (double)(qn.QuadPart - from.QuadPart) * 1000.0 / (double)qf.QuadPart; }; bool gotThisCall = false; + // A settled screen with crop debt owed: ask for a fresh grab (the next capture re-creates the + // duplication, whose first frame is the whole current desktop). The cached copy stays on screen + // meanwhile; haveDesktop is kept so nothing goes black. Quiet means no desktop image change: + // a timeout, or a pointer-only frame (while panning DDA keeps returning those). + auto askCropRefresh = [&]() { + if (fresh) return false; + const unsigned long long nowMs = GetTickCount64(); + if (!cropDebt.refreshDue(nowMs, 250)) return false; + cropDebt.refreshAsked(nowMs); + dupl.Reset(); + quietRecreate = true; + freshCapture = true; + return true; + }; for (int a = 0; a < firstAttempts; ++a) { if (fresh && !FreshDrainContinue(gotThisCall, msSince(q0), gotThisCall ? msSince(qFirst) : 0.0)) @@ -472,22 +492,11 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { HRESULT hr = dupl->AcquireNextFrame(to, &fi, &res); if (hr == DXGI_ERROR_WAIT_TIMEOUT) { SafeRelease(res); - // A settled screen with crop debt owed: ask for a fresh grab (the next capture re-creates - // the duplication, whose first frame is the whole current desktop). The cached copy - // stays on screen meanwhile; haveDesktop is kept so nothing goes black. - if (!fresh) { - const unsigned long long nowMs = GetTickCount64(); - if (cropDebt.refreshDue(nowMs, 250)) { - cropDebt.refreshAsked(nowMs); - dupl.Reset(); - freshCapture = true; - return haveDesktop; - } - } + if (askCropRefresh()) return haveDesktop; if (gotThisCall) break; continue; } - if (hr == DXGI_ERROR_ACCESS_LOST) { SafeRelease(res); dupl.Reset(); return gotThisCall || haveDesktop; } + if (hr == DXGI_ERROR_ACCESS_LOST) { SafeRelease(res); dupl.Reset(); quietRecreate = false; return gotThisCall || haveDesktop; } if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET) { SafeRelease(res); deviceLost = true; RLog("capture: device lost hr=0x%08lX", (unsigned long)hr); return gotThisCall || haveDesktop; @@ -499,7 +508,8 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { // LastPresentTime == 0 means only the pointer moved (no desktop image change). We ignore // the OS pointer (drawn from GetCursorInfo), so the cached copy is still valid - copy // nothing. This keeps a busy-pointer-but-static desktop from forcing any copy at all. - if (tex && fi.LastPresentTime.QuadPart != 0) { + const bool desktopChanged = tex && fi.LastPresentTime.QuadPart != 0; + if (desktopChanged) { // Match the copy texture to the captured format so the copy never mismatches (incl. // after a runtime HDR toggle changes the duplication format). The tonemap is gated on // capFp16, not on the format, so BGRA8 captures stay passthrough. @@ -546,7 +556,10 @@ bool RenderEngine::State::capture(const RECT& view, bool crop) { SafeRelease(res); dupl->ReleaseFrame(); - if (!fresh) return true; // settled desktop: the single frame is enough + if (!fresh) { // settled desktop: the single frame is enough + if (!desktopChanged && askCropRefresh()) return haveDesktop; // pointer-only = quiet + return true; + } // fresh: keep looping to drain to the latest frame (the short `to` breaks us out) } return gotThisCall || haveDesktop; // no frame within budget; keep whatever we had @@ -820,6 +833,7 @@ bool RenderEngine::State::buildDeviceResources() { // while, and capture() re-creates it lazily, paced by duplRetry. Failing here made a start during // a UAC prompt transform-only until Wind restarted, and a device rebuild loop every 0.5 s. Only // a target output missing from our adapter stays fatal: no retry can capture it. + quietRecreate = false; duplRetry.reset(); if (!recreateDupl()) { if (duplNoOutput) { RLog("buildDeviceResources: recreateDupl failed (no output)"); return false; } @@ -984,6 +998,7 @@ bool RenderEngine::revealFrameDone(double spinBudgetMs) { void RenderEngine::invalidateCapture() { if (!s_) return; s_->dupl.Reset(); + s_->quietRecreate = false; s_->duplRetry.reset(); // a deliberate fresh start (zoom-in/out) tries at once s_->haveDesktop = false; s_->freshCapture = true; @@ -1108,6 +1123,7 @@ bool RenderEngine::retarget(const MonitorTarget& m) { SetWindowPos(s_->hwnd, HWND_TOPMOST, px, m.y, m.w, m.h, SWP_NOACTIVATE); lstrcpynW(s_->targetDevice, m.device, 32); s_->dupl.Reset(); + s_->quietRecreate = false; s_->duplRetry.reset(); s_->haveDesktop = false; s_->freshCapture = true; diff --git a/src/track/focus_track.cpp b/src/track/focus_track.cpp index 2b694f70..e59d5826 100644 --- a/src/track/focus_track.cpp +++ b/src/track/focus_track.cpp @@ -340,15 +340,15 @@ void FocusTracker::setActive(bool on, bool wantCaret, bool wantFocus, bool log) if (t && on != was) PostThreadMessageW(t, kActiveMsg, on ? 1 : 0, 0); // hooks + poll rate if (on && !was && t) PostThreadMessageW(t, kWakeMsg, 0, 0); } -void FocusTracker::publish(TrackKind k, double l, double t, double r, double b, const char* src, unsigned gen) { +bool FocusTracker::publish(TrackKind k, double l, double t, double r, double b, const char* src, bool missed) { { std::lock_guard g(mu_); - if (TrackPublishIsRepeat(snap_, pubGen_, k, l, t, r, b, gen)) return; // unchanged, same focus + if (TrackPublishIsRepeat(snap_, k, l, t, r, b, missed)) return false; // unchanged snap_ = { k, ++seq_, l, t, r, b }; - pubGen_ = gen; } if (log_) wind::Log(wind::LogLevel::Info, "track", "%s via %s: %.0f,%.0f %.0fx%.0f", k == TrackKind::Caret ? "caret" : "focus", src, l, t, r - l, b - t); + return true; } // FocusTracker::run's state and steps, all on the tracker thread (#477: split out of run()). The @@ -363,7 +363,15 @@ struct FocusRun { std::atomic& wantCaret_; std::atomic& wantFocus_; std::atomic& log_; - void publish(TrackKind k, double l, double t, double r, double b, const char* src) { tracker.publish(k, l, t, r, b, src, focusGen); } + // A focus change since the last publish resolved to nothing published (a skipped container, an + // unanswered lookup), or the tracker was (re)activated: the next report is published even at + // the last snapshot's rect (TrackPublishIsRepeat). + bool missedFocus = true; + bool publish(TrackKind k, double l, double t, double r, double b, const char* src) { + const bool done = tracker.publish(k, l, t, r, b, src, missedFocus); + if (done) missedFocus = false; + return done; + } void setUp(); // COM, UIA and its timeouts, the focus lookup, the event hooks void messageLoop(MSG& m); // wakes, coalescing and the backstop poll until WM_QUIT @@ -624,6 +632,7 @@ struct FocusRun { const bool focusUnanswered = focusChanged && wantFocus_.load() && !win32Only && !java && focusLookup && !el; if (el && FAILED(el->get_CurrentBoundingRectangle(&b))) b = RECT{}; + bool focusPublished = false; // this focus change reached publish() (a same-rect repeat counts) // 1. A focus change: follow the focused control (if wanted). Its caret becomes the baseline. // A container-sized focus (the page after leaving a text box, a pane, the window) is not // something the user moved to, so it is skipped (issue #278). @@ -636,8 +645,10 @@ struct FocusRun { b.left, b.top, b.right - b.left, b.bottom - b.top); } else { publish(TrackKind::Focus, b.left, b.top, b.right, b.bottom, "uia-focus"); + focusPublished = true; } } + if (focusChanged && wantFocus_.load() && !focusPublished) missedFocus = true; followCaret(el, b, fg, java, fromPoll, rs0, rs1); if (el) el->Release(); return focusUnanswered; @@ -766,6 +777,7 @@ void FocusRun::messageLoop(MSG& m) { pendingCaret = true; ++focusGen; // (re)activation: the caret found now is a baseline too baseKey = wind::FocusKey{}; // never a "repeat" of a focus from before the zoom-in + missedFocus = true; // nor a repeat of a report from before it } else if (m.wParam == JavaBridge::kJavaFocus) { pendingFocus = true; javaDirty = true; ++javaEvents; ++focusGen; // a Java focus change: its caret is a baseline, like any other diff --git a/src/track/focus_track.h b/src/track/focus_track.h index 9f4afbba..fe4d40dd 100644 --- a/src/track/focus_track.h +++ b/src/track/focus_track.h @@ -31,7 +31,7 @@ class FocusTracker { mutable std::mutex mu_; TrackSnapshot snap_; unsigned seq_ = 0; - unsigned pubGen_ = ~0u; // focus generation of snap_ (TrackPublishIsRepeat) - void publish(TrackKind k, double l, double t, double r, double b, const char* src, unsigned gen); + // false when it was a repeat of snap_ (TrackPublishIsRepeat; missed bypasses that) + bool publish(TrackKind k, double l, double t, double r, double b, const char* src, bool missed); }; } // namespace wind diff --git a/src/track/java_bridge.cpp b/src/track/java_bridge.cpp index e62df370..88b33f23 100644 --- a/src/track/java_bridge.cpp +++ b/src/track/java_bridge.cpp @@ -175,6 +175,7 @@ static HMODULE LoadVerified(const std::wstring& path, bool log) { const std::wstring dir = cut == std::wstring::npos ? std::wstring() : path.substr(0, cut); std::vector pending; // dependency names still to look for in the folder std::set seen; // lower-case names already handled + size_t fromFolder = 0; // dependencies actually opened from the bridge folder seen.insert("windowsaccessbridge-64.dll"); // Signed, and its import table read: the names go on the pending list. auto verify = [&](const std::wstring& p, HANDLE h) { @@ -200,10 +201,16 @@ static HMODULE LoadVerified(const std::wstring& path, bool log) { const std::string name = pending.back(); pending.pop_back(); if (!seen.insert(LowerAscii(name)).second) continue; - if (seen.size() > 64) { ok = false; break; } // a dependency tree no bridge has if (IsApiSetName(name)) continue; const std::wstring d = dir + L"\\" + std::wstring(name.begin(), name.end()); if (GetFileAttributesW(d.c_str()) == INVALID_FILE_ATTRIBUTES) continue; // not from the folder + // Only files taken from the folder count: system and API-set names are free, so a real + // signed JBR/Temurin bridge (a handful of bundled DLLs) is never refused for its import list. + if (++fromFolder > 32) { + ok = false; + wind::Log(wind::LogLevel::Warn, "track", "java: %ls pulls over 32 DLLs from its folder; refused", path.c_str()); + break; + } HANDLE h = hold(d); ok = h != INVALID_HANDLE_VALUE && verify(d, h); if (!ok) wind::Log(wind::LogLevel::Warn, "track", "java: %ls needs %ls from its folder, which is unsigned or unreadable", diff --git a/src/track/view_target.h b/src/track/view_target.h index a69f2098..2534a51a 100644 --- a/src/track/view_target.h +++ b/src/track/view_target.h @@ -21,14 +21,15 @@ namespace wind { enum class ViewOwner { Mouse, Caret, Focus, Keys }; // Keys: keyboard panning (#287) enum class TrackKind { None = 0, Caret = 1, Focus = 2 }; struct TrackSnapshot { TrackKind kind = TrackKind::None; unsigned seq = 0; double l = 0, t = 0, r = 0, b = 0; }; -// The tracker publishes a report only when it differs from the last one, but only WITHIN one focus -// generation (gen: the tracker's focusGen when the report was made). A report from a new focus is -// always published, even at the same rect: returning to the same text box after a focus that was -// never published (a skipped container, a timed-out lookup) must still reach StepViewOwner, which -// acts on a seq change only (review 2026-10-16 #30). -inline bool TrackPublishIsRepeat(const TrackSnapshot& last, unsigned lastGen, TrackKind k, - double l, double t, double r, double b, unsigned gen) { - return lastGen == gen && last.kind == k && last.l == l && last.t == t && last.r == r && last.b == b; +// The tracker publishes a report only when it differs from the last one. The exception is a focus +// change that resolved to something NOT published since then (missed: a skipped container, a +// timed-out or empty lookup): returning to the same text box after it must still reach +// StepViewOwner, which acts on a seq change only (review 2026-10-16 #30). A plain repeat focus +// event on the same control (Chromium re-firing, a menu or tooltip closing) stays a repeat, so it +// cannot take the view from the mouse. +inline bool TrackPublishIsRepeat(const TrackSnapshot& last, TrackKind k, + double l, double t, double r, double b, bool missed) { + return !missed && last.kind == k && last.l == l && last.t == t && last.r == r && last.b == b; } struct ViewOwnerState { ViewOwner owner = ViewOwner::Mouse; diff --git a/tests/test_view_target.cpp b/tests/test_view_target.cpp index efe2bc11..d6a01103 100644 --- a/tests/test_view_target.cpp +++ b/tests/test_view_target.cpp @@ -10,14 +10,14 @@ static TrackSnapshot Snap(TrackKind k, unsigned seq) { TrackSnapshot s; s.kind = k; s.seq = seq; s.l = 100; s.t = 100; s.r = 102; s.b = 120; return s; } -TEST_CASE("publish dedupe holds within one focus generation only (review 2026-10-16 #30)") { +TEST_CASE("publish dedupe is bypassed only after an unpublished focus change (review 2026-10-16 #30)") { TrackSnapshot last; last.kind = TrackKind::Focus; last.seq = 4; last.l = 10; last.t = 20; last.r = 110; last.b = 40; - CHECK(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 10, 20, 110, 40, 7)); // same focus, same rect - CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 10, 20, 110, 40, 8)); // focus came back: publish - CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Caret, 10, 20, 110, 40, 7)); // another kind - CHECK_FALSE(TrackPublishIsRepeat(last, 7, TrackKind::Focus, 11, 20, 110, 40, 7)); // moved - TrackSnapshot none; // nothing published yet - CHECK_FALSE(TrackPublishIsRepeat(none, ~0u, TrackKind::Focus, 0, 0, 0, 0, 0)); + CHECK(TrackPublishIsRepeat(last, TrackKind::Focus, 10, 20, 110, 40, false)); // repeat focus event, same control + CHECK_FALSE(TrackPublishIsRepeat(last, TrackKind::Focus, 10, 20, 110, 40, true)); // back after a skipped focus: publish + CHECK_FALSE(TrackPublishIsRepeat(last, TrackKind::Caret, 10, 20, 110, 40, false)); // another kind + CHECK_FALSE(TrackPublishIsRepeat(last, TrackKind::Focus, 11, 20, 110, 40, false)); // moved + TrackSnapshot none; // nothing published yet + CHECK_FALSE(TrackPublishIsRepeat(none, TrackKind::Focus, 1, 1, 2, 2, false)); } TEST_CASE("a new caret event takes the view; a repeat of the same seq does not re-trigger") {