Undo

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 = 300;
+const CLICK_FRAME_MAX_AGE_MS = 600;
-// How long a click waits for the next buffered frame before falling back to
+// How long a click waits for the in-flight grab before falling back to a
-// a one-off fresh shot.
+// one-off fresh shot.
 const CLICK_FRAME_WAIT_MS = 2000;
 const CLICK_CAPTURE_HIDE_DELAY_MS = 25;
 
@@ -44,13 +44,6 @@ 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' });
@@ -74,9 +67,8 @@ class CaptureService {
     this.frameWaiters = [];
     this.latestFrame = null;
     this.lastClickCapture = 0;
+    this.clickWatcherButtonDown = false;
     this.frameLoopInFlight = false;
-    this.clickWatcherTextBuffer = '';
-    this.clickWatcherEventType = null;
     this.shooting = false;
   }
 
@@ -417,7 +409,7 @@ class CaptureService {
         && sameDisplay;
     };
     if (usable(this.latestFrame)) return this.latestFrame;
-    if (!this.frameLoopRunning) return null;
+    if (!this.frameLoopRunning || !this.frameLoopInFlight) 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()));
@@ -429,20 +421,24 @@ class CaptureService {
   startClickWatcher() {
     this.stopClickWatcher();
     try {
-      this.clickWatcherTextBuffer = '';
+      this.clickWatcherButtonDown = false;
-      this.clickWatcherEventType = null;
       if (process.platform === 'linux' && hasBinary('xinput')) {
-        // Stream XI2 events from X and use the ButtonPress coordinates that
+        // Stream raw button events from the X server; one capture per press.
-        // 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') {
-        // Use a low-level mouse hook so the click coordinates arrive with the
+        // Poll the left mouse button via GetAsyncKeyState; print one line per click.
-        // event itself instead of being sampled after the fact.
+        const ps = `
-        const ps = this.windowsClickWatcherScript();
+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
+}`;
         this.clickWatcher = spawn('powershell.exe', ['-NoProfile', '-Command', ps], { stdio: ['ignore', 'pipe', 'ignore'] });
         this.clickWatcher.stdout.on('data', (chunk) => {
           this.processClickWatcherData(chunk.toString(), 'win32');
@@ -457,169 +453,53 @@ class CaptureService {
     }
   }
 
-  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.clickWatcherTextBuffer = '';
+    this.clickWatcherButtonDown = false;
-    this.clickWatcherEventType = null;
   }
 
   processClickWatcherData(text, platform = process.platform) {
-    this.clickWatcherTextBuffer += String(text);
+    const lines = String(text).split(/\r?\n/);
-    const lines = this.clickWatcherTextBuffer.split(/\r?\n/);
-    this.clickWatcherTextBuffer = lines.pop() || '';
     if (platform === 'linux') {
       for (const line of lines) {
         if (!line) continue;
-        const eventMatch = /^\s*EVENT type \d+ \(([^)]+)\)/.exec(line);
+        // xinput batches multiple events into one chunk, so parse line by
-        if (eventMatch) {
+        // line and track press/release state instead of collapsing the chunk.
-          this.clickWatcherEventType = eventMatch[1];
+        if (/RawButtonPress|ButtonPress/.test(line)) {
-          continue;
+          if (!this.clickWatcherButtonDown) {
-        }
+            this.clickWatcherButtonDown = true;
-        if (this.clickWatcherEventType && /ButtonPress$/.test(this.clickWatcherEventType)) {
+            this.onOsClick();
-          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) {
-        const clickMatch = /^\s*CLICK\s+([-\d.]+)\s+([-\d.]+)/.exec(line);
+        if (line.includes('CLICK')) this.onOsClick();
-        if (clickMatch) {
-          this.onOsClick(this.normalizeClickPoint(parsePoint({ x: clickMatch[1], y: clickMatch[2] })));
-        }
       }
     }
   }
 
-  normalizeClickPoint(point) {
+  onOsClick(at = Date.now()) {
-    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;
-    if (!clickPos) {
+    // Grab the cursor position synchronously, right when the click is
-      // Fallback for platforms or event streams that do not provide
+    // detected, so the marker lands exactly where the user clicked even if
-      // coordinates.
+    // the shot itself takes a moment to grab.
-      clickPos = screen.getCursorScreenPoint();
+    const 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,
@@ -627,7 +507,7 @@ try {
       size: grabbed.image.getSize(),
       display: grabbed.display,
       cursor: capturePoint || grabbed.cursor,
-      capturedAt,
+      capturedAt: Date.now(),
     };
   }
 

tests/unit/capture.test.js

@@ -63,51 +63,31 @@ function makeFrame(name, ageMs = 0, overrides = {}) {
 
 test('rapid click watcher bursts are parsed one click at a time', () => {
   const service = makeService();
-  const clicks = [];
+  let clicks = 0;
-  service.normalizeClickPoint = (point) => point;
+  service.onOsClick = () => {
-  service.onOsClick = (clickPos) => {
+    clicks += 1;
-    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.deepEqual(clicks, [
+  assert.equal(clicks, 2);
-    { x: 10, y: 20 },
-    { x: 30, y: 40 },
-  ]);
 });
 
-test('windows click watcher output is buffered and parsed with coordinates', () => {
+test('windows click watcher output is counted line by line', () => {
   const service = makeService();
-  const clicks = [];
+  let clicks = 0;
-  service.normalizeClickPoint = (point) => point;
+  service.onOsClick = () => {
-  service.onOsClick = (clickPos) => {
+    clicks += 1;
-    clicks.push(clickPos);
   };
 
-  service.processClickWatcherData('CL', 'win32');
+  service.processClickWatcherData('CLICK\r\nCLICK\r\n', 'win32');
-  service.processClickWatcherData('ICK 100 200\r\nCLICK 300 400\r\n', 'win32');
 
-  assert.deepEqual(clicks, [
+  assert.equal(clicks, 2);
-    { 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 () => {
@@ -177,7 +157,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('a running frame loop waits for the next buffered frame when the cache is empty', async () => {
+test('an idle click capture does not wait for the next frame loop tick', async () => {
   const service = makeService();
   service.session = { guideId: 'guide-idle', paused: false, count: 0, intervalSec: 0 };
   service.frameLoopRunning = true;
@@ -186,7 +166,7 @@ test('a running frame loop waits for the next buffered frame when the cache is e
   let nextFrameCalled = false;
   service.nextFrame = async () => {
     nextFrameCalled = true;
-    return makeFrame('loop-frame');
+    throw new Error('idle clicks must not wait for a new frame');
   };
 
   let shootCalled = false;
@@ -194,18 +174,12 @@ test('a running frame loop waits for the next buffered frame when the cache is e
     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, false);
+  assert.equal(shootCalled, true);
-  assert.equal(nextFrameCalled, true);
+  assert.equal(nextFrameCalled, false);
-  assert.deepEqual(added, ['loop-frame']);
 });
 
 test('clicks during an in-flight grab wait for the frame instead of being dropped', async () => {