From 521cc16e44aa28ec7c6574ad58969198422ff9f8 Mon Sep 17 00:00:00 2001 From: Eugene Petrenko Date: Tue, 28 Jul 2026 13:43:10 +0200 Subject: [PATCH 1/2] Add optional @PPI hint to --display for HiDPI pixel displays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- Sources/tart/Commands/Set.swift | 12 ++++++++- Sources/tart/Platform/Darwin.swift | 6 +++-- Sources/tart/VMConfig.swift | 18 ++++++++++--- Tests/TartTests/VMConfigTests.swift | 42 +++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/Sources/tart/Commands/Set.swift b/Sources/tart/Commands/Set.swift index 384fda8c..e0016c89 100644 --- a/Sources/tart/Commands/Set.swift +++ b/Sources/tart/Commands/Set.swift @@ -14,7 +14,7 @@ struct Set: AsyncParsableCommand { @Option(help: "VM memory size in megabytes") var memory: UInt64? - @Option(help: "VM display resolution in a format of WIDTHxHEIGHT[pt|px]. For example, 1200x800, 1200x800pt or 1920x1080px. Units are treated as hints and default to \"pt\" (points) for macOS VMs and \"px\" (pixels) for Linux VMs when not specified.") + @Option(help: "VM display resolution in a format of WIDTHxHEIGHT[pt|px][@PPI]. For example, 1200x800, 1200x800pt, 1920x1080px or 3200x1800px@220. Units are treated as hints and default to \"pt\" (points) for macOS VMs and \"px\" (pixels) for Linux VMs when not specified. The optional @PPI (pixels-per-inch) hint applies to pixel-unit displays and, at a Retina-class value such as 220, makes a macOS guest expose a HiDPI (2x) mode regardless of the host display; it defaults to 72 (non-Retina) when omitted.") var display: VMDisplayConfig? @Flag(inversion: .prefixedNo, help: ArgumentHelp("Whether to automatically reconfigure the VM's display to fit the window")) @@ -57,6 +57,7 @@ struct Set: AsyncParsableCommand { vmConfig.display.height = display.height } vmConfig.display.unit = display.unit + vmConfig.display.ppi = display.ppi } vmConfig.displayRefit = displayRefit @@ -91,6 +92,14 @@ extension VMDisplayConfig: ExpressibleByArgument { public init(argument: String) { var argument = argument var unit: Unit? = nil + var ppi: Int? = nil + + // Optional "@PPI" pixels-per-inch hint, e.g. "3200x1800px@220". Parsed + // before the unit suffix since it always trails the whole spec. + if let atIndex = argument.lastIndex(of: "@") { + ppi = Int(argument[argument.index(after: atIndex)...]) + argument = String(argument[.. Date: Tue, 28 Jul 2026 15:49:28 +0200 Subject: [PATCH 2/2] Reject non-positive @PPI hints 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 --- Sources/tart/Commands/Set.swift | 6 ++++-- Sources/tart/VMConfig.swift | 13 +++++++++---- Tests/TartTests/VMConfigTests.swift | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/Sources/tart/Commands/Set.swift b/Sources/tart/Commands/Set.swift index e0016c89..6a8b737b 100644 --- a/Sources/tart/Commands/Set.swift +++ b/Sources/tart/Commands/Set.swift @@ -95,9 +95,11 @@ extension VMDisplayConfig: ExpressibleByArgument { var ppi: Int? = nil // Optional "@PPI" pixels-per-inch hint, e.g. "3200x1800px@220". Parsed - // before the unit suffix since it always trails the whole spec. + // before the unit suffix since it always trails the whole spec. Only a + // positive value is kept; zero/negative (or non-numeric) is ignored so the + // display falls back to the 72 default rather than a broken configuration. if let atIndex = argument.lastIndex(of: "@") { - ppi = Int(argument[argument.index(after: atIndex)...]) + ppi = Int(argument[argument.index(after: atIndex)...]).flatMap { $0 > 0 ? $0 : nil } argument = String(argument[.. 0 { + return ppi + } + return 72 } } diff --git a/Tests/TartTests/VMConfigTests.swift b/Tests/TartTests/VMConfigTests.swift index f85740c1..b590a847 100644 --- a/Tests/TartTests/VMConfigTests.swift +++ b/Tests/TartTests/VMConfigTests.swift @@ -57,4 +57,19 @@ final class VMConfigTests: XCTestCase { XCTAssertEqual(VMDisplayConfig(width: 3200, height: 1800, unit: .pixel, ppi: nil), config) XCTAssertEqual(72, config.effectivePixelsPerInch) } + + func testRejectsNonPositivePixelsPerInchWhenParsing() throws { + // A zero or negative PPI is meaningless and must not persist — it would + // otherwise reach VZMacGraphicsDisplayConfiguration and fail to build the + // display instead of falling back to the 72 default. + XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@0").ppi) + XCTAssertNil(VMDisplayConfig(argument: "3200x1800px@-5").ppi) + } + + func testEffectivePixelsPerInchIgnoresNonPositiveStoredValue() throws { + // Defense for a hand-edited config.json: a stored value <= 0 falls back to + // 72 rather than being handed to VZMacGraphicsDisplayConfiguration. + XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: 0).effectivePixelsPerInch) + XCTAssertEqual(72, VMDisplayConfig(width: 100, height: 100, unit: .pixel, ppi: -5).effectivePixelsPerInch) + } }