diff --git a/app/capture.js b/app/capture.js index f5d4f78..f157760 100644 --- a/app/capture.js +++ b/app/capture.js @@ -25,18 +25,25 @@ const { encodePng } = require('../core/png'); // Dedupe duplicate watcher events for one physical click while still // allowing intentionally fast clicking. const CLICK_DEBOUNCE_MS = 40; -// Idle gap between frame-loop grabs. Keep this at zero so the buffered -// frame stays as close to real time as possible while recording. -const FRAME_LOOP_IDLE_MS = 0; +// Idle gap between frame-loop grabs. Must stay well above zero: grabbing +// back-to-back starves the main-process event loop, which delays delivery +// of click events from the OS watcher by whole seconds. The frame history +// plus hook-side click timestamps tolerate the coarser cadence. +const FRAME_LOOP_IDLE_MS = 200; // 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; +// A loop grab that started at most this long after the click still shows +// the screen the user clicked on (UI reactions render slower than this). +const CLICK_FRAME_START_SLACK_MS = 300; const CLICK_CAPTURE_HIDE_DELAY_MS = 25; -const RECENT_FRAME_RETENTION_MS = 2000; -const RECENT_FRAME_LIMIT = 8; +// Frames now hold raw images (~20MB each at 2880x1800), so keep the history +// window wide enough to outlast any processing hiccup but the count low. +const RECENT_FRAME_RETENTION_MS = 4000; +const RECENT_FRAME_LIMIT = 4; function pointInBounds(point, bounds) { if (!point || !bounds) return false; @@ -423,10 +430,15 @@ class CaptureService { const sameDisplay = !clickPos || pointInBounds(clickPos, f && f.display && f.display.bounds); const startedAt = Number.isFinite(f && f.startedAt) ? f.startedAt : (f && f.capturedAt); const completedBeforeClick = Number.isFinite(f && f.capturedAt) && f.capturedAt <= clickTime; - const startedBeforeClick = Number.isFinite(startedAt) && startedAt <= clickTime; + // A grab that began within the slack window after the click still + // shows the click-instant screen (UI reactions take longer than the + // slack to render), and it beats the alternative — a fresh shot that + // both starts later and stalls the loop for every queued click. + const startedNearClick = Number.isFinite(startedAt) + && startedAt <= clickTime + CLICK_FRAME_START_SLACK_MS; const timingMatches = completedBeforeClick ? clickTime - f.capturedAt <= CLICK_FRAME_MAX_AGE_MS - : allowInFlight && startedBeforeClick && clickTime - startedAt <= CLICK_FRAME_MAX_AGE_MS; + : allowInFlight && startedNearClick; return Boolean(f) && f.mode === grabMode && timingMatches @@ -436,11 +448,18 @@ class CaptureService { .filter((f, i, arr) => f && arr.indexOf(f) === i && usable(f)) .sort((a, b) => b.capturedAt - a.capturedAt)[0]; if (buffered) return buffered; - if (!this.frameLoopRunning || !this.frameLoopInFlight || this.frameLoopGrabStartedAt > clickTime) return null; + // As long as the loop is running, the next grab is at most one idle gap + // away — wait for it rather than racing it with a one-off shot. + 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())); if (usable(next, { allowInFlight: true })) return next; + if (next && Number.isFinite(next.startedAt) + && next.startedAt > clickTime + CLICK_FRAME_START_SLACK_MS) { + // Grabs only get later from here; let the fresh-shot path handle it. + return null; + } } return null; } @@ -759,7 +778,12 @@ public static class SFMouseHook { const grabbed = await this.grab(mode, capturePoint); return { mode, - png: grabbed.image.toPNG(), + // Keep the raw image and defer PNG encoding to storeFrameAsStep: + // toPNG() on a full-resolution frame blocks the main thread for + // hundreds of ms, and doing it every frame-loop tick starved the + // event loop so badly that click events arrived seconds late. + // Encoding once per *stored* step is cheap; encoding per grab is not. + image: grabbed.image, size: grabbed.image.getSize(), display: grabbed.display, cursor: capturePoint || grabbed.cursor, @@ -796,7 +820,7 @@ public static class SFMouseHook { enabled: Boolean(this.settings.get('editor.focusedViewDefaultForNewSteps')), zoom: 1, panX: 0.5, panY: 0.5, }, - }, frame.png, frame.size); + }, frame.png || frame.image.toPNG(), frame.size); return { ok: true, step }; } diff --git a/tests/unit/capture.test.js b/tests/unit/capture.test.js index 2a0b547..8c0524d 100644 --- a/tests/unit/capture.test.js +++ b/tests/unit/capture.test.js @@ -328,29 +328,60 @@ 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('an idle click capture waits for the imminent loop frame instead of racing it', async () => { + // Grabs take seconds while the idle gap is ~200ms, so the loop's next + // frame both starts sooner and avoids stalling the loop the way a + // competing one-off shot would. const service = makeService(); service.session = { guideId: 'guide-idle', paused: false, count: 0, intervalSec: 0 }; service.frameLoopRunning = true; service.frameLoopInFlight = false; - let nextFrameCalled = false; + const clickAt = Date.now(); service.nextFrame = async () => { - nextFrameCalled = true; - throw new Error('idle clicks must not wait for a new frame'); + const f = makeFrame('next-loop-frame'); + f.startedAt = clickAt + 100; // grab began one idle gap after the click + f.capturedAt = clickAt + 350; + return f; + }; + service.shoot = async () => { + throw new Error('idle clicks must wait for the loop frame, not take a fresh shot'); + }; + 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 }, { at: clickAt }); + + assert.equal(result.ok, true); + assert.deepEqual(added, ['next-loop-frame']); +}); + +test('a loop frame started too long after the click falls back to a fresh shot', async () => { + const service = makeService(); + service.session = { guideId: 'guide-late', paused: false, count: 0, intervalSec: 0 }; + service.frameLoopRunning = true; + service.frameLoopInFlight = false; + + const clickAt = Date.now(); + service.nextFrame = async () => { + const f = makeFrame('too-late-frame'); + f.startedAt = clickAt + 5000; // way past the slack window + f.capturedAt = clickAt + 6000; + return f; + }; let shootCalled = false; service.shoot = async () => { shootCalled = true; - return { ok: true, step: { stepId: 'idle-step' } }; + return { ok: true, step: { stepId: 'fresh-step' } }; }; - const result = await service.sessionCapture('click', { x: 1, y: 1 }); + const result = await service.sessionCapture('click', { x: 1, y: 1 }, { at: clickAt }); assert.equal(result.ok, true); - assert.equal(shootCalled, true); - assert.equal(nextFrameCalled, false); + assert.equal(shootCalled, true, 'late frames must not be passed off as the click-time screen'); }); test('clicks during an in-flight grab wait for the frame instead of being dropped', async () => {