From a9c3a5e6790a10d827f5e8c028e286c7d774b94e Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 3 May 2026 03:55:30 -0700 Subject: [PATCH] Preserve overlay across macOS flaps and mpv playlist changes - keep visible overlays alive during transient macOS tracker loss - reuse the running mpv overlay path on playlist navigation - update regression coverage and changelog fragments --- ...overlay-hiding-while-mpv-remains-active.md | 60 ++++++++++++++++++ ...playlist-changes-re-running-app-warmups.md | 63 +++++++++++++++++++ changes/323-macos-overlay-tracker-flaps.md | 4 ++ changes/324-mpv-playlist-overlay-reuse.md | 4 ++ plugin/subminer/process.lua | 11 +--- scripts/test-plugin-start-gate.lua | 18 +++--- src/core/services/overlay-visibility.test.ts | 59 +++++++++++++++++ src/core/services/overlay-visibility.ts | 16 +++-- 8 files changed, 212 insertions(+), 23 deletions(-) create mode 100644 backlog/tasks/task-323 - Fix-macOS-overlay-hiding-while-mpv-remains-active.md create mode 100644 backlog/tasks/task-324 - Fix-mpv-playlist-changes-re-running-app-warmups.md create mode 100644 changes/323-macos-overlay-tracker-flaps.md create mode 100644 changes/324-mpv-playlist-overlay-reuse.md diff --git a/backlog/tasks/task-323 - Fix-macOS-overlay-hiding-while-mpv-remains-active.md b/backlog/tasks/task-323 - Fix-macOS-overlay-hiding-while-mpv-remains-active.md new file mode 100644 index 00000000..d2a480e6 --- /dev/null +++ b/backlog/tasks/task-323 - Fix-macOS-overlay-hiding-while-mpv-remains-active.md @@ -0,0 +1,60 @@ +--- +id: TASK-323 +title: Fix macOS overlay hiding while mpv remains active +status: Done +assignee: + - '@codex' +created_date: '2026-05-03 07:41' +updated_date: '2026-05-03 07:48' +labels: + - bug + - macos + - overlay +dependencies: [] +references: + - src/core/services/overlay-visibility.ts + - src/window-trackers/macos-tracker.ts +priority: high +--- + +## Description + + +macOS visible overlay can hide/reload during normal playback even while mpv, or the overlay over mpv, remains the active viewing surface. The fix should preserve overlay visibility and subtitle continuity during transient macOS focus/tracker flaps, while still hiding the overlay when the tracked mpv window is genuinely unavailable or another app is brought forward. + + +## Acceptance Criteria + +- [x] #1 When the macOS tracker has recent valid mpv geometry, transient focus/helper misses do not hide the visible overlay or force a reload. +- [x] #2 The overlay still hides when the tracked mpv window is genuinely lost beyond the existing tracking grace behavior. +- [x] #3 A regression test covers the macOS active-playback case where mpv/overlay focus is preserved despite a transient non-tracking state. +- [x] #4 Relevant docs or task notes are updated if behavior or verification guidance changes. + + +## Implementation Plan + + +1. Add a failing regression in `src/core/services/overlay-visibility.test.ts`: on macOS, after the overlay is visible/tracked, a transient tracker state with `isTracking() === false` but non-null `getGeometry()` keeps the overlay visible, updates bounds, and does not call `hide()` or loading OSD. +2. Implement the minimal macOS preserve path in `src/core/services/overlay-visibility.ts`, mirroring the existing Windows transient non-minimized branch but without Windows z-order binding. +3. Preserve existing startup/lost-window behavior: `windowTracker: null` and `isTracking() === false` with `getGeometry() === null` still hide and show the first loading OSD. +4. Run focused tests for `src/core/services/overlay-visibility.test.ts`; then typecheck or the repo runtime verification lane if the focused patch passes. +5. Update TASK-323 notes/acceptance criteria with verification results. + + +## Implementation Notes + + +Added a macOS overlay visibility regression for transient tracker loss with retained geometry. The test failed first because the old path marked tracker-not-ready and hid the overlay. Implemented a scoped preserve path in `src/core/services/overlay-visibility.ts`: macOS now keeps the visible overlay alive only when the tracker still has retained geometry; true loss with null geometry still hides and emits the existing loading OSD behavior. Added changelog fragment `changes/323-macos-overlay-tracker-flaps.md`. + +Verification: `bun test src/core/services/overlay-visibility.test.ts` passed after the fix; `bun test src/window-trackers/macos-tracker.test.ts src/core/services/overlay-visibility.test.ts` passed; `bun run typecheck` passed; `bun run test:env` passed; isolated `bun test src/core/services/subsync.test.ts` passed; `bun run build` passed; `bun run test:smoke:dist` passed; `bun run changelog:lint` passed. `bun run test:fast` failed twice in an unrelated broad-suite interaction where `src/renderer/handlers/keyboard.ts` tried to use missing `window.electronAPI` while `src/core/services/subsync.test.ts` was running, followed by Bun node:test nested-test cascade errors. + + +## Final Summary + + +Fixed the macOS visible-overlay hide/reload path during normal playback by preserving the overlay when the tracker briefly reports non-tracking but still has retained mpv geometry. The overlay visibility service now treats that macOS state like a transient tracker flap: it keeps bounds/layer/order refreshed and leaves the overlay click-through instead of hiding or showing the loading OSD. True macOS loss remains unchanged: no tracker or null geometry still hides the overlay and uses the existing loading behavior. + +Added regression coverage in `src/core/services/overlay-visibility.test.ts` for the active-playback case and added changelog fragment `changes/323-macos-overlay-tracker-flaps.md`. + +Verification passed: focused overlay tests, macOS tracker + overlay tests, typecheck, `test:env`, isolated `subsync.test.ts`, build, dist smoke, and changelog lint. Full `test:fast` remains blocked by an unrelated broad-suite interaction where renderer keyboard state fires without `window.electronAPI` during `subsync.test.ts`, then Bun reports node:test cascade errors. + diff --git a/backlog/tasks/task-324 - Fix-mpv-playlist-changes-re-running-app-warmups.md b/backlog/tasks/task-324 - Fix-mpv-playlist-changes-re-running-app-warmups.md new file mode 100644 index 00000000..f7a9f62d --- /dev/null +++ b/backlog/tasks/task-324 - Fix-mpv-playlist-changes-re-running-app-warmups.md @@ -0,0 +1,63 @@ +--- +id: TASK-324 +title: Fix mpv playlist changes re-running app warmups +status: Done +assignee: [] +created_date: '2026-05-03 07:48' +updated_date: '2026-05-03 07:52' +labels: + - bug + - mpv + - overlay +dependencies: [] +references: + - launcher/ + - src/core/services/mpv.ts + - src/main/runtime/ +priority: medium +--- + +## Description + + +When moving to the next or previous mpv playlist entry, SubMiner should reconnect the existing app/runtime to mpv instead of treating the new video like a fresh app startup. Re-running startup warmups or creating another app session after the first video can interfere with overlay behavior. + + +## Acceptance Criteria + +- [x] #1 Changing to next or previous mpv playlist item reuses the existing app/runtime instead of launching a new app session. +- [x] #2 Startup warmups are not repeated for playlist item changes after the first app startup. +- [x] #3 Overlay behavior remains available after playlist navigation. +- [x] #4 Regression test covers the playlist-change/reconnect path. + + +## Implementation Plan + + +1. Reproduce the plugin auto-start regression with a failing Lua start-gate test. +2. Update mpv plugin auto-start handling so playlist/file changes with an already-running overlay reuse the existing app path and do not re-arm pause-until-ready warmup. +3. Add changelog fragment and run plugin/launcher verification. + + +## Implementation Notes + + +RED: `lua scripts/test-plugin-start-gate.lua` failed after changing the duplicate pause-until-ready auto-start expectations; it showed the loading gate was armed twice while overlay was already running. + +GREEN: `plugin/subminer/process.lua` now disarms any old ready gate and only reasserts visible overlay state when auto-start fires while `state.overlay_running` is already true. + + +## Final Summary + + +Summary: +- Updated the mpv Lua plugin auto-start reuse path so a file/playlist load with an already-running overlay no longer re-arms the pause-until-ready tokenization gate. +- Kept the existing app/control command reuse behavior: subsequent auto-starts reassert visible/hidden overlay state without issuing another `--start` subprocess. +- Added a changelog fragment for the mpv playlist overlay reuse fix. + +Tests: +- `lua scripts/test-plugin-start-gate.lua` (red before fix, green after) +- `bun run test:plugin:src` +- `bun run changelog:lint` +- `bun run test:launcher:env:src` + diff --git a/changes/323-macos-overlay-tracker-flaps.md b/changes/323-macos-overlay-tracker-flaps.md new file mode 100644 index 00000000..0f026c8a --- /dev/null +++ b/changes/323-macos-overlay-tracker-flaps.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Kept the macOS visible overlay alive during transient mpv tracker flaps when the last tracked video geometry is still available. diff --git a/changes/324-mpv-playlist-overlay-reuse.md b/changes/324-mpv-playlist-overlay-reuse.md new file mode 100644 index 00000000..31e64981 --- /dev/null +++ b/changes/324-mpv-playlist-overlay-reuse.md @@ -0,0 +1,4 @@ +type: fixed +area: mpv + +- mpv: Playlist navigation now reuses the running SubMiner overlay without repeating the pause-until-ready warmup gate. diff --git a/plugin/subminer/process.lua b/plugin/subminer/process.lua index 79c6225c..b3ba1bac 100644 --- a/plugin/subminer/process.lua +++ b/plugin/subminer/process.lua @@ -299,16 +299,7 @@ function M.create(ctx) if overrides.auto_start_trigger == true then subminer_log("debug", "process", "Auto-start ignored because overlay is already running") local socket_path = overrides.socket_path or opts.socket_path - local should_pause_until_ready = ( - resolve_visible_overlay_startup() - and resolve_pause_until_ready() - and has_matching_mpv_ipc_socket(socket_path) - ) - if should_pause_until_ready then - arm_auto_play_ready_gate() - else - disarm_auto_play_ready_gate() - end + disarm_auto_play_ready_gate() local visibility_action = resolve_visible_overlay_startup() and "show-visible-overlay" or "hide-visible-overlay" diff --git a/scripts/test-plugin-start-gate.lua b/scripts/test-plugin-start-gate.lua index 9cf0c2ac..70b26bdc 100644 --- a/scripts/test-plugin-start-gate.lua +++ b/scripts/test-plugin-start-gate.lua @@ -906,23 +906,23 @@ do ) assert_true( count_control_calls(recorded.async_calls, "--show-visible-overlay") == 4, - "duplicate pause-until-ready auto-start should re-assert visible overlay on both start and ready events" + "duplicate pause-until-ready auto-start should re-assert visible overlay on initial start, ready, and later file load" ) assert_true( - count_osd_message(recorded.osd, "SubMiner: Loading subtitle tokenization...") == 2, - "duplicate pause-until-ready auto-start should arm tokenization loading gate for each file" + count_osd_message(recorded.osd, "SubMiner: Loading subtitle tokenization...") == 1, + "duplicate pause-until-ready auto-start should not repeat tokenization loading gate after overlay is running" ) assert_true( - count_osd_message(recorded.osd, "SubMiner: Subtitle tokenization ready") == 2, - "duplicate pause-until-ready auto-start should release tokenization gate for each file" + count_osd_message(recorded.osd, "SubMiner: Subtitle tokenization ready") == 1, + "duplicate pause-until-ready auto-start should not wait for a second readiness signal after overlay is running" ) assert_true( - count_property_set(recorded.property_sets, "pause", true) == 2, - "duplicate pause-until-ready auto-start should force pause for each file" + count_property_set(recorded.property_sets, "pause", true) == 1, + "duplicate pause-until-ready auto-start should not force pause after overlay is running" ) assert_true( - count_property_set(recorded.property_sets, "pause", false) == 2, - "duplicate pause-until-ready auto-start should resume playback for each file" + count_property_set(recorded.property_sets, "pause", false) == 1, + "duplicate pause-until-ready auto-start should not resume a gate that was never rearmed" ) end diff --git a/src/core/services/overlay-visibility.test.ts b/src/core/services/overlay-visibility.test.ts index c265a07b..3624f9c9 100644 --- a/src/core/services/overlay-visibility.test.ts +++ b/src/core/services/overlay-visibility.test.ts @@ -1192,6 +1192,65 @@ test('macOS keeps visible overlay hidden while tracker is not initialized yet', assert.ok(!calls.includes('update-bounds')); }); +test('macOS preserves visible overlay during transient tracker loss with retained geometry', () => { + const { window, calls } = createMainWindowRecorder(); + const osdMessages: string[] = []; + let trackerWarning = false; + let tracking = true; + const tracker: WindowTrackerStub = { + isTracking: () => tracking, + getGeometry: () => ({ x: 0, y: 0, width: 1280, height: 720 }), + isTargetWindowFocused: () => true, + }; + + const run = () => + updateVisibleOverlayVisibility({ + visibleOverlayVisible: true, + mainWindow: window as never, + windowTracker: tracker as never, + trackerNotReadyWarningShown: trackerWarning, + setTrackerNotReadyWarningShown: (shown: boolean) => { + trackerWarning = shown; + }, + updateVisibleOverlayBounds: () => { + calls.push('update-bounds'); + }, + ensureOverlayWindowLevel: () => { + calls.push('ensure-level'); + }, + syncPrimaryOverlayWindowLayer: () => { + calls.push('sync-layer'); + }, + enforceOverlayLayerOrder: () => { + calls.push('enforce-order'); + }, + syncOverlayShortcuts: () => { + calls.push('sync-shortcuts'); + }, + isMacOSPlatform: true, + showOverlayLoadingOsd: (message: string) => { + osdMessages.push(message); + }, + } as never); + + run(); + calls.length = 0; + tracking = false; + + run(); + + assert.equal(trackerWarning, false); + assert.deepEqual(osdMessages, []); + assert.ok(calls.includes('update-bounds')); + assert.ok(calls.includes('sync-layer')); + assert.ok(calls.includes('mouse-ignore:true:forward')); + assert.ok(calls.includes('ensure-level')); + assert.ok(calls.includes('enforce-order')); + assert.ok(calls.includes('sync-shortcuts')); + assert.ok(!calls.includes('hide')); + assert.ok(!calls.includes('show')); +}); + test('macOS suppresses immediate repeat loading OSD after tracker recovery until cooldown expires', () => { const { window } = createMainWindowRecorder(); const osdMessages: string[] = []; diff --git a/src/core/services/overlay-visibility.ts b/src/core/services/overlay-visibility.ts index 768dcc61..10913cd8 100644 --- a/src/core/services/overlay-visibility.ts +++ b/src/core/services/overlay-visibility.ts @@ -260,11 +260,16 @@ export function updateVisibleOverlayVisibility(args: { return; } + const hasRetainedTrackedGeometry = args.windowTracker.getGeometry() !== null; + const shouldPreserveTransientTrackedOverlay = + (args.isMacOSPlatform && hasRetainedTrackedGeometry) || + (args.isWindowsPlatform && + typeof args.windowTracker.isTargetWindowMinimized === 'function' && + !args.windowTracker.isTargetWindowMinimized()); + if ( - args.isWindowsPlatform && - typeof args.windowTracker.isTargetWindowMinimized === 'function' && - !args.windowTracker.isTargetWindowMinimized() && - (mainWindow.isVisible() || args.windowTracker.getGeometry() !== null) + shouldPreserveTransientTrackedOverlay && + (mainWindow.isVisible() || hasRetainedTrackedGeometry) ) { args.setTrackerNotReadyWarningShown(false); const geometry = args.windowTracker.getGeometry(); @@ -273,6 +278,9 @@ export function updateVisibleOverlayVisibility(args: { } args.syncPrimaryOverlayWindowLayer('visible'); showPassiveVisibleOverlay(); + if (!args.forceMousePassthrough && !args.isWindowsPlatform) { + args.enforceOverlayLayerOrder(); + } args.syncOverlayShortcuts(); return; }