Never take a post-click screenshot when a pre-click frame exists
The remaining 'captured slightly after the click' reports came from the fresh-shot fallback, which grabs the screen when the click is processed (after it). The previous lead change made that fallback *more* likely: a frame now had to be >=120ms before the click to qualify, so on machines where the capture stream can't always keep a frame that old buffered, more clicks fell through to the post-click shot. Make the click-lead a two-tier preference instead of a hard gate in selectFrameForClick: 1. newest frame captured at least leadMs before the click (ideal margin), else 2. newest frame captured before the click at all. Only when no pre-click frame exists does the caller fresh-shot. leadMs is threaded through the stream backend to the worker so both selection paths agree. Verified end to end: frames land ~120-170ms before each click, markers stay at 0.00%, and the 8-click burst still saves all 8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+11
-13
@@ -557,31 +557,29 @@ class CaptureService {
|
|||||||
async frameForClick(clickPos = null, clickAt = Date.now()) {
|
async frameForClick(clickPos = null, clickAt = Date.now()) {
|
||||||
const mode = this.settings.get('capture.mode') || 'fullscreen';
|
const mode = this.settings.get('capture.mode') || 'fullscreen';
|
||||||
const grabMode = mode === 'region' ? 'fullscreen' : mode;
|
const grabMode = mode === 'region' ? 'fullscreen' : mode;
|
||||||
const rawClickTime = Number.isFinite(clickAt) ? clickAt : Date.now();
|
const clickTime = Number.isFinite(clickAt) ? clickAt : Date.now();
|
||||||
// Click lead: aim selection at a moment slightly *before* the hook
|
// Click lead: prefer a frame captured a little *before* the hook
|
||||||
// timestamp. The hook fires on button-down, but the visible UI often
|
// timestamp. The hook fires on button-down, but the visible UI often
|
||||||
// starts reacting within a frame or two of that (hover→press states,
|
// starts reacting within a frame or two (hover→press states, the cursor
|
||||||
// the cursor settling), and capture-stream pixels lag the real screen
|
// settling) and capture-stream pixels lag the real screen slightly, so a
|
||||||
// by a frame. Targeting clickTime - leadMs keeps the saved screenshot
|
// frame timestamped right at the click can still show the click's onset.
|
||||||
// clear of the click's own onset so the step shows the screen the user
|
// The lead is a *preference*: selection falls back to any pre-click
|
||||||
// was about to act on. Tunable via capture.clickLeadMs.
|
// frame when none is old enough, so it never forces a post-click fresh
|
||||||
|
// shot. Tunable via capture.clickLeadMs.
|
||||||
const leadMs = Math.max(0, Number(this.settings.get('capture.clickLeadMs')) || 0);
|
const leadMs = Math.max(0, Number(this.settings.get('capture.clickLeadMs')) || 0);
|
||||||
const clickTime = rawClickTime - leadMs;
|
|
||||||
const strict = this.strictClickFrames();
|
const strict = this.strictClickFrames();
|
||||||
const opts = {
|
const opts = {
|
||||||
clickAt: clickTime,
|
clickAt: clickTime,
|
||||||
|
leadMs,
|
||||||
clickPos,
|
clickPos,
|
||||||
mode: grabMode,
|
mode: grabMode,
|
||||||
strict,
|
strict,
|
||||||
// The lead shifts the target earlier, so widen the staleness budget by
|
maxAgeMs: CLICK_FRAME_MAX_AGE_MS,
|
||||||
// the same amount — a frame that was fresh enough for the real click
|
|
||||||
// is still fresh enough for the lead-adjusted target.
|
|
||||||
maxAgeMs: CLICK_FRAME_MAX_AGE_MS + leadMs,
|
|
||||||
startSlackMs: CLICK_FRAME_START_SLACK_MS,
|
startSlackMs: CLICK_FRAME_START_SLACK_MS,
|
||||||
};
|
};
|
||||||
|
|
||||||
if (this.streamBackend && this.streamBackend.isActive() && grabMode === 'fullscreen') {
|
if (this.streamBackend && this.streamBackend.isActive() && grabMode === 'fullscreen') {
|
||||||
const frame = await this.streamBackend.frameForClick({ clickPos, clickAt: clickTime, strict });
|
const frame = await this.streamBackend.frameForClick({ clickPos, clickAt: clickTime, strict, leadMs });
|
||||||
if (frame) return frame;
|
if (frame) return frame;
|
||||||
// No qualifying frame (or the backend just went unhealthy): fall
|
// No qualifying frame (or the backend just went unhealthy): fall
|
||||||
// through to the loop buffer / fresh-shot fallbacks below.
|
// through to the loop buffer / fresh-shot fallbacks below.
|
||||||
|
|||||||
+29
-8
@@ -125,14 +125,7 @@ function frameUsableForClick(frame, {
|
|||||||
return startedAt <= clickTime + startSlackMs;
|
return startedAt <= clickTime + startSlackMs;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
function newestUsableFrame(frames, opts) {
|
||||||
* Best already-buffered frame for a click: the newest frame that qualifies
|
|
||||||
* under frameUsableForClick. Buffered frames are by definition completed, so
|
|
||||||
* in-flight acceptance never applies here. Returns null when nothing
|
|
||||||
* qualifies and the caller must wait for the in-flight grab or fall back to
|
|
||||||
* a fresh shot.
|
|
||||||
*/
|
|
||||||
function selectFrameForClick(frames, opts = {}) {
|
|
||||||
let best = null;
|
let best = null;
|
||||||
for (const frame of frames || []) {
|
for (const frame of frames || []) {
|
||||||
if (!frameUsableForClick(frame, { ...opts, allowInFlight: false })) continue;
|
if (!frameUsableForClick(frame, { ...opts, allowInFlight: false })) continue;
|
||||||
@@ -141,6 +134,34 @@ function selectFrameForClick(frames, opts = {}) {
|
|||||||
return best;
|
return best;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Best already-buffered frame for a click, in two tiers:
|
||||||
|
* 1. with a click lead (opts.leadMs > 0): the newest frame captured at least
|
||||||
|
* leadMs *before* the click, so the step shows the screen the user was
|
||||||
|
* about to act on — clear of the click's own onset;
|
||||||
|
* 2. failing that, the newest frame captured before the click at all.
|
||||||
|
*
|
||||||
|
* The two tiers matter for correctness, not just polish: the lead is a
|
||||||
|
* *preference*, never a hard gate. If it were a gate, a click with no frame
|
||||||
|
* old enough to satisfy the lead would fall through to the caller's fresh
|
||||||
|
* shot — which captures the screen *after* the click. The tier-2 fallback
|
||||||
|
* guarantees that as long as any pre-click frame exists, we use it rather
|
||||||
|
* than shooting post-click. Buffered frames are always completed, so
|
||||||
|
* in-flight acceptance never applies here.
|
||||||
|
*/
|
||||||
|
function selectFrameForClick(frames, opts = {}) {
|
||||||
|
const leadMs = Math.max(0, Number(opts.leadMs) || 0);
|
||||||
|
const clickAt = Number.isFinite(opts.clickAt) ? opts.clickAt : Date.now();
|
||||||
|
if (leadMs > 0) {
|
||||||
|
// Widen the staleness budget by the lead so a frame that was fresh
|
||||||
|
// enough for the real click is still fresh enough for the lead target.
|
||||||
|
const maxAgeMs = (opts.maxAgeMs == null ? DEFAULT_MAX_AGE_MS : opts.maxAgeMs) + leadMs;
|
||||||
|
const led = newestUsableFrame(frames, { ...opts, clickAt: clickAt - leadMs, maxAgeMs });
|
||||||
|
if (led) return led;
|
||||||
|
}
|
||||||
|
return newestUsableFrame(frames, { ...opts, clickAt });
|
||||||
|
}
|
||||||
|
|
||||||
const api = {
|
const api = {
|
||||||
FrameRing,
|
FrameRing,
|
||||||
frameUsableForClick,
|
frameUsableForClick,
|
||||||
|
|||||||
@@ -147,6 +147,7 @@
|
|||||||
await sampleFrame(state);
|
await sampleFrame(state);
|
||||||
const frame = StepForgeClickFrames.selectFrameForClick(state.ring.frames(), {
|
const frame = StepForgeClickFrames.selectFrameForClick(state.ring.frames(), {
|
||||||
clickAt: cmd.clickAt,
|
clickAt: cmd.clickAt,
|
||||||
|
leadMs: cmd.leadMs || 0,
|
||||||
mode: 'fullscreen',
|
mode: 'fullscreen',
|
||||||
strict: cmd.strict !== false,
|
strict: cmd.strict !== false,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -210,7 +210,7 @@ class StreamCaptureBackend {
|
|||||||
* Resolves null when no frame qualifies (caller falls back) — and also on
|
* Resolves null when no frame qualifies (caller falls back) — and also on
|
||||||
* timeout, which additionally counts toward unhealthiness.
|
* timeout, which additionally counts toward unhealthiness.
|
||||||
*/
|
*/
|
||||||
frameForClick({ clickPos = null, clickAt = Date.now(), strict = true } = {}) {
|
frameForClick({ clickPos = null, clickAt = Date.now(), strict = true, leadMs = 0 } = {}) {
|
||||||
if (!this.active || !this.host) return Promise.resolve(null);
|
if (!this.active || !this.host) return Promise.resolve(null);
|
||||||
const displays = [...this.streams.values()].filter((s) => s.ready).map((s) => s.display);
|
const displays = [...this.streams.values()].filter((s) => s.ready).map((s) => s.display);
|
||||||
const display = clickPos ? displayForDipPoint(clickPos, displays) : (displays[0] || null);
|
const display = clickPos ? displayForDipPoint(clickPos, displays) : (displays[0] || null);
|
||||||
@@ -234,6 +234,7 @@ class StreamCaptureBackend {
|
|||||||
displayId: display.id,
|
displayId: display.id,
|
||||||
clickAt,
|
clickAt,
|
||||||
strict,
|
strict,
|
||||||
|
leadMs,
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
+6
-4
@@ -24,10 +24,12 @@ Keep-a-Changelog conventions; versions follow semver.
|
|||||||
- Physical→DIP coordinate conversion is multi-monitor and scale-factor
|
- Physical→DIP coordinate conversion is multi-monitor and scale-factor
|
||||||
aware (`screen.screenToDipPoint` on Windows, display-geometry math
|
aware (`screen.screenToDipPoint` on Windows, display-geometry math
|
||||||
elsewhere), fixing marker drift on displays scaled away from 100%.
|
elsewhere), fixing marker drift on displays scaled away from 100%.
|
||||||
- A configurable click-lead (`capture.clickLeadMs`, default 120ms) targets
|
- A configurable click-lead (`capture.clickLeadMs`, default 120ms) prefers
|
||||||
the screen just before each click so the saved step shows what the user
|
a frame captured a little before each click so the saved step shows what
|
||||||
was about to act on, not the click's onset; the stream sampling cadence
|
the user was about to act on, not the click's onset; the stream sampling
|
||||||
was tightened to 50ms so a frame near that target always exists.
|
cadence was tightened to 50ms so a frame near that target always exists.
|
||||||
|
The lead is a preference, not a gate: selection falls back to the newest
|
||||||
|
frame still before the click, so it never forces a post-click screenshot.
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
|||||||
@@ -798,8 +798,8 @@ test('click frames come from the stream backend when it is active', async () =>
|
|||||||
|
|
||||||
assert.equal(result.ok, true);
|
assert.equal(result.ok, true);
|
||||||
assert.deepEqual(added, ['stream-frame']);
|
assert.deepEqual(added, ['stream-frame']);
|
||||||
assert.deepEqual(requests, [{ clickPos: { x: 10, y: 10 }, clickAt, strict: true }],
|
assert.deepEqual(requests, [{ clickPos: { x: 10, y: 10 }, clickAt, strict: true, leadMs: 0 }],
|
||||||
'the worker receives the hook-time click timestamp and strictness');
|
'the worker receives the hook-time click timestamp, strictness, and lead');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('a stream backend with no qualifying frame falls through to the fresh-shot path', async () => {
|
test('a stream backend with no qualifying frame falls through to the fresh-shot path', async () => {
|
||||||
|
|||||||
@@ -124,6 +124,39 @@ test('frames of the wrong capture mode are rejected', () => {
|
|||||||
assert.equal(frameUsableForClick(f, { clickAt, mode: 'window' }), true);
|
assert.equal(frameUsableForClick(f, { clickAt, mode: 'window' }), true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('the click lead prefers a frame captured at least leadMs before the click', () => {
|
||||||
|
const clickAt = 10_000;
|
||||||
|
const frames = [
|
||||||
|
frame('with-margin', { startedAt: 9780, capturedAt: 9800 }), // 200ms before
|
||||||
|
frame('right-at-click', { startedAt: 9970, capturedAt: 9985 }), // 15ms before
|
||||||
|
];
|
||||||
|
|
||||||
|
const chosen = selectFrameForClick(frames, { clickAt, mode: 'fullscreen', strict: true, leadMs: 120 });
|
||||||
|
|
||||||
|
assert.equal(chosen.name, 'with-margin',
|
||||||
|
'with a lead, the frame clear of the click onset wins over the one right at it');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the click lead falls back to any pre-click frame rather than forcing a post-click shot', () => {
|
||||||
|
// The whole point of the two-tier rule: when nothing satisfies the lead,
|
||||||
|
// we still return a pre-click frame (caller would otherwise fresh-shot
|
||||||
|
// *after* the click). Only "right-before" exists here.
|
||||||
|
const clickAt = 10_000;
|
||||||
|
const frames = [frame('right-before', { startedAt: 9960, capturedAt: 9980 })];
|
||||||
|
|
||||||
|
const chosen = selectFrameForClick(frames, { clickAt, mode: 'fullscreen', strict: true, leadMs: 120 });
|
||||||
|
|
||||||
|
assert.equal(chosen.name, 'right-before');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the click lead still returns null when no frame precedes the click at all', () => {
|
||||||
|
const clickAt = 10_000;
|
||||||
|
const frames = [frame('after', { startedAt: 10_050, capturedAt: 10_080 })];
|
||||||
|
|
||||||
|
assert.equal(selectFrameForClick(frames, { clickAt, strict: true, leadMs: 120 }), null,
|
||||||
|
'a post-click frame is never selected; the caller takes the fresh-shot fallback');
|
||||||
|
});
|
||||||
|
|
||||||
test('a frame without startedAt falls back to capturedAt for the strict check', () => {
|
test('a frame without startedAt falls back to capturedAt for the strict check', () => {
|
||||||
const clickAt = 10_000;
|
const clickAt = 10_000;
|
||||||
const before = frame('legacy-before', { capturedAt: 9950 });
|
const before = frame('legacy-before', { capturedAt: 9950 });
|
||||||
|
|||||||
Reference in New Issue
Block a user