Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Windows “Taskbar Lyric” feature by embedding an Electron BrowserWindow into the Windows taskbar via a new Rust N-API module, plus associated settings/UI wiring and some shared lyric-sync refactoring.
Changes:
- Introduce
windows/taskbar-lyricrenderer entry (Vue UI + karaoke/scrolling line component). - Add a new native module
native/taskbar-lyric(Win10/Win11 strategies, UIA/tray/registry watchers) and integrate it into build/packaging. - Refactor
useNowPlayingSyncinto a shared composable with pluggable index-picking + log tag, and update Desktop Lyric / Dynamic Island to use it.
Reviewed changes
Copilot reviewed 51 out of 55 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| windows/taskbar-lyric/main.ts | New Vue entrypoint for taskbar lyric window |
| windows/taskbar-lyric/index.html | New HTML entry for taskbar lyric window |
| windows/taskbar-lyric/components/TaskbarLyricLine.vue | New scrolling + word-by-word highlight lyric line renderer |
| windows/taskbar-lyric/App.vue | New taskbar lyric UI (cover, controls, double-line rendering, theme alignment) |
| windows/shared/composables/useNowPlayingSync.ts | Shared now-playing sync with configurable index picker + log tag |
| windows/dynamic-island/components/IslandLyricLine.vue | Update to use shared getNowPlayingCurrentMs import |
| windows/dynamic-island/App.vue | Switch to shared useNowPlayingSync with pickLatestStartedIndex |
| windows/desktop-lyric/composables/useNowPlayingSync.ts | Remove local now-playing sync implementation |
| windows/desktop-lyric/components/LyricLine.vue | Update to use shared getNowPlayingCurrentMs import |
| windows/desktop-lyric/App.vue | Switch to shared useNowPlayingSync with pickPrimaryIndex |
| tsconfig.web.json | Add @windows/* path alias for renderer |
| tsconfig.node.json | Add @splayer/taskbar-lyric path alias for node/main tooling |
| src/types/settings-schema.ts | Add tag support to setting items/sections |
| src/stores/settings.ts | Track isTaskbarLyricOpen and subscribe to visibility updates |
| src/settings/virtualBindings.ts | Add virtual binding to toggle taskbar lyric window |
| src/settings/schema.ts | Add Taskbar Lyric settings section (Windows-only) + Beta tags |
| src/i18n/locales/zh-CN.json | Add zh-CN strings for taskbar lyric settings |
| src/i18n/locales/en-US.json | Add en-US strings for taskbar lyric settings |
| src/components/ui/STag.vue | New UI tag/badge component |
| src/components/settings/SettingsSection.vue | Render section-level tag badge |
| src/components/settings/SettingsItem.vue | Render item-level tag badge |
| shared/types/window.ts | Extend window API types for taskbar lyric + events |
| shared/types/settings.ts | Add TaskbarLyricSettings + attach to SystemConfig |
| shared/defaults/settings.ts | Add default taskbar lyric config |
| scripts/build-native.ts | Add native build target for taskbar-lyric (win32-only) |
| native/taskbar-lyric/src/utils.rs | Win32 helpers (registry, taskbar HWND, style mutation, theme detection) |
| native/taskbar-lyric/src/uia_watcher.rs | UIA-based taskbar layout change watcher |
| native/taskbar-lyric/src/uia.rs | UIA taskbar scanner for Win11 bridge content bounds |
| native/taskbar-lyric/src/tray_watcher.rs | WinEvent hook watcher for tray location changes |
| native/taskbar-lyric/src/strategy/win11.rs | Win11 embedding + layout computation strategy |
| native/taskbar-lyric/src/strategy/win10.rs | Win10 embedding + rebar/tasklist carving strategy |
| native/taskbar-lyric/src/strategy/mod.rs | Strategy trait + shared layout structs |
| native/taskbar-lyric/src/logger.rs | No-op logging macros for the native module |
| native/taskbar-lyric/src/lib.rs | N-API exports (TaskbarService, RegistryWatcher, UIA/Tray watchers) + worker loop |
| native/taskbar-lyric/package.json | Native module package metadata/scripts |
| native/taskbar-lyric/index.d.ts | Generated TS typings for the native module |
| native/taskbar-lyric/build.rs | NAPI build setup |
| native/taskbar-lyric/Cargo.toml | New Rust crate definition + deps/features |
| electron/preload/index.ts | Expose taskbar lyric window + taskbar lyric event APIs to renderer |
| electron/preload/index.d.ts | Add TaskbarLyricApi to window.api typing |
| electron/main/window/taskbarLyric.ts | Main-process window creation/embedding + watcher wiring + layout application |
| electron/main/window/index.ts | Export taskbar lyric window helpers from window module |
| electron/main/window/dynamicIsland.ts | Update BrowserWindow flags (disable min/max/fullscreen) |
| electron/main/window/desktopLyric.ts | Update BrowserWindow flags (disable min/max/fullscreen) |
| electron/main/utils/nativeLoader.ts | Use centralized nativeLog logger scope |
| electron/main/utils/logger.ts | Add taskbarLog and nativeLog scopes |
| electron/main/utils/i18n.ts | Add tray labels for taskbar lyric open/close |
| electron/main/services/tray.ts | Add tray menu item for taskbar lyric (Windows-only) |
| electron/main/ipc/window.ts | Add IPC handlers for taskbar lyric toggling/closing/query |
| electron/main/ipc/config.ts | Apply/broadcast taskbar lyric config changes + trigger relayout |
| electron.vite.config.ts | Add new renderer entry + aliases (@windows, native module alias) |
| electron-builder.config.ts | Package taskbar-lyric .node into app resources |
| components.d.ts | Register STag for auto component typing |
| Cargo.toml | Add taskbar-lyric crate to workspace |
| Cargo.lock | Lock new dependencies for taskbar-lyric crate |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** 关闭任务栏歌词窗口并清理所有资源 */ | ||
| export const closeTaskbarLyricWindow = (): void => { | ||
| if (taskbarLyricWindow && !taskbarLyricWindow.isDestroyed()) { | ||
| taskbarLyricWindow.close(); | ||
| } | ||
| taskbarLyricWindow = null; | ||
| cleanupWatchers(); | ||
| setTrayTaskbarLyric(false); | ||
| broadcast("taskbarLyric:visibilityChange", false); |
There was a problem hiding this comment.
closeTaskbarLyricWindow() sets taskbarLyricWindow = null, stops watchers, updates tray state, and broadcasts visibility change immediately after calling win.close(), but the window may still be alive until the "closed" event fires. This also duplicates the same cleanup/broadcast logic already registered in the "closed" handler, which can lead to double stop() calls and inconsistent state if the close is prevented. Align this with the Desktop Lyric / Dynamic Island patterns: let the "closed" handler own teardown/state updates (or call destroy() if immediate teardown is required).
| /** 关闭任务栏歌词窗口并清理所有资源 */ | |
| export const closeTaskbarLyricWindow = (): void => { | |
| if (taskbarLyricWindow && !taskbarLyricWindow.isDestroyed()) { | |
| taskbarLyricWindow.close(); | |
| } | |
| taskbarLyricWindow = null; | |
| cleanupWatchers(); | |
| setTrayTaskbarLyric(false); | |
| broadcast("taskbarLyric:visibilityChange", false); | |
| /** 请求关闭任务栏歌词窗口,实际清理由窗口的 "closed" 事件统一处理 */ | |
| export const closeTaskbarLyricWindow = (): void => { | |
| if (taskbarLyricWindow && !taskbarLyricWindow.isDestroyed()) { | |
| taskbarLyricWindow.close(); | |
| } |
| if name == "TrayNotifyWnd" | ||
| && let Ok(guard) = GLOBAL_CALLBACK.lock() | ||
| && let Some(cb) = guard.as_ref() | ||
| { | ||
| cb(); |
There was a problem hiding this comment.
The tray event hook calls cb() while holding the GLOBAL_CALLBACK mutex. If the callback is slow or re-entrant, this can unnecessarily block other lock users and increases deadlock risk. Prefer cloning the Arc (or extracting a raw function pointer) while locked, then dropping the guard before invoking the callback.
| if name == "TrayNotifyWnd" | |
| && let Ok(guard) = GLOBAL_CALLBACK.lock() | |
| && let Some(cb) = guard.as_ref() | |
| { | |
| cb(); | |
| if name == "TrayNotifyWnd" { | |
| let callback = GLOBAL_CALLBACK | |
| .lock() | |
| .ok() | |
| .and_then(|guard| guard.as_ref().cloned()); | |
| if let Some(cb) = callback { | |
| cb(); | |
| } |
| thread::spawn(move || unsafe { | ||
| let _ = CoInitializeEx(None, COINIT_MULTITHREADED); | ||
|
|
There was a problem hiding this comment.
CoInitializeEx's result is ignored, but CoUninitialize() is called unconditionally at the end of the thread. CoUninitialize should only be called if COM initialization succeeded (and not when CoInitializeEx returns errors like RPC_E_CHANGED_MODE). Track the HRESULT/boolean and only uninitialize when appropriate (similar to TaskbarScanner's should_uninitialize handling).
| import { t } from "@main/utils/i18n"; | ||
| import { trayLog } from "@main/utils/logger"; | ||
| import { toggleDesktopLyricWindow, toggleDynamicIslandWindow, focusMainWindow } from "@main/window"; | ||
| import { |
There was a problem hiding this comment.
This import line is significantly longer than the surrounding imports and likely to be reformatted by the project's formatter/linter. Splitting the named imports across multiple lines will improve readability and avoid churn in future diffs.
| }); | ||
|
|
||
| // 切换任务栏歌词窗口 | ||
| ipcMain.handle("window:toggleTaskbarLyric", () => toggleTaskbarLyricWindow()); |
There was a problem hiding this comment.
ipcMain.handle("window:toggleTaskbarLyric") currently returns the result of toggleTaskbarLyricWindow(), but that function returns void. This makes window.api.window.toggleTaskbarLyric() resolve to undefined even though WindowApi.toggleTaskbarLyric is typed as Promise<boolean>. Please return a boolean open/close result (consistent with toggleDesktopLyricWindow / toggleDynamicIslandWindow) and forward that value from the IPC handler.
| ipcMain.handle("window:toggleTaskbarLyric", () => toggleTaskbarLyricWindow()); | |
| ipcMain.handle("window:toggleTaskbarLyric", () => { | |
| toggleTaskbarLyricWindow(); | |
| return !!getTaskbarLyricWindow(); | |
| }); |
| const currentWidth = 300; | ||
|
|
There was a problem hiding this comment.
currentWidth is a hard-coded constant, but it's used as the lyric_width parameter for the native module (Win10 uses it to carve space). This means user settings like taskbarLyric.maxWidth / taskbarLyric.autoMaxWidth won't actually affect the reserved taskbar space on Win10, and applyTaskbarLyricLayout() will never reflect those config changes. Consider deriving the update width from the current config (e.g., use maxWidth when autoMaxWidth is false) and updating the native layout whenever config changes.
| const currentWidth = 300; | |
| const DEFAULT_LYRIC_WIDTH = 300; | |
| type TaskbarLyricStoreSettings = { | |
| autoMaxWidth?: boolean; | |
| maxWidth?: number; | |
| }; | |
| const readTaskbarLyricSettings = (): TaskbarLyricStoreSettings => { | |
| const typedStore = store as { | |
| get?: (key: string) => unknown; | |
| [key: string]: unknown; | |
| }; | |
| const settings = | |
| typeof typedStore.get === "function" | |
| ? typedStore.get("taskbarLyric") | |
| : typedStore.taskbarLyric; | |
| return settings && typeof settings === "object" | |
| ? (settings as TaskbarLyricStoreSettings) | |
| : {}; | |
| }; | |
| const resolveCurrentWidth = (): number => { | |
| const { autoMaxWidth, maxWidth } = readTaskbarLyricSettings(); | |
| if (autoMaxWidth === false) { | |
| const width = Number(maxWidth); | |
| if (Number.isFinite(width) && width > 0) { | |
| return width; | |
| } | |
| } | |
| return DEFAULT_LYRIC_WIDTH; | |
| }; | |
| let currentWidth = resolveCurrentWidth(); | |
| const refreshTaskbarLyricWidth = (): void => { | |
| currentWidth = resolveCurrentWidth(); | |
| }; | |
| const applyTaskbarLyricLayoutIfAvailable = (): void => { | |
| const layoutApplier = ( | |
| globalThis as typeof globalThis & { | |
| applyTaskbarLyricLayout?: () => void | Promise<void>; | |
| } | |
| ).applyTaskbarLyricLayout; | |
| if (typeof layoutApplier === "function") { | |
| void layoutApplier(); | |
| } | |
| }; | |
| const registerTaskbarLyricConfigListener = (): void => { | |
| const typedStore = store as { | |
| onDidChange?: (key: string, listener: () => void) => unknown; | |
| onDidAnyChange?: (listener: () => void) => unknown; | |
| subscribe?: (listener: () => void) => unknown; | |
| }; | |
| const onChange = (): void => { | |
| refreshTaskbarLyricWidth(); | |
| applyTaskbarLyricLayoutIfAvailable(); | |
| }; | |
| if (typeof typedStore.onDidChange === "function") { | |
| typedStore.onDidChange("taskbarLyric", onChange); | |
| return; | |
| } | |
| if (typeof typedStore.onDidAnyChange === "function") { | |
| typedStore.onDidAnyChange(onChange); | |
| return; | |
| } | |
| if (typeof typedStore.subscribe === "function") { | |
| typedStore.subscribe(onChange); | |
| } | |
| }; | |
| registerTaskbarLyricConfigListener(); |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 54 out of 58 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let callback_arc = Arc::new(callback); | ||
|
|
||
| if let Ok(mut guard) = GLOBAL_CALLBACK.lock() { | ||
| *guard = Some(callback_arc); | ||
| } |
There was a problem hiding this comment.
TrayWatcher::new() 会把回调存到全局 GLOBAL_CALLBACK,但 stop()/Drop 没有把它清空,导致 watcher 停止后回调闭包仍被静态 Arc 持有(资源无法释放)。建议在 stop()(或 Drop)里把 GLOBAL_CALLBACK 置回 None(与 TaskbarCreatedWatcher 的清理逻辑对齐)。
| unsafe { | ||
| let hr = CoInitializeEx(None, COINIT_MULTITHREADED); | ||
| if hr.is_err() { | ||
| return; | ||
| } |
There was a problem hiding this comment.
worker_loop 里 CoInitializeEx 只要返回 Err 就直接退出,但 RPC_E_CHANGED_MODE 在其它实现(如 uia.rs)属于“可继续运行但不需要 CoUninitialize”的情况。这里直接 return 会让服务在少数环境下静默失效。建议参照 TaskbarScanner::new() 的 should_uninitialize 模式:对 RPC_E_CHANGED_MODE 允许继续,同时只在真正初始化成功时才调用 CoUninitialize()。
| createTaskbarLyricWindow(); | ||
| return true; |
There was a problem hiding this comment.
toggleTaskbarLyricWindow() 在非 Windows 平台会走到 createTaskbarLyricWindow(); return true;,但 createTaskbarLyricWindow() 此时返回 null,导致 IPC 调用得到“已打开”的假状态。建议在 toggle 内先判 process.platform !== "win32" 直接返回 false,或根据 createTaskbarLyricWindow() 的返回值决定最终返回值。
| createTaskbarLyricWindow(); | |
| return true; | |
| if (process.platform !== "win32") { | |
| return false; | |
| } | |
| return createTaskbarLyricWindow() !== null; |
| // Windows 上 HWND 是 64 位指针(x64),读为 BigInt 再转 number | ||
| const hwndPtr = Number(win.getNativeWindowHandle().readBigUInt64LE(0)); | ||
| taskbarLog.info(`嵌入窗口 hwnd=${hwndPtr}`); |
There was a problem hiding this comment.
这里把 HWND(u64)从 BigInt 转成 Number 再传给原生模块,存在精度丢失风险(超过 2^53 时会截断),可能导致嵌入失败且难定位。建议把 NAPI 方法参数改为 BigInt,或直接接收 Buffer/Uint8Array(从 getNativeWindowHandle() 直接传递)并在 Rust 侧解析为 usize,避免 JS number 精度问题。
| // Windows 上 HWND 是 64 位指针(x64),读为 BigInt 再转 number | |
| const hwndPtr = Number(win.getNativeWindowHandle().readBigUInt64LE(0)); | |
| taskbarLog.info(`嵌入窗口 hwnd=${hwndPtr}`); | |
| // Windows 上 HWND 可能是 64 位值,先保留为 BigInt,避免直接转成 number 产生静默精度丢失 | |
| const hwndPtrBigInt = win.getNativeWindowHandle().readBigUInt64LE(0); | |
| if (hwndPtrBigInt > BigInt(Number.MAX_SAFE_INTEGER)) { | |
| taskbarLog.error( | |
| `嵌入窗口失败:hwnd=${hwndPtrBigInt.toString()} 超出 JS Number 安全整数范围,无法安全传递给原生模块`, | |
| ); | |
| return; | |
| } | |
| const hwndPtr = Number(hwndPtrBigInt); | |
| taskbarLog.info(`嵌入窗口 hwnd=${hwndPtrBigInt.toString()}`); |
| onConfigChange: (callback: (config: unknown) => void) => | ||
| subscribe("taskbarLyric:configChange", callback), |
There was a problem hiding this comment.
taskbarLyric.onConfigChange 这里把回调参数标成 unknown,但 electron/preload/index.d.ts 中 TaskbarLyricApi.onConfigChange 的回调类型是 TaskbarLyricSettings。实现与类型声明不一致,后续改动时更容易把非预期数据透传到渲染端而不自知。建议在 preload 侧引入 TaskbarLyricSettings 并使用 subscribe<TaskbarLyricSettings>("taskbarLyric:configChange", ...),保持实现与共享类型一致。
| onConfigChange: (callback: (config: unknown) => void) => | |
| subscribe("taskbarLyric:configChange", callback), | |
| onConfigChange: (callback: (config: TaskbarLyricSettings) => void) => | |
| subscribe<TaskbarLyricSettings>("taskbarLyric:configChange", callback), |
|
@copilot 审查 |
Agent-Logs-Url: https://github.com/SPlayer-Dev/SPlayer-Next/sessions/d3f48708-e204-4599-8d39-dfd1832ef8df Co-authored-by: imsyy <42232682+imsyy@users.noreply.github.com>
Agent-Logs-Url: https://github.com/SPlayer-Dev/SPlayer-Next/sessions/d3f48708-e204-4599-8d39-dfd1832ef8df Co-authored-by: imsyy <42232682+imsyy@users.noreply.github.com>
审查完毕,共审查 58 个变更文件。发现并修复了 3 个问题( 必修:
确认已处理的历史审查项:
|
No description provided.