From f0d8c297db533879befd0d352a5d38d1421208f5 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 17 Oct 2026 01:02:27 -0700 Subject: [PATCH] fix: align macOS mouse pointer with captured content The capture helper now reports the filter content rect as the capture frame on macOS 14 and later, and keeps the source frame only on macOS 13. Window recordings, including windows chosen in Apple's picker, seed normalization from the source frame before the first cursor sample, then replace it with the helper's capture bounds and rebase samples already stored. Display recordings keep normalizing against the display bounds. Fixes #1130 --- electron/ipc/handlers.ts | 53 +++- .../macNativeCursorRecordingSession.test.ts | 235 ++++++++++++++++++ .../ScreenCaptureRecorder.swift | 28 ++- 3 files changed, 299 insertions(+), 17 deletions(-) create mode 100644 electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.test.ts diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 1724d695d..e7c57d250 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -879,7 +879,9 @@ const nativeMacCaptureOutputs = new WeakMap) { const bounds = event.captureBounds as Rectangle | undefined; - if (bounds && bounds.width > 0 && bounds.height > 0) { + // Display takes never adopt this rect: their cursor stays on the display bounds. + // Adopting it there would move a capture whose alignment is already right. + if ( + isMacWindowCaptureSourceId(selectedSource?.id) && + bounds && + bounds.width > 0 && + bounds.height > 0 + ) { activeMacCaptureBounds = bounds; + cursorRecordingSession?.updateCaptureBounds?.(bounds); } nativeMacCaptureEvents.emit("helper-event", event); } @@ -3222,6 +3241,9 @@ export function registerIpcHandlers( ? (screen.getAllDisplays().find((display) => display.id === request.source.displayId) ?? null) : getSelectedDisplay(); + // Drop the previous take before this lookup. A window with no frame of its + // own would otherwise seed the cursor from that take's capture rect. + activeMacCaptureBounds = null; const bounds = pick?.bounds ?? request.source.bounds ?? sourceDisplay?.bounds ?? getSelectedSourceBounds(); const captureExcludedWindowSourceIds: string[] = []; @@ -3285,7 +3307,14 @@ export function registerIpcHandlers( nativeMacIsPaused = false; nativeMacStopInFlight = false; nativeMacRecordingWarning = null; - activeMacCaptureBounds = null; + // Before the cursor session samples. A legacy window otherwise normalises + // its opening samples against the display, and a picker window would keep + // the picker's frame after the helper has reported the content rect it is + // actually recording (#1130). Display takes stay unset, so the display + // bounds remain authoritative. + if (isMacWindowCaptureSourceId(selectedSource?.id) && bounds.width > 0 && bounds.height > 0) { + activeMacCaptureBounds = bounds; + } const cursorStartTimeMs = Date.now(); if (cursorCaptureMode === "editable-overlay") { diff --git a/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.test.ts b/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.test.ts new file mode 100644 index 000000000..960cfd906 --- /dev/null +++ b/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.test.ts @@ -0,0 +1,235 @@ +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * The cast on `actual` is written out in the factory rather than shared in a + * helper: `vi.mock` calls are HOISTED above every top-level statement, so a + * module-scope helper is still in its temporal dead zone when the factory runs. + */ +type WithDefault = { default?: Record }; + +const mocks = vi.hoisted(() => ({ + cursor: { x: 0, y: 0 }, + nearestDisplay: { + scaleFactor: 2, + bounds: { x: 0, y: 0, width: 1920, height: 1080 }, + }, + isTrustedAccessibilityClient: vi.fn(() => true), + accessSync: vi.fn(), +})); + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + const spawn = vi.fn(); + return { ...actual, spawn, default: { ...((actual as WithDefault).default ?? {}), spawn } }; +}); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + accessSync: mocks.accessSync, + default: { ...((actual as WithDefault).default ?? {}), accessSync: mocks.accessSync }, + }; +}); + +vi.mock("electron", () => ({ + systemPreferences: { isTrustedAccessibilityClient: mocks.isTrustedAccessibilityClient }, + screen: { + getCursorScreenPoint: () => mocks.cursor, + getDisplayNearestPoint: () => mocks.nearestDisplay, + }, +})); + +import { spawn } from "node:child_process"; +import type { Rectangle } from "electron"; +import { MacNativeCursorRecordingSession } from "./macNativeCursorRecordingSession"; + +/** Minimal stand-in for the cursor helper: stdio pipes plus kill bookkeeping. */ +class FakeHelper extends EventEmitter { + stdout = new PassThrough(); + stderr = new PassThrough(); + killed = false; + + kill() { + this.killed = true; + return true; + } + + /** Feeds one NDJSON line, the way the real helper emits them. */ + emitEvent(event: Record) { + this.stdout.write(`${JSON.stringify(event)}\n`); + } +} + +const spawnMock = vi.mocked(spawn); +let helper: FakeHelper; + +beforeEach(() => { + helper = new FakeHelper(); + spawnMock.mockReset(); + spawnMock.mockReturnValue(helper as unknown as ReturnType); + mocks.cursor = { x: 0, y: 0 }; + mocks.nearestDisplay = { + scaleFactor: 2, + bounds: { x: 0, y: 0, width: 1920, height: 1080 }, + }; + mocks.isTrustedAccessibilityClient.mockReset(); + mocks.isTrustedAccessibilityClient.mockReturnValue(true); + mocks.accessSync.mockReset(); + vi.spyOn(console, "warn").mockImplementation(() => undefined); + vi.spyOn(console, "error").mockImplementation(() => undefined); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const WINDOW_BOUNDS: Rectangle = { x: 200, y: 100, width: 1000, height: 800 }; + +async function recordWithHelper( + getDisplayBounds: () => Rectangle | null, + sample: (emit: (event: Record) => void) => void, +) { + const session = new MacNativeCursorRecordingSession({ + getDisplayBounds, + maxSamples: 20, + sampleIntervalMs: 33, + startTimeMs: 1_000, + }); + const pending = session.start(); + await Promise.resolve(); + helper.emitEvent({ type: "ready", timestampMs: 1_000 }); + await pending; + sample((event) => helper.emitEvent(event)); + return session.stop(); +} + +function click(timestampMs: number) { + return { type: "sample", timestampMs, leftButtonPressed: true }; +} + +describe("MacNativeCursorRecordingSession capture bounds", () => { + it("places click hotspots on the top and bottom of a window with a non-zero origin", async () => { + // 10px inside each edge. A display-sized frame, or any fixed y shift, misses both. + mocks.cursor = { x: 400, y: 110 }; + const data = await recordWithHelper( + () => WINDOW_BOUNDS, + (emit) => { + emit(click(1_100)); + mocks.cursor = { x: 400, y: 890 }; + emit(click(1_200)); + }, + ); + + expect(data.samples[0]).toMatchObject({ + cx: 0.2, + cy: 10 / 800, + visible: true, + interactionType: "click", + }); + expect(data.samples[1]).toMatchObject({ + cx: 0.2, + cy: 790 / 800, + visible: true, + interactionType: "click", + }); + }); + + it("replaces the source frame with later bounds and leaves a display frame unshifted", async () => { + // The picker frame sits below the pixels. The helper's capture rect replaces it; + // the next sample must not keep the old origin. A full display then maps as itself. + let bounds: Rectangle = { x: 200, y: 180, width: 1000, height: 800 }; + mocks.cursor = { x: 700, y: 500 }; + const data = await recordWithHelper( + () => bounds, + (emit) => { + emit(click(1_100)); + bounds = { x: 200, y: 120, width: 1000, height: 860 }; + emit(click(1_200)); + bounds = { x: 0, y: 0, width: 1920, height: 1080 }; + mocks.cursor = { x: 960, y: 540 }; + emit(click(1_300)); + }, + ); + + expect(data.samples[0].cy).toBeCloseTo((500 - 180) / 800, 5); + expect(data.samples[1].cx).toBeCloseTo((700 - 200) / 1000, 5); + expect(data.samples[1].cy).toBeCloseTo((500 - 120) / 860, 5); + expect(data.samples[2]).toMatchObject({ cx: 0.5, cy: 0.5 }); + }); + + it("rebases samples collected before the authoritative bounds arrive", async () => { + let bounds: Rectangle = { x: 0, y: 0, width: 1920, height: 1080 }; + mocks.cursor = { x: 400, y: 300 }; + const session = new MacNativeCursorRecordingSession({ + getDisplayBounds: () => bounds, + maxSamples: 20, + sampleIntervalMs: 33, + startTimeMs: 1_000, + }); + const pending = session.start(); + await Promise.resolve(); + helper.emitEvent({ type: "ready", timestampMs: 1_000 }); + await pending; + helper.emitEvent(click(1_100)); + + bounds = WINDOW_BOUNDS; + session.updateCaptureBounds(bounds); + + const data = await session.stop(); + expect(data.samples[0]).toMatchObject({ + cx: 0.2, + cy: 0.25, + visible: true, + interactionType: "click", + }); + }); + + it("falls back to the display under the pointer when no capture bounds exist", async () => { + // No helper on disk: position-only sampling, and nothing has reported a capture rect. + mocks.accessSync.mockImplementation(() => { + throw new Error("ENOENT"); + }); + mocks.nearestDisplay = { + scaleFactor: 2, + bounds: { x: 0, y: 0, width: 1440, height: 900 }, + }; + mocks.cursor = { x: 720, y: 450 }; + const session = new MacNativeCursorRecordingSession({ + getDisplayBounds: () => null, + maxSamples: 5, + sampleIntervalMs: 60_000, + startTimeMs: 1_000, + }); + + await session.start(); + const data = await session.stop(); + + expect(spawnMock).not.toHaveBeenCalled(); + expect(data.provider).toBe("none"); + expect(data.samples[0]).toMatchObject({ cx: 0.5, cy: 0.5, visible: true }); + }); + + it("clamps a brief exit and hides the pointer only after a sustained one", async () => { + // Outside the window, above its top edge. The first two stay visible at the + // clamp; the third crosses the hide threshold. One sample back inside clears it. + mocks.cursor = { x: 1_400, y: 40 }; + const data = await recordWithHelper( + () => WINDOW_BOUNDS, + (emit) => { + emit({ type: "sample", timestampMs: 1_100 }); + emit({ type: "sample", timestampMs: 1_133 }); + emit({ type: "sample", timestampMs: 1_166 }); + mocks.cursor = { x: 700, y: 500 }; + emit({ type: "sample", timestampMs: 1_200 }); + }, + ); + + expect(data.samples.slice(0, 3).map((sample) => sample.visible)).toEqual([true, true, false]); + expect(data.samples[0]).toMatchObject({ cx: 1, cy: 0 }); + expect(data.samples[2]).toMatchObject({ cx: 1, cy: 0, visible: false }); + expect(data.samples[3]).toMatchObject({ cx: 0.5, cy: 0.5, visible: true }); + }); +}); diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 1450f130d..1c3fdd913 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -119,8 +119,9 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { let filter: SCContentFilter let width: Int let height: Int - // Global frame (points, top-left origin) of the captured region. Used by the - // renderer to normalize cursor positions into the captured window's space. + // Global frame (points, top-left origin) of the captured region. The cursor is + // normalised against this, so it is the filter's content rect — the same + // rectangle the output buffer is sized from — wherever that exists. let captureFrame: CGRect } @@ -623,7 +624,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { filter: picked.filter, width: size.width, height: size.height, - captureFrame: picked.frame + captureFrame: captureContentFrame(for: picked.filter, fallback: picked.frame) ) } @@ -716,7 +717,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { filter: filter, width: size.width, height: size.height, - captureFrame: display.frame + captureFrame: captureContentFrame(for: filter, fallback: display.frame) ) case "window": guard let windowId = request.source.windowId else { @@ -741,7 +742,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { filter: filter, width: size.width, height: size.height, - captureFrame: window.frame + captureFrame: captureContentFrame(for: filter, fallback: window.frame) ) default: throw HelperError.invalidSourceType(request.source.type) @@ -1150,6 +1151,23 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { return status == .complete } + /// On-screen rectangle of the pixels this filter will rasterise, in points. + /// + /// `captureSize` already takes its dimensions from `contentRect`: that product is + /// the buffer ScreenCaptureKit fills. The cursor is a global Electron point + /// normalised against a frame reported separately, so the frame has to be that + /// same rectangle, origin included. `SCWindow.frame` and the frame stored with a + /// system-picker choice describe the window, which on macOS 26 sits below the + /// pixels the stream writes; the pointer then draws above the click (#1130). + /// No constant is added here to compensate. `contentRect` is screen points and + /// exists from macOS 14. On 13 the source frame is the only measurement available. + private func captureContentFrame(for filter: SCContentFilter, fallback: CGRect) -> CGRect { + guard #available(macOS 14.0, *) else { + return fallback + } + return filter.contentRect + } + /// Output-buffer size for a filter, capped by the resolution the app asked for. /// /// The filter is asked first, because `contentRect × pointPixelScale` is by definition the