feat(windows): DX12 swap chain resize + Target double-free fix#158
Merged
feat(windows): DX12 swap chain resize + Target double-free fix#158
Conversation
deblasis
force-pushed
the
feat/winui3-shell-followup
branch
from
10d735b to
35d666f
Compare
deblasis
force-pushed
the
feat/winui3-shell-dx12-fixes
branch
from
6029d75 to
6ee61e7
Compare
Target.resource is set in beginFrame to one of the API's back_buffers slots without an AddRef -- it is a borrowed reference whose lifetime is owned by DirectX12.back_buffers. Target.deinit was Releasing it anyway, which was a latent double-free that the previous no-op setTargetSize masked because the back buffers were never actually released. As soon as ResizeBuffers releases them on resize, the next frame.resize call hits Target.deinit on the dangling pointer and the process dies silently.
setTargetSize previously only cached the requested dimensions and the swap chain back buffers stayed at their initial size for the lifetime of the surface. DXGI composition stretched the small back buffer onto the larger SwapChainPanel, which is why the terminal looked smaller and blurry inside the window. Resize on the renderer thread, not the apprt thread. setTargetSize runs synchronously from ghostty_surface_set_size on the C# UI thread and must not touch GPU state. It now stores the desired size in atomics and beginFrame compares them to applied_width/height at the top -- if they differ it drains the GPU, releases back buffers, calls IDXGISwapChain1::ResizeBuffers, re-acquires buffers and recreates RTVs at the same descriptor slots, then resets per-frame fence values. Device-removed HRESULTs route through the existing handleDeviceRemoved path.
A managed exception on the UI thread silently exits the process with a non-descriptive code, leaving nothing to debug from. Hook the three unhandled-exception entry points (UI dispatcher, AppDomain, task scheduler) and write the exception to stderr. Debug-only -- in Release we want WER to capture a real crash dump instead.
- Pack desired_width/desired_height into a single u64 atomic so a
drag-resize cannot tear and briefly resize the swap chain to a
width/height pair from two different setTargetSize calls.
- resizeSwapChain now returns error.NoDevice / error.NoSwapChain on
the missing-handle paths instead of silently returning. The previous
silent return left applied_width/height stale, so beginFrame would
loop on the same desired size every frame with nothing logged.
- Drop the unconditional log.info("presentLastTarget"...) that fired
every idle frame.
- Document the IDXGISwapChain3 -> IDXGISwapChain1 ptrCast as a COM
v-table prefix reinterpret (cheaper than QueryInterface for a hot
path).
deblasis
force-pushed
the
feat/winui3-shell-dx12-fixes
branch
from
6ee61e7 to
d8ce97a
Compare
Three changes that together remove the visible white frame at the leading edge of an interactive drag-resize: - Replace the WNDCLASS hbrBackground brush on the WinUI 3 host HWND with a dark solid brush. DWM paints uncovered window pixels with the class brush before WinUI 3 extends its XAML content into the new area, and the default brush is white. - Set RootGrid Background to the same dark color so the XAML layer matches the Win32 layer underneath the SwapChainPanel. - Drop the 30 ms resize debounce in TerminalControl. It was a workaround for the old DX12 renderer that recreated the swap chain on every set_size; now that setTargetSize records desired dimensions atomically and beginFrame applies them via ResizeBuffers, every pixel of drag costs only an atomic store and matches the macOS apprt's zero-debounce path.
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
1 participant
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.
Important
Stacked PR. Order:
Two bugs from the followup backlog. They surfaced together but had separate root causes.
Target.deinit double-free
Target.resourceis set inbeginFrameto one ofDirectX12.back_buffers[i]without an AddRef -- it is a borrowed reference owned by the API.Target.deinitwas Releasing it anyway. The previous no-opsetTargetSizemasked this because the back buffers were never released, so the dangling pointer was never used. As soon asResizeBuffersreleases a back buffer, the nextframe.resizehitsTarget.deiniton the dangling pointer and the process dies silently (no Zig panic, no managed exception).Fix: document
Target.resourceas borrowed and stop releasing it indeinit.DX12 swap chain resize
setTargetSizeonly cached the requested dimensions. The back buffers stayed at the initial size for the lifetime of the surface and DXGI composition stretched the small buffer onto the larger SwapChainPanel, which is why the terminal looked smaller and blurry.setTargetSizeruns synchronously fromghostty_surface_set_sizeon the C# UI thread, so it cannot touch GPU state. It now stores the desired dims as atomics.beginFrameruns on the renderer thread (the only thread allowed near back buffers, fences, and RTVs) and compares desired vs applied at the top -- if they differ it calls a newresizeSwapChain:waitForGpuIDXGISwapChain1::ResizeBuffers(frame_count, w, h, UNKNOWN, 0)UNKNOWNformat and0flags preserve creation values. Device-removed HRESULTs route through the existinghandleDeviceRemovedpath. Guarded against zero/no-op sizes so the C# 30ms debounce drops identical calls cheaply.Debug-only unhandled-exception logger
The crash here was masked by the C# host silently exiting on an unhandled managed exception with no diagnostic. Added a Debug-only hook on the three exception entry points (UI dispatcher, AppDomain, task scheduler) that writes the exception to stderr. Release builds leave this off so WER can capture a real crash dump.
Test plan
zig build -Dapp-runtime=nonecleandotnet build windows/Ghostty/Ghostty.slncleanjust run-win, drag-resize aggressively across many sizes, rundir, close window. No crash, no DX12 debug-layer warnings, terminal grid fills the panel pixel-perfect.