From 10c422101316e0d3e2871dc2d0dab84d3e412f7f Mon Sep 17 00:00:00 2001 From: Mr P-Tech Date: Mon, 10 Aug 2026 14:57:08 +0100 Subject: [PATCH] feat(control): add opt-in input tracing to locate mis-placed clicks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining click failure cannot be diagnosed by reading the code. Whether the pointer has actually arrived by the time a button event is delivered depends on when the OS applies a synthetic move, which is only observable at runtime. Adds PAIRUX_DEBUG_INPUT=1 tracing that records, per button event, the normalized coordinates, the absolute pixels we asked for, the position read back from the OS immediately before the press, and the drift between them. Non-zero drift means the settle delay is too short and the click is landing somewhere other than where the guest aimed. Also logs nut-js's screen geometry at init, which is the other half of a mis-placed click: if that disagrees with the display's real geometry — Retina reporting physical pixels where setPosition wants logical points — every coordinate is scaled wrong and no delay fixes it. On the injector side it records the two-cursor bookkeeping around each click, so a restore firing between a down and its up (which would yank the pointer mid-click, and is invisible in the backend's own trace) shows up. Off unless explicitly set to "1": it costs a window-server round trip per button event and prints the coordinates of everything the remote peer clicks. --- packages/remote-input/src/backends/nutjs.ts | 36 +++++++++++++++++++++ packages/remote-input/src/debug.test.ts | 33 +++++++++++++++++++ packages/remote-input/src/debug.ts | 15 +++++++++ packages/remote-input/src/injector.ts | 16 +++++++++ 4 files changed, 100 insertions(+) create mode 100644 packages/remote-input/src/debug.test.ts create mode 100644 packages/remote-input/src/debug.ts diff --git a/packages/remote-input/src/backends/nutjs.ts b/packages/remote-input/src/backends/nutjs.ts index 64b1c0f..8304268 100644 --- a/packages/remote-input/src/backends/nutjs.ts +++ b/packages/remote-input/src/backends/nutjs.ts @@ -1,4 +1,5 @@ import { resolveModifiers } from '../modifiers.js'; +import { isInputDebugEnabled } from '../debug.js'; import type { InputEvent, MouseMoveEvent, @@ -142,6 +143,19 @@ export class NutJsInputBackend implements InputBackend { const { screen } = await getNut(); this.screenWidth = await screen.width(); this.screenHeight = await screen.height(); + + if (isInputDebugEnabled()) { + // The other half of a mis-placed click: if this disagrees with the + // display's real geometry (Retina reporting physical pixels while + // setPosition expects logical points, say), every coordinate is scaled + // wrong and no settle delay will save it. + console.log('[InputInjector:debug] screen geometry from nut-js', { + width: this.screenWidth, + height: this.screenHeight, + platform: process.platform, + }); + } + return { screenWidth: this.screenWidth, screenHeight: this.screenHeight }; } @@ -169,6 +183,28 @@ export class NutJsInputBackend implements InputBackend { // a delay is a sub-millisecond blip that many controls simply ignore. await settle(); + if (isInputDebugEnabled()) { + // Read the pointer back from the OS. If `actual` does not match + // `requested` here, the settle above is too short and the click is + // landing somewhere other than where the guest aimed. + let actual: { x: number; y: number } | null = null; + try { + actual = await mouse.getPosition(); + } catch (error) { + console.warn('[InputInjector:debug] could not read pointer back', { error }); + } + + console.log('[InputInjector:debug] button', { + action: event.action, + button: event.button, + normalized: { x: event.x, y: event.y }, + requested: { x, y }, + actual, + drift: actual ? { x: actual.x - x, y: actual.y - y } : null, + screen: { width: this.screenWidth, height: this.screenHeight }, + }); + } + switch (event.action) { case 'down': await mouse.pressButton(button); diff --git a/packages/remote-input/src/debug.test.ts b/packages/remote-input/src/debug.test.ts new file mode 100644 index 0000000..6987eca --- /dev/null +++ b/packages/remote-input/src/debug.test.ts @@ -0,0 +1,33 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { isInputDebugEnabled } from './debug.js'; + +describe('isInputDebugEnabled', () => { + const original = process.env.PAIRUX_DEBUG_INPUT; + + afterEach(() => { + if (original === undefined) delete process.env.PAIRUX_DEBUG_INPUT; + else process.env.PAIRUX_DEBUG_INPUT = original; + }); + + // The tracing prints the coordinates of everything the remote peer clicks + // and costs a window-server round trip per button event, so silence has to + // be the default rather than something a build flag happens to give us. + it('is off unless explicitly enabled', () => { + delete process.env.PAIRUX_DEBUG_INPUT; + expect(isInputDebugEnabled()).toBe(false); + }); + + it('is on for exactly "1"', () => { + process.env.PAIRUX_DEBUG_INPUT = '1'; + expect(isInputDebugEnabled()).toBe(true); + }); + + // Anything truthy-looking but not "1" stays off, so a stray value in a shell + // profile cannot quietly turn tracing on in a packaged build. + it('ignores other values', () => { + for (const value of ['0', 'true', 'yes', '']) { + process.env.PAIRUX_DEBUG_INPUT = value; + expect(isInputDebugEnabled()).toBe(false); + } + }); +}); diff --git a/packages/remote-input/src/debug.ts b/packages/remote-input/src/debug.ts new file mode 100644 index 0000000..d81ad65 --- /dev/null +++ b/packages/remote-input/src/debug.ts @@ -0,0 +1,15 @@ +/** + * Per-event input tracing, off unless `PAIRUX_DEBUG_INPUT=1`. + * + * Exists to answer questions that cannot be settled by reading the code — + * above all, whether the pointer is actually where we asked it to be by the + * time a remote click's button event goes down. The OS applies synthetic + * moves on its own schedule, so that ordering is only observable at runtime. + * + * Off by default: it reads the cursor position on every button event, which + * is a round trip into the window server, and it prints the coordinates of + * everything the remote peer clicks. + */ +export function isInputDebugEnabled(): boolean { + return process.env.PAIRUX_DEBUG_INPUT === '1'; +} diff --git a/packages/remote-input/src/injector.ts b/packages/remote-input/src/injector.ts index 2b8d184..73f461c 100644 --- a/packages/remote-input/src/injector.ts +++ b/packages/remote-input/src/injector.ts @@ -13,6 +13,7 @@ import { type InputBackendSelection, } from './factory.js'; import { InputRateLimiter, validateInputEvent, type RejectionReason } from './safety.js'; +import { isInputDebugEnabled } from './debug.js'; import type { InputBackend, InputDiagnostics, @@ -287,6 +288,21 @@ export class RemoteInputInjector { const remaining = new Set(this.heldButtons); if (event.action === 'down') remaining.add(event.button); else if (event.action === 'up') remaining.delete(event.button); + + if (isInputDebugEnabled()) { + // The two-cursor bookkeeping around a click. A restore firing between a + // down and its up would yank the pointer mid-click and is invisible from + // the backend's own trace, so it is recorded here. + this.logger.log('[RemoteInput:debug] click dispatch', { + action: event.action, + dragging, + heldBefore: [...this.heldButtons], + remainingAfter: [...remaining], + borrowedFrom: this.borrowedFrom, + willRestore: remaining.size === 0, + }); + } + if (remaining.size > 0) return; await this.restoreLocalPointer();