mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-05-04 00:41:33 -07:00
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
This commit is contained in:
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [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.
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Implementation Plan
|
||||||
|
|
||||||
|
<!-- SECTION:PLAN:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:PLAN:END -->
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
<!-- SECTION:NOTES:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:NOTES:END -->
|
||||||
|
|
||||||
|
## Final Summary
|
||||||
|
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||||
@@ -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
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [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.
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Implementation Plan
|
||||||
|
|
||||||
|
<!-- SECTION:PLAN:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:PLAN:END -->
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
<!-- SECTION:NOTES:BEGIN -->
|
||||||
|
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.
|
||||||
|
<!-- SECTION:NOTES:END -->
|
||||||
|
|
||||||
|
## Final Summary
|
||||||
|
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||||
|
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`
|
||||||
|
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||||
@@ -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.
|
||||||
@@ -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.
|
||||||
@@ -299,16 +299,7 @@ function M.create(ctx)
|
|||||||
if overrides.auto_start_trigger == true then
|
if overrides.auto_start_trigger == true then
|
||||||
subminer_log("debug", "process", "Auto-start ignored because overlay is already running")
|
subminer_log("debug", "process", "Auto-start ignored because overlay is already running")
|
||||||
local socket_path = overrides.socket_path or opts.socket_path
|
local socket_path = overrides.socket_path or opts.socket_path
|
||||||
local should_pause_until_ready = (
|
disarm_auto_play_ready_gate()
|
||||||
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
|
|
||||||
local visibility_action = resolve_visible_overlay_startup()
|
local visibility_action = resolve_visible_overlay_startup()
|
||||||
and "show-visible-overlay"
|
and "show-visible-overlay"
|
||||||
or "hide-visible-overlay"
|
or "hide-visible-overlay"
|
||||||
|
|||||||
@@ -906,23 +906,23 @@ do
|
|||||||
)
|
)
|
||||||
assert_true(
|
assert_true(
|
||||||
count_control_calls(recorded.async_calls, "--show-visible-overlay") == 4,
|
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(
|
assert_true(
|
||||||
count_osd_message(recorded.osd, "SubMiner: Loading subtitle tokenization...") == 2,
|
count_osd_message(recorded.osd, "SubMiner: Loading subtitle tokenization...") == 1,
|
||||||
"duplicate pause-until-ready auto-start should arm tokenization loading gate for each file"
|
"duplicate pause-until-ready auto-start should not repeat tokenization loading gate after overlay is running"
|
||||||
)
|
)
|
||||||
assert_true(
|
assert_true(
|
||||||
count_osd_message(recorded.osd, "SubMiner: Subtitle tokenization ready") == 2,
|
count_osd_message(recorded.osd, "SubMiner: Subtitle tokenization ready") == 1,
|
||||||
"duplicate pause-until-ready auto-start should release tokenization gate for each file"
|
"duplicate pause-until-ready auto-start should not wait for a second readiness signal after overlay is running"
|
||||||
)
|
)
|
||||||
assert_true(
|
assert_true(
|
||||||
count_property_set(recorded.property_sets, "pause", true) == 2,
|
count_property_set(recorded.property_sets, "pause", true) == 1,
|
||||||
"duplicate pause-until-ready auto-start should force pause for each file"
|
"duplicate pause-until-ready auto-start should not force pause after overlay is running"
|
||||||
)
|
)
|
||||||
assert_true(
|
assert_true(
|
||||||
count_property_set(recorded.property_sets, "pause", false) == 2,
|
count_property_set(recorded.property_sets, "pause", false) == 1,
|
||||||
"duplicate pause-until-ready auto-start should resume playback for each file"
|
"duplicate pause-until-ready auto-start should not resume a gate that was never rearmed"
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -1192,6 +1192,65 @@ test('macOS keeps visible overlay hidden while tracker is not initialized yet',
|
|||||||
assert.ok(!calls.includes('update-bounds'));
|
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', () => {
|
test('macOS suppresses immediate repeat loading OSD after tracker recovery until cooldown expires', () => {
|
||||||
const { window } = createMainWindowRecorder();
|
const { window } = createMainWindowRecorder();
|
||||||
const osdMessages: string[] = [];
|
const osdMessages: string[] = [];
|
||||||
|
|||||||
@@ -260,11 +260,16 @@ export function updateVisibleOverlayVisibility(args: {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const hasRetainedTrackedGeometry = args.windowTracker.getGeometry() !== null;
|
||||||
|
const shouldPreserveTransientTrackedOverlay =
|
||||||
|
(args.isMacOSPlatform && hasRetainedTrackedGeometry) ||
|
||||||
|
(args.isWindowsPlatform &&
|
||||||
|
typeof args.windowTracker.isTargetWindowMinimized === 'function' &&
|
||||||
|
!args.windowTracker.isTargetWindowMinimized());
|
||||||
|
|
||||||
if (
|
if (
|
||||||
args.isWindowsPlatform &&
|
shouldPreserveTransientTrackedOverlay &&
|
||||||
typeof args.windowTracker.isTargetWindowMinimized === 'function' &&
|
(mainWindow.isVisible() || hasRetainedTrackedGeometry)
|
||||||
!args.windowTracker.isTargetWindowMinimized() &&
|
|
||||||
(mainWindow.isVisible() || args.windowTracker.getGeometry() !== null)
|
|
||||||
) {
|
) {
|
||||||
args.setTrackerNotReadyWarningShown(false);
|
args.setTrackerNotReadyWarningShown(false);
|
||||||
const geometry = args.windowTracker.getGeometry();
|
const geometry = args.windowTracker.getGeometry();
|
||||||
@@ -273,6 +278,9 @@ export function updateVisibleOverlayVisibility(args: {
|
|||||||
}
|
}
|
||||||
args.syncPrimaryOverlayWindowLayer('visible');
|
args.syncPrimaryOverlayWindowLayer('visible');
|
||||||
showPassiveVisibleOverlay();
|
showPassiveVisibleOverlay();
|
||||||
|
if (!args.forceMousePassthrough && !args.isWindowsPlatform) {
|
||||||
|
args.enforceOverlayLayerOrder();
|
||||||
|
}
|
||||||
args.syncOverlayShortcuts();
|
args.syncOverlayShortcuts();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user