fix(overlay): seed display info from launch args instead of racing IPC - #7
Open
pango07 wants to merge 2 commits into
Open
fix(overlay): seed display info from launch args instead of racing IPC#7pango07 wants to merge 2 commits into
pango07 wants to merge 2 commits into
Conversation
createOverlayWindow pushed display-info on a once('did-finish-load')
handler, but OverlayApp attaches its listener inside a useEffect. When
the send landed first the message was dropped and displayRef stayed null
for the lifetime of the window, so cursor positions fell through to the
unmapped branch that treats global screen coordinates as display-local.
That happens to be correct on a single display at origin (0,0), which is
why it went unnoticed, and wrong everywhere else.
Pass the display through webPreferences.additionalArguments so the
preload can parse it synchronously and the renderer has its coordinate
space before the first cursor-position message arrives. The IPC push
stays for reloads, and is now `on` rather than `once` since the argv
value is only correct for the first load.
Refs #5
The comment claimed the argv value was only correct for the first load. It isn't: a reload re-executes the preload in the same process with the same argv, so the re-push sends an identical snapshot. It's redundancy, not an update path, and bounds changes don't flow through it at all — rebuildOverlays destroys and recreates the window instead. Say so. Also warn instead of silently returning null when the launch argument fails to parse, since that failure would otherwise reproduce the exact symptom this change exists to fix, and note why plain JSON in argv is safe here (all-numeric fields, so no spaces for Windows to split on).
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while looking into #5.
the bug
createOverlayWindowpusheddisplay-infoon aonce('did-finish-load')handler (windows.ts:96), butOverlayAppattaches its listener inside auseEffect(OverlayApp.tsx:197). Nothing orders those two. When the send lands first the message is dropped, and because it'sonceit never comes again, sodisplayRef.currentstays null for the lifetime of the window.Every cursor position then falls through to the else branch:
That happens to be correct on a single display at origin (0,0), which is why it hasn't bitten anyone yet. On a second monitor, or any display with a non-zero origin, the companion cursor renders at the wrong offset.
onElementDetectedhas the same dependency atOverlayApp.tsx:230, so pointing targets land wrong too.the fix
Pass the display through
webPreferences.additionalArguments, so the preload parses it out ofprocess.argvand the renderer has its coordinate space synchronously on mount. No ordering to get wrong.The IPC push stays for reloads and is now
onrather thanonce, since the argv value is only correct for the first load.DisplayInfoand the arg prefix moved intoshared/types.ts(the shape was inlined in three places)window.flicky.getDisplayInfo(), returns null in non-overlay windowsonDisplayInfokeeps working, now typed off the shared interfacetesting
npm run typecheckpasses.npm run lintfails on master too, unrelated: eslint 9 wants a flateslint.config.jsand the repo doesn't have one.Not runtime-tested, I don't have a display here. Worth a quick check on a multi-monitor setup that the companion still tracks correctly on the non-primary display.
note
This is not the mic bug in #5, that's a separate chain I commented on in the issue. This is the second symptom the reporter mentioned (cursor not trailing), though I suspect their specific case is Wayland's
getCursorScreenPoint()rather than this. Worth fixing either way.Generated by Claude Code