From 3d4135b494e7af739f1b1ea3d0fa4f1e47d353a4 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 26 Aug 2026 12:17:05 +0300 Subject: [PATCH 1/7] config: close the file created by openPath createFileAbsolute's handle was dropped on the floor rather than closed. On POSIX that costs nothing. On Windows the create denies read sharing for as long as the handle lives, so every later read of the config in the same process fails with a sharing violation on a file nothing else has open. On a first run that is every read there is: the config does not exist, the open creates it, and the app dies with 0xC000027B before it draws. --- src/config/edit.zig | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/config/edit.zig b/src/config/edit.zig index 41119c7e19..2131235313 100644 --- a/src/config/edit.zig +++ b/src/config/edit.zig @@ -47,17 +47,24 @@ pub fn openPath(alloc_gpa: Allocator) ![:0]const u8 { dir.close(global.io()); } - // Try to create file and go on if it already exists - _ = std.Io.Dir.createFileAbsolute( + // Try to create file and go on if it already exists. The handle is + // closed immediately: all this call needs is for the file to exist. + // + // Holding it open costs nothing on POSIX but is fatal on Windows, + // where the create denies read sharing for as long as the handle + // lives. Every later read of the config in the same process then + // fails with a sharing violation on a file no other process is + // touching, which on a first run is every read there is. + if (std.Io.Dir.createFileAbsolute( global.io(), config_path.name, .{ .exclusive = true }, - ) catch |err| { - switch (err) { - error.PathAlreadyExists => {}, - else => return err, - } - }; + )) |file| { + file.close(global.io()); + } else |err| switch (err) { + error.PathAlreadyExists => {}, + else => return err, + } } return try alloc_gpa.dupeZ(u8, config_path.name); From 562f4c4d759bb34ab7bf2b1cb3e6d92d0b947881 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 26 Aug 2026 12:17:23 +0300 Subject: [PATCH 2/7] config: use a built-in light/dark pair when no theme is set Upstream ships no default theme: with an empty config the terminal comes up on the compile-time colours, and the surrounding chrome is left to guess. That is fine when the terminal is the whole window. It is not fine here, where the terminal sits inside chrome that has to agree with it, and a "zero configuration" install is the common case rather than the exception. wintty_theme.zig holds the pair, derived from the logo and checked by wintty_theme_test.zig: every slot the terminal actually renders text in clears WCAG AA against its background, with palette 0 exempted since it is the shadow colour and only has to stay distinguishable. It is applied through the same overlay a user theme file goes through, so precedence is unchanged: an explicit theme wins, an explicit setting wins over the theme, and nothing at all now lands on the pair instead of on the compile-time defaults. window-theme moves from auto to system at the same time, since auto derives the window theme from the background, which now follows the desktop, and would fight it. ghostty_config_set_color_scheme lets the host say which half to take before finalize. ghostty_config_builtin_theme hands the same text out so the host can read colours it needs before a surface exists. --- include/ghostty.h | 3 + src/config/CApi.zig | 39 ++++++++ src/config/Config.zig | 124 ++++++++++++++++++++--- src/config/wintty_theme.zig | 99 ++++++++++++++++++ src/config/wintty_theme_test.zig | 166 +++++++++++++++++++++++++++++++ 5 files changed, 419 insertions(+), 12 deletions(-) create mode 100644 src/config/wintty_theme.zig create mode 100644 src/config/wintty_theme_test.zig diff --git a/include/ghostty.h b/include/ghostty.h index 700002673f..f0dad62ba0 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -1217,6 +1217,9 @@ GHOSTTY_API void ghostty_config_load_cli_args(ghostty_config_t); GHOSTTY_API void ghostty_config_load_file(ghostty_config_t, const char*); GHOSTTY_API void ghostty_config_load_default_files(ghostty_config_t); GHOSTTY_API void ghostty_config_load_recursive_files(ghostty_config_t); +GHOSTTY_API void ghostty_config_set_color_scheme(ghostty_config_t, + ghostty_color_scheme_e); +GHOSTTY_API ghostty_string_s ghostty_config_builtin_theme(ghostty_color_scheme_e); GHOSTTY_API void ghostty_config_finalize(ghostty_config_t); GHOSTTY_API bool ghostty_config_get(ghostty_config_t, void*, const char*, uintptr_t); GHOSTTY_API ghostty_input_trigger_s ghostty_config_trigger(ghostty_config_t, diff --git a/src/config/CApi.zig b/src/config/CApi.zig index a57745e1cd..f31d96734c 100644 --- a/src/config/CApi.zig +++ b/src/config/CApi.zig @@ -1,5 +1,6 @@ const builtin = @import("builtin"); const std = @import("std"); +const apprt = @import("../apprt.zig"); const inputpkg = @import("../input.zig"); const global = @import("../global.zig"); const String = @import("../main_c.zig").String; @@ -8,6 +9,7 @@ const Config = @import("Config.zig"); const c_get = @import("c_get.zig"); const edit = @import("edit.zig"); const Key = @import("key.zig").Key; +const wintty_theme = @import("wintty_theme.zig"); const log = std.log.scoped(.config); @@ -84,6 +86,43 @@ export fn ghostty_config_load_recursive_files(self: *Config) void { }; } +/// Set the desktop colour scheme this config resolves against, for the +/// conditional `theme = light:...,dark:...` form and for the built-in +/// theme pair. Must be called before ghostty_config_finalize to have any +/// effect, since that is where the theme is applied. +/// +/// Without this an embedder holding its own config handle resolves every +/// conditional against the default scheme (light) no matter what the +/// desktop is set to, and reads back colours the terminal never renders. +export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { + const scheme = std.enums.fromInt(apprt.ColorScheme, scheme_raw) orelse { + log.warn("invalid color scheme value={}", .{scheme_raw}); + return; + }; + self._conditional_state.theme = switch (scheme) { + .light => .light, + .dark => .dark, + }; +} + +/// The built-in theme applied when no theme is configured, in config file +/// syntax, or an empty string on a build that has no built-in theme. +/// +/// Exists so an embedder drawing chrome around the terminal can resolve the +/// same colours the terminal resolved without a second copy of the palette +/// to keep in step. Points at static storage; the caller must not free it. +export fn ghostty_config_builtin_theme(scheme_raw: c_int) String { + if (comptime !wintty_theme.enabled) return .empty; + const scheme = std.enums.fromInt(apprt.ColorScheme, scheme_raw) orelse { + log.warn("invalid color scheme value={}", .{scheme_raw}); + return .empty; + }; + return .fromSlice(wintty_theme.forScheme(switch (scheme) { + .light => .light, + .dark => .dark, + })); +} + export fn ghostty_config_finalize(self: *Config) void { self.finalize() catch |err| { log.err("error finalizing config err={}", .{err}); diff --git a/src/config/Config.zig b/src/config/Config.zig index fe07b2a47b..51c6aed845 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -28,6 +28,7 @@ const Conditional = conditional.Conditional; const file_load = @import("file_load.zig"); const formatterpkg = @import("formatter.zig"); const themepkg = @import("theme.zig"); +const wintty_theme = @import("wintty_theme.zig"); const url = @import("url.zig"); pub const Key = @import("key.zig").Key; const MetricModifier = fontpkg.Metrics.Modifier; @@ -4695,6 +4696,38 @@ fn loadTheme(self: *Config, theme: Theme) !void { const file = themefile.file; defer file.close(global.io()); + var buf: [2048]u8 = undefined; + var file_reader = file.reader(global.io(), &buf); + var iter: cli.args.LineIterator = .{ + .r = &file_reader.interface, + .filepath = path, + }; + try self.applyThemeOverlay(&iter); +} + +/// Load the built-in Wintty theme for the current conditional theme state. +/// +/// Goes through the same overlay as a user theme file, so the user's own +/// config still overrides it and the replay steps are made conditional the +/// same way. See `wintty_theme.zig` for why a default pair exists at all. +fn loadBuiltinTheme(self: *Config) !void { + var reader: std.Io.Reader = .fixed( + wintty_theme.forScheme(self._conditional_state.theme), + ); + var iter: cli.args.LineIterator = .{ + .r = &reader, + .filepath = "", + }; + try self.applyThemeOverlay(&iter); +} + +/// Load a theme from `iter` underneath the config already loaded into self. +/// +/// Split out of loadTheme so the built-in theme, which is a string rather +/// than a file, gets byte-for-byte the same precedence and replay handling. +/// Warning: this deinits self and replaces it, so anything borrowed from +/// self before the call is freed after it. +fn applyThemeOverlay(self: *Config, iter: *cli.args.LineIterator) !void { // From this point onwards, we load the theme and do a bit of a dance // to achieve two separate goals: // @@ -4714,11 +4747,7 @@ fn loadTheme(self: *Config, theme: Theme) !void { errdefer new_config.deinit(); // Load our theme - var buf: [2048]u8 = undefined; - var file_reader = file.reader(global.io(), &buf); - const reader = &file_reader.interface; - var iter: cli.args.LineIterator = .{ .r = reader, .filepath = path }; - try new_config.loadIter(alloc_gpa, &iter); + try new_config.loadIter(alloc_gpa, iter); // Setup our replay to be conditional. conditional: for (new_config._replay_steps.items) |*item| { @@ -4800,6 +4829,18 @@ pub fn finalize(self: *Config) !void { // Mark that we use a conditional theme self._conditional_set.insert(.theme); } + } else if (comptime wintty_theme.enabled) { + // No theme configured, so fall back to the built-in pair rather + // than to the compile-time colour defaults. See wintty_theme.zig + // for why this fork has a default theme where upstream has none. + try self.loadBuiltinTheme(); + + // Same reasoning as the different-light-and-dark branch above: + // auto derives the window theme from the terminal background, which + // now moves with the desktop, so it would fight the desktop instead + // of following it. + if (self.@"window-theme" == .auto) self.@"window-theme" = .system; + self._conditional_set.insert(.theme); } // Used for a variety of defaults. See the function docs as well the @@ -11945,26 +11986,85 @@ test "issue 228: non-empty foreground still overrides theme" { }, cfg.foreground); } -test "issue 228: empty foreground with no theme stays compile-time default" { +test "issue 228: empty foreground with no theme defers to the layer below" { const testing = std.testing; const alloc = testing.allocator; var cfg = try Config.default(alloc); defer cfg.deinit(); - // With no theme there is no lower layer to defer to, so an empty value - // keeps the compile-time default (#FFFFFF). This guards against the skip - // logic leaking into the non-theme path. + // An empty value defers to whatever is underneath it. What that is + // depends on the build: on a build with a built-in theme the theme is + // the layer below, and everywhere else there is no layer at all and the + // compile-time default stands. Either way this guards the same thing as + // before, that the skip logic does not turn an empty value into + // something other than the layer below it. + const expected: Color = if (comptime wintty_theme.enabled) + // The light half, since that is Config.default's conditional state. + .{ .r = 0x1E, .g = 0x23, .b = 0x33 } + else + .{ .r = 0xFF, .g = 0xFF, .b = 0xFF }; + var it: TestIterator = .{ .data = &.{ "--foreground=", } }; try cfg.loadIter(alloc, &it); try cfg.finalize(); + try testing.expectEqual(expected, cfg.foreground); +} + +test "built-in theme applies when nothing is configured" { + if (comptime !wintty_theme.enabled) return error.SkipZigTest; + + const testing = std.testing; + const alloc = testing.allocator; + + for ([_]struct { theme: conditional.State.Theme, bg: Color }{ + .{ .theme = .light, .bg = .{ .r = 0xF4, .g = 0xF6, .b = 0xFB } }, + .{ .theme = .dark, .bg = .{ .r = 0x13, .g = 0x16, .b = 0x20 } }, + }) |tc| { + var cfg = try Config.default(alloc); + defer cfg.deinit(); + cfg._conditional_state.theme = tc.theme; + try cfg.finalize(); + + try testing.expectEqual(tc.bg, cfg.background); + + // window-theme must stop deriving itself from the background, which + // now moves with the desktop, and follow the desktop directly. + try testing.expectEqual(WindowTheme.system, cfg.@"window-theme"); + + // Registered as conditional, or an OS light/dark flip would not + // rebuild the config and the pair would never switch. + try testing.expect(cfg._conditional_set.contains(.theme)); + } +} + +test "built-in theme loses to an explicit setting" { + if (comptime !wintty_theme.enabled) return error.SkipZigTest; + + const testing = std.testing; + const alloc = testing.allocator; + + var cfg = try Config.default(alloc); + defer cfg.deinit(); + var it: TestIterator = .{ .data = &.{"--background=#abcdef"} }; + try cfg.loadIter(alloc, &it); + try cfg.finalize(); + try testing.expectEqual(Color{ - .r = 0xFF, - .g = 0xFF, - .b = 0xFF, + .r = 0xAB, + .g = 0xCD, + .b = 0xEF, + }, cfg.background); + + // Only the key the user set is theirs; the rest still comes from the + // built-in theme. + try testing.expectEqual(Color{ + .r = 0x1E, + .g = 0x23, + .b = 0x33, }, cfg.foreground); } diff --git a/src/config/wintty_theme.zig b/src/config/wintty_theme.zig new file mode 100644 index 0000000000..aad52edf0c --- /dev/null +++ b/src/config/wintty_theme.zig @@ -0,0 +1,99 @@ +//! The built-in light/dark theme pair Wintty falls back to when the user +//! has not configured a theme. +//! +//! Upstream Ghostty has no default theme: an unconfigured install is always +//! dark (`#282c34` on the Tomorrow Night palette) whatever the desktop +//! around it is set to. That is a reasonable default on macOS and Linux, +//! where Ghostty is usually installed deliberately by someone who will go +//! on to configure it. On Windows it reads as a bug, because the terminal +//! is frequently the first thing launched on a fresh machine and it lands +//! beside a light-themed shell. +//! +//! So Wintty ships a pair instead, selected from the conditional theme +//! state the app feeds in from the OS. Both halves are applied through the +//! same overlay path a user theme file uses, so anything set in the user's +//! own config still wins. +//! +//! The colours are taken from the application icon: the electric blue of +//! the ghost's glow is the accent, the ghost's own silver is the dark-mode +//! foreground, and the near-black indigo of the icon's corners is the +//! dark-mode field. Every colour here clears WCAG AA (4.5:1) against its +//! background except palette slot 0, which is the "black" slot and is +//! deliberately close to the background: programs use it as a fill, not as +//! text. `wintty_theme_test.zig` asserts that property so a future palette +//! tweak cannot quietly regress it. + +const std = @import("std"); +const builtin = @import("builtin"); +const conditional = @import("conditional.zig"); + +/// Whether an unconfigured install gets the built-in pair. +/// +/// Windows only: the macOS and Linux builds share this source tree and are +/// expected to behave like upstream Ghostty, whose unconfigured default is +/// the compile-time colours in Config.zig. +pub const enabled = builtin.os.tag == .windows; + +/// The theme source for a given desktop colour scheme, in Ghostty config +/// syntax. Parsed by the same iterator that reads a theme file, so any +/// valid config key is allowed here. +pub fn forScheme(theme: conditional.State.Theme) []const u8 { + return switch (theme) { + .light => light, + .dark => dark, + }; +} + +pub const dark: []const u8 = + \\background = #131620 + \\foreground = #d5d9e5 + \\cursor-color = #4babef + \\selection-background = #2b3350 + \\selection-foreground = #f2f4fa + \\palette = 0=#2a2f3d + \\palette = 1=#f0787f + \\palette = 2=#7fd69b + \\palette = 3=#edc77a + \\palette = 4=#4babef + \\palette = 5=#b98cf0 + \\palette = 6=#5bd5e8 + \\palette = 7=#d5d9e5 + \\palette = 8=#7a8296 + \\palette = 9=#ff9aa0 + \\palette = 10=#9ce6b4 + \\palette = 11=#ffd99a + \\palette = 12=#7bc5ff + \\palette = 13=#d3abff + \\palette = 14=#8ae7f5 + \\palette = 15=#f2f4fa + \\ +; + +pub const light: []const u8 = + \\background = #f4f6fb + \\foreground = #1e2333 + \\cursor-color = #1668c4 + \\selection-background = #cfe0f5 + \\selection-foreground = #141828 + \\palette = 0=#1e2333 + \\palette = 1=#c0334a + \\palette = 2=#1f7a4d + \\palette = 3=#8a6410 + \\palette = 4=#1668c4 + \\palette = 5=#7a3fbf + \\palette = 6=#0f6e80 + \\palette = 7=#4a5265 + \\palette = 8=#666e81 + \\palette = 9=#a82a3e + \\palette = 10=#186540 + \\palette = 11=#73530c + \\palette = 12=#0f55a6 + \\palette = 13=#65329f + \\palette = 14=#0b5a69 + \\palette = 15=#1e2333 + \\ +; + +test { + _ = @import("wintty_theme_test.zig"); +} diff --git a/src/config/wintty_theme_test.zig b/src/config/wintty_theme_test.zig new file mode 100644 index 0000000000..75044d9c2b --- /dev/null +++ b/src/config/wintty_theme_test.zig @@ -0,0 +1,166 @@ +//! Contrast guarantees for the built-in theme pair in `wintty_theme.zig`. +//! +//! These parse the theme source the same way a reader would see it, so they +//! also catch a malformed line, and then hold every colour to a WCAG ratio +//! against its own background. The point is that "the default theme is +//! legible" is a property the next person to retouch the palette has to +//! keep, not a thing that was true once. + +const std = @import("std"); +const testing = std.testing; +const wintty_theme = @import("wintty_theme.zig"); + +/// WCAG 2.x relative luminance of an sRGB colour. +fn luminance(rgb: [3]u8) f64 { + var channels: [3]f64 = undefined; + for (rgb, &channels) |raw, *out| { + const c = @as(f64, @floatFromInt(raw)) / 255.0; + out.* = if (c <= 0.03928) + c / 12.92 + else + std.math.pow(f64, (c + 0.055) / 1.055, 2.4); + } + return 0.2126 * channels[0] + 0.7152 * channels[1] + 0.0722 * channels[2]; +} + +fn contrast(a: [3]u8, b: [3]u8) f64 { + const la = luminance(a); + const lb = luminance(b); + return (@max(la, lb) + 0.05) / (@min(la, lb) + 0.05); +} + +fn parseHex(s: []const u8) ![3]u8 { + const body = if (s.len > 0 and s[0] == '#') s[1..] else s; + if (body.len != 6) return error.BadHexLength; + return .{ + try std.fmt.parseInt(u8, body[0..2], 16), + try std.fmt.parseInt(u8, body[2..4], 16), + try std.fmt.parseInt(u8, body[4..6], 16), + }; +} + +const Parsed = struct { + background: [3]u8 = undefined, + foreground: [3]u8 = undefined, + cursor: [3]u8 = undefined, + selection_background: [3]u8 = undefined, + selection_foreground: [3]u8 = undefined, + palette: [16][3]u8 = undefined, + palette_seen: [16]bool = @splat(false), +}; + +/// Minimal reader for the subset of config syntax the theme source uses. +/// Deliberately strict: an unrecognised key is an error rather than a +/// silent skip, so a typo in the theme fails the test instead of leaving +/// a colour at its compile-time default. +fn parse(source: []const u8) !Parsed { + var out: Parsed = .{}; + var seen_background = false; + + var lines = std.mem.tokenizeScalar(u8, source, '\n'); + while (lines.next()) |raw| { + const line = std.mem.trim(u8, raw, " \r\t"); + if (line.len == 0) continue; + + const eq = std.mem.indexOfScalar(u8, line, '=') orelse + return error.MissingEquals; + const key = std.mem.trim(u8, line[0..eq], " "); + const value = std.mem.trim(u8, line[eq + 1 ..], " "); + + if (std.mem.eql(u8, key, "background")) { + out.background = try parseHex(value); + seen_background = true; + } else if (std.mem.eql(u8, key, "foreground")) { + out.foreground = try parseHex(value); + } else if (std.mem.eql(u8, key, "cursor-color")) { + out.cursor = try parseHex(value); + } else if (std.mem.eql(u8, key, "selection-background")) { + out.selection_background = try parseHex(value); + } else if (std.mem.eql(u8, key, "selection-foreground")) { + out.selection_foreground = try parseHex(value); + } else if (std.mem.eql(u8, key, "palette")) { + const inner = std.mem.indexOfScalar(u8, value, '=') orelse + return error.MissingPaletteIndex; + const idx = try std.fmt.parseInt(u8, value[0..inner], 10); + if (idx >= 16) return error.PaletteIndexOutOfRange; + out.palette[idx] = try parseHex(value[inner + 1 ..]); + out.palette_seen[idx] = true; + } else { + return error.UnknownKey; + } + } + + if (!seen_background) return error.MissingBackground; + for (out.palette_seen) |seen| if (!seen) return error.IncompletePalette; + return out; +} + +/// WCAG AA for body text. Everything a program can put on screen as text +/// has to clear this against the theme's own background. +const aa_text = 4.5; + +fn expectAtLeast(actual: f64, minimum: f64) !void { + if (actual >= minimum) return; + std.debug.print( + "contrast {d:.2} is below the required {d:.2}\n", + .{ actual, minimum }, + ); + return error.InsufficientContrast; +} + +fn checkTheme(source: []const u8) !void { + const t = try parse(source); + + try expectAtLeast(contrast(t.background, t.foreground), aa_text); + try expectAtLeast(contrast(t.background, t.cursor), aa_text); + try expectAtLeast( + contrast(t.selection_background, t.selection_foreground), + aa_text, + ); + + // Slot 0 is the "black" slot. Programs use it as a fill behind other + // colours rather than as text, and on a dark theme it sits close to the + // background by convention, so it cannot be held to the text rule. It + // still has to be told apart from the background, which is the failure + // that would actually matter: a slot 0 equal to the background makes + // anything drawn in it disappear. + for (t.palette, 0..) |color, i| { + if (i == 0) { + try testing.expect(contrast(t.background, color) > 1.2); + continue; + } + expectAtLeast(contrast(t.background, color), aa_text) catch |err| { + std.debug.print("palette slot {d} failed\n", .{i}); + return err; + }; + } +} + +test "built-in dark theme is legible" { + try checkTheme(wintty_theme.dark); +} + +test "built-in light theme is legible" { + try checkTheme(wintty_theme.light); +} + +test "the two halves actually differ in polarity" { + const d = try parse(wintty_theme.dark); + const l = try parse(wintty_theme.light); + + // A pair whose halves are both dark would pass every contrast test + // above and still defeat the entire point of having a pair. + try testing.expect(luminance(d.background) < 0.1); + try testing.expect(luminance(l.background) > 0.7); +} + +test "forScheme selects the matching half" { + try testing.expectEqualStrings( + wintty_theme.dark, + wintty_theme.forScheme(.dark), + ); + try testing.expectEqualStrings( + wintty_theme.light, + wintty_theme.forScheme(.light), + ); +} From f727d44b7c615b90721cdd7c2f70577dfd179c71 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 26 Aug 2026 12:17:52 +0300 Subject: [PATCH 3/7] windows: follow the desktop light/dark setting with no config The shell now tells the config which half of the built-in pair to take, before finalize, and re-tells it on reload. With nothing configured the terminal, the chrome and the splash all come from the same source and move together when the desktop setting changes. What this fixes, measured on a light desktop with an empty config: window chrome glyphs 1.87:1 -> 6.56:1 and up splash ink a fixed step in linear luminance -> a fixed step in L*, so it reads the same in both halves The chrome number came from Mica tinting off the desktop while the window element theme was derived from palette luminance. With window-theme now system, both sides read the desktop and the split is gone. ThemeResolution gains StepLightness, which walks CIE L* rather than multiplying luminance, so "one step lighter" means the same thing on a light background as on a dark one. LaunchTexture uses it for the splash ink; the old fixed contrast ratio produced a visible step in one half and a nearly invisible one in the other. ConfigService's ReadFlags is now guarded: it ran unguarded in the constructor, so any throw there took the app down before it drew, which is how the first-run file sharing violation presented. Its palette fallbacks also move to libghostty's own defaults, since the previous ones were a different theme's colours and only showed up when everything else failed. IThemeProvider loses the resolved colours and the font. Nothing read them and nothing refreshed them, so they sat at fixed values regardless of config, waiting for a first caller to trust them. --- windows/Ghostty.Core/Config/ConfigIniFile.cs | 21 +++- windows/Ghostty.Core/Config/IThemeProvider.cs | 20 ++-- windows/Ghostty.Core/Shell/LaunchTexture.cs | 33 +++--- .../Ghostty.Core/Windows/ThemeResolution.cs | 75 +++++++++++++ .../Ghostty.Tests/Shell/LaunchTextureTests.cs | 66 +++++++++-- windows/Ghostty/Interop/NativeMethods.cs | 9 ++ windows/Ghostty/Services/ConfigService.cs | 103 +++++++++++++++--- windows/Ghostty/Services/ThemeProvider.cs | 7 -- windows/Ghostty/Settings/WindowState.cs | 9 ++ windows/Ghostty/Shell/SplashWindow.cs | 49 ++++++++- 10 files changed, 330 insertions(+), 62 deletions(-) diff --git a/windows/Ghostty.Core/Config/ConfigIniFile.cs b/windows/Ghostty.Core/Config/ConfigIniFile.cs index a6aab3bada..3b5ca1ffb8 100644 --- a/windows/Ghostty.Core/Config/ConfigIniFile.cs +++ b/windows/Ghostty.Core/Config/ConfigIniFile.cs @@ -33,11 +33,26 @@ public static class ConfigIniFile /// public static Dictionary> Load(string? path) { - var dict = new Dictionary>(StringComparer.OrdinalIgnoreCase); if (string.IsNullOrEmpty(path) || !File.Exists(path)) - return dict; + return new Dictionary>(StringComparer.OrdinalIgnoreCase); + + return Parse(File.ReadLines(path)); + } + + /// + /// Parse ini text that is already in memory, by the same rules as + /// . Used for the built-in theme libghostty hands back + /// as a string rather than a file. + /// + public static Dictionary> ParseText(string? text) + => string.IsNullOrEmpty(text) + ? new Dictionary>(StringComparer.OrdinalIgnoreCase) + : Parse(text.Split('\n')); - foreach (var line in File.ReadLines(path)) + private static Dictionary> Parse(IEnumerable lines) + { + var dict = new Dictionary>(StringComparer.OrdinalIgnoreCase); + foreach (var line in lines) { var trimmed = line.TrimStart(); if (trimmed.Length == 0 || trimmed.StartsWith('#')) continue; diff --git a/windows/Ghostty.Core/Config/IThemeProvider.cs b/windows/Ghostty.Core/Config/IThemeProvider.cs index 21cc667692..bb5f2be39c 100644 --- a/windows/Ghostty.Core/Config/IThemeProvider.cs +++ b/windows/Ghostty.Core/Config/IThemeProvider.cs @@ -3,20 +3,18 @@ namespace Ghostty.Core.Config; /// -/// Provides resolved theme values from config. Colors are -/// represented as uint (ARGB) to avoid WinUI dependencies -/// in Ghostty.Core. +/// Enumerates the theme files available to the user. /// +/// +/// This used to also carry resolved background/foreground/cursor/selection +/// colours and the font. Nothing read them and nothing refreshed them, so +/// they sat at fixed Catppuccin values regardless of config, waiting for +/// the first caller to trust them. Resolved colours come from +/// , which is the side that actually tracks +/// the config and the OS colour scheme. +/// public interface IThemeProvider { - uint BackgroundColor { get; } - uint ForegroundColor { get; } - uint CursorColor { get; } - uint SelectionColor { get; } - string? FontFamily { get; } - double FontSize { get; } - string? ThemeName { get; } - /// Available theme names (bundled + user). IReadOnlyList AvailableThemes { get; } } diff --git a/windows/Ghostty.Core/Shell/LaunchTexture.cs b/windows/Ghostty.Core/Shell/LaunchTexture.cs index e8dd9dcac5..05436bf786 100644 --- a/windows/Ghostty.Core/Shell/LaunchTexture.cs +++ b/windows/Ghostty.Core/Shell/LaunchTexture.cs @@ -27,13 +27,18 @@ namespace Ghostty.Core.Shell; public static class LaunchTexture { /// - /// How far the ink sits from the background, per channel. This is the - /// dial for how visible the texture is. + /// How far the ink sits from the background, in CIE L* units. This is + /// the dial for how visible the texture is. /// /// - /// A fixed step rather than a fraction of the remaining headroom. - /// A fraction lands very differently on a near-black background than on - /// a mid-grey one, and the point is that it reads the same on both. + /// Measured perceptually rather than as a step per channel, which + /// is what this used to be. A fixed channel step is not a fixed amount + /// of visible difference: sRGB is gamma encoded, so the same ten counts + /// buy far more separation down at the black end than up at the white + /// end. Off a near-black background it came to dL* 5.0 and off a + /// near-white one to dL* 3.5, so the same texture that read as a faint + /// grain in dark mode all but vanished in light mode. Holding L* + /// constant is what actually makes it read the same on both. /// /// The sheet does not spend all of this. It is a mask that tops /// out below full, so the strongest mark on screen lands at roughly @@ -41,7 +46,8 @@ public static class LaunchTexture /// visible; the sheet needs no regenerating for it, since the tint is /// applied at draw time. /// - public const int Contrast = 10; + public const double ContrastLStar = 5.0; + /// /// The narrowest crop, as a fraction of the sheet's shorter edge. @@ -141,16 +147,15 @@ public static uint ResolveInkRgb(uint backgroundRgb) var g = (int)((backgroundRgb >> 8) & 0xFF); var b = (int)(backgroundRgb & 0xFF); - // Rec. 601 luma. Which side of mid the background falls on is the - // only question being asked, so the cheap weighting is enough: a - // gamma-correct one would only move the answer on colours where - // either direction reads about the same anyway. + // Which way to step. Rec. 601 luma is enough for the direction: the + // only question is which side of mid the background falls on, and a + // gamma-correct weighting would only move the answer on colours + // where either direction reads about the same anyway. var luma = ((299 * r) + (587 * g) + (114 * b)) / 1000; - var step = luma < 128 ? Contrast : -Contrast; - - return (uint)((Channel(r + step) << 16) | (Channel(g + step) << 8) | Channel(b + step)); - static int Channel(int value) => value < 0 ? 0 : value > 255 ? 255 : value; + return Ghostty.Core.Windows.ThemeResolution.StepLightness( + backgroundRgb & 0x00FFFFFFu, + luma < 128 ? ContrastLStar : -ContrastLStar); } /// diff --git a/windows/Ghostty.Core/Windows/ThemeResolution.cs b/windows/Ghostty.Core/Windows/ThemeResolution.cs index 013968d065..0128325054 100644 --- a/windows/Ghostty.Core/Windows/ThemeResolution.cs +++ b/windows/Ghostty.Core/Windows/ThemeResolution.cs @@ -132,6 +132,81 @@ public static double ContrastRatio(uint a, uint b) return (hi + 0.05) / (lo + 0.05); } + /// + /// CIE L* for a relative luminance: 0 for black, 100 for white, spaced + /// so that equal differences look equal. + /// + private static double Lightness(double luminance) + => luminance > 0.008856 + ? (116.0 * Math.Cbrt(luminance)) - 16.0 + : 903.3 * luminance; + + /// + /// A tint of that sits + /// away from it perceptually: positive for lighter, negative for darker. + /// Both argument and result are packed 0x00RRGGBB. + /// + /// + /// Perceptual rather than a step per channel, because those are not + /// the same thing. sRGB is gamma encoded, so a fixed number of counts + /// buys markedly more visible separation down at the black end than up + /// at the white end. Anything tuned to look right on a dark background + /// and then reused on a light one comes out weaker than intended, which + /// is how a texture calibrated in dark mode ends up nearly invisible in + /// light mode. + /// + /// Every channel moves by the same number of counts, so the result + /// stays a tint of the input rather than becoming a colour of its own. + /// Near either end of the range the target is unreachable and the result + /// is the closest step available, but never the input itself: a tint + /// equal to its background draws nothing. + /// + public static uint StepLightness(uint rgb, double deltaLStar) + { + const int maxChannelStep = 48; + + var r = (int)((rgb >> 16) & 0xFF); + var g = (int)((rgb >> 8) & 0xFF); + var b = (int)(rgb & 0xFF); + + var direction = deltaLStar >= 0 ? 1 : -1; + var target = Lightness(LuminanceOf(r, g, b)) + deltaLStar; + + // Walk out a count at a time and stop on the first step that has + // covered the distance. Lightness is monotonic in the step, so the + // first hit is the closest one at or past the target, and the range + // is small enough that searching it beats solving it. + var step = maxChannelStep; + for (var candidate = 1; candidate <= maxChannelStep; candidate++) + { + var offset = direction * candidate; + var lightness = Lightness(LuminanceOf( + Clamp(r + offset), Clamp(g + offset), Clamp(b + offset))); + + if (direction > 0 ? lightness >= target : lightness <= target) + { + step = candidate; + break; + } + } + + step *= direction; + return (uint)((Clamp(r + step) << 16) | (Clamp(g + step) << 8) | Clamp(b + step)); + + static int Clamp(int value) => value < 0 ? 0 : value > 255 ? 255 : value; + } + + private static double LuminanceOf(int r, int g, int b) + { + static double Linearize(int channel) + { + var c = channel / 255.0; + return c <= 0.03928 ? c / 12.92 : Math.Pow((c + 0.055) / 1.055, 2.4); + } + + return (0.2126 * Linearize(r)) + (0.7152 * Linearize(g)) + (0.0722 * Linearize(b)); + } + /// /// Pick a legible foreground for text drawn over /// . Keeps when it diff --git a/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs b/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs index df87c8b4bc..3d01b655a6 100644 --- a/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs +++ b/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs @@ -277,25 +277,77 @@ public void The_whole_turn_range_gets_used() [Theory] [InlineData(0x000000u, true)] // black has only one direction to go [InlineData(0xFFFFFFu, false)] // and so does white - [InlineData(0x1E1E2Eu, true)] // the default dark background + [InlineData(0x131620u, true)] // the built-in dark background + [InlineData(0xF4F6FBu, false)] // the built-in light background [InlineData(0x808080u, false)] // mid grey, just past the luma split public void Ink_steps_away_from_the_background(uint background, bool expectLighter) { - // Derived from the constant rather than written out, because the - // constant is a dial: it gets turned whenever the texture reads as - // too faint or too loud, and a test that hard-codes the answer turns - // every such adjustment into a failing test with nothing wrong. - var step = expectLighter ? LaunchTexture.Contrast : -LaunchTexture.Contrast; var ink = LaunchTexture.ResolveInkRgb(background); foreach (var shift in new[] { 16, 8, 0 }) { var before = (int)((background >> shift) & 0xFF); var after = (int)((ink >> shift) & 0xFF); - Assert.Equal(Math.Clamp(before + step, 0, 255), after); + if (expectLighter) Assert.True(after >= before); + else Assert.True(after <= before); } } + [Theory] + [InlineData(0x131620u)] // the built-in dark background + [InlineData(0xF4F6FBu)] // the built-in light background + [InlineData(0x282C34u)] // libghostty's compile-time default + [InlineData(0x808080u)] + [InlineData(0x404040u)] + [InlineData(0xE0E0E0u)] + public void Ink_sits_the_same_perceptual_distance_from_any_background(uint background) + { + // The whole point of the L* solve. A per-channel step gave dL* 5.0 + // off the dark background and 3.5 off the light one, so the texture + // that read as a grain in dark mode was nearly gone in light mode. + // + // Half a unit of slack: the step is a whole number of counts, so it + // lands on the first count at or past the target rather than exactly + // on it. Compared against the constant rather than a written-out + // number, because the constant is a dial and turning it must not + // fail a test with nothing wrong. + var delta = Math.Abs( + LStar(LaunchTexture.ResolveInkRgb(background)) - LStar(background)); + + Assert.InRange( + delta, + LaunchTexture.ContrastLStar - 0.5, + LaunchTexture.ContrastLStar + 0.5); + } + + [Theory] + [InlineData(0x000000u)] // nothing below to step down to + [InlineData(0xFFFFFFu)] // nothing above to step up to + [InlineData(0x020202u)] + [InlineData(0xFDFDFDu)] + public void Ink_at_the_ends_still_differs_from_the_background(uint background) + { + // The solve cannot reach its target from here, and must not answer + // with the background itself: an ink equal to the background draws + // nothing at all. + Assert.NotEqual(background, LaunchTexture.ResolveInkRgb(background)); + } + + private static double LStar(uint rgb) + { + static double Linearize(uint channel) + { + var c = channel / 255.0; + return c <= 0.03928 ? c / 12.92 : Math.Pow((c + 0.055) / 1.055, 2.4); + } + + var y = (0.2126 * Linearize((rgb >> 16) & 0xFF)) + + (0.7152 * Linearize((rgb >> 8) & 0xFF)) + + (0.0722 * Linearize(rgb & 0xFF)); + + return y > 0.008856 ? (116.0 * Math.Cbrt(y)) - 16.0 : 903.3 * y; + } + [Fact] public void Ink_stays_within_a_channel() { diff --git a/windows/Ghostty/Interop/NativeMethods.cs b/windows/Ghostty/Interop/NativeMethods.cs index b48c1bf9f6..c4e1c53d8f 100644 --- a/windows/Ghostty/Interop/NativeMethods.cs +++ b/windows/Ghostty/Interop/NativeMethods.cs @@ -357,6 +357,15 @@ internal static int InitWideFromProcess() [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] internal static partial void ConfigLoadDefaultFiles(GhosttyConfig config); + [LibraryImport(Dll, EntryPoint = "ghostty_config_set_color_scheme")] + [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] + internal static partial void ConfigSetColorScheme( + GhosttyConfig config, GhosttyColorScheme scheme); + + [LibraryImport(Dll, EntryPoint = "ghostty_config_builtin_theme")] + [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] + internal static partial GhosttyString ConfigBuiltinTheme(GhosttyColorScheme scheme); + [LibraryImport(Dll, EntryPoint = "ghostty_config_finalize")] [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] internal static partial void ConfigFinalize(GhosttyConfig config); diff --git a/windows/Ghostty/Services/ConfigService.cs b/windows/Ghostty/Services/ConfigService.cs index 4504b10140..04caa12582 100644 --- a/windows/Ghostty/Services/ConfigService.cs +++ b/windows/Ghostty/Services/ConfigService.cs @@ -94,7 +94,10 @@ internal sealed partial class ConfigService : IConfigService, Ghostty.Core.Profi public Ghostty.Core.Hosting.WindowSaveState WindowSaveState { get; private set; } = Ghostty.Core.Hosting.WindowSaveState.Default; public uint ForegroundColor { get; private set; } = 0x00FFFFFF; - public uint BackgroundColor { get; private set; } = 0x001E1E2E; + // libghostty's own compile-time default. Held only between construction + // and the first ReadFlags; anything else here reads as a terminal + // background that no terminal is painted with. + public uint BackgroundColor { get; private set; } = 0x00282C34; public uint? CursorColor { get; private set; } public uint? CursorTextColor { get; private set; } // Explicit chrome accent. Null when the user hasn't set accent-color; @@ -347,8 +350,16 @@ public ConfigService(DispatcherQueue dispatcher) "libghostty export that touches global state."); } + var isOsDark = OsTheme.IsDark(); + _config = NativeMethods.ConfigNew(); NativeMethods.ConfigLoadDefaultFiles(_config); + // Before finalize: that is where the theme is applied, and the + // scheme decides which half of a light/dark pair (the user's, or + // the built-in one) gets applied. Without it this handle resolves + // every conditional against light and reports colours the terminal + // is not rendering. + NativeMethods.ConfigSetColorScheme(_config, ToScheme(isOsDark)); NativeMethods.ConfigFinalize(_config); var pathStr = NativeMethods.ConfigOpenPath(); @@ -361,9 +372,31 @@ public ConfigService(DispatcherQueue dispatcher) SeedConfigIfEmpty(); CacheDiagnostics(); - ReadFlags(OsTheme.IsDark()); + + try + { + ReadFlags(isOsDark); + } + catch (Exception ex) + { + // ReadFlags reads files, so it can fail on a config another + // process holds. Reload and RefreshForOsColorScheme already + // treat that as recoverable; this one used to be the exception, + // and an unhandled throw here kills the process inside + // App.OnLaunched with no window and no message. + // + // Every value it would have set has a default, so a failed read + // leaves a usable snapshot rather than a torn one, and the + // first successful reload replaces it wholesale. + StaticLoggers.ConfigService.LogSnapshotRefreshFailed(ex); + } } + private static Ghostty.Core.Interop.GhosttyColorScheme ToScheme(bool isDark) + => isDark + ? Ghostty.Core.Interop.GhosttyColorScheme.Dark + : Ghostty.Core.Interop.GhosttyColorScheme.Light; + /// /// Mac Ghostty seeds a comment header when it creates the config /// file for the first time. On Windows, ghostty_config_open_path() @@ -450,6 +483,7 @@ public bool Reload() if (hcPath is not null) NativeMethods.ConfigLoadFile(newConfig, hcPath); } + NativeMethods.ConfigSetColorScheme(newConfig, ToScheme(OsTheme.IsDark())); NativeMethods.ConfigFinalize(newConfig); } catch (Exception ex) @@ -747,8 +781,20 @@ private void ReadFlags(bool isOsDark) // File.ReadLines calls regardless of how many keys we probe. _configFileCache = LoadIniFile(ConfigFilePath); var activeTheme = ResolveActiveThemeName(isOsDark); - var themePath = string.IsNullOrEmpty(activeTheme) ? null : ResolveThemePath(activeTheme); - _activeThemeFileCache = themePath is null ? null : LoadIniFile(themePath); + // No theme configured is not "no theme": libghostty applies its + // built-in light/dark pair in that case, so the chrome has to + // resolve against the same one or it frames a pane in colours the + // pane is not filled with. Asked for by scheme rather than cached, + // because a flip re-enters here with the other one. + // + // A configured-but-unresolvable theme deliberately does not land + // here: libghostty leaves the compile-time colours in place for + // that, and substituting the built-in pair would drift again. + _activeThemeFileCache = string.IsNullOrEmpty(activeTheme) + ? LoadBuiltinTheme(isOsDark) + : ResolveThemePath(activeTheme) is { } themePath + ? LoadIniFile(themePath) + : null; // Immediately after the assignment it certifies, so the two cannot // disagree. Both failure legs then stay consistent: a throw from @@ -866,12 +912,17 @@ private void ReadFlagsCore() WindowSaveState = Ghostty.Core.Hosting.WindowSaveStateExtensions.Parse( GetString("window-save-state", "default")); - // For background and foreground we go through GetThemeValue first - // because libghostty's _config was finalized with the default - // (.light) conditional state, so for a pair theme in dark mode - // it would return the LIGHT theme's colors. GetThemeValue resolves - // the active theme name (light vs dark) based on OS state. - BackgroundColor = ResolveThemedColor("background", 0x001E1E2E); + // Resolved from the config and theme text rather than through + // ghostty_config_get. The native handle is finalized against the + // scheme that was current when it was built, and an OS light/dark + // flip re-enters here without rebuilding it, so on the far side of + // a flip it still answers for the outgoing scheme. The text path + // takes isOsDark per call and does not have that problem. + // + // The defaults are libghostty's own compile-time colours, reached + // only on a build with no built-in theme; otherwise the built-in + // theme has already supplied both. + BackgroundColor = ResolveThemedColor("background", 0x00282C34); ForegroundColor = ResolveThemedColor("foreground", 0x00FFFFFF); // cursor-color is a TerminalColor (tagged union) in the Zig @@ -1296,6 +1347,24 @@ private IReadOnlyList GetAllFileValues(string key) private static Dictionary> LoadIniFile(string? path) => Ghostty.Core.Config.ConfigIniFile.Load(path); + /// + /// The built-in theme libghostty applies for + /// when nothing is configured, parsed into the same shape a theme file + /// gets. Null when the build has no built-in theme, which leaves the + /// per-key defaults below in charge exactly as before. + /// + private static Dictionary>? LoadBuiltinTheme(bool isOsDark) + { + var str = NativeMethods.ConfigBuiltinTheme(ToScheme(isOsDark)); + if (str.Ptr == IntPtr.Zero || str.Len == 0) return null; + + var text = Marshal.PtrToStringUTF8(str.Ptr, (int)str.Len); + if (string.IsNullOrEmpty(text)) return null; + + // Static storage on the native side, so there is nothing to free. + return Ghostty.Core.Config.ConfigIniFile.ParseText(text); + } + /// /// Read a color config value. libghostty returns colors as /// ghostty_config_color_s { r: u8, g: u8, b: u8 }. @@ -1351,12 +1420,18 @@ private unsafe uint GetColor(string key, uint defaultValue) /// private uint[] GetAllPaletteColors() { + // libghostty's own defaults, from Name.default in + // src/terminal/color.zig -- NOT the xterm primaries. These were + // xterm's, which meant an unconfigured install had the chrome + // deriving from one palette while the terminal rendered another. + // Reached only when neither the built-in theme nor a configured + // one sets an index. uint[] defaults = [ - 0x000000, 0xCC0000, 0x00CC00, 0xCCCC00, - 0x0000CC, 0xCC00CC, 0x00CCCC, 0xCCCCCC, - 0x666666, 0xFF0000, 0x00FF00, 0xFFFF00, - 0x0000FF, 0xFF00FF, 0x00FFFF, 0xFFFFFF, + 0x1D1F21, 0xCC6666, 0xB5BD68, 0xF0C674, + 0x81A2BE, 0xB294BB, 0x8ABEB7, 0xC5C8C6, + 0x666666, 0xD54E53, 0xB9CA4A, 0xE7C547, + 0x7AA6DA, 0xC397D8, 0x70C0B1, 0xEAEAEA, ]; // Apply theme palette first (lower priority). Use the cached diff --git a/windows/Ghostty/Services/ThemeProvider.cs b/windows/Ghostty/Services/ThemeProvider.cs index a194aace85..cb843314f1 100644 --- a/windows/Ghostty/Services/ThemeProvider.cs +++ b/windows/Ghostty/Services/ThemeProvider.cs @@ -10,13 +10,6 @@ internal sealed partial class ThemeProvider : IThemeProvider, IDisposable { private readonly IConfigService _configService; - public uint BackgroundColor { get; private set; } = 0xFF1E1E2E; - public uint ForegroundColor { get; private set; } = 0xFFCDD6F4; - public uint CursorColor { get; private set; } = 0xFFF5E0DC; - public uint SelectionColor { get; private set; } = 0xFF585B70; - public string? FontFamily { get; private set; } - public double FontSize { get; private set; } = 13.0; - public string? ThemeName { get; private set; } public IReadOnlyList AvailableThemes { get; private set; } = Array.Empty(); public ThemeProvider(IConfigService configService) diff --git a/windows/Ghostty/Settings/WindowState.cs b/windows/Ghostty/Settings/WindowState.cs index ae1e36e312..2c812c07fe 100644 --- a/windows/Ghostty/Settings/WindowState.cs +++ b/windows/Ghostty/Settings/WindowState.cs @@ -45,6 +45,15 @@ internal sealed class WindowState // until the first close; the splash falls back to the config default. public uint? BackgroundRgb { get; set; } + // Whether the background above came from the built-in theme rather than + // from anything the user set, which makes it a value that can go stale + // while the app is not running: the built-in theme follows the desktop's + // light/dark setting, and that can be flipped between two launches. The + // splash re-derives instead of trusting the saved colour when this is + // set, which is the difference between coming up in the colour the + // terminal is about to be and flashing the one it used to be. + public bool BackgroundFollowsOsTheme { get; set; } + // User-resized height of the quake / drop-down window, remembered // per-user so a manual resize survives restarts. Null until the user // first resizes. Distinct from WindowHeight (regular window placement); diff --git a/windows/Ghostty/Shell/SplashWindow.cs b/windows/Ghostty/Shell/SplashWindow.cs index 0ef0ccafa8..882e567f2d 100644 --- a/windows/Ghostty/Shell/SplashWindow.cs +++ b/windows/Ghostty/Shell/SplashWindow.cs @@ -41,10 +41,13 @@ namespace Ghostty.Shell; /// internal static unsafe partial class SplashWindow { - // Fallback background when no colour has been persisted yet (first - // ever launch). Matches ConfigService's default background so the - // handoff to the real window is not a visible colour jump. - private const uint DefaultBackgroundRgb = 0x1E1E2E; + // Fallback backgrounds when no colour has been persisted yet (first + // ever launch). These are the backgrounds of libghostty's built-in + // theme pair, so the handoff to the real window is not a visible + // colour jump -- which now means picking the half the desktop is on, + // because the terminal will. + private const uint DefaultDarkBackgroundRgb = 0x131620; + private const uint DefaultLightBackgroundRgb = 0xF4F6FB; // Fallback rect when there is no saved window state, in physical // pixels. Close enough to the app's default window that the splash @@ -1181,8 +1184,42 @@ private static bool TryGetWorkArea( private static uint ResolveBackgroundRgb(Ghostty.Settings.WindowState? state) { - if (state?.BackgroundRgb is uint saved) return saved & 0x00FFFFFFu; - return DefaultBackgroundRgb; + // The saved colour is the better answer whenever it can still be + // right: it is what the terminal actually came up as last time, + // config and all. It stops being right when it was the built-in + // theme's and the desktop has flipped since, because the terminal is + // about to come up in the other half of the pair. + if (state is { BackgroundFollowsOsTheme: false, BackgroundRgb: uint saved }) + return saved & 0x00FFFFFFu; + + return IsSystemDark() + ? DefaultDarkBackgroundRgb + : DefaultLightBackgroundRgb; + } + + /// + /// The OS light/dark setting, read straight from the registry. + /// + /// + /// Not OsTheme.IsDark: this runs before the XAML application + /// exists, and activating a UISettings here costs more than the + /// whole splash is allowed. Defaults to dark on any failure, matching + /// what the splash did unconditionally before. + /// + private static bool IsSystemDark() + { + try + { + var value = Microsoft.Win32.Registry.GetValue( + @"HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize", + "AppsUseLightTheme", + null); + return value is int i ? i == 0 : true; + } + catch (Exception) + { + return true; + } } private static bool RegisterWindowClass() From 2128b5fc76a2da6446f99eff1effeca847d2a51f Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 26 Aug 2026 13:21:06 +0300 Subject: [PATCH 4/7] config: fix what review found in the zero-config theme Palette. In the light half, slot 0 and slot 15 were the same colour, and slot 7 was dark. Anything pairing black with bright white rendered invisible, and `ESC[47m` gave a dark background carrying dark text. The cause was the test: it held every slot to a contrast ratio against the background and exempted only slot 0, which is the near-background slot in a DARK theme. Clearing that rule is what pushed the light half's white slots dark. The rule is polarity-aware now: fills are slot 0 in the dark half and slots 7 and 15 in the light one, and a fill is held to the opposite pair of rules -- distinguishable from the background, and readable with foreground text on top. Slot 0 against slot 15 is pinned separately. Verified by restoring the old palette and watching it fail. The test could also pass on a theme with a line missing: the non-palette colours defaulted to undefined, and 0xAA-filled bytes clear every ratio it asserts. They are optional and required now. C API. ghostty_config_set_color_scheme returns bool and refuses after finalize instead of accepting and doing nothing. Accepting was worse than useless: the recorded scheme would then disagree with the colours already resolved, and the next real desktop change would compare equal to it, be dropped as "no change", and leave the config stuck on the wrong half until the user flipped twice. ghostty_config_builtin_theme returns static storage through the one ABI type that everywhere else means "you own this". Both the header and the managed import now say so, since freeing it hands the allocator a pointer it never owned. ghostty_config_theme_is_builtin is new, and fixes a real regression: the chrome decided whether the built-in pair applied by reading `theme` out of the top-level config file, but `theme` can be set in a file reached through `config-file`. That combination rendered the terminal in the user's theme and painted the chrome from the built-in pair. Windows. A failed ReadFlags in the constructor left the "which scheme are my caches for" flag at its default, which on a light desktop is accidentally correct, so the retry guard declined every retry for the life of the process. It now points at the scheme we do not have. Reload sampled the desktop scheme twice with a config rebuild in between; it samples once. ConfigIniFile.Load opens with FileShare.ReadWrite rather than File.ReadLines' default of FileShare.Read, since this file has writers. edit.zig's comment named the wrong mechanism. Both create and open pass FILE_SHARE_READ|WRITE|DELETE, so a leaked handle cannot block another Zig reader. What it holds is GENERIC_WRITE, which locks out a reader asking for FILE_SHARE_READ alone -- the .NET default, and the host reads this file immediately after asking for its path. Also: the perceptual-distance test asserted a symmetric window around the target, but the step is a whole number of counts and near black one count is worth over half a unit of L*. It now asserts at-or-past and no more than one count past, with a near-black row that would have failed the old bound. The direction test was satisfied by ink equal to the background. The luminance formula had a third copy. StepLightness' doc promised a tint that never equals its input and never drifts in hue; both stop holding once a channel clamps, so it says what it does. --- include/ghostty.h | 7 +- src/config/CApi.zig | 41 ++++++++-- src/config/Config.zig | 38 +++++++--- src/config/edit.zig | 11 +-- src/config/wintty_theme.zig | 20 +++-- src/config/wintty_theme_test.zig | 76 +++++++++++++------ windows/Ghostty.Core/Config/ConfigIniFile.cs | 18 ++++- .../Ghostty.Core/Windows/ThemeResolution.cs | 27 +++---- .../Ghostty.Tests/Shell/LaunchTextureTests.cs | 22 ++++-- windows/Ghostty/Interop/NativeMethods.cs | 32 +++++++- windows/Ghostty/Services/ConfigService.cs | 43 +++++++++-- 11 files changed, 255 insertions(+), 80 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index f0dad62ba0..9702d359fa 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -1217,8 +1217,13 @@ GHOSTTY_API void ghostty_config_load_cli_args(ghostty_config_t); GHOSTTY_API void ghostty_config_load_file(ghostty_config_t, const char*); GHOSTTY_API void ghostty_config_load_default_files(ghostty_config_t); GHOSTTY_API void ghostty_config_load_recursive_files(ghostty_config_t); -GHOSTTY_API void ghostty_config_set_color_scheme(ghostty_config_t, +// Must be called before ghostty_config_finalize. Returns false if it was +// not, or if the scheme is out of range, in which case nothing changed. +GHOSTTY_API bool ghostty_config_set_color_scheme(ghostty_config_t, ghostty_color_scheme_e); +GHOSTTY_API bool ghostty_config_theme_is_builtin(ghostty_config_t); +// Static storage: do NOT pass the result to ghostty_string_free. The ptr is +// NULL on a build with no built-in theme. GHOSTTY_API ghostty_string_s ghostty_config_builtin_theme(ghostty_color_scheme_e); GHOSTTY_API void ghostty_config_finalize(ghostty_config_t); GHOSTTY_API bool ghostty_config_get(ghostty_config_t, void*, const char*, uintptr_t); diff --git a/src/config/CApi.zig b/src/config/CApi.zig index f31d96734c..6497c93934 100644 --- a/src/config/CApi.zig +++ b/src/config/CApi.zig @@ -88,21 +88,47 @@ export fn ghostty_config_load_recursive_files(self: *Config) void { /// Set the desktop colour scheme this config resolves against, for the /// conditional `theme = light:...,dark:...` form and for the built-in -/// theme pair. Must be called before ghostty_config_finalize to have any -/// effect, since that is where the theme is applied. +/// theme pair. Returns false if the call did nothing. /// /// Without this an embedder holding its own config handle resolves every /// conditional against the default scheme (light) no matter what the /// desktop is set to, and reads back colours the terminal never renders. -export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void { +/// +/// Must be called before ghostty_config_finalize, which is where the theme +/// is applied, and is refused afterwards rather than accepted-and-ignored. +/// Accepting it would be worse than useless: the recorded scheme would then +/// disagree with the colours already resolved, and the next real desktop +/// change would compare equal to it and be dropped as "no change", leaving +/// the config stuck on the wrong half until the user flipped twice. To +/// react to a scheme change after finalize, rebuild the config instead. +export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) bool { const scheme = std.enums.fromInt(apprt.ColorScheme, scheme_raw) orelse { log.warn("invalid color scheme value={}", .{scheme_raw}); - return; + return false; }; + + if (self._finalized) { + log.warn("color scheme set after finalize, ignoring", .{}); + return false; + } + self._conditional_state.theme = switch (scheme) { .light => .light, .dark => .dark, }; + return true; +} + +/// Whether this config resolved its colours from the built-in theme pair, +/// i.e. whether nothing anywhere in the loaded config set `theme`. +/// +/// An embedder cannot answer this by reading the user's config file itself: +/// `theme` can be set in a file pulled in by `config-file`, and following +/// that recursion means reimplementing the loader's include rules. Ask the +/// config that actually did the loading instead. +export fn ghostty_config_theme_is_builtin(self: *Config) bool { + if (comptime !wintty_theme.enabled) return false; + return self.theme == null; } /// The built-in theme applied when no theme is configured, in config file @@ -110,7 +136,12 @@ export fn ghostty_config_set_color_scheme(self: *Config, scheme_raw: c_int) void /// /// Exists so an embedder drawing chrome around the terminal can resolve the /// same colours the terminal resolved without a second copy of the palette -/// to keep in step. Points at static storage; the caller must not free it. +/// to keep in step. +/// +/// Unlike every other ghostty_string_s producer, this one points at static +/// storage and must NOT be passed to ghostty_string_free. Freeing it hands +/// the allocator a pointer it never owned. The ptr is null on a build with +/// no built-in theme, so callers must check before reading. export fn ghostty_config_builtin_theme(scheme_raw: c_int) String { if (comptime !wintty_theme.enabled) return .empty; const scheme = std.enums.fromInt(apprt.ColorScheme, scheme_raw) orelse { diff --git a/src/config/Config.zig b/src/config/Config.zig index 51c6aed845..601ae0b23b 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -4014,6 +4014,13 @@ _conditional_state: conditional.State = .{}, /// loading. This is used to speed up the conditional evaluation process. _conditional_set: std.EnumSet(conditional.Key) = .{}, +/// Whether finalize has run on this config. +/// +/// Only used to refuse the C API's set-color-scheme after the point where +/// it would silently do nothing. Not copied by clone: a clone is expected +/// to be finalized again. +_finalized: bool = false, + /// The steps we can use to reload the configuration after it has been loaded /// without reopening the files. This is used in very specific cases such /// as loadTheme which has more details on why. @@ -4705,6 +4712,23 @@ fn loadTheme(self: *Config, theme: Theme) !void { try self.applyThemeOverlay(&iter); } +/// Everything finalize does when no theme is configured, in one call so the +/// upstream theme block stays a one-line diff and rebases cleanly. +fn applyBuiltinTheme(self: *Config) !void { + // Warning: this deinits our existing config and replaces it, so all + // memory from self prior to this point is freed. + try self.loadBuiltinTheme(); + + // Same reasoning as the different-light-and-dark themes above: auto + // derives the window theme from the terminal background, which now + // moves with the desktop, so it would fight the desktop rather than + // follow it. + if (self.@"window-theme" == .auto) self.@"window-theme" = .system; + + // Mark that we use a conditional theme. + self._conditional_set.insert(.theme); +} + /// Load the built-in Wintty theme for the current conditional theme state. /// /// Goes through the same overlay as a user theme file, so the user's own @@ -4830,17 +4854,7 @@ pub fn finalize(self: *Config) !void { self._conditional_set.insert(.theme); } } else if (comptime wintty_theme.enabled) { - // No theme configured, so fall back to the built-in pair rather - // than to the compile-time colour defaults. See wintty_theme.zig - // for why this fork has a default theme where upstream has none. - try self.loadBuiltinTheme(); - - // Same reasoning as the different-light-and-dark branch above: - // auto derives the window theme from the terminal background, which - // now moves with the desktop, so it would fight the desktop instead - // of following it. - if (self.@"window-theme" == .auto) self.@"window-theme" = .system; - self._conditional_set.insert(.theme); + try self.applyBuiltinTheme(); } // Used for a variety of defaults. See the function docs as well the @@ -5036,6 +5050,8 @@ pub fn finalize(self: *Config) !void { // Finalize key remapping set for efficient lookups self.@"key-remap".finalize(); + + self._finalized = true; } /// Callback for src/cli/args.zig to allow us to handle special cases diff --git a/src/config/edit.zig b/src/config/edit.zig index 2131235313..d142cb629e 100644 --- a/src/config/edit.zig +++ b/src/config/edit.zig @@ -50,11 +50,12 @@ pub fn openPath(alloc_gpa: Allocator) ![:0]const u8 { // Try to create file and go on if it already exists. The handle is // closed immediately: all this call needs is for the file to exist. // - // Holding it open costs nothing on POSIX but is fatal on Windows, - // where the create denies read sharing for as long as the handle - // lives. Every later read of the config in the same process then - // fails with a sharing violation on a file no other process is - // touching, which on a first run is every read there is. + // Leaking it costs nothing on POSIX. On Windows the create asks for + // GENERIC_WRITE, so while the handle lives the file cannot be opened + // by anyone requesting FILE_SHARE_READ alone, which is what .NET's + // File.ReadLines and friends default to. The host reads this file + // right after asking for its path, and on a first run that read is + // the one that creates it, so it denies itself. if (std.Io.Dir.createFileAbsolute( global.io(), config_path.name, diff --git a/src/config/wintty_theme.zig b/src/config/wintty_theme.zig index aad52edf0c..c9e22fc39a 100644 --- a/src/config/wintty_theme.zig +++ b/src/config/wintty_theme.zig @@ -18,12 +18,18 @@ //! the ghost's glow is the accent, the ghost's own silver is the dark-mode //! foreground, and the near-black indigo of the icon's corners is the //! dark-mode field. Every colour here clears WCAG AA (4.5:1) against its -//! background except palette slot 0, which is the "black" slot and is -//! deliberately close to the background: programs use it as a fill, not as -//! text. `wintty_theme_test.zig` asserts that property so a future palette -//! tweak cannot quietly regress it. +//! background, except the two or three slots programs use as a fill rather +//! than as text: slot 0 in the dark half, slots 7 and 15 in the light half. +//! Those are held to the other half of the same bargain instead -- they must +//! stay distinguishable from the background, and text in the foreground +//! colour drawn on top of them must itself clear AA. +//! +//! Which slots those are flips with the polarity, which is the trap here. A +//! light theme whose "white" slots are dark passes a naive +//! every-slot-against-the-background check and then renders `ESC[47m` as +//! dark-on-dark. `wintty_theme_test.zig` encodes the polarity-aware rule, and +//! separately pins that slot 0 and slot 15 can never collide. -const std = @import("std"); const builtin = @import("builtin"); const conditional = @import("conditional.zig"); @@ -82,7 +88,7 @@ pub const light: []const u8 = \\palette = 4=#1668c4 \\palette = 5=#7a3fbf \\palette = 6=#0f6e80 - \\palette = 7=#4a5265 + \\palette = 7=#b4bacb \\palette = 8=#666e81 \\palette = 9=#a82a3e \\palette = 10=#186540 @@ -90,7 +96,7 @@ pub const light: []const u8 = \\palette = 12=#0f55a6 \\palette = 13=#65329f \\palette = 14=#0b5a69 - \\palette = 15=#1e2333 + \\palette = 15=#cfd5e3 \\ ; diff --git a/src/config/wintty_theme_test.zig b/src/config/wintty_theme_test.zig index 75044d9c2b..2514327c4b 100644 --- a/src/config/wintty_theme_test.zig +++ b/src/config/wintty_theme_test.zig @@ -39,12 +39,16 @@ fn parseHex(s: []const u8) ![3]u8 { }; } +/// Every field is optional and every field is required. A theme that drops +/// a line has to fail parsing: left as `undefined`, a missing colour is +/// whatever the stack held, and 0xAA-filled bytes happen to clear every +/// ratio below, so the test would pass by not testing anything. const Parsed = struct { - background: [3]u8 = undefined, - foreground: [3]u8 = undefined, - cursor: [3]u8 = undefined, - selection_background: [3]u8 = undefined, - selection_foreground: [3]u8 = undefined, + background: ?[3]u8 = null, + foreground: ?[3]u8 = null, + cursor: ?[3]u8 = null, + selection_background: ?[3]u8 = null, + selection_foreground: ?[3]u8 = null, palette: [16][3]u8 = undefined, palette_seen: [16]bool = @splat(false), }; @@ -55,7 +59,6 @@ const Parsed = struct { /// a colour at its compile-time default. fn parse(source: []const u8) !Parsed { var out: Parsed = .{}; - var seen_background = false; var lines = std.mem.tokenizeScalar(u8, source, '\n'); while (lines.next()) |raw| { @@ -69,7 +72,6 @@ fn parse(source: []const u8) !Parsed { if (std.mem.eql(u8, key, "background")) { out.background = try parseHex(value); - seen_background = true; } else if (std.mem.eql(u8, key, "foreground")) { out.foreground = try parseHex(value); } else if (std.mem.eql(u8, key, "cursor-color")) { @@ -90,7 +92,11 @@ fn parse(source: []const u8) !Parsed { } } - if (!seen_background) return error.MissingBackground; + if (out.background == null) return error.MissingBackground; + if (out.foreground == null) return error.MissingForeground; + if (out.cursor == null) return error.MissingCursor; + if (out.selection_background == null) return error.MissingSelectionBackground; + if (out.selection_foreground == null) return error.MissingSelectionForeground; for (out.palette_seen) |seen| if (!seen) return error.IncompletePalette; return out; } @@ -110,30 +116,56 @@ fn expectAtLeast(actual: f64, minimum: f64) !void { fn checkTheme(source: []const u8) !void { const t = try parse(source); + const background = t.background.?; + const foreground = t.foreground.?; - try expectAtLeast(contrast(t.background, t.foreground), aa_text); - try expectAtLeast(contrast(t.background, t.cursor), aa_text); + try expectAtLeast(contrast(background, foreground), aa_text); + try expectAtLeast(contrast(background, t.cursor.?), aa_text); try expectAtLeast( - contrast(t.selection_background, t.selection_foreground), + contrast(t.selection_background.?, t.selection_foreground.?), aa_text, ); - // Slot 0 is the "black" slot. Programs use it as a fill behind other - // colours rather than as text, and on a dark theme it sits close to the - // background by convention, so it cannot be held to the text rule. It - // still has to be told apart from the background, which is the failure - // that would actually matter: a slot 0 equal to the background makes - // anything drawn in it disappear. + // Some slots are fills, not text. Programs paint them behind other + // colours, so by convention they sit near the background and cannot be + // held to the text rule. Which slots those are depends on the polarity: + // on a dark theme it is the "black" end, on a light theme the "white" + // end. Getting this backwards is the bug this exists to catch, since a + // light theme whose white slots are dark clears an every-slot-against- + // the-background check and still renders `ESC[47m` as dark-on-dark. + const dark_theme = luminance(background) < 0.5; + const fill_slots: []const usize = if (dark_theme) &.{0} else &.{ 7, 15 }; + for (t.palette, 0..) |color, i| { - if (i == 0) { - try testing.expect(contrast(t.background, color) > 1.2); + if (std.mem.indexOfScalar(usize, fill_slots, i) != null) { + // A fill has the opposite job to text, so it is held to the + // opposite pair of rules: it must be distinguishable from the + // background (or anything drawn in it vanishes), and text in the + // foreground colour on top of it must itself be readable. + testing.expect(contrast(background, color) > 1.2) catch |err| { + std.debug.print("fill slot {d} is invisible against the background\n", .{i}); + return err; + }; + expectAtLeast(contrast(color, foreground), aa_text) catch |err| { + std.debug.print("fill slot {d} cannot carry foreground text\n", .{i}); + return err; + }; continue; } - expectAtLeast(contrast(t.background, color), aa_text) catch |err| { + expectAtLeast(contrast(background, color), aa_text) catch |err| { std.debug.print("palette slot {d} failed\n", .{i}); return err; }; } + + // Slot 0 is "black" and slot 15 is "bright white". Whatever the polarity, + // a program that pairs them expects opposites: powerline segments and + // `fzf --color=bw` both do. They collided once already, when the light + // half pushed 15 dark to clear the text rule against a light background. + expectAtLeast(contrast(t.palette[0], t.palette[15]), aa_text) catch |err| { + std.debug.print("palette slot 0 and slot 15 are not opposites\n", .{}); + return err; + }; } test "built-in dark theme is legible" { @@ -150,8 +182,8 @@ test "the two halves actually differ in polarity" { // A pair whose halves are both dark would pass every contrast test // above and still defeat the entire point of having a pair. - try testing.expect(luminance(d.background) < 0.1); - try testing.expect(luminance(l.background) > 0.7); + try testing.expect(luminance(d.background.?) < 0.1); + try testing.expect(luminance(l.background.?) > 0.7); } test "forScheme selects the matching half" { diff --git a/windows/Ghostty.Core/Config/ConfigIniFile.cs b/windows/Ghostty.Core/Config/ConfigIniFile.cs index 3b5ca1ffb8..4e6c06a930 100644 --- a/windows/Ghostty.Core/Config/ConfigIniFile.cs +++ b/windows/Ghostty.Core/Config/ConfigIniFile.cs @@ -36,7 +36,23 @@ public static Dictionary> Load(string? path) if (string.IsNullOrEmpty(path) || !File.Exists(path)) return new Dictionary>(StringComparer.OrdinalIgnoreCase); - return Parse(File.ReadLines(path)); + // FileShare.ReadWrite rather than File.ReadLines' default of + // FileShare.Read. This file has writers: the settings UI rewrites it, + // and libghostty holds a write handle across its own config edits. A + // reader that refuses to share writes turns any of those into a + // sharing violation on a file that is merely open, not locked. + using var stream = new FileStream( + path, + FileMode.Open, + FileAccess.Read, + FileShare.ReadWrite | FileShare.Delete); + using var reader = new StreamReader(stream); + return Parse(ReadLines(reader)); + } + + private static IEnumerable ReadLines(StreamReader reader) + { + while (reader.ReadLine() is { } line) yield return line; } /// diff --git a/windows/Ghostty.Core/Windows/ThemeResolution.cs b/windows/Ghostty.Core/Windows/ThemeResolution.cs index 0128325054..2ab708bb69 100644 --- a/windows/Ghostty.Core/Windows/ThemeResolution.cs +++ b/windows/Ghostty.Core/Windows/ThemeResolution.cs @@ -156,13 +156,22 @@ private static double Lightness(double luminance) /// light mode. /// /// Every channel moves by the same number of counts, so the result - /// stays a tint of the input rather than becoming a colour of its own. - /// Near either end of the range the target is unreachable and the result - /// is the closest step available, but never the input itself: a tint - /// equal to its background draws nothing. + /// normally stays a tint of the input rather than becoming a colour of + /// its own. Two things stop holding once a channel hits a rail, since a + /// clamped channel stops moving while the others keep going: the hue + /// drifts (stepping #F4F6FB up far enough reaches #FFFFFF, and the blue + /// cast is gone), and a colour with no headroom at all in the requested + /// direction comes back unchanged. Callers that cannot use the input + /// itself must pick the direction with room -- which is what + /// LaunchTexture.ResolveInkRgb does with its luma split. /// public static uint StepLightness(uint rgb, double deltaLStar) { + // Caps the walk below. Comfortable for a deltaLStar in single digits + // (the worst case, pure black, needs 17 counts for 5.0), but it is a + // ceiling on the dial as well as on the loop: past roughly 15 the + // walk starts running out for mid greys and quietly under-delivering + // rather than failing. Raise it alongside any larger step. const int maxChannelStep = 48; var r = (int)((rgb >> 16) & 0xFF); @@ -197,15 +206,7 @@ public static uint StepLightness(uint rgb, double deltaLStar) } private static double LuminanceOf(int r, int g, int b) - { - static double Linearize(int channel) - { - var c = channel / 255.0; - return c <= 0.03928 ? c / 12.92 : Math.Pow((c + 0.055) / 1.055, 2.4); - } - - return (0.2126 * Linearize(r)) + (0.7152 * Linearize(g)) + (0.0722 * Linearize(b)); - } + => RelativeLuminance((uint)((r << 16) | (g << 8) | b)); /// /// Pick a legible foreground for text drawn over diff --git a/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs b/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs index 3d01b655a6..499a7368d4 100644 --- a/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs +++ b/windows/Ghostty.Tests/Shell/LaunchTextureTests.cs @@ -284,6 +284,12 @@ public void Ink_steps_away_from_the_background(uint background, bool expectLight { var ink = LaunchTexture.ResolveInkRgb(background); + // Per-channel the assertion can only be one-sided, since a channel + // that is already at the rail does not move. Something has to move + // though, or the texture is invisible and every check below still + // passes -- so pin that separately. + Assert.NotEqual(background, ink); + foreach (var shift in new[] { 16, 8, 0 }) { var before = (int)((background >> shift) & 0xFF); @@ -300,24 +306,26 @@ public void Ink_steps_away_from_the_background(uint background, bool expectLight [InlineData(0x808080u)] [InlineData(0x404040u)] [InlineData(0xE0E0E0u)] + [InlineData(0x0A0A0Au)] // near black, where one count is worth most public void Ink_sits_the_same_perceptual_distance_from_any_background(uint background) { // The whole point of the L* solve. A per-channel step gave dL* 5.0 // off the dark background and 3.5 off the light one, so the texture // that read as a grain in dark mode was nearly gone in light mode. // - // Half a unit of slack: the step is a whole number of counts, so it - // lands on the first count at or past the target rather than exactly - // on it. Compared against the constant rather than a written-out - // number, because the constant is a dial and turning it must not - // fail a test with nothing wrong. + // At or past the target, never more than one count past it. The step + // is a whole number of counts, so it overshoots, and near black one + // count is worth over half a unit of L* -- a symmetric window around + // the target fails there with nothing wrong. Compared against the + // constant rather than a written-out number, because the constant is + // a dial and turning it must not fail a test either. var delta = Math.Abs( LStar(LaunchTexture.ResolveInkRgb(background)) - LStar(background)); Assert.InRange( delta, - LaunchTexture.ContrastLStar - 0.5, - LaunchTexture.ContrastLStar + 0.5); + LaunchTexture.ContrastLStar, + LaunchTexture.ContrastLStar + 1.0); } [Theory] diff --git a/windows/Ghostty/Interop/NativeMethods.cs b/windows/Ghostty/Interop/NativeMethods.cs index c4e1c53d8f..3c0a14afd0 100644 --- a/windows/Ghostty/Interop/NativeMethods.cs +++ b/windows/Ghostty/Interop/NativeMethods.cs @@ -359,9 +359,39 @@ internal static int InitWideFromProcess() [LibraryImport(Dll, EntryPoint = "ghostty_config_set_color_scheme")] [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] - internal static partial void ConfigSetColorScheme( + private static partial byte ConfigSetColorSchemeNative( GhosttyConfig config, GhosttyColorScheme scheme); + /// + /// Tell the config which desktop colour scheme to resolve against. + /// Returns false if nothing changed, which for a well-formed scheme + /// means the config was already finalized and the call came too late. + /// + internal static bool ConfigSetColorScheme(GhosttyConfig config, GhosttyColorScheme scheme) + => ConfigSetColorSchemeNative(config, scheme) != 0; + + [LibraryImport(Dll, EntryPoint = "ghostty_config_theme_is_builtin")] + [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] + private static partial byte ConfigThemeIsBuiltinNative(GhosttyConfig config); + + /// + /// Whether the config resolved its colours from the built-in theme pair, + /// i.e. nothing anywhere in the loaded config set theme. Asked of + /// libghostty rather than of the config file, because theme can be + /// set in a file reached through config-file. + /// + internal static bool ConfigThemeIsBuiltin(GhosttyConfig config) + => ConfigThemeIsBuiltinNative(config) != 0; + + /// + /// The built-in theme text for a scheme, in config file syntax. + /// + /// + /// The returned string points at static storage on the native side. It + /// must not be freed, which makes it the one exception among this file's + /// producers -- see the note on the + /// declaration in ghostty.h. + /// [LibraryImport(Dll, EntryPoint = "ghostty_config_builtin_theme")] [UnmanagedCallConv(CallConvs = new[] { typeof(System.Runtime.CompilerServices.CallConvCdecl) })] internal static partial GhosttyString ConfigBuiltinTheme(GhosttyColorScheme scheme); diff --git a/windows/Ghostty/Services/ConfigService.cs b/windows/Ghostty/Services/ConfigService.cs index 04caa12582..78b86c1304 100644 --- a/windows/Ghostty/Services/ConfigService.cs +++ b/windows/Ghostty/Services/ConfigService.cs @@ -386,9 +386,20 @@ public ConfigService(DispatcherQueue dispatcher) // App.OnLaunched with no window and no message. // // Every value it would have set has a default, so a failed read - // leaves a usable snapshot rather than a torn one, and the + // leaves a consistent snapshot rather than a torn one, and the // first successful reload replaces it wholesale. StaticLoggers.ConfigService.LogSnapshotRefreshFailed(ex); + + // Consistent is not the same as usable, and the retry has to + // stay open. ReadFlags records the scheme its caches hold as its + // last act, so a throw leaves the field at its default of false. + // On a light desktop that is accidentally the truth, and + // RefreshForOsColorScheme's "already on this scheme" guard would + // then decline every retry for the life of the process, leaving + // the chrome on field defaults that no build renders. Point the + // flag at the scheme we do not have so the next flip goes + // through. + _themedValuesAreForDarkOs = !isOsDark; } } @@ -469,6 +480,13 @@ public bool Reload() // teardown, so _shuttingDown is the only guard against the freed app. if (_app.Handle == IntPtr.Zero) return false; + // Sampled once for the whole reload. Sampling again for ReadFlags + // would let a desktop flip between the two land a config resolved + // against one scheme in caches recorded as holding the other, and + // the second sample is the one the retry guard believes -- so the + // first would never be corrected. + var isOsDark = OsTheme.IsDark(); + GhosttyConfig newConfig; try { @@ -483,7 +501,7 @@ public bool Reload() if (hcPath is not null) NativeMethods.ConfigLoadFile(newConfig, hcPath); } - NativeMethods.ConfigSetColorScheme(newConfig, ToScheme(OsTheme.IsDark())); + NativeMethods.ConfigSetColorScheme(newConfig, ToScheme(isOsDark)); NativeMethods.ConfigFinalize(newConfig); } catch (Exception ex) @@ -517,7 +535,7 @@ public bool Reload() try { CacheDiagnostics(); - ReadFlags(OsTheme.IsDark()); + ReadFlags(isOsDark); } catch (Exception ex) { @@ -780,21 +798,32 @@ private void ReadFlags(bool isOsDark) // these caches, so the whole reload is bounded by at most two // File.ReadLines calls regardless of how many keys we probe. _configFileCache = LoadIniFile(ConfigFilePath); - var activeTheme = ResolveActiveThemeName(isOsDark); // No theme configured is not "no theme": libghostty applies its // built-in light/dark pair in that case, so the chrome has to // resolve against the same one or it frames a pane in colours the // pane is not filled with. Asked for by scheme rather than cached, // because a flip re-enters here with the other one. // + // Whether that happened is asked of libghostty, not of the config + // file. `theme` can be set in a file pulled in by `config-file`, + // which ResolveActiveThemeName cannot see, and reading it as "no + // theme" would paint the chrome from the built-in pair while the + // terminal renders the theme the user actually asked for. + // // A configured-but-unresolvable theme deliberately does not land // here: libghostty leaves the compile-time colours in place for // that, and substituting the built-in pair would drift again. - _activeThemeFileCache = string.IsNullOrEmpty(activeTheme) - ? LoadBuiltinTheme(isOsDark) - : ResolveThemePath(activeTheme) is { } themePath + if (NativeMethods.ConfigThemeIsBuiltin(_config)) + { + _activeThemeFileCache = LoadBuiltinTheme(isOsDark); + } + else + { + var activeTheme = ResolveActiveThemeName(isOsDark); + _activeThemeFileCache = ResolveThemePath(activeTheme) is { } themePath ? LoadIniFile(themePath) : null; + } // Immediately after the assignment it certifies, so the two cannot // disagree. Both failure legs then stay consistent: a throw from From 158970eecacbe8ae1d20876910dee775906173c7 Mon Sep 17 00:00:00 2001 From: Alessandro De Blasis Date: Wed, 26 Aug 2026 12:18:07 +0300 Subject: [PATCH 5/7] windows: draw the selected tab as a folder joined to the terminal The selected tab is stroked in the accent colour on the three sides that do not meet the pane, and the strip behind it is the same colour as that stroke, so the tab reads as a card lifted out of the strip. On the fourth side, where the tab meets the terminal, a cover the width of the tab is drawn over the pane border, so the tab fill runs into the terminal with no line across the join. Horizontal tabs open at the top, vertical tabs at the left, and both get the same treatment. The cover cannot live in the strip: content drawn there is clipped to the strip and never reaches the pane row. It is drawn in the pane row instead, positioned from the selected tab's offset within the strip viewport and clipped to that viewport, so a tab scrolled half out of view gets half a cover rather than one hanging in space. Three things move it, and all three are handled: opening or closing a tab (the new tab has no bounds on the first dispatcher pass, so a one-shot LayoutUpdated re-places it once it does), resizing or moving the window, and switching the layout (placed from the completion callback, since placing it from pre-animation geometry left it drifted by the delta). The vertical strip's selected title also gains its accent colour at startup. UpdateCursorAccentColors ran before the layout snap and so never reached the strip on the first pass, leaving the selected title at 1.11:1. It now runs after, and measures 14.46:1. The covers are gated on whether vertical tabs are wanted, not on the hidden strip's Visibility. The hidden strip is Visible by XAML default and is deliberately flipped by the priming pass, so the Visibility check hid the cover on every launch. --- windows/Ghostty/MainWindow.xaml.cs | 244 +++++++++++++++++- windows/Ghostty/Tabs/TabHost.xaml | 3 +- windows/Ghostty/Tabs/TabHost.xaml.cs | 237 ++++++++++++++++- windows/Ghostty/Tabs/VerticalTabHost.xaml.cs | 12 + windows/Ghostty/Tabs/VerticalTabStrip.xaml.cs | 41 ++- 5 files changed, 523 insertions(+), 14 deletions(-) diff --git a/windows/Ghostty/MainWindow.xaml.cs b/windows/Ghostty/MainWindow.xaml.cs index 0cc0bab110..c9b2d6d26f 100644 --- a/windows/Ghostty/MainWindow.xaml.cs +++ b/windows/Ghostty/MainWindow.xaml.cs @@ -607,6 +607,54 @@ void OnContentLoadedOnce(object s, RoutedEventArgs e) Canvas.SetZIndex(_verticalTabHost, -1); RootGrid.Children.Add(_verticalTabHost); + // Covers the active pane's top border across the selected tab, so + // the tab's fill runs into the terminal with no line between them. + // Lives in the pane's row rather than the strip's: drawn from the + // strip it would have to overhang its own parent to reach the + // border, and that overhang is clipped. + _tabSeamCover = new Microsoft.UI.Xaml.Shapes.Rectangle + { + HorizontalAlignment = HorizontalAlignment.Left, + VerticalAlignment = VerticalAlignment.Top, + IsHitTestVisible = false, + Visibility = Visibility.Collapsed, + // Deep enough to take the stroke and the hairline above it, both + // of which sit inside the pane's own gutter. + Height = Math.Ceiling(Core.Panes.PaneChrome.ActiveBorderThickness) + 1, + }; + Grid.SetRow(_tabSeamCover, 1); + Grid.SetColumn(_tabSeamCover, 1); + RootGrid.Children.Add(_tabSeamCover); + _horizontalTabHost.SelectedTabSeamChanged += OnSelectedTabSeamChanged; + + // The same seam on the vertical strip, rotated: there the selected + // row meets the pane along its right edge, so the cover is a + // vertical bar over the pane's left border. Placed across the whole + // RootGrid rather than in one cell, because the vertical strip spans + // both rows and a per-cell margin would need the row heights to + // convert; a margin in the grid's own space needs nothing. + _verticalSeamCover = new Microsoft.UI.Xaml.Shapes.Rectangle + { + HorizontalAlignment = HorizontalAlignment.Left, + VerticalAlignment = VerticalAlignment.Top, + IsHitTestVisible = false, + Visibility = Visibility.Collapsed, + // Wide enough to swallow the pane's gutter and its stroke, plus + // the overlap below. Erring wide is free: the fill is the + // terminal's own colour and the row's own fill is the same, so + // any overshoot in either direction lands on the colour that is + // already there. Erring narrow is what leaves a line. + Width = VerticalSeamOverlap + + Core.Panes.PaneChrome.SurfaceInset + + Math.Ceiling(Core.Panes.PaneChrome.ActiveBorderThickness) + 1, + }; + Grid.SetRow(_verticalSeamCover, 0); + Grid.SetRowSpan(_verticalSeamCover, 2); + Grid.SetColumn(_verticalSeamCover, 0); + Grid.SetColumnSpan(_verticalSeamCover, 2); + RootGrid.Children.Add(_verticalSeamCover); + _verticalTabHost.SelectionRowChanged += OnVerticalSeamChanged; + // Apply initial shell theme now that tab hosts exist, then // paint RootGrid.Background from the resolved state. ApplyShellTheme(); @@ -658,12 +706,7 @@ void OnContentLoadedOnce(object s, RoutedEventArgs e) // or crashes never runs the close path, and the splash would // then keep falling back to the built-in default and flash a // mismatched colour on every subsequent start. - var splashBackground = _configService.BackgroundColor & 0x00FFFFFFu; - if (_windowState.BackgroundRgb != splashBackground) - { - _windowState.BackgroundRgb = splashBackground; - _windowState.Save(); - } + if (RecordSplashBackground()) _windowState.Save(); } _tabManager.TabAdded += (_, t) => @@ -723,6 +766,22 @@ void OnContentLoadedOnce(object s, RoutedEventArgs e) if (_verticalTabsVisible) _verticalTabHost.SyncSelectionFromManager(); + // Tell both strips the terminal's colours. This only ever ran from + // OnConfigReloadedChrome, so a session whose config was never + // reloaded left both hosts on their own fallbacks -- survivable in + // the horizontal strip, but the vertical strip's fallback calibrates + // the selected row's title against the system accent rather than the + // row it is drawn on, which put a white title on the light half of + // the theme at 1.11:1. + // + // Deliberately here and not earlier beside ApplyShellTheme. It drives + // the vertical strip's NavigationView (theme refresh, per-item + // brushes, selection chrome), and doing that before Snap has decided + // which strip is live -- and before the control is loaded -- left + // MUXC in a state where a later SelectedItem assignment took an + // access violation inside NavigationView. + UpdateCursorAccentColors(); + _titleBar = new TitleBarCoordinator( this, _tabManager, @@ -958,6 +1017,12 @@ private void AnimateTabLayoutTo(bool vertical) _pendingLayoutTarget = null; _verticalTabsVisible = vertical; _tabHost = vertical ? _verticalTabHost : _horizontalTabHost; + // The seam covers are gated on the flag just set, and the strip that + // is coming back may not raise anything on its own (a switch does not + // resize it or move its selection). Ask both for a fresh placement so + // whichever one now owns the seam draws it, and the other hides. + _tabSeamCover.Visibility = Visibility.Collapsed; + _verticalSeamCover.Visibility = Visibility.Collapsed; // Paint caption/title chrome before the cross-fade so the OS // buttons and drag row do not flash stale horizontal colors. ApplyVerticalTitleBarChrome(); @@ -989,6 +1054,14 @@ private void AnimateTabLayoutTo(bool vertical) RefreshTabHostChrome(); if (vertical) _verticalTabHost.SyncSelectionFromManager(); + + // Place the seam only now. The switch is animated, so the strip + // that is arriving has no final geometry until it lands -- a + // placement made when the switch was requested reads the offsets + // the strip had before it, which are non-zero and therefore look + // valid, and the cover ends up rubbing out a stretch of border + // nowhere near the tab. + _horizontalTabHost.RefreshSeam(); _titleBar.ApplyForCurrentMode(); var leaf = _tabManager.ActiveTab?.PaneHost?.ActiveLeaf; if (leaf is not null) @@ -1494,7 +1567,7 @@ private async void OnClosedAsync(object sender, WindowEventArgs args) // Carried purely for the next cold start's splash, which runs // before any theme has been resolved and would otherwise have // to guess this colour. - _windowState.BackgroundRgb = _configService.BackgroundColor & 0x00FFFFFFu; + RecordSplashBackground(); _windowState.Save(); } @@ -2330,6 +2403,159 @@ private void ApplyButtonColors( /// blends with the terminal background so the active tab connects to the /// pane below it. Called on every config reload so theme changes apply. /// + /// + /// Copy the resolved terminal background into the window state for the + /// next cold start's splash, which runs before any theme is resolved and + /// would otherwise have to guess. Returns true when anything moved, so a + /// caller can skip a write. + /// + /// + /// One place for both callers because they used to be two, and the one + /// that ran at startup wrote the colour without the flag beside it. That + /// left every session claiming a background the desktop could not flip + /// out from under, and the splash went on trusting a stale colour. + /// + private bool RecordSplashBackground() + { + var background = _configService.BackgroundColor & 0x00FFFFFFu; + + // Neither a configured background nor a configured theme means the + // colour is the built-in theme's, which tracks the desktop and so is + // only good for as long as that does not move. + var followsOs = !_configService.IsConfiguredInFile("background") + && string.IsNullOrEmpty(_configService.CurrentTheme); + + if (_windowState.BackgroundRgb == background + && _windowState.BackgroundFollowsOsTheme == followsOs) + { + return false; + } + + _windowState.BackgroundRgb = background; + _windowState.BackgroundFollowsOsTheme = followsOs; + return true; + } + + private readonly Microsoft.UI.Xaml.Shapes.Rectangle _tabSeamCover; + + /// + /// Place the seam cover under the selected tab, or hide it when the + /// strip has nothing to join to (vertical layout, or before the strip + /// has arranged). + /// + private void OnSelectedTabSeamChanged(double left, double width, Brush? fill) + { + if (_isClosed) return; + + // Only meaningful in horizontal layout: in vertical the strip is + // beside the pane, not above it, and the seam is a different edge. + // + // Gated on the layout MainWindow last applied, NOT on the hosts' + // Visibility. Visibility is not a layout signal: both hosts are + // Visible by default until the first Snap, and PrimeHiddenStrip + // deliberately makes the collapsed one Visible at zero opacity for a + // few frames. Reading it here meant the first placement of every + // session decided it was in vertical layout and hid the cover, and + // nothing re-fired until the window happened to be resized. + if (width <= 0 || fill is null || _verticalTabsVisible) + { + _tabSeamCover.Visibility = Visibility.Collapsed; + return; + } + + _tabSeamCover.Margin = new Thickness(left, 0, 0, 0); + _tabSeamCover.Width = width; + _tabSeamCover.Fill = fill; + _tabSeamCover.Visibility = Visibility.Visible; + } + + private readonly Microsoft.UI.Xaml.Shapes.Rectangle _verticalSeamCover; + + /// + /// How far back into the selected row the vertical seam cover starts. + /// + private const double VerticalSeamOverlap = 4.0; + + /// + /// Place the vertical strip's seam cover over the pane's left border, + /// for the height of the selected row. + /// + private void OnVerticalSeamChanged() + { + if (_isClosed) return; + + var row = _verticalTabHost.SelectionRowElement; + // Same reasoning as the horizontal gate: the layout MainWindow last + // applied, not the host's Visibility. + if (!_verticalTabsVisible + || row.Visibility != Visibility.Visible + || row.ActualWidth <= 0 + || row.ActualHeight <= 2 + || row is not Border { Background: { } fill }) + { + _verticalSeamCover.Visibility = Visibility.Collapsed; + return; + } + + // Start at the row's own right edge, which is already the terminal + // colour, so the cover cannot bleed back over the strip. + Windows.Foundation.Point start; + try + { + start = row.TransformToVisual(RootGrid) + .TransformPoint(new Windows.Foundation.Point(row.ActualWidth, 0)); + } + catch (Exception ex) when (ex is ArgumentException or InvalidOperationException) + { + // The row is not in the tree yet, or is being torn out of it. + // The next SelectionRowChanged places it. + _verticalSeamCover.Visibility = Visibility.Collapsed; + return; + } + + // Started a few pixels back inside the row rather than exactly at its + // edge: the row's right edge and the pane border are not flush, and + // the strip's own surface shows through whatever is left between + // them. Backing into the row costs nothing since both are filled + // with the same colour. + // + // Inside the row's top and bottom strokes, so those still close onto + // the pane border the way the horizontal tab's corners do. + const double edgeStroke = 1.0; + var top = start.Y + edgeStroke; + var bottom = start.Y + row.ActualHeight - edgeStroke; + + // Clip to the strip. With more tabs than fit, the selected row can be + // scrolled out of the list while its layout offset still reports + // where it would have been, and a cover placed there is a bar of + // terminal colour drawn across the pane at a height with no tab + // beside it. + try + { + var stripTop = _verticalTabHost.TransformToVisual(RootGrid) + .TransformPoint(new Windows.Foundation.Point(0, 0)).Y; + top = Math.Max(top, stripTop); + bottom = Math.Min(bottom, stripTop + _verticalTabHost.ActualHeight); + } + catch (Exception ex) when (ex is ArgumentException or InvalidOperationException) + { + _verticalSeamCover.Visibility = Visibility.Collapsed; + return; + } + + if (bottom - top <= 0) + { + _verticalSeamCover.Visibility = Visibility.Collapsed; + return; + } + + _verticalSeamCover.Margin = new Thickness( + start.X - VerticalSeamOverlap, top, 0, 0); + _verticalSeamCover.Height = bottom - top; + _verticalSeamCover.Fill = fill; + _verticalSeamCover.Visibility = Visibility.Visible; + } + private void UpdateCursorAccentColors() { var bg = _configService.BackgroundColor; @@ -2345,6 +2571,10 @@ private void UpdateCursorAccentColors() var cc = _configService.CursorColor ?? _configService.ForegroundColor; var wuiColor = Windows.UI.Color.FromArgb(0xFF, (byte)(cc >> 16), (byte)(cc >> 8), (byte)cc); + // Both hosts, from the one value that also draws the pane border + // below: the selected tab is stroked in it on the three sides that + // do not meet the pane, so tab and pane read as a single shape. + _horizontalTabHost.SetAccentColor(wuiColor); _verticalTabHost.SetAccentColor(wuiColor); ApplyPerTabChrome(); diff --git a/windows/Ghostty/Tabs/TabHost.xaml b/windows/Ghostty/Tabs/TabHost.xaml index 0c9e935a60..029630dfdc 100644 --- a/windows/Ghostty/Tabs/TabHost.xaml +++ b/windows/Ghostty/Tabs/TabHost.xaml @@ -42,7 +42,7 @@ both tab hosts can share a single container without ever reparenting the SwapChainPanels. This UserControl now renders only the tab strip chrome. --> - + - +