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

app/capture.js

@@ -23,8 +23,6 @@ const { encodePng } = require('../core/png');
  */
 
 const CLICK_DEBOUNCE_MS = 700;
-const CLICK_CAPTURE_CACHE_MS = 75;
-const CLICK_CAPTURE_CACHE_MAX_AGE_MS = 400;
 const CLICK_CAPTURE_HIDE_DELAY_MS = 25;
 
 function hasBinary(name) {
@@ -45,9 +43,6 @@ class CaptureService {
     this.session = null; // { guideId, paused, count, intervalSec }
     this.intervalTimer = null;
     this.clickWatcher = null;
-    this.captureCacheTimer = null;
-    this.captureCache = null;
-    this.captureCacheRunning = false;
     this.lastClickCapture = 0;
     this.shooting = false;
   }
@@ -189,27 +184,15 @@ 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) and arms the click-capture
+    // a brief delay so the user sees it happen).
-    // cache so the very next click is captured instantly.
     if (wasPaused && !this.session.paused) {
       const win = this.getWindow();
-      const arm = () => {
+      const tuck = () => {
         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.startClickCaptureCache();
-        }
       };
-      if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(arm, 400);
+      if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(tuck, 400);
-      else arm();
+      else tuck();
-    } else if (!wasPaused && this.session.paused) {
-      // Pausing stops the background cache refresh loop, but it does so by
-      // returning early (see startClickCaptureCache's refresh) without
-      // resetting captureCacheRunning or clearing captureCache. Without this
-      // call, resuming would find captureCacheRunning still true, so
-      // startClickCaptureCache() would no-op and every click afterwards
-      // would reuse the stale pre-pause frame instead of a fresh one.
-      this.stopClickCaptureCache();
     }
     if (this.rebuildTrayMenu) this.rebuildTrayMenu();
     this.notify('capture:state', this.state());
@@ -221,7 +204,6 @@ class CaptureService {
       this.intervalTimer = null;
     }
     this.stopClickWatcher();
-    this.stopClickCaptureCache();
     this.destroySessionTray();
     this.session = null;
     if (this.hiddenForSession) {
@@ -258,23 +240,18 @@ class CaptureService {
     try {
       const mode = this.settings.get('capture.mode') || 'fullscreen';
       const grabMode = mode === 'region' ? 'fullscreen' : mode;
-      // The background refresh loop (startClickCaptureCache) keeps this
+      // Always a fresh shot, started right now. Click captures pass the
-      // updated every ~75ms; if it's gone stale (refresh errors silently and
+      // cursor position grabbed at the instant of the click (onOsClick), so
-      // stops updating, or the cache was never refreshed after a resume),
+      // the marker always lands on the screen the user actually clicked —
-      // fall back to a fresh shot rather than reusing an old background.
+      // never on a pre-click frame from some moment earlier.
-      const cacheFresh = this.captureCache && this.captureCache.mode === grabMode
+      const finalResult = await this.shoot({
-        && Date.now() - this.captureCache.capturedAt <= CLICK_CAPTURE_CACHE_MAX_AGE_MS;
+        guideId: this.session.guideId,
-      const cached = trigger === 'click' && cacheFresh ? this.captureCache : null;
+        mode: grabMode,
-      const finalResult = cached
+        delayMs: 0,
-        ? this.storeFrameAsStep(this.session.guideId, grabMode, cached, clickPos)
+        hideWindowDelayMs: trigger === 'click' ? CLICK_CAPTURE_HIDE_DELAY_MS : null,
-        : await this.shoot({
+        refocus: false, // don't steal focus from the app the user is documenting
-          guideId: this.session.guideId,
+        clickPos,
-          mode: grabMode,
+      });
-          delayMs: 0,
-          hideWindowDelayMs: trigger === 'click' ? CLICK_CAPTURE_HIDE_DELAY_MS : null,
-          refocus: false, // don't steal focus from the app the user is documenting
-          clickPos,
-        });
       if (finalResult.ok) {
         this.session.count += 1;
         this.notify('capture:added', { guideId: this.session.guideId, step: finalResult.step, trigger });
@@ -293,40 +270,6 @@ class CaptureService {
 
   // ---- click-triggered capture --------------------------------------------
 
-  startClickCaptureCache() {
-    if (this.captureCacheRunning) return;
-    this.captureCacheRunning = true;
-    const refresh = async () => {
-      if (!this.session || this.session.paused || !this.captureCacheRunning) 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.captureCacheRunning && this.session && !this.session.paused) {
-            this.captureCache = frame;
-          }
-        }
-      } catch {
-        // Cache misses are fine; click capture falls back to a fresh shot.
-      } finally {
-        if (this.session && !this.session.paused && this.captureCacheRunning) {
-          this.captureCacheTimer = setTimeout(refresh, CLICK_CAPTURE_CACHE_MS);
-        }
-      }
-    };
-    this.captureCacheTimer = setTimeout(refresh, 0);
-  }
-
-  stopClickCaptureCache() {
-    if (this.captureCacheTimer) {
-      clearTimeout(this.captureCacheTimer);
-      this.captureCacheTimer = null;
-    }
-    this.captureCacheRunning = false;
-    this.captureCache = null;
-  }
-
   startClickWatcher() {
     this.stopClickWatcher();
     try {
@@ -383,7 +326,7 @@ while ($true) {
     this.lastClickCapture = now;
     // Grab the cursor position synchronously, right when the click is
     // detected, so the marker lands exactly where the user clicked even if
-    // the cached frame is a beat stale or a fresh shot takes a moment.
+    // the shot itself takes a moment to grab.
     const clickPos = screen.getCursorScreenPoint();
     this.sessionCapture('click', clickPos).catch(() => {});
   }

docs/CHANGELOG.md

@@ -92,12 +92,13 @@ 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 no longer reuse the same stale background screenshot
+- Click captures now always take a brand-new screenshot at the instant
-  for every step (only the click marker moved). Pausing now fully resets
+  of the click, with the click marker at the click-time cursor position.
-  the click-capture cache so resuming starts a fresh background refresh
+  Previously they reused a pre-click frame from a background cache that
-  loop, and a cached frame older than 400ms (e.g. if the background
+  could be stale by the time of the click — so the background image and
-  refresh silently stops working) is now discarded in favor of a fresh
+  the click marker didn't always line up, and after pause/resume the
-  screenshot.
+  same frozen frame could be reused for every step with only the marker
+  moving. The pre-click frame cache has been removed entirely.
 
 ### Added (initial feature set)
 

tests/unit/capture.test.js

@@ -50,127 +50,41 @@ test('click-triggered session capture uses the low-latency hide pause', async ()
   });
 });
 
-test('click-triggered session capture prefers the cached frame when ready', async () => {
+test('every click capture takes a fresh shot — pre-grabbed frames are never reused', async () => {
   const service = makeService();
-  service.settings.get = (key) => {
-    if (key === 'capture.mode') return 'fullscreen';
-    if (key === 'capture.delayMs') return 0;
-    if (key === 'capture.clickMarker') return true;
-    if (key === 'capture.clickMarkerColor') return '#E5484D';
-    if (key === 'editor.focusedViewDefaultForNewSteps') return false;
-    return null;
-  };
   service.session = { guideId: 'guide-2', paused: false, count: 0, intervalSec: 0 };
-  service.captureCache = {
+
-    mode: 'fullscreen',
+  let shots = 0;
-    png: Buffer.from('cached-png'),
+  service.captureCurrentFrame = async (mode) => {
-    size: { width: 120, height: 80 },
+    shots += 1;
-    display: { bounds: { x: 10, y: 20, width: 120, height: 80 } },
+    return {
-    cursor: { x: 70, y: 40 },
+      mode,
-    capturedAt: Date.now(),
+      png: Buffer.from(`frame-${shots}`),
+      size: { width: 100, height: 100 },
+      display: { bounds: { x: 0, y: 0, width: 100, height: 100 } },
+      cursor: { x: 50, y: 50 },
+      capturedAt: Date.now(),
+    };
   };
 
-  let shootCalled = false;
+  const captured = [];
-  service.shoot = async () => {
+  service.store.addStep = (guideId, fields, png) => {
-    shootCalled = true;
+    captured.push(png.toString());
-    throw new Error('fresh shot should not run when cache is ready');
+    return { stepId: `step-${captured.length}` };
-  };
-
-  const added = [];
-  service.store.addStep = (guideId, fields, png, size) => {
-    added.push({ guideId, fields, png, size });
-    return { stepId: 'step-2', ...fields };
-  };
-  service.notify = (channel, payload) => {
-    added.push({ channel, payload });
-  };
-
-  const result = await service.sessionCapture('click');
-
-  assert.equal(result.ok, true);
-  assert.equal(shootCalled, false);
-  assert.equal(service.session.count, 1);
-  assert.equal(added[0].guideId, 'guide-2');
-  assert.deepEqual(added[0].png, Buffer.from('cached-png'));
-  assert.deepEqual(added[0].size, { width: 120, height: 80 });
-  assert.equal(added[0].fields.annotations.length, 1);
-  assert.equal(added[0].fields.annotations[0].type, 'oval');
-});
-
-test('click-triggered capture marks the click-time cursor position, not the cached frame\'s (possibly stale) cursor', async () => {
-  const service = makeService();
-  service.settings.get = (key) => {
-    if (key === 'capture.mode') return 'fullscreen';
-    if (key === 'capture.delayMs') return 0;
-    if (key === 'capture.clickMarker') return true;
-    if (key === 'capture.clickMarkerColor') return '#E5484D';
-    if (key === 'editor.focusedViewDefaultForNewSteps') return false;
-    return null;
-  };
-  service.session = { guideId: 'guide-3', paused: false, count: 0, intervalSec: 0 };
-  service.captureCache = {
-    mode: 'fullscreen',
-    png: Buffer.from('cached-png'),
-    size: { width: 120, height: 80 },
-    display: { bounds: { x: 0, y: 0, width: 120, height: 80 } },
-    // Stale cursor position from the cache-refresh loop, well outside the
-    // display — if this were used for the marker, no annotation would be
-    // placed at all.
-    cursor: { x: 9999, y: 9999 },
-    capturedAt: Date.now(),
-  };
-
-  service.shoot = async () => {
-    throw new Error('fresh shot should not run when cache is ready');
-  };
-
-  let added = null;
-  service.store.addStep = (guideId, fields, png, size) => {
-    added = { guideId, fields, png, size };
-    return { stepId: 'step-3', ...fields };
   };
   service.notify = () => {};
 
-  // The user clicked dead center of the display.
+  await service.sessionCapture('click', { x: 10, y: 10 });
-  const result = await service.sessionCapture('click', { x: 60, y: 40 });
+  await service.sessionCapture('click', { x: 20, y: 20 });
+  service.togglePause(true);
+  service.togglePause(false);
+  await service.sessionCapture('click', { x: 30, y: 30 });
 
-  assert.equal(result.ok, true);
+  assert.deepEqual(captured, ['frame-1', 'frame-2', 'frame-3'],
-  assert.equal(added.fields.annotations.length, 1);
+    'each click must capture a brand-new frame, including after pause/resume');
-  const marker = added.fields.annotations[0];
-  assert.equal(marker.type, 'oval');
-  const d = 0.035;
-  assert.ok(Math.abs(marker.x - (0.5 - d / 2)) < 1e-9);
-  assert.ok(Math.abs(marker.y - (0.5 - (d * 120 / 80) / 2)) < 1e-9);
 });
 
-test('click-triggered session capture falls back to a fresh shot when the cached frame is stale', async () => {
+test('click capture marks the click-time cursor position', async () => {
-  const service = makeService();
-  service.session = { guideId: 'guide-stale', paused: false, count: 0, intervalSec: 0 };
-  // A frame that's well past the cache's max age — e.g. the background
-  // refresh loop died (errored silently, or never restarted after a
-  // pause/resume) and left a frozen, increasingly-stale frame behind.
-  service.captureCache = {
-    mode: 'fullscreen',
-    png: Buffer.from('stale-png'),
-    size: { width: 120, height: 80 },
-    display: { bounds: { x: 0, y: 0, width: 120, height: 80 } },
-    cursor: { x: 60, y: 40 },
-    capturedAt: Date.now() - 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 cached frame must not be reused');
-});
-
-test('live-shot click capture also marks the click-time cursor position', async () => {
   const service = makeService();
   service.settings.get = (key) => {
     if (key === 'capture.mode') return 'fullscreen';
@@ -206,7 +120,7 @@ test('live-shot click capture also marks the click-time cursor position', async
   assert.equal(added.fields.annotations[0].type, 'oval');
 });
 
-test('a new session starts paused and does not hide the window or arm the click cache until "Start recording" is pressed', async () => {
+test('a new session starts paused and does not hide the window until "Start recording" is pressed', async () => {
   const service = makeService();
   const win = {
     destroyed: false, visible: true, minimized: false, hidden: 0, shown: 0,
@@ -222,8 +136,6 @@ test('a new session starts paused and does not hide the window or arm the click
   };
   service.getWindow = () => win;
   service.clickCaptureAvailable = () => true;
-  let cacheStarted = 0;
-  service.startClickCaptureCache = () => { cacheStarted += 1; };
 
   try {
     service.startSession('guide-5');
@@ -231,7 +143,6 @@ test('a new session starts paused and does not hide the window or arm the click
     assert.equal(service.session.paused, true, 'sessions start paused');
     assert.equal(service.state().paused, true);
     assert.equal(win.hidden, 0, 'window must stay visible until recording starts');
-    assert.equal(cacheStarted, 0, 'click-capture cache must not start before recording starts');
 
     // User clicks "Start recording" (the resume action).
     service.togglePause(false);
@@ -240,7 +151,6 @@ test('a new session starts paused and does not hide the window or arm the click
 
     await new Promise((r) => setTimeout(r, 450));
     assert.equal(win.hidden, 1, 'window hides once recording actually starts');
-    assert.equal(cacheStarted, 1, 'click-capture cache is armed once recording starts');
   } finally {
     service.finishSession();
   }