Fixed an issue where clicking wouldn't line up with screenshot part 2

app/capture.js

@@ -22,7 +22,18 @@ const { encodePng } = require('../core/png');
  * failures surface as { ok: false, reason } instead of crashing.
  */
 
-const CLICK_DEBOUNCE_MS = 700;
+// Dedupe duplicate watcher events for one physical click while still
+// allowing intentionally fast clicking.
+const CLICK_DEBOUNCE_MS = 150;
+// Idle gap between frame-loop grabs; the effective refresh rate is
+// grab-duration + this.
+const FRAME_LOOP_IDLE_MS = 50;
+// 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_WAIT_MS = 2000;
 const CLICK_CAPTURE_HIDE_DELAY_MS = 25;
 
 function hasBinary(name) {
@@ -43,6 +54,10 @@ class CaptureService {
     this.session = null; // { guideId, paused, count, intervalSec }
     this.intervalTimer = null;
     this.clickWatcher = null;
+    this.frameLoopTimer = null;
+    this.frameLoopRunning = false;
+    this.frameWaiters = [];
+    this.latestFrame = null;
     this.lastClickCapture = 0;
     this.shooting = false;
   }
@@ -184,15 +199,22 @@ class CaptureService {
     const wasPaused = this.session.paused;
     this.session.paused = typeof force === 'boolean' ? force : !this.session.paused;
     // Starting/resuming tucks the window away again for clean shots (after
-    // a brief delay so the user sees it happen).
+    // a brief delay so the user sees it happen) and starts the frame loop
+    // that serves click captures. Pausing stops the loop and discards the
+    // buffered frame, so a resume can never serve a pre-pause screen.
     if (wasPaused && !this.session.paused) {
       const win = this.getWindow();
-      const tuck = () => {
+      const arm = () => {
         if (!this.session || this.session.paused) return;
         if (this.hiddenForSession && win && !win.isDestroyed() && win.isVisible()) win.hide();
+        if (this.settings.get('capture.captureOutsideClicks') !== false && this.clickCaptureAvailable()) {
+          this.startFrameLoop();
+        }
       };
-      if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(tuck, 400);
+      if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(arm, 400);
-      else tuck();
+      else arm();
+    } else if (!wasPaused && this.session.paused) {
+      this.stopFrameLoop();
     }
     if (this.rebuildTrayMenu) this.rebuildTrayMenu();
     this.notify('capture:state', this.state());
@@ -204,6 +226,7 @@ class CaptureService {
       this.intervalTimer = null;
     }
     this.stopClickWatcher();
+    this.stopFrameLoop();
     this.destroySessionTray();
     this.session = null;
     if (this.hiddenForSession) {
@@ -230,20 +253,34 @@ class CaptureService {
   /** One capture inside the active session (hotkey/click/interval/manual). */
   async sessionCapture(trigger = 'hotkey', clickPos = null) {
     if (!this.session || this.session.paused) return { ok: false, reason: 'no active capture session' };
-    if (this.shooting) return { ok: false, reason: 'capture already in progress' };
     // Automatic triggers stand down while the user is in StepForge, so the
     // app stays clickable mid-session and never screenshots itself.
     if (trigger !== 'manual' && this.userIsInApp()) {
       return { ok: false, reason: 'skipped — StepForge is focused' };
     }
+
+    // Clicks are served from the frame loop: the buffered frame was grabbed
+    // at (or moments before) the click instant, so the background matches
+    // what the user clicked on. A click that lands while a grab is in
+    // flight waits for that frame instead of being dropped, so fast
+    // clicking still yields one step per click.
+    if (trigger === 'click') {
+      const frame = await this.frameForClick();
+      if (!this.session || this.session.paused) return { ok: false, reason: 'no active capture session' };
+      if (frame) {
+        const result = this.storeFrameAsStep(this.session.guideId, frame.mode, frame, clickPos);
+        if (result.ok) this.noteStepAdded(result.step, trigger);
+        return result;
+      }
+      // No usable frame (loop not running or grab failing): fall through
+      // to a one-off fresh shot.
+    }
+
+    if (this.shooting) return { ok: false, reason: 'capture already in progress' };
     this.shooting = true;
     try {
       const mode = this.settings.get('capture.mode') || 'fullscreen';
       const grabMode = mode === 'region' ? 'fullscreen' : mode;
-      // Always a fresh shot, started right now. Click captures pass the
-      // cursor position grabbed at the instant of the click (onOsClick), so
-      // the marker always lands on the screen the user actually clicked —
-      // never on a pre-click frame from some moment earlier.
       const finalResult = await this.shoot({
         guideId: this.session.guideId,
         mode: grabMode,
@@ -252,24 +289,111 @@ class CaptureService {
         refocus: false, // don't steal focus from the app the user is documenting
         clickPos,
       });
-      if (finalResult.ok) {
+      if (finalResult.ok) this.noteStepAdded(finalResult.step, trigger);
-        this.session.count += 1;
-        this.notify('capture:added', { guideId: this.session.guideId, step: finalResult.step, trigger });
-        this.notify('capture:state', this.state());
-        if (this.rebuildTrayMenu) this.rebuildTrayMenu(); // refresh step counter
-      }
       return finalResult;
     } finally {
       this.shooting = false;
     }
   }
 
+  noteStepAdded(step, trigger) {
+    this.session.count += 1;
+    this.notify('capture:added', { guideId: this.session.guideId, step, trigger });
+    this.notify('capture:state', this.state());
+    if (this.rebuildTrayMenu) this.rebuildTrayMenu(); // refresh step counter
+  }
+
   hotkeyCapture() {
     return this.sessionCapture('hotkey');
   }
 
   // ---- click-triggered capture --------------------------------------------
 
+  /**
+   * Continuous screen-grab loop that runs while recording. It keeps the most
+   * recent frame in `latestFrame` so a click can be served from a frame
+   * grabbed at (or moments before) the instant of the click — a fresh grab
+   * started after the click would land hundreds of ms late and show the
+   * click's effects instead of what the user clicked on.
+   */
+  startFrameLoop() {
+    if (this.frameLoopRunning) return;
+    this.frameLoopRunning = true;
+    const tick = async () => {
+      if (!this.frameLoopRunning) return;
+      if (!this.session || this.session.paused) {
+        this.frameLoopRunning = false;
+        return;
+      }
+      try {
+        if (!this.shooting) {
+          const mode = this.settings.get('capture.mode') || 'fullscreen';
+          const grabMode = mode === 'region' ? 'fullscreen' : mode;
+          const frame = await this.captureCurrentFrame(grabMode);
+          if (this.frameLoopRunning) this.acceptFrame(frame);
+        }
+      } catch {
+        // Grab failures are fine — clicks fall back to a one-off fresh shot.
+      } finally {
+        if (this.frameLoopRunning && this.session && !this.session.paused) {
+          this.frameLoopTimer = setTimeout(tick, FRAME_LOOP_IDLE_MS);
+        }
+      }
+    };
+    this.frameLoopTimer = setTimeout(tick, 0);
+  }
+
+  /** Store a grabbed frame and hand it to any clicks waiting on it. */
+  acceptFrame(frame) {
+    this.latestFrame = frame;
+    const waiters = this.frameWaiters;
+    this.frameWaiters = [];
+    for (const resolve of waiters) resolve(frame);
+  }
+
+  /** Resolves with the next frame the loop grabs (null on timeout/stop). */
+  nextFrame(timeoutMs) {
+    return new Promise((resolve) => {
+      const entry = (frame) => {
+        clearTimeout(timer);
+        resolve(frame);
+      };
+      const timer = setTimeout(() => {
+        this.frameWaiters = this.frameWaiters.filter((w) => w !== entry);
+        resolve(null);
+      }, timeoutMs);
+      this.frameWaiters.push(entry);
+    });
+  }
+
+  stopFrameLoop() {
+    if (this.frameLoopTimer) {
+      clearTimeout(this.frameLoopTimer);
+      this.frameLoopTimer = null;
+    }
+    this.frameLoopRunning = false;
+    this.latestFrame = null;
+    const waiters = this.frameWaiters;
+    this.frameWaiters = [];
+    for (const resolve of waiters) resolve(null);
+  }
+
+  /**
+   * Freshest frame usable for a click capture: the buffered frame when it's
+   * recent enough, otherwise the next frame the loop delivers. Null when the
+   * loop isn't running or can't deliver in time.
+   */
+  async frameForClick() {
+    const mode = this.settings.get('capture.mode') || 'fullscreen';
+    const grabMode = mode === 'region' ? 'fullscreen' : mode;
+    const usable = (f) => f && f.mode === grabMode
+      && Date.now() - f.capturedAt <= CLICK_FRAME_MAX_AGE_MS;
+    if (usable(this.latestFrame)) return this.latestFrame;
+    if (!this.frameLoopRunning) return null;
+    const next = await this.nextFrame(CLICK_FRAME_WAIT_MS);
+    return usable(next) ? next : null;
+  }
+
   startClickWatcher() {
     this.stopClickWatcher();
     try {

docs/CHANGELOG.md

@@ -92,13 +92,15 @@ Initial release.
   literal text "undefined" by an old bug); a corrupted file is now
   treated as empty instead of crashing the dialog, and is overwritten
   with valid JSON the next time settings are saved.
-- Click captures now always take a brand-new screenshot at the instant
+- Click captures now line up with the click. While recording, a
-  of the click, with the click marker at the click-time cursor position.
+  continuous screen-grab loop keeps the latest frame buffered, and each
-  Previously they reused a pre-click frame from a background cache that
+  click becomes a step from a frame grabbed at (or moments before) the
-  could be stale by the time of the click — so the background image and
+  click instant, with the marker at the click-time cursor position — a
-  the click marker didn't always line up, and after pause/resume the
+  frame older than 600ms is never used. Fast clicks are no longer
-  same frozen frame could be reused for every step with only the marker
+  dropped: a click that lands while a grab is in flight waits for that
-  moving. The pre-click frame cache has been removed entirely.
+  frame instead of being discarded, and the click debounce was lowered
+  from 700ms to 150ms. Pausing stops the loop and discards the buffered
+  frame, so resuming can never reuse a stale pre-pause screenshot.
 
 ### Added (initial feature set)
 

tests/unit/capture.test.js

@@ -50,38 +50,90 @@ test('click-triggered session capture uses the low-latency hide pause', async ()
   });
 });
 
-test('every click capture takes a fresh shot — pre-grabbed frames are never reused', async () => {
+function makeFrame(name, ageMs = 0) {
-  const service = makeService();
-  service.session = { guideId: 'guide-2', paused: false, count: 0, intervalSec: 0 };
-
-  let shots = 0;
-  service.captureCurrentFrame = async (mode) => {
-    shots += 1;
   return {
-      mode,
+    mode: 'fullscreen',
-      png: Buffer.from(`frame-${shots}`),
+    png: Buffer.from(name),
     size: { width: 100, height: 100 },
     display: { bounds: { x: 0, y: 0, width: 100, height: 100 } },
     cursor: { x: 50, y: 50 },
-      capturedAt: Date.now(),
+    capturedAt: Date.now() - ageMs,
-    };
   };
+}
 
-  const captured = [];
+test('a click is served instantly from the freshly buffered frame', async () => {
+  const service = makeService();
+  service.session = { guideId: 'guide-2', paused: false, count: 0, intervalSec: 0 };
+  service.latestFrame = makeFrame('buffered-png');
+  service.shoot = async () => {
+    throw new Error('must not take a fresh shot when a buffered frame is ready');
+  };
+  const added = [];
   service.store.addStep = (guideId, fields, png) => {
-    captured.push(png.toString());
+    added.push(png.toString());
-    return { stepId: `step-${captured.length}` };
+    return { stepId: 'step-1' };
   };
-  service.notify = () => {};
 
-  await service.sessionCapture('click', { x: 10, y: 10 });
+  const result = await service.sessionCapture('click', { x: 10, y: 10 });
-  await service.sessionCapture('click', { x: 20, y: 20 });
+
+  assert.equal(result.ok, true);
+  assert.deepEqual(added, ['buffered-png']);
+  assert.equal(service.session.count, 1);
+});
+
+test('a stale buffered frame is not reused — the click falls back to a fresh shot', async () => {
+  const service = makeService();
+  service.session = { guideId: 'guide-stale', paused: false, count: 0, intervalSec: 0 };
+  service.latestFrame = makeFrame('stale-png', 10_000);
+
+  let shootCalled = false;
+  service.shoot = async () => {
+    shootCalled = true;
+    return { ok: true, step: { stepId: 'fresh-step' } };
+  };
+
+  const result = await service.sessionCapture('click', { x: 1, y: 1 });
+
+  assert.equal(result.ok, true);
+  assert.equal(shootCalled, true, 'a stale buffered frame must not be reused');
+});
+
+test('clicks during an in-flight grab wait for the frame instead of being dropped', async () => {
+  const service = makeService();
+  service.session = { guideId: 'guide-fast', paused: false, count: 0, intervalSec: 0 };
+  service.frameLoopRunning = true; // a grab is in flight, no frame buffered yet
+  service.shoot = async () => {
+    throw new Error('waiting clicks must use the loop frame, not a competing shot');
+  };
+  const added = [];
+  service.store.addStep = (guideId, fields, png) => {
+    added.push(png.toString());
+    return { stepId: `step-${added.length}` };
+  };
+
+  // Two rapid clicks land before the grab completes.
+  const first = service.sessionCapture('click', { x: 1, y: 1 });
+  const second = service.sessionCapture('click', { x: 2, y: 2 });
+  service.acceptFrame(makeFrame('loop-frame'));
+  const [r1, r2] = await Promise.all([first, second]);
+
+  assert.equal(r1.ok, true);
+  assert.equal(r2.ok, true);
+  assert.deepEqual(added, ['loop-frame', 'loop-frame'],
+    'both clicks must become steps from the frame that was in flight');
+  assert.equal(service.session.count, 2);
+});
+
+test('pausing stops the frame loop and discards the buffered frame', () => {
+  const service = makeService();
+  service.session = { guideId: 'guide-pause', paused: false, count: 0, intervalSec: 0 };
+  service.frameLoopRunning = true;
+  service.latestFrame = makeFrame('pre-pause');
+
   service.togglePause(true);
-  service.togglePause(false);
-  await service.sessionCapture('click', { x: 30, y: 30 });
 
-  assert.deepEqual(captured, ['frame-1', 'frame-2', 'frame-3'],
+  assert.equal(service.frameLoopRunning, false);
-    'each click must capture a brand-new frame, including after pause/resume');
+  assert.equal(service.latestFrame, null, 'a resume must never serve a pre-pause frame');
 });
 
 test('click capture marks the click-time cursor position', async () => {