diff --git a/changes/macos-controller-double-press.md b/changes/macos-controller-double-press.md new file mode 100644 index 00000000..4d9aae46 --- /dev/null +++ b/changes/macos-controller-double-press.md @@ -0,0 +1,5 @@ +type: fixed +area: overlay + +- On macOS and Windows, one controller press no longer runs its action twice, which cancelled out toggles like mpv pause. +- On macOS, the press that brings the overlay forward from mpv no longer runs its action a second time. diff --git a/changes/popup-pause-controller-lookup.md b/changes/popup-pause-controller-lookup.md new file mode 100644 index 00000000..997b3a57 --- /dev/null +++ b/changes/popup-pause-controller-lookup.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Fixed controller and keyboard lookups sometimes pausing playback without a popup, leaving playback paused after the popup closed until the mouse moved, or not pausing when the popup opened. diff --git a/e2e/playback.e2e.test.ts b/e2e/playback.e2e.test.ts index f5fb5b5c..a65237e2 100644 --- a/e2e/playback.e2e.test.ts +++ b/e2e/playback.e2e.test.ts @@ -15,7 +15,7 @@ let started: Promise | undefined; function useSession(): Promise { started ??= (async () => { - const session = await startE2eSession(); + const session = await startE2eSession({ config: { controller: { enabled: true } } }); try { return { session, overlay: await session.openPage(OVERLAY_PAGE) }; } catch (error) { @@ -273,6 +273,64 @@ test( }, ); +// macOS and Windows keep a hidden modal renderer alive, and hidden windows keep polling because +// background throttling is off. A fake pad goes to every renderer, as a real pad would. +// Linux has no such window, and under Xvfb without a window manager the overlay reports hidden, +// so requestAnimationFrame and with it the controller poll never run. Bun's node:test ignores the +// `skip` option, so this picks `test.skip` instead. +const testOutsideLinux = process.platform === 'linux' ? test.skip : test; +testOutsideLinux( + 'one controller press toggles mpv pause once across every window', + { timeout: TEST_TIMEOUT_MS }, + async () => { + const { session } = await useSession(); + const START_BUTTON = 9; + const targets = (await listCdpTargets(session.cdpPort)).filter((target) => + target.url.includes('renderer/index.html'), + ); + const renderers = await Promise.all(targets.map(connectCdpPage)); + const everyRenderer = (expression: string) => + Promise.all(renderers.map((renderer) => renderer.evaluate(expression))); + const nextPolls = `new Promise((done) => + requestAnimationFrame(() => requestAnimationFrame(() => done(true))))`; + try { + await everyRenderer(`(() => { + window.__fakePadPressed = false; + Object.defineProperty(navigator, 'getGamepads', { + configurable: true, + value: () => [{ + id: 'E2E Pad (STANDARD GAMEPAD)', + index: 0, + connected: true, + mapping: 'standard', + axes: [0, 0, 0, 0], + buttons: Array.from({ length: 17 }, (_, index) => { + const pressed = window.__fakePadPressed && index === ${START_BUTTON}; + return { value: pressed ? 1 : 0, pressed, touched: pressed }; + }), + }], + }); + return true; + })()`); + await everyRenderer(nextPolls); + await session.mpv.command('set_property', 'pause', true); + + await everyRenderer('window.__fakePadPressed = true'); + await everyRenderer(nextPolls); + await everyRenderer('window.__fakePadPressed = false'); + await everyRenderer(nextPolls); + // Long enough for a second `cycle pause` to reach mpv and cancel the first. + await new Promise((done) => setTimeout(done, 300)); + + assert.equal(await session.mpv.command('get_property', 'pause'), false); + } finally { + await everyRenderer('delete navigator.getGamepads'); + renderers.forEach((renderer) => renderer.close()); + await session.mpv.command('set_property', 'pause', true); + } + }, +); + // Runs last: it leaves the lookup popup open over the overlay. test( 'holding Shift over a word opens the Yomitan popup with its dictionary entry', diff --git a/src/renderer/handlers/controller-press-replay.test.ts b/src/renderer/handlers/controller-press-replay.test.ts index af3e5caf..da6ed6e9 100644 --- a/src/renderer/handlers/controller-press-replay.test.ts +++ b/src/renderer/handlers/controller-press-replay.test.ts @@ -20,12 +20,39 @@ function pad(pressed: number[] = [], mapping = 'standard'): GamepadLike { const pressedIndices = (gamepad: GamepadLike | null | undefined): number[] => gamepad?.buttons.flatMap((button, index) => (button.pressed ? [index] : [])) ?? []; -test('a queued press reads as pressed for exactly one poll', () => { +test('a queued press the pad never reports is released after the window', () => { const replay = createControllerPressReplay(); replay.queue(0, 1000); assert.deepEqual(pressedIndices(replay.apply([pad()], 1010)[0]), [0]); - assert.deepEqual(pressedIndices(replay.apply([pad()], 1026)[0]), []); + assert.deepEqual(pressedIndices(replay.apply([pad()], 1026)[0]), [0]); + assert.deepEqual(pressedIndices(replay.apply([pad()], 1499)[0]), [0]); + assert.deepEqual(pressedIndices(replay.apply([pad()], 1500)[0]), []); +}); + +// The helper's IPC beats Chromium, which only starts reading the pad once the overlay has focus. +// Going unpressed before the late report would give the controller a second press edge. +test('a press the pad reports late reads as one continuous press', () => { + const replay = createControllerPressReplay(); + replay.queue(11, 1000); + + const pressedAt = (real: number[], now: number) => + pressedIndices(replay.apply([pad(real)], now)[0]); + assert.deepEqual(pressedAt([], 1016), [11]); + assert.deepEqual(pressedAt([], 1032), [11]); + assert.deepEqual(pressedAt([11], 1048), [11]); + assert.deepEqual(pressedAt([11], 1064), [11]); + assert.deepEqual(pressedAt([], 1080), []); +}); + +test('a fresh press after the replayed one was released reads as a new press', () => { + const replay = createControllerPressReplay(); + replay.queue(11, 1000); + + assert.deepEqual(pressedIndices(replay.apply([pad([11])], 1016)[0]), [11]); + assert.deepEqual(pressedIndices(replay.apply([pad()], 1032)[0]), []); + assert.deepEqual(pressedIndices(replay.apply([pad([11])], 1048)[0]), [11]); + assert.deepEqual(pressedIndices(replay.apply([pad()], 1064)[0]), []); }); test('a press the Gamepad API already reported is not replayed', () => { diff --git a/src/renderer/handlers/controller-press-replay.ts b/src/renderer/handlers/controller-press-replay.ts index 5818fed9..21e590ec 100644 --- a/src/renderer/handlers/controller-press-replay.ts +++ b/src/renderer/handlers/controller-press-replay.ts @@ -1,16 +1,18 @@ import type { GamepadLike } from './gamepad-controller.js'; /** - * How far back a press the Gamepad API reported itself still counts as the same press the - * main process is replaying. Covers the helper -> main -> renderer IPC hop. + * How far apart the helper's report and the Gamepad API's report of one physical press can be. + * The Gamepad API can lead (the helper -> main -> renderer IPC hop) or lag (Chromium only reads + * the pad once the overlay has focus). */ const SAME_PRESS_WINDOW_MS = 500; /** * Replays macOS background presses (see `src/main/runtime/macos-controller-watch.ts`) into the - * controller poll. A queued button reads as pressed for one poll, so the controller sees a - * normal press edge; the next poll reads the real pad again, so a button still held does not - * fire twice. A press the Gamepad API already reported is dropped. + * controller poll. A queued button reads as pressed until the Gamepad API has reported that + * press and its release, or until the window passes if it never does. Holding it across the gap + * before a late report keeps one physical press to one press edge. A press the Gamepad API + * already reported is dropped. * * Only a lone standard-mapping pad gets the replay: the press carries no pad identity, so with * several visible it could run an action on the wrong one. With none visible yet, or a pad @@ -18,6 +20,7 @@ const SAME_PRESS_WINDOW_MS = 500; */ export function createControllerPressReplay() { const pendingPresses: Array<{ buttonIndex: number; queuedAt: number }> = []; + const heldPresses = new Map(); const lastRealPressAt = new Map(); function recordRealPresses(gamepads: GamepadLike[], now: number): void { @@ -55,20 +58,37 @@ export function createControllerPressReplay() { Boolean(gamepad?.connected), ); recordRealPresses(connected, now); - if (pendingPresses.length === 0) return gamepads; + if (pendingPresses.length === 0 && heldPresses.size === 0) return gamepads; - const replayed = pendingPresses - .filter(({ buttonIndex, queuedAt }) => { - const seenAt = lastRealPressAt.get(buttonIndex); - return seenAt === undefined || seenAt < queuedAt - SAME_PRESS_WINDOW_MS; - }) - .map(({ buttonIndex }) => buttonIndex); + for (const { buttonIndex, queuedAt } of pendingPresses) { + const seenAt = lastRealPressAt.get(buttonIndex); + if (seenAt === undefined || seenAt < queuedAt - SAME_PRESS_WINDOW_MS) { + heldPresses.set(buttonIndex, { + releaseAt: queuedAt + SAME_PRESS_WINDOW_MS, + realPressSeen: false, + }); + } + } pendingPresses.length = 0; - const onlyPad = connected.length === 1 ? connected[0] : undefined; - if (replayed.length === 0 || onlyPad?.mapping !== 'standard') return gamepads; + const onlyPad = connected.length === 1 ? connected[0] : undefined; + if (onlyPad?.mapping !== 'standard') { + heldPresses.clear(); + return gamepads; + } + + for (const [buttonIndex, hold] of heldPresses) { + if (onlyPad.buttons[buttonIndex]?.pressed) { + hold.realPressSeen = true; + } else if (hold.realPressSeen || now >= hold.releaseAt) { + heldPresses.delete(buttonIndex); + } + } + if (heldPresses.size === 0) return gamepads; + + const held = [...heldPresses.keys()]; return gamepads.map((gamepad) => - gamepad === onlyPad ? withPressed(onlyPad, replayed) : gamepad, + gamepad === onlyPad ? withPressed(onlyPad, held) : gamepad, ); }, }; diff --git a/src/renderer/handlers/mouse.test.ts b/src/renderer/handlers/mouse.test.ts index 43b81e44..de23198a 100644 --- a/src/renderer/handlers/mouse.test.ts +++ b/src/renderer/handlers/mouse.test.ts @@ -147,7 +147,7 @@ function createMouseHarness( /** Moves the tracked pointer onto `hovered` and fires a document mousemove. */ move: (hovered: HoverTarget, clientX = 120, clientY = 240) => { harness.hovered = hovered; - harness.fire('document', 'mousemove', { clientX, clientY }); + harness.fire('document', 'mousemove', { clientX, clientY, isTrusted: true }); }, isInteractive: () => overlayClassList.contains('interactive'), isSecondaryHoverActive: () => @@ -357,6 +357,69 @@ test('popup open pauses and popup close resumes when yomitan popup auto-pause is assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]); }); +// The lookup's fallback scan dispatches untrusted mouse events on the selected word, and +// those bubble to the pointer tracker while the real cursor is elsewhere. +test('popup close resumes after a fallback scan event lands on the subtitle', async (t) => { + const h = createMouseHarness(t, { hoverPause: true, popupPause: true }); + h.handlers.setupPointerTracking(); + h.handlers.setupYomitanObserver(); + + h.hovered = 'primary'; + h.fire('document', 'pointermove', { clientX: 120, clientY: 240, isTrusted: false }); + h.fire('document', 'mousemove', { clientX: 120, clientY: 240, isTrusted: false }); + await waitForNextTick(); + assert.equal(h.ctx.state.isOverSubtitle, false); + + h.fire('window', YOMITAN_POPUP_SHOWN_EVENT); + await waitForNextTick(); + h.fire('window', YOMITAN_POPUP_HIDDEN_EVENT); + + assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]); +}); + +// Controller and keyboard pause toggles change playback without touching the hover state. +test('popup open pauses when the user resumed playback during a hover pause', async (t) => { + let paused = false; + const h = createMouseHarness(t, { + hoverPause: true, + popupPause: true, + getPlaybackPaused: async () => paused, + }); + h.handlers.setupYomitanObserver(); + + // `paused` stays false: the pause button resumed playback after this hover pause. + await h.handlers.handleMouseEnter(); + assert.deepEqual(h.mpvCommands, [PAUSE]); + + // The lookup event lands while the shown event's pause check is still in flight. + h.popupHostVisible = true; + h.fire('window', YOMITAN_POPUP_SHOWN_EVENT); + h.fire('window', YOMITAN_LOOKUP_EVENT); + await waitForNextTick(); + + assert.deepEqual(h.mpvCommands, [PAUSE, PAUSE]); +}); + +test('popup taking over a hover pause leaves playback paused without a second command', async (t) => { + let paused = false; + const h = createMouseHarness(t, { + hoverPause: true, + popupPause: true, + getPlaybackPaused: async () => paused, + }); + h.handlers.setupYomitanObserver(); + + await h.handlers.handleMouseEnter(); + paused = true; + h.fire('window', YOMITAN_POPUP_SHOWN_EVENT); + await waitForNextTick(); + await h.handlers.handleMouseLeave(); + assert.deepEqual(h.mpvCommands, [PAUSE]); + + h.fire('window', YOMITAN_POPUP_HIDDEN_EVENT); + assert.deepEqual(h.mpvCommands, [PAUSE, RESUME]); +}); + for (const c of [ { name: 'nested popup close reasserts interactive state and focus when another popup remains visible on Windows', diff --git a/src/renderer/handlers/mouse.ts b/src/renderer/handlers/mouse.ts index 23bdf661..908a59d1 100644 --- a/src/renderer/handlers/mouse.ts +++ b/src/renderer/handlers/mouse.ts @@ -275,13 +275,14 @@ export function createMouseHandlers( return; } - const requestId = ++popupPauseRequestId; if (pausedByYomitanPopup) return; + const requestId = ++popupPauseRequestId; + // The popup takes over the hover pause so that closing it owns the resume. A pause toggle + // can resume playback without clearing the hover flag, so mpv still decides below. if (pausedBySubtitleHover) { pausedBySubtitleHover = false; pausedByYomitanPopup = true; - return; } let paused: boolean | null = null; @@ -442,16 +443,17 @@ export function createMouseHandlers( } function setupPointerTracking(): void { - document.addEventListener('mousemove', (event: MouseEvent) => { + // The lookup's fallback scan dispatches untrusted events on the selected word, which say + // nothing about where the cursor is. Counting them as hover pauses playback until the next + // real pointer move. + const trackPointer = (event: MouseEvent | PointerEvent): void => { + if (!event.isTrusted) return; updatePointerPosition(event); syncHoverStateFromTrackedPointer(event); maybeResyncPointerHoverState(event); - }); - document.addEventListener('pointermove', (event: PointerEvent) => { - updatePointerPosition(event); - syncHoverStateFromTrackedPointer(event); - maybeResyncPointerHoverState(event); - }); + }; + document.addEventListener('mousemove', trackPointer); + document.addEventListener('pointermove', trackPointer); document.addEventListener('visibilitychange', () => { if (document.visibilityState !== 'visible') { return; diff --git a/src/renderer/renderer.ts b/src/renderer/renderer.ts index ed1bb68d..d49eb539 100644 --- a/src/renderer/renderer.ts +++ b/src/renderer/renderer.ts @@ -657,7 +657,12 @@ function startControllerPolling(): void { : ctx.state.kikuModalOpen ? fieldGroupingControllerLayout : null, - getSuspended: () => ctx.state.controllerSuspendedByModalWindow, + // macOS and Windows keep the modal window's renderer alive while hidden, and it keeps + // polling. It only owns the controller with a modal open; otherwise every press would run + // here and in the main overlay. + getSuspended: () => + ctx.state.controllerSuspendedByModalWindow || + (ctx.platform.isModalLayer && !isAnyModalOpen()), }); const poll = (now: number): void => {