fix(windows): WinUI 3 shell focus, input topology, and action plumbing#157
Merged
fix(windows): WinUI 3 shell focus, input topology, and action plumbing#157
Conversation
3 tasks
ghostty_target_s is a 16-byte struct and on Windows x64 is passed by hidden pointer. The current delegate declared it as a value struct, which is ABI-incorrect. It has never crashed only because the handler returns false without reading target. Switch both target and action to IntPtr to match the ABI before adding real decode logic.
Mirrors ghostty_action_tag_e from ghostty.h so the runtime action callback can dispatch on tag without blind pointer reads. Also adds MessageBeep for the upcoming RING_BELL handling.
Plumbing for the next task, which decodes the ghostty action union and raises these when the core asks us to update the title or close.
First real handling of ghostty_action_s. Reads the tag at offset 0, dispatches the three variants this single-window shell can act on, and leaves everything else falling through to core defaults. SET_TITLE and CLOSE_WINDOW marshal onto the UI dispatcher before raising the events MainWindow consumes. RING_BELL calls MessageBeep directly because it is thread-safe.
Subscribe MainWindow to the TerminalControl events so cmd.exe title changes reach the window chrome and exit closes the window. Switch the resize source from SizeChangedEventArgs to Panel.ActualWidth/ActualHeight to eliminate a DPI-rounding gap that was letterboxing the terminal by a pixel or two at the edges. Also add a comment on IsTabStop="False" explaining why it must stay.
The previous WinUI 3 shell put GotFocus/LostFocus, KeyDown/Up,
CharacterReceived, and IsTabStop on the SwapChainPanel itself, which
caused continuous focus ping-pong. Two reasons:
1) SwapChainPanel inherits from Grid, not Control, so it cannot
hold keyboard focus directly - Panel.Focus(...) was a silent
no-op.
2) Subscribing CharacterReceived/KeyDown on a SwapChainPanel
causes the framework to insert a TSF/IME input scope which
transiently steals and returns focus on every layout pass.
Move all input/focus to the outer UserControl. Leave SizeChanged and
CompositionScaleChanged on the SwapChainPanel. Replace the OnLoaded
TryEnqueue retry loop with a one-shot Panel.LayoutUpdated subscription
so initial sizing sees a settled layout. CompositionScaleChanged now
also re-kicks the resize debounce so pixel dimensions are recomputed
when DPI changes. Dedupe focus state forwarding so libghostty never
sees redundant focus events.
Pixel-perfect fill and DX12 resize-crash issues remain - they live
below this layer in the Zig DX12 renderer and will be addressed in a
follow-up stacked PR.
- Return false from OnAction on null dispatcher so the core falls back to its default instead of treating a silently dropped action as handled. - Replace hand-rolled PtrToStringUtf8 with Marshal.PtrToStringUTF8. - Pin GhosttyActionTag to the three variants we actually dispatch on, with explicit upstream-matched indices, so a reorder in ghostty.h cannot silently misroute one tag to another handler. - Unsubscribe Panel.LayoutUpdated in OnUnloaded so the one-shot init handler cannot keep firing if the control is torn down before its size ever settles. - Don't flip _focused before the surface check in SetFocusState: a stale value would dedupe the next focus change after the surface is recreated. - Drop unused app parameter shadowing _app in OnAction; remove stale step-number comments in OnLoaded that no longer match.
10d735b to
35d666f
Compare
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
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.
Follow-up fixes on top of the WinUI 3 shell scaffold. Three areas:
Action callback plumbing
GhosttyActionCbP/Invoke signature:ghostty_target_sandghostty_action_sare passed by hidden pointer on Windows x64, not by value.ghostty_action_sand dispatchSET_TITLE,RING_BELL,CLOSE_WINDOW. Everything else falls through to core defaults.TitleChangedandCloseRequestedevents onTerminalControl;MainWindowsubscribes to update the chrome.MessageBeep(MB_OK)for the bell.Focus and input topology
GotFocus/LostFocus/KeyDown/KeyUp/CharacterReceivedfrom theSwapChainPanelto the outerUserControl. SwapChainPanel inherits from Grid, not Control, so it cannot hold keyboard focus directly —Panel.Focus(...)was a silent no-op. SubscribingCharacterReceivedto a SwapChainPanel also inserts a TSF input scope that thrashes focus on every layout pass.this.Focus(FocusState.Pointer)only when not already focused.Resize and initial sizing
OnLoadedTryEnqueueretry loop with a one-shotPanel.LayoutUpdatedsubscription so the initial size is primed once layout is settled andCompositionScaleX/Yis valid.OnCompositionScaleChangednow re-kicks the resize debounce so pixel dimensions are recomputed when DPI changes.KickResizeDebouncehelper to avoid duplicating the timer init.Out of scope (deferred to next stacked PR
feat/winui3-shell-dx12-fixes)Target:
feat/winui3-shell