diff --git a/app/capture.js b/app/capture.js index 99c286e..dacde1c 100644 --- a/app/capture.js +++ b/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); - else tuck(); + if (this.hiddenForSession && win && !win.isDestroyed()) setTimeout(arm, 400); + 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) { - 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 - } + if (finalResult.ok) this.noteStepAdded(finalResult.step, trigger); 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 { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 30c8954..e320f2b 100644 --- a/docs/CHANGELOG.md +++ b/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 - of the click, with the click marker at the click-time cursor position. - Previously they reused a pre-click frame from a background cache that - could be stale by the time of the click — so the background image and - the click marker didn't always line up, and after pause/resume the - same frozen frame could be reused for every step with only the marker - moving. The pre-click frame cache has been removed entirely. +- Click captures now line up with the click. While recording, a + continuous screen-grab loop keeps the latest frame buffered, and each + click becomes a step from a frame grabbed at (or moments before) the + click instant, with the marker at the click-time cursor position — a + frame older than 600ms is never used. Fast clicks are no longer + dropped: a click that lands while a grab is in flight waits for that + 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) diff --git a/tests/unit/capture.test.js b/tests/unit/capture.test.js index 5974a83..2fb8f18 100644 --- a/tests/unit/capture.test.js +++ b/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) { + return { + mode: 'fullscreen', + 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() - ageMs, + }; +} + +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 }; - - let shots = 0; - service.captureCurrentFrame = async (mode) => { - shots += 1; - return { - mode, - 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(), - }; + service.latestFrame = makeFrame('buffered-png'); + service.shoot = async () => { + throw new Error('must not take a fresh shot when a buffered frame is ready'); }; - - const captured = []; + const added = []; service.store.addStep = (guideId, fields, png) => { - captured.push(png.toString()); - return { stepId: `step-${captured.length}` }; + added.push(png.toString()); + return { stepId: 'step-1' }; }; - service.notify = () => {}; - await service.sessionCapture('click', { x: 10, y: 10 }); - await service.sessionCapture('click', { x: 20, y: 20 }); + const result = await service.sessionCapture('click', { x: 10, y: 10 }); + + 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'], - 'each click must capture a brand-new frame, including after pause/resume'); + assert.equal(service.frameLoopRunning, false); + assert.equal(service.latestFrame, null, 'a resume must never serve a pre-pause frame'); }); test('click capture marks the click-time cursor position', async () => {