Add optional @PPI hint to --display for HiDPI pixel displays - #1296
Add optional @PPI hint to --display for HiDPI pixel displays#1296jonnyzzz wants to merge 2 commits into
Conversation
`tart set --display WIDTHxHEIGHTpx` always configured the guest's pixel framebuffer at a hardcoded 72 PPI, so a macOS guest never exposed a HiDPI (2x/Retina) mode when the host display was non-Retina — or absent, which is common on headless/CI hosts. There was no way to ask for a Retina guest in that case. Allow an optional "@ppi" pixels-per-inch suffix on --display, e.g. `--display 3200x1800px@220`. At a Retina-class density the pixel display then offers a scaling:on (HiDPI) mode. The pixel path never consults NSScreen, so this works regardless of the host display or whether a user is logged in on the host. The hint lives in VMConfig (VMDisplayConfig.ppi) and defaults to the existing 72 when omitted, so behavior is unchanged for current VMs and configs. Covered by VMConfigTests (parsing, description round-trip, effective-PPI default, backward compatibility, malformed input). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 521cc16e44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| vmConfig.display.height = display.height | ||
| } | ||
| vmConfig.display.unit = display.unit | ||
| vmConfig.display.ppi = display.ppi |
There was a problem hiding this comment.
When a user mistypes the new suffix as @0 or @-1, the parser produces a non-nil ppi and this assignment persists it even though width and height have positive guards. The macOS pixel path later feeds effectivePixelsPerInch straight into VZMacGraphicsDisplayConfiguration, so the saved config no longer falls back to 72 and can fail to construct or validate the display until the user manually resets it; reject or ignore ppi <= 0 before saving.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 60bf256. The --display parser now keeps only a positive PPI (@0/@-1 are ignored like non-numeric input), and effectivePixelsPerInch also treats a non-positive stored value (e.g. a hand-edited config) as unset, so a non-positive value can never reach VZMacGraphicsDisplayConfiguration — it falls back to 72. Covered by two new tests in VMConfigTests.
Validated end-to-end on a real macOS guestBooted
So the |
Codex review flagged that a mistyped "@0" or "@-1" parsed to a non-nil ppi that persisted and was fed straight into VZMacGraphicsDisplayConfiguration, bypassing the 72 fallback and risking a display that fails to build until the user manually resets it. Guard both ends: the --display parser now keeps only a positive PPI (zero/negative ignored just like non-numeric input), and effectivePixelsPerInch treats a non-positive stored value (e.g. from a hand-edited config) as unset, falling back to 72. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Problem
tart set --display WIDTHxHEIGHTpxbuilds the guest's pixel-unit framebuffer with a hardcoded 72 PPI (Sources/tart/Platform/Darwin.swift). As a result a macOS guest never exposes a HiDPI / Retina (scaling:on, 2x) mode when the host display is non-Retina — or when there is no host display at all, which is the normal case on headless / CI hosts. The point-unit (pt) path can produce a HiDPI guest, but only by copying the hostNSScreen.mainbacking scale, so it depends on a Retina host console being attached and a GUI session being logged in. There was previously no way to ask for a Retina guest on such a host.Change
Allow an optional
@PPI(pixels-per-inch) suffix on--display:At a Retina-class density (e.g.
220) the pixel display then offers ascaling:on(HiDPI, 2x) mode inside the guest. Because the pixel path never consultsNSScreen, this works regardless of the host display or whether a user is logged in on the host — it only needs the (already-required) unlocked login keychain that Virtualization.framework needs to boot any VM on macOS 15+.VMDisplayConfig.ppi(persisted inconfig.json; omitted when nil, so existing configs are untouched).--displayparser acceptsWIDTHxHEIGHT[pt|px][@PPI]; the@PPIis parsed before the unit suffix and degrades to “unset” on malformed input.VMDisplayConfig.effectivePixelsPerInchreturnsppi ?? 72, so behavior is unchanged when the hint is omitted.CustomStringConvertibleround-trips the hint (3200x1800px@220).The hint applies to the pixel path; on the point path it is stored but not used (that path derives scale from the host screen).
Tests
Added to
Tests/TartTests/VMConfigTests.swift(TDD — each was watched failing before implementation):@PPIsuffix (with and without an explicit unit);effectivePixelsPerInchuses the configured value and defaults to 72 when unset;WIDTHxHEIGHT[pt|px]strings carry no PPI;@notanumber) degrades to the 72 default rather than failing the parse.Full suite green (
swift test), andswiftformat --lintreports the changed files clean.