From 298d7728b8e6c12dd98720db2f8b184a4b130b8e Mon Sep 17 00:00:00 2001 From: Iisyourdad Date: Thu, 11 Jun 2026 13:02:37 -0500 Subject: [PATCH] Fixed an issue where clicking wouldn't line up with screenshot part 5 --- app/capture.js | 188 ++++++++++++++++++++++++++++++------- tests/unit/capture.test.js | 54 ++++++++--- 2 files changed, 194 insertions(+), 48 deletions(-) diff --git a/app/capture.js b/app/capture.js index bbbb2ed..80b3034 100644 --- a/app/capture.js +++ b/app/capture.js @@ -30,9 +30,9 @@ const CLICK_DEBOUNCE_MS = 40; const FRAME_LOOP_IDLE_MS = 0; // A buffered frame older than this is too stale to pass off as "the screen // at the instant of the click". -const CLICK_FRAME_MAX_AGE_MS = 600; -// How long a click waits for the in-flight grab before falling back to a -// one-off fresh shot. +const CLICK_FRAME_MAX_AGE_MS = 300; +// How long a click waits for the next buffered frame before falling back to +// a one-off fresh shot. const CLICK_FRAME_WAIT_MS = 2000; const CLICK_CAPTURE_HIDE_DELAY_MS = 25; @@ -44,6 +44,13 @@ function pointInBounds(point, bounds) { && point.y <= bounds.y + bounds.height; } +function parsePoint(text) { + const x = Number(text.x); + const y = Number(text.y); + if (!Number.isFinite(x) || !Number.isFinite(y)) return null; + return { x, y }; +} + function hasBinary(name) { try { execFileSync('which', [name], { stdio: 'pipe' }); @@ -67,8 +74,9 @@ class CaptureService { this.frameWaiters = []; this.latestFrame = null; this.lastClickCapture = 0; - this.clickWatcherButtonDown = false; this.frameLoopInFlight = false; + this.clickWatcherTextBuffer = ''; + this.clickWatcherEventType = null; this.shooting = false; } @@ -409,7 +417,7 @@ class CaptureService { && sameDisplay; }; if (usable(this.latestFrame)) return this.latestFrame; - if (!this.frameLoopRunning || !this.frameLoopInFlight) return null; + if (!this.frameLoopRunning) return null; const deadline = Date.now() + CLICK_FRAME_WAIT_MS; while (this.frameLoopRunning && Date.now() < deadline) { const next = await this.nextFrame(Math.max(1, deadline - Date.now())); @@ -421,24 +429,20 @@ class CaptureService { startClickWatcher() { this.stopClickWatcher(); try { - this.clickWatcherButtonDown = false; + this.clickWatcherTextBuffer = ''; + this.clickWatcherEventType = null; if (process.platform === 'linux' && hasBinary('xinput')) { - // Stream raw button events from the X server; one capture per press. + // Stream XI2 events from X and use the ButtonPress coordinates that + // are already stamped into the event instead of sampling the cursor + // after the OS has had time to move on. this.clickWatcher = spawn('xinput', ['test-xi2', '--root'], { stdio: ['ignore', 'pipe', 'ignore'] }); this.clickWatcher.stdout.on('data', (chunk) => { this.processClickWatcherData(chunk.toString(), 'linux'); }); } else if (process.platform === 'win32') { - // Poll the left mouse button via GetAsyncKeyState; print one line per click. - const ps = ` -Add-Type -Namespace W -Name U -MemberDefinition '[DllImport("user32.dll")] public static extern short GetAsyncKeyState(int k);' -$down = $false -while ($true) { - $s = [W.U]::GetAsyncKeyState(0x01) -band 0x8000 - if ($s -and -not $down) { Write-Output CLICK } - $down = [bool]$s - Start-Sleep -Milliseconds 10 -}`; + // Use a low-level mouse hook so the click coordinates arrive with the + // event itself instead of being sampled after the fact. + const ps = this.windowsClickWatcherScript(); this.clickWatcher = spawn('powershell.exe', ['-NoProfile', '-Command', ps], { stdio: ['ignore', 'pipe', 'ignore'] }); this.clickWatcher.stdout.on('data', (chunk) => { this.processClickWatcherData(chunk.toString(), 'win32'); @@ -453,53 +457,169 @@ while ($true) { } } + windowsClickWatcherScript() { + return ` +$source = @" +using System; +using System.Diagnostics; +using System.Runtime.InteropServices; + +public static class MouseHook { + public const int WH_MOUSE_LL = 14; + public const int WM_LBUTTONDOWN = 0x0201; + + public delegate IntPtr LowLevelMouseProc(int nCode, IntPtr wParam, IntPtr lParam); + + [StructLayout(LayoutKind.Sequential)] + public struct POINT { + public int X; + public int Y; + } + + [StructLayout(LayoutKind.Sequential)] + public struct MSLLHOOKSTRUCT { + public POINT pt; + public uint mouseData; + public uint flags; + public uint time; + public IntPtr dwExtraInfo; + } + + [StructLayout(LayoutKind.Sequential)] + public struct MSG { + public IntPtr hwnd; + public uint message; + public IntPtr wParam; + public IntPtr lParam; + public uint time; + public POINT pt; + } + + [DllImport("user32.dll", SetLastError = true)] + public static extern IntPtr SetWindowsHookEx(int idHook, LowLevelMouseProc lpfn, IntPtr hMod, uint dwThreadId); + + [DllImport("user32.dll", SetLastError = true)] + public static extern bool UnhookWindowsHookEx(IntPtr hhk); + + [DllImport("user32.dll", SetLastError = true)] + public static extern IntPtr CallNextHookEx(IntPtr hhk, int nCode, IntPtr wParam, IntPtr lParam); + + [DllImport("user32.dll")] + public static extern bool GetMessage(out MSG lpMsg, IntPtr hWnd, uint wMsgFilterMin, uint wMsgFilterMax); + + [DllImport("user32.dll")] + public static extern bool TranslateMessage([In] ref MSG lpMsg); + + [DllImport("user32.dll")] + public static extern IntPtr DispatchMessage([In] ref MSG lpMsg); + + [DllImport("kernel32.dll", CharSet = CharSet.Auto, SetLastError = true)] + public static extern IntPtr GetModuleHandle(string lpModuleName); +} +"@ +Add-Type -TypeDefinition $source -Language CSharp + +$script:mouseHookProc = [MouseHook+LowLevelMouseProc]{ + param([int]$nCode, [IntPtr]$wParam, [IntPtr]$lParam) + + if ($nCode -ge 0 -and $wParam.ToInt64() -eq [MouseHook]::WM_LBUTTONDOWN) { + $hookStruct = [Runtime.InteropServices.Marshal]::PtrToStructure($lParam, [Type][MouseHook+MSLLHOOKSTRUCT]) + [Console]::Out.WriteLine(("CLICK {0} {1}" -f $hookStruct.pt.X, $hookStruct.pt.Y)) + [Console]::Out.Flush() + } + + return [MouseHook]::CallNextHookEx([IntPtr]::Zero, $nCode, $wParam, $lParam) +} + +$module = [MouseHook]::GetModuleHandle([System.Diagnostics.Process]::GetCurrentProcess().MainModule.ModuleName) +if ($module -eq [IntPtr]::Zero) { + throw "GetModuleHandle failed" +} + +$hook = [MouseHook]::SetWindowsHookEx([MouseHook]::WH_MOUSE_LL, $script:mouseHookProc, $module, 0) +if ($hook -eq [IntPtr]::Zero) { + throw "SetWindowsHookEx(WH_MOUSE_LL) failed" +} + +try { + $msg = New-Object 'MouseHook+MSG' + while ([MouseHook]::GetMessage([ref]$msg, [IntPtr]::Zero, 0, 0)) { + [MouseHook]::TranslateMessage([ref]$msg) | Out-Null + [MouseHook]::DispatchMessage([ref]$msg) | Out-Null + } +} finally { + [MouseHook]::UnhookWindowsHookEx($hook) | Out-Null +} +`; + } + stopClickWatcher() { if (this.clickWatcher) { try { this.clickWatcher.kill(); } catch { /* already gone */ } this.clickWatcher = null; } - this.clickWatcherButtonDown = false; + this.clickWatcherTextBuffer = ''; + this.clickWatcherEventType = null; } processClickWatcherData(text, platform = process.platform) { - const lines = String(text).split(/\r?\n/); + this.clickWatcherTextBuffer += String(text); + const lines = this.clickWatcherTextBuffer.split(/\r?\n/); + this.clickWatcherTextBuffer = lines.pop() || ''; if (platform === 'linux') { for (const line of lines) { if (!line) continue; - // xinput batches multiple events into one chunk, so parse line by - // line and track press/release state instead of collapsing the chunk. - if (/RawButtonPress|ButtonPress/.test(line)) { - if (!this.clickWatcherButtonDown) { - this.clickWatcherButtonDown = true; - this.onOsClick(); + const eventMatch = /^\s*EVENT type \d+ \(([^)]+)\)/.exec(line); + if (eventMatch) { + this.clickWatcherEventType = eventMatch[1]; + continue; + } + if (this.clickWatcherEventType && /ButtonPress$/.test(this.clickWatcherEventType)) { + const pointMatch = /^\s*root:\s*([-\d.]+)\/([-\d.]+)/.exec(line); + if (pointMatch) { + this.onOsClick(this.normalizeClickPoint(parsePoint({ x: pointMatch[1], y: pointMatch[2] }))); + this.clickWatcherEventType = null; } - } else if (/RawButtonRelease|ButtonRelease/.test(line)) { - this.clickWatcherButtonDown = false; } } return; } if (platform === 'win32') { for (const line of lines) { - if (line.includes('CLICK')) this.onOsClick(); + const clickMatch = /^\s*CLICK\s+([-\d.]+)\s+([-\d.]+)/.exec(line); + if (clickMatch) { + this.onOsClick(this.normalizeClickPoint(parsePoint({ x: clickMatch[1], y: clickMatch[2] }))); + } } } } - onOsClick(at = Date.now()) { + normalizeClickPoint(point) { + if (!point) return null; + try { + const dip = screen.screenToDipPoint(point); + return dip && Number.isFinite(dip.x) && Number.isFinite(dip.y) ? dip : point; + } catch { + return point; + } + } + + onOsClick(clickPos = null, at = Date.now()) { if (!this.session || this.session.paused) return; // Ignore clicks on StepForge itself (pausing, finishing, editing). if (BrowserWindow.getFocusedWindow()) return; if (at - this.lastClickCapture < CLICK_DEBOUNCE_MS) return; this.lastClickCapture = at; - // Grab the cursor position synchronously, right when the click is - // detected, so the marker lands exactly where the user clicked even if - // the shot itself takes a moment to grab. - const clickPos = screen.getCursorScreenPoint(); + if (!clickPos) { + // Fallback for platforms or event streams that do not provide + // coordinates. + clickPos = screen.getCursorScreenPoint(); + } this.sessionCapture('click', clickPos).catch(() => {}); } async captureCurrentFrame(mode, capturePoint = null) { + const capturedAt = Date.now(); const grabbed = await this.grab(mode, capturePoint); return { mode, @@ -507,7 +627,7 @@ while ($true) { size: grabbed.image.getSize(), display: grabbed.display, cursor: capturePoint || grabbed.cursor, - capturedAt: Date.now(), + capturedAt, }; } diff --git a/tests/unit/capture.test.js b/tests/unit/capture.test.js index 8f1ecdf..97a1d25 100644 --- a/tests/unit/capture.test.js +++ b/tests/unit/capture.test.js @@ -63,31 +63,51 @@ function makeFrame(name, ageMs = 0, overrides = {}) { test('rapid click watcher bursts are parsed one click at a time', () => { const service = makeService(); - let clicks = 0; - service.onOsClick = () => { - clicks += 1; + const clicks = []; + service.normalizeClickPoint = (point) => point; + service.onOsClick = (clickPos) => { + clicks.push(clickPos); }; service.processClickWatcherData([ 'EVENT type 17 (RawButtonPress)', + 'root: 10/20', 'EVENT type 18 (RawButtonRelease)', 'EVENT type 17 (RawButtonPress)', + 'root: 30/40', 'EVENT type 18 (RawButtonRelease)', ].join('\n'), 'linux'); - assert.equal(clicks, 2); + assert.deepEqual(clicks, [ + { x: 10, y: 20 }, + { x: 30, y: 40 }, + ]); }); -test('windows click watcher output is counted line by line', () => { +test('windows click watcher output is buffered and parsed with coordinates', () => { const service = makeService(); - let clicks = 0; - service.onOsClick = () => { - clicks += 1; + const clicks = []; + service.normalizeClickPoint = (point) => point; + service.onOsClick = (clickPos) => { + clicks.push(clickPos); }; - service.processClickWatcherData('CLICK\r\nCLICK\r\n', 'win32'); + service.processClickWatcherData('CL', 'win32'); + service.processClickWatcherData('ICK 100 200\r\nCLICK 300 400\r\n', 'win32'); - assert.equal(clicks, 2); + assert.deepEqual(clicks, [ + { x: 100, y: 200 }, + { x: 300, y: 400 }, + ]); +}); + +test('the windows click watcher script installs a low-level mouse hook', () => { + const service = makeService(); + const script = service.windowsClickWatcherScript(); + + assert.match(script, /WH_MOUSE_LL/); + assert.match(script, /WM_LBUTTONDOWN/); + assert.match(script, /CLICK \{0\} \{1\}/); }); test('a click is served instantly from the freshly buffered frame', async () => { @@ -157,7 +177,7 @@ test('a stale buffered frame is not reused — the click falls back to a fresh s assert.equal(shootCalled, true, 'a stale buffered frame must not be reused'); }); -test('an idle click capture does not wait for the next frame loop tick', async () => { +test('a running frame loop waits for the next buffered frame when the cache is empty', async () => { const service = makeService(); service.session = { guideId: 'guide-idle', paused: false, count: 0, intervalSec: 0 }; service.frameLoopRunning = true; @@ -166,7 +186,7 @@ test('an idle click capture does not wait for the next frame loop tick', async ( let nextFrameCalled = false; service.nextFrame = async () => { nextFrameCalled = true; - throw new Error('idle clicks must not wait for a new frame'); + return makeFrame('loop-frame'); }; let shootCalled = false; @@ -174,12 +194,18 @@ test('an idle click capture does not wait for the next frame loop tick', async ( shootCalled = true; return { ok: true, step: { stepId: 'idle-step' } }; }; + const added = []; + service.store.addStep = (guideId, fields, png) => { + added.push(png.toString()); + return { stepId: 'idle-step' }; + }; const result = await service.sessionCapture('click', { x: 1, y: 1 }); assert.equal(result.ok, true); - assert.equal(shootCalled, true); - assert.equal(nextFrameCalled, false); + assert.equal(shootCalled, false); + assert.equal(nextFrameCalled, true); + assert.deepEqual(added, ['loop-frame']); }); test('clicks during an in-flight grab wait for the frame instead of being dropped', async () => {