Fix/mouse click screenshot align #2

Merged
Tyler merged 11 commits from fix/mouse_click_screenshot_align into main 2026-06-12 14:41:42 +00:00
4 changed files with 238 additions and 30 deletions
Showing only changes of commit f2c5831315 - Show all commits
+56 -12
View File
@@ -55,6 +55,10 @@ const DEFAULT_CLICK_DEBOUNCE_MS = 200;
// How long a Linux raw button event waits for its regular twin (the // How long a Linux raw button event waits for its regular twin (the
// representation that carries root coordinates) before firing without them. // representation that carries root coordinates) before firing without them.
const LINUX_CLICK_TWIN_MS = 25; const LINUX_CLICK_TWIN_MS = 25;
// Longest the window stays visible warming up the recorder at recording
// start. A slow capture-stream start (Windows can take several seconds) must
// not keep the window up and recording un-armed indefinitely.
const WARMUP_MAX_MS = 1500;
// Idle gap between legacy frame-loop grabs. Must stay well above zero: // Idle gap between legacy frame-loop grabs. Must stay well above zero:
// grabbing back-to-back starves the main-process event loop, which delays // grabbing back-to-back starves the main-process event loop, which delays
// delivery of click events from the OS watcher by whole seconds. (The // delivery of click events from the OS watcher by whole seconds. (The
@@ -126,6 +130,10 @@ class CaptureService {
this.lastAcceptedClickByButton = new Map(); this.lastAcceptedClickByButton = new Map();
this.streamBackend = null; this.streamBackend = null;
this.streamBackendStarting = false; this.streamBackendStarting = false;
this.captureGen = 0; // bumped on stop to invalidate in-flight backend starts
// True only while a resume is warming up (window still visible, buffer
// not yet primed). Clicks are ignored until it clears — see armRecording.
this.warmingUp = false;
} }
state() { state() {
@@ -317,6 +325,7 @@ class CaptureService {
if (wasPaused && !this.session.paused) { if (wasPaused && !this.session.paused) {
this.armRecording(); this.armRecording();
} else if (!wasPaused && this.session.paused) { } else if (!wasPaused && this.session.paused) {
this.warmingUp = false; // cancel any in-flight warmup
this.stopFrameLoop(); this.stopFrameLoop();
this.stopClickFrameBackend(); this.stopClickFrameBackend();
} }
@@ -339,14 +348,28 @@ class CaptureService {
const wantHide = Boolean(this.hiddenForSession && win && !win.isDestroyed()); const wantHide = Boolean(this.hiddenForSession && win && !win.isDestroyed());
const recorderWanted = this.settings.get('capture.captureOutsideClicks') !== false const recorderWanted = this.settings.get('capture.captureOutsideClicks') !== false
&& this.clickCaptureAvailable(); && this.clickCaptureAvailable();
// Recording is not "live" until the window is hidden and the buffer is
// primed. While warming up, the window is still visible and over the
// user's work, so clicks in this period are ignored (onOsClick checks
// warmingUp) instead of being skipped erratically or shot post-click —
// the bug that made a restarted recording "stop after one click".
this.warmingUp = Boolean(wantHide || recorderWanted);
const settleMs = Number(this.settings.get('capture.postHideSettleMs'));
const run = async () => { const run = async () => {
if (!this.session || this.session.paused) return; if (!this.session || this.session.paused) { this.warmingUp = false; return; }
const startedAt = Date.now(); const startedAt = Date.now();
if (recorderWanted) { if (recorderWanted) {
// Resolves once at least one stream is delivering frames (or the // Warm the recorder, but never let a slow backend start (it waits up
// loop fallback is running), so the buffer is primed before the hide. // to several seconds for the capture stream) keep the window visible
try { await this.startClickFrameBackend(); } catch { /* falls back internally */ } // and recording un-armed. Cap the wait; if it isn't ready by then,
if (!this.session || this.session.paused) return; // hide anyway and let the first click or two take the fresh-shot
// fallback while the stream finishes coming up in the background.
const warm = this.startClickFrameBackend().catch(() => {});
let capTimer = null;
const cap = new Promise((r) => { capTimer = setTimeout(r, WARMUP_MAX_MS); });
await Promise.race([warm, cap]);
if (capTimer) clearTimeout(capTimer);
if (!this.session || this.session.paused) { this.warmingUp = false; return; }
} }
// Keep the window visible briefly so the user sees the transition even // Keep the window visible briefly so the user sees the transition even
// when warmup was instant; warmup time counts toward this. // when warmup was instant; warmup time counts toward this.
@@ -354,17 +377,19 @@ class CaptureService {
const elapsed = Date.now() - startedAt; const elapsed = Date.now() - startedAt;
if (elapsed < minVisibleMs) { if (elapsed < minVisibleMs) {
await new Promise((r) => setTimeout(r, minVisibleMs - elapsed)); await new Promise((r) => setTimeout(r, minVisibleMs - elapsed));
if (!this.session || this.session.paused) return; if (!this.session || this.session.paused) { this.warmingUp = false; return; }
} }
if (wantHide && win && !win.isDestroyed() && win.isVisible()) { if (wantHide && win && !win.isDestroyed() && win.isVisible()) {
win.hide(); win.hide();
// Let a couple of frames of the now-unobscured screen land before // Let a couple of frames of the now-unobscured screen land before
// the user's first click, so that frame shows their work, not the // the user's first click, so that frame shows their work, not the
// app window that was just dismissed. // app window that was just dismissed.
await new Promise((r) => setTimeout(r, this.settings.get('capture.postHideSettleMs') || 150)); await new Promise((r) => setTimeout(r, Number.isFinite(settleMs) ? settleMs : 150));
} }
// Window hidden and buffer primed — clicks now count.
this.warmingUp = false;
}; };
run().catch(() => {}); run().catch(() => { this.warmingUp = false; });
} }
finishSession() { finishSession() {
@@ -372,6 +397,7 @@ class CaptureService {
clearInterval(this.intervalTimer); clearInterval(this.intervalTimer);
this.intervalTimer = null; this.intervalTimer = null;
} }
this.warmingUp = false;
this.stopClickWatcher(); this.stopClickWatcher();
this.stopFrameLoop(); this.stopFrameLoop();
this.stopClickFrameBackend(); this.stopClickFrameBackend();
@@ -673,6 +699,12 @@ class CaptureService {
return; return;
} }
if (this.streamBackend || this.streamBackendStarting) return; if (this.streamBackend || this.streamBackendStarting) return;
// Generation token: a stop/finish/pause bumps it. If it changes while
// this async start is in flight (e.g. the user finishes and restarts
// before a slow start resolves), the backend we built belongs to a dead
// session — discard it instead of installing it, and never leave
// streamBackendStarting stuck so the new session can start its own.
const gen = this.captureGen;
this.streamBackendStarting = true; this.streamBackendStarting = true;
try { try {
// eslint-disable-next-line global-require // eslint-disable-next-line global-require
@@ -691,9 +723,10 @@ class CaptureService {
sources: sources.map((s) => ({ id: s.id, display_id: s.display_id })), sources: sources.map((s) => ({ id: s.id, display_id: s.display_id })),
sampleMs: this.settings.get('capture.frameSampleMs') || 100, sampleMs: this.settings.get('capture.frameSampleMs') || 100,
}); });
if (!ok || !this.session || this.session.paused) { const stale = gen !== this.captureGen;
if (!ok || stale || !this.session || this.session.paused) {
backend.stop(); backend.stop();
if (this.session && !this.session.paused) { if (!stale && this.session && !this.session.paused) {
console.error('[stepforge] stream capture backend failed to start — using in-process frame loop'); console.error('[stepforge] stream capture backend failed to start — using in-process frame loop');
this.startFrameLoop(); this.startFrameLoop();
} }
@@ -703,16 +736,20 @@ class CaptureService {
clog('stream capture backend active'); clog('stream capture backend active');
this.notify('capture:state', this.state()); this.notify('capture:state', this.state());
} catch (err) { } catch (err) {
if (this.session && !this.session.paused) { if (gen === this.captureGen && this.session && !this.session.paused) {
console.error(`[stepforge] stream capture backend error (${err && err.message}) — using in-process frame loop`); console.error(`[stepforge] stream capture backend error (${err && err.message}) — using in-process frame loop`);
this.startFrameLoop(); this.startFrameLoop();
} }
} finally { } finally {
this.streamBackendStarting = false; if (gen === this.captureGen) this.streamBackendStarting = false;
} }
} }
stopClickFrameBackend() { stopClickFrameBackend() {
// Invalidate any in-flight start (see captureGen above) and free the
// guard so the next session can start a fresh backend immediately.
this.captureGen += 1;
this.streamBackendStarting = false;
if (!this.streamBackend) return; if (!this.streamBackend) return;
const backend = this.streamBackend; const backend = this.streamBackend;
this.streamBackend = null; this.streamBackend = null;
@@ -1111,6 +1148,13 @@ public static class SFMouseHook {
onOsClick(at = Date.now(), osPoint = null, button = 'mouse') { onOsClick(at = Date.now(), osPoint = null, button = 'mouse') {
if (!this.session || this.session.paused) return; if (!this.session || this.session.paused) return;
// Recording isn't live until the window is hidden and the buffer primed
// (see armRecording). Clicks during warmup land on the still-visible app
// window, not the user's work, so ignore them rather than capturing junk.
if (this.warmingUp) {
clog('click@', Number.isFinite(at) ? at : Date.now(), button, 'ignored — still warming up');
return;
}
const clickAt = Number.isFinite(at) ? at : Date.now(); const clickAt = Number.isFinite(at) ? at : Date.now();
// Leading-edge debounce: ignore a click that lands within the debounce // Leading-edge debounce: ignore a click that lands within the debounce
// window of the last accepted click of the same button. This makes fast // window of the last accepted click of the same button. This makes fast
+28 -18
View File
@@ -195,11 +195,22 @@ function createWindow() {
console.log('CLICK-SELFTEST burst:', burstSteps, 'of', burstCount, console.log('CLICK-SELFTEST burst:', burstSteps, 'of', burstCount,
burstSteps === burstCount ? 'OK — no clicks dropped on finish' : 'FAIL — clicks lost'); burstSteps === burstCount ? 'OK — no clicks dropped on finish' : 'FAIL — clicks lost');
// Helper: wait until armRecording has finished warming (window
// hidden, buffer primed) so an injected click counts as a real
// recording click rather than being ignored as a warmup click.
const waitArmed = async () => {
for (let i = 0; i < 80 && capture.warmingUp; i++) {
await new Promise((res) => setTimeout(res, 50));
}
};
// Third scenario: the real "Start recording" path. armRecording // Third scenario: the real "Start recording" path. armRecording
// must warm the recorder *before* hiding, so a click right after // warms the recorder while the window is visible and only arms the
// start still gets a pre-click frame instead of the post-click // session once it hides; the first click *after* arming must get a
// fresh shot that made "the first screenshot late". (This host may // pre-click frame (not the post-click shot that made "the first
// lack xinput, which gates the recorder, so force availability.) // screenshot late"), and a click *during* warmup must be ignored,
// not mishandled. (This host may lack xinput, which gates the
// recorder, so force availability.)
const armGuide = store.createGuide({ title: 'arm selftest' }); const armGuide = store.createGuide({ title: 'arm selftest' });
mainWindow.show(); mainWindow.show();
await new Promise((res) => setTimeout(res, 300)); await new Promise((res) => setTimeout(res, 300));
@@ -207,25 +218,23 @@ function createWindow() {
capture.stopClickWatcher(); capture.stopClickWatcher();
capture.clickCaptureAvailable = () => true; capture.clickCaptureAvailable = () => true;
capture.hiddenForSession = true; // window was visible at session start capture.hiddenForSession = true; // window was visible at session start
capture.togglePause(false); // armRecording: warm → hide capture.togglePause(false); // armRecording: warm → hide → arm
// Click while the old code would still be warming up (~250ms in). // A click during warmup must be ignored (window still visible).
await new Promise((res) => setTimeout(res, 250)); await new Promise((res) => setTimeout(res, 200));
const warmupClicks = store.getGuide(armGuide.guideId).stepsOrder.length;
capture.onOsClick(Date.now(), toPhysical({ x: bounds.x + 100, y: bounds.y + 100 }), 'button-1');
await waitArmed();
const armPoint = { const armPoint = {
x: Math.round(bounds.x + bounds.width * 0.4), x: Math.round(bounds.x + bounds.width * 0.4),
y: Math.round(bounds.y + bounds.height * 0.4), y: Math.round(bounds.y + bounds.height * 0.4),
}; };
const armClickAt = Date.now(); capture.onOsClick(Date.now(), toPhysical(armPoint), 'button-1');
capture.onOsClick(armClickAt, toPhysical(armPoint), 'button-1');
await capture.clickQueue; await capture.clickQueue;
await new Promise((res) => setTimeout(res, 800)); await new Promise((res) => setTimeout(res, 800));
const armStepIds = store.getGuide(armGuide.guideId).stepsOrder; const armSteps = store.getGuide(armGuide.guideId).stepsOrder.length;
let armPreClick = false; console.log('CLICK-SELFTEST arm: warmup-click steps', warmupClicks,
if (armStepIds.length) { '-> after-arm steps', armSteps,
// A pre-click frame is the win; the log line shows the margin. armSteps === 1 ? 'OK — warmup click ignored, first armed click captured' : 'FAIL');
armPreClick = true;
}
console.log('CLICK-SELFTEST arm: first click ->', armStepIds.length,
'step(s)', armPreClick ? '(see margin in [capture] log above)' : 'FAIL — first click lost');
capture.finishSession(); capture.finishSession();
// Fourth scenario: the debounce itself, exercised end to end through // Fourth scenario: the debounce itself, exercised end to end through
@@ -241,7 +250,8 @@ function createWindow() {
capture.hiddenForSession = true; capture.hiddenForSession = true;
capture.togglePause(false); capture.togglePause(false);
await capture.startClickFrameBackend(); await capture.startClickFrameBackend();
await new Promise((res) => setTimeout(res, 1500)); await waitArmed();
await new Promise((res) => setTimeout(res, 300));
const dbPoint = { const dbPoint = {
x: Math.round(bounds.x + bounds.width * 0.55), x: Math.round(bounds.x + bounds.width * 0.55),
y: Math.round(bounds.y + bounds.height * 0.55), y: Math.round(bounds.y + bounds.height * 0.55),
+10
View File
@@ -51,6 +51,16 @@ Keep-a-Changelog conventions; versions follow semver.
### Fixed ### Fixed
- **Restarting a recording no longer drops clicks or "stops after one
click."** While the recorder warmed up at the start of a session the
window was still visible, so clicks over it were skipped and clicks
elsewhere were shot post-click — and on a restart the backend start could
take several seconds, stretching that bad window out. Recording is now
"armed" only once the window is actually hidden and the buffer is primed:
clicks during warmup are cleanly ignored (the window is covering the
user's work anyway), the window hides within a bounded time even if the
backend is slow to start, and a slow start left over from a finished
session can no longer block the next session from starting its own.
- **Fast click bursts no longer lose screenshots.** Finishing or pausing a - **Fast click bursts no longer lose screenshots.** Finishing or pausing a
recording used to cancel every screenshot still being encoded, so a quick recording used to cancel every screenshot still being encoded, so a quick
series of clicks saved only the first two or three. The capture worker now series of clicks saved only the first two or three. The capture worker now
+144
View File
@@ -554,6 +554,150 @@ test('debounced clicks never reach the capture queue (end to end through onOsCli
assert.equal(service.session.count, 2); assert.equal(service.session.count, 2);
}); });
// ---- warmup gating (recording goes live only after the window hides) -----------
test('clicks during warmup are ignored, not captured as junk', () => {
const service = makeService();
service.session = { guideId: 'g-warm', paused: false, count: 0, intervalSec: 0 };
service.warmingUp = true;
const seen = [];
service.enqueueClickCapture = (clickPos, at) => { seen.push(at); };
service.onOsClick(1000, { x: 10, y: 20 }, 'left');
assert.deepEqual(seen, [], 'a click while warming up must not be captured');
// Once armed, clicks are captured again.
service.warmingUp = false;
service.onOsClick(2000, { x: 10, y: 20 }, 'left');
assert.deepEqual(seen, [2000]);
});
test('pausing and finishing clear the warmup flag', () => {
const service = makeService();
service.session = { guideId: 'g-warm2', paused: false, count: 0, intervalSec: 0 };
service.warmingUp = true;
service.togglePause(true);
assert.equal(service.warmingUp, false, 'pause cancels an in-flight warmup');
service.session = { guideId: 'g-warm3', paused: false, count: 0, intervalSec: 0 };
service.warmingUp = true;
service.finishSession();
assert.equal(service.warmingUp, false, 'finish cancels an in-flight warmup');
});
test('armRecording warms while visible, then hides and arms the session', async () => {
// Reproduces the restart path: recording must not be "live" until the
// window is hidden. A click injected during warmup is ignored; a click
// after arming is captured.
const service = makeService();
const win = {
destroyed: false, visible: true,
isDestroyed() { return this.destroyed; },
isVisible() { return this.visible; },
isMinimized() { return false; },
hide() { this.visible = false; },
show() { this.visible = true; },
focus() {}, getTitle() { return 'StepForge'; },
getBounds() { return { x: 0, y: 0, width: 800, height: 600 }; },
};
service.getWindow = () => win;
service.clickCaptureAvailable = () => true;
// Stub the recorder so warmup resolves fast without real Electron.
service.startClickFrameBackend = async () => {};
service.session = { guideId: 'g-arm', paused: false, count: 0, intervalSec: 0 };
service.hiddenForSession = true;
const captured = [];
service.enqueueClickCapture = (clickPos, at) => { captured.push(at); };
service.armRecording();
assert.equal(service.warmingUp, true, 'warming up begins immediately');
service.onOsClick(1, { x: 10, y: 10 }, 'left');
assert.deepEqual(captured, [], 'a click during warmup is ignored');
// Wait out the warmup (min-visible 400ms + settle 150ms here).
for (let i = 0; i < 40 && service.warmingUp; i++) {
await new Promise((r) => setTimeout(r, 50));
}
assert.equal(service.warmingUp, false, 'warmup clears');
assert.equal(win.visible, false, 'the window is hidden once armed');
service.onOsClick(2, { x: 10, y: 10 }, 'left');
assert.deepEqual(captured, [2], 'a click after arming is captured');
service.finishSession();
});
test('a slow recorder start still arms within the warmup cap', async () => {
// If the backend start hangs (Windows can take seconds), the window must
// still hide and recording must still arm — the restart bug was the
// window staying up for many seconds, dropping every click over it.
const service = makeService();
const win = {
destroyed: false, visible: true,
isDestroyed() { return this.destroyed; },
isVisible() { return this.visible; },
isMinimized() { return false; },
hide() { this.visible = false; },
show() { this.visible = true; },
focus() {}, getTitle() { return 'StepForge'; },
getBounds() { return { x: 0, y: 0, width: 800, height: 600 }; },
};
service.getWindow = () => win;
service.clickCaptureAvailable = () => true;
// Backend start that never resolves within the test.
service.startClickFrameBackend = () => new Promise(() => {});
service.session = { guideId: 'g-slow', paused: false, count: 0, intervalSec: 0 };
service.hiddenForSession = true;
service.armRecording();
// The cap is 1500ms; wait a bit beyond it.
for (let i = 0; i < 60 && service.warmingUp; i++) {
await new Promise((r) => setTimeout(r, 50));
}
assert.equal(service.warmingUp, false, 'a hung backend start must not keep recording un-armed');
assert.equal(win.visible, false, 'the window hides even when the recorder is slow to start');
service.finishSession();
});
test('a slow backend start from a finished session does not install itself or block a restart', async () => {
// Rapid restart: session A starts a backend that resolves slowly; the user
// finishes A and starts B before it resolves. A's late backend must not
// install into B, and the starting-guard must not block B from starting
// its own.
const service = makeService();
service.session = { guideId: 'A', paused: false, count: 0, intervalSec: 0 };
let releaseA;
const aReady = new Promise((r) => { releaseA = r; });
const built = [];
// Model the generation guard at the seam: a slow start that checks
// captureGen before installing the backend it built.
service.startClickFrameBackend = async function patched() {
const gen = this.captureGen;
this.streamBackendStarting = true;
try {
await aReady; // slow start
const backend = { isActive: () => true, stop: () => built.push('stopped') };
if (gen !== this.captureGen || !this.session || this.session.paused) {
backend.stop();
return;
}
this.streamBackend = backend;
built.push('installed');
} finally {
if (gen === this.captureGen) this.streamBackendStarting = false;
}
};
const startA = service.startClickFrameBackend();
service.finishSession(); // bumps captureGen, A is now stale
releaseA();
await startA;
assert.deepEqual(built, ['stopped'], 'the stale backend is stopped, never installed');
assert.equal(service.streamBackend, null);
assert.equal(service.streamBackendStarting, false, 'the guard is freed for the next session');
});
// ---- coordinate conversion ------------------------------------------------------ // ---- coordinate conversion ------------------------------------------------------
test('hook coordinates are converted physical → DIP via screenToDipPoint when available', () => { test('hook coordinates are converted physical → DIP via screenToDipPoint when available', () => {