feat: space actor#392
Conversation
…displays PR acsandmann#392 (space actor) changed handle_window_server_appeared to reassign a known window to the WindowServer-reported space on every SpaceWindowCreated event. That event also fires as a side effect of Rift's own SetWindowFrame dragging a window across a display seam, so Rift chased the echo of its own move and re-tiled endlessly. Apps that resist AX frame changes (e.g. Zen, Outlook) never settle, producing a ~170 ms flap between displays. Unlike the WindowFrameChanged path, this appeared->reassign path had no self-move guard. Skip the reassignment when a Rift frame transaction is still in flight for the window (transaction_manager.get_target_frame is Some); the authoritative space record is still updated. Genuine external moves (manual drag, no pending transaction) continue to reassign. Adds appeared_does_not_reassign_window_while_rift_move_is_in_flight (the regression guard) and appeared_reassigns_window_without_pending_rift_move (ensures genuine external moves are still followed).
The observer refcon-hint optimization (ec8d197) encodes the notification kind + window into the refcon so the callback can recover them without a lookup. Per-window notifications were registered with the encoded hint, but per-application notifications kept using the plain `add_notification`, which attaches a zero refcon. Kind tags start at 1, so `from_tag(0)` returns None and `decode_notification_data` discards the event - silently dropping every app-level notification (ApplicationActivated/Deactivated/Hidden/Shown and MainWindowChanged on the app element). The user-visible symptom is that switching apps via Cmd-Tab no longer switches to the app's workspace, because the auto workspace switch is driven by ApplicationActivated, which never reaches the reactor. This affects all apps (including native ones like Finder), not just Electron apps. Register app-level notifications with their encoded kind hint, mirroring the per-window path, so the callback decodes them correctly again.
This reverts commit 723ff9b.
…, and Dock click (#399) (#400) * fix(focus): synchronize layout engine selection on mouse click, hover, and Dock click (#399) - Intercepts Event::MouseUp to immediately synchronize layout selection when a window is focused via a click. - Exposes LayoutEngine::focused_window to allow external querying of the layout engine's selected/focused window. - Updates handle_mouse_moved_over_window to perform focus selection synchronization on hover, even if the window is already the main macOS window. - Forces visible window refresh and queries app main window via AX upon receiving ApplicationGloballyActivated (e.g. Dock click activation) to bypass macOS AX event drops. * fix: trigger workspace switch on global app activation from dock click * fix: Master Stack Layout Reordering and Arrangement (#397) (#398) * refactor: simplify should_raise_on_mouse_over signature and caller logic * fix moving into empty stack * fix --------- Co-authored-by: acsandmann <157552025+acsandmann@users.noreply.github.com>
|
I think this is a good step. I noticed few issues:
|
have fixes for all these locally. some good steps there too, like truly centralizing all window state as the ghost window issue is due to state being in virutal workspaces, reactor, etc |
That would be awesome. Would it also react to when changing the display arrangement/rotation without having to restart rift? I think this might be emitting the same events as it would on wake? |
yes, but we hold state here and if there is a difference we will forward new space/display to reactor. but on wake if there is no change then no events so nothing should be messed up.. |
Tighten active-space window reconciliation so moved or hidden windows keep the correct workspace ownership across displays, ignore stale WindowServer/user-space events, and restore windows cleanly after topology/fullscreen changes. Also separate menu-bar space from command context so layout/workspace commands follow the focused display, update workspace-change event handling, and expand tests for multi-display, fullscreen, and hidden-window behavior.
|
i think this is quite close to being merged |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c47dc31256
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let cfg = reactor.activation_cfg(); | ||
| let current_screens = reactor.screens_for_current_spaces(); | ||
| reactor.space_activation_policy.on_spaces_updated(cfg, ¤t_screens); | ||
| reactor.apply_authoritative_active_spaces(active_spaces); |
There was a problem hiding this comment.
When default_disable, one_space, or ToggleSpaceActivated has disabled a space/display, this installs the spaces actor's raw active_spaces set directly after merely updating space_activation_policy. The spaces actor does not know the user's activation policy, so every normal SpaceStateChanged snapshot re-enables all current user spaces; for example, starting with default_disable = true will still make the reactor manage/layout windows on all spaces as soon as the first snapshot is forwarded. This should go through the policy-backed recompute path instead of applying the forwarded set verbatim.
Useful? React with 👍 / 👎.
|
@hajiboy95 does this branch have the correct updated master stack behavior or is it on main? as in, there are conflicts and im not sure which one is the correct one, this or main. |
|
The updated master stack behaviour is already on the main. I use it 😁 #367 was the PR |
4 participants
this splits out spaces from the reactor and cleans up a lot of decision making