From 132f7c27ba00562c22ed957dbaa4428106df17e3 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 16:23:54 +0200 Subject: [PATCH] fix: inspector font/formatting, FreeType-only backend on Apple Resolves the parity items of docs/rewrite/notes/review-rv17-codex.md (final-Z20.md): - FreeType-only font backend is also available on Apple targets (discovery-free, embedded emoji fonts); ghostty_surface_quicklook_font returns null for every backend except plain CoreText like Zig. - The +list-fonts test keys off the font crate's selected backend (HAS_DISCOVERY), correct under feature unification and on Apple. - Inspector font: 32 px rasterized with FreeType (dear-imgui freetype) like Ghostty; a new `visual-parity.sh inspector` scenario shows 0.031% differing pixels (hover highlight) vs the original on Linux. Windows, Apple and wasm keep stb_truetype at 32 px (registered as a deliberate deviation). - Inspector DCS/APC descriptions follow Zig 0.15.2's formatting (checked by running Zig), incl. the terminal.Parser.Action prefix. - The ImGui demo window appears only in dev-profile builds, mirroring Zig's .Debug-only gate. Test Plan: - cargo test --workspace --no-fail-fast (3582 passed) - just check-macos (both, also with --features font-backend-freetype) - scripts/visual-parity.sh inspector --- Cargo.lock | 2 + .../src/class/inspector_widget.rs | 2 +- crates/ghostty-cli/src/list_fonts.rs | 30 ++++++---- crates/ghostty-core/src/surface/mod.rs | 11 ++-- crates/ghostty-font/Cargo.toml | 15 ++--- crates/ghostty-font/build.rs | 32 +++++----- crates/ghostty-font/src/backend.rs | 39 ++++++++---- crates/ghostty-font/src/face.rs | 19 ++++++ crates/ghostty-inspector/Cargo.toml | 7 +++ crates/ghostty-inspector/build.rs | 14 +++++ crates/ghostty-inspector/src/inspector.rs | 55 ++++++++++++----- .../ghostty-inspector/src/widgets/surface.rs | 18 +++++- .../ghostty-inspector/src/widgets/termio.rs | 59 +++++++++++++++++-- crates/ghostty-lib/src/inspector.rs | 3 +- docs/rewrite/DEVIANT_FIXES.md | 1 + docs/rewrite/notes/final-Z20.md | 20 +++++++ scripts/macos-crosscheck.sh | 10 +++- scripts/visual-parity.sh | 13 ++-- 18 files changed, 267 insertions(+), 83 deletions(-) create mode 100644 crates/ghostty-inspector/build.rs create mode 100644 docs/rewrite/notes/final-Z20.md diff --git a/Cargo.lock b/Cargo.lock index 4ed3e64..f515728 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -706,8 +706,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "728af47c10cb2b12aa36b942e610f894ecafc5d61d0056ab61f9629f02b3d5dc" dependencies = [ "cc", + "pkg-config", "serde", "serde_json", + "vcpkg", ] [[package]] diff --git a/crates/ghostty-apprt-gtk/src/class/inspector_widget.rs b/crates/ghostty-apprt-gtk/src/class/inspector_widget.rs index fce2911..0361b86 100644 --- a/crates/ghostty-apprt-gtk/src/class/inspector_widget.rs +++ b/crates/ghostty-apprt-gtk/src/class/inspector_widget.rs @@ -97,7 +97,7 @@ mod imp { let weak = obj.downgrade(); obj.upcast_ref::().set_hooks( // Called to do initial setup of the UI. - Box::new(ghostty_inspector::setup), + Box::new(|ctx, _scale| ghostty_inspector::setup(ctx)), // Called for every frame to draw the UI. Box::new(move |ui| { let Some(this) = weak.upgrade() else { diff --git a/crates/ghostty-cli/src/list_fonts.rs b/crates/ghostty-cli/src/list_fonts.rs index 8f77280..239d5fd 100644 --- a/crates/ghostty-cli/src/list_fonts.rs +++ b/crates/ghostty-cli/src/list_fonts.rs @@ -134,20 +134,28 @@ pub fn run(args: &[impl AsRef<[u8]>]) -> Result { #[cfg(test)] mod tests { + /// Keyed off the font crate's actually selected backend, not this + /// crate's own feature flag: cargo feature unification can enable + /// `ghostty-font/font-backend-freetype` without the CLI forwarding + /// feature, and on Apple targets the default backend (CoreText) has + /// discovery. + /// /// Without a discovery mechanism (the `freetype` font backend, Zig /// `Discover == void`) `+list-fonts` prints the compile-time-option /// message and exits with 1, after parsing the arguments. - #[cfg(feature = "font-backend-freetype")] #[test] - fn freetype_backend_reports_no_discovery() { - const { assert!(!ghostty_font::HAS_DISCOVERY) }; - assert_eq!(super::run(&[] as &[&[u8]]).expect("run"), 1); - assert!(super::NO_DISCOVERY_MESSAGE.ends_with("try again.")); - } - - #[cfg(not(feature = "font-backend-freetype"))] - #[test] - fn default_backend_has_discovery() { - const { assert!(ghostty_font::HAS_DISCOVERY) }; + fn discovery_presence_follows_selected_backend() { + let backend = ghostty_font::options.backend; + assert_eq!( + ghostty_font::HAS_DISCOVERY, + backend.has_fontconfig() || backend.has_coretext() + ); + if ghostty_font::HAS_DISCOVERY { + assert_ne!(backend, ghostty_font::Backend::Freetype); + } else { + assert_eq!(backend, ghostty_font::Backend::Freetype); + assert_eq!(super::run(&[] as &[&[u8]]).expect("run"), 1); + assert!(super::NO_DISCOVERY_MESSAGE.ends_with("try again.")); + } } } diff --git a/crates/ghostty-core/src/surface/mod.rs b/crates/ghostty-core/src/surface/mod.rs index 4e8c029..d65f0b4 100644 --- a/crates/ghostty-core/src/surface/mod.rs +++ b/crates/ghostty-core/src/surface/mod.rs @@ -745,6 +745,10 @@ impl Surface { #[cfg(target_vendor = "apple")] #[must_use] pub fn quicklook_font(&self, scale: f32) -> Option<*mut std::ffi::c_void> { + // For non-CoreText (and the other CoreText variants) we return none. + if ghostty_font::options.backend != ghostty_font::Backend::Coretext { + return None; + } let grid = self.renderer.font_grid(); let mut state = grid.write(); let face = state @@ -752,12 +756,7 @@ impl Surface { .collection .get_face(ghostty_font::collection::Index::default()) .ok()?; - let font = face.quicklook_font(f64::from(scale)); - Some( - objc2_core_foundation::CFRetained::into_raw(font) - .as_ptr() - .cast(), - ) + ghostty_font::face::quicklook_font_raw(face, f64::from(scale)) } /// Current sizes. diff --git a/crates/ghostty-font/Cargo.toml b/crates/ghostty-font/Cargo.toml index 8a7c4ca..368b51b 100644 --- a/crates/ghostty-font/Cargo.toml +++ b/crates/ghostty-font/Cargo.toml @@ -40,7 +40,7 @@ harfbuzz-sys = { version = "0.8.0", features = ["bundled"] } # Like upstream (which builds them from source) the bundled copies are used, # so no system library is needed on macOS. freetype-sys = { version = "0.23.0", optional = true, features = ["bundled"] } -harfbuzz-sys = { version = "0.8.0", optional = true, default-features = false, features = ["coretext", "bundled"] } +harfbuzz-sys = { version = "0.8.0", optional = true, default-features = false, features = ["bundled"] } objc2 = "0.6.5" objc2-core-foundation = "0.3.2" objc2-core-graphics = "0.3.2" @@ -51,18 +51,19 @@ objc2-foundation = "0.3.2" default = [] # CoreText for discovery, FreeType for rendering, HarfBuzz for shaping # (Zig `-Dfont-backend=coretext_freetype`; Apple targets only). -coretext-freetype = ["dep:freetype-sys", "dep:harfbuzz-sys", "harfbuzz-sys/freetype"] +coretext-freetype = ["dep:freetype-sys", "dep:harfbuzz-sys", "harfbuzz-sys/freetype", "harfbuzz-sys/coretext"] # CoreText for discovery and rendering, HarfBuzz for shaping # (Zig `-Dfont-backend=coretext_harfbuzz`; Apple targets only). -coretext-harfbuzz = ["dep:harfbuzz-sys"] +coretext-harfbuzz = ["dep:harfbuzz-sys", "harfbuzz-sys/coretext"] # CoreText for discovery and rendering, no shaping # (Zig `-Dfont-backend=coretext_noshape`; Apple targets only). coretext-noshape = [] # FreeType for rendering and HarfBuzz for shaping with no font discovery -# (Zig `-Dfont-backend=freetype`; non-Apple, non-wasm targets only; the -# default there is `fontconfig_freetype`). Not enabled by `--all-features` -# in any meaningful sense: it switches the whole build to the backend. -font-backend-freetype = [] +# (Zig `-Dfont-backend=freetype`; every target but wasm32, Apple included; +# the default on Linux is `fontconfig_freetype`). Not enabled by +# `--all-features` in any meaningful sense: it switches the whole build to +# the backend. +font-backend-freetype = ["dep:freetype-sys", "dep:harfbuzz-sys", "harfbuzz-sys/freetype"] # This crate wraps the freetype / harfbuzz / fontconfig C libraries through # their `-sys` crates; unsafe is denied by default and allowed on minimal, diff --git a/crates/ghostty-font/build.rs b/crates/ghostty-font/build.rs index a0d16ee..a6bfe12 100644 --- a/crates/ghostty-font/build.rs +++ b/crates/ghostty-font/build.rs @@ -3,9 +3,10 @@ //! //! Backends (see `src/backend.rs`): //! * Linux & other non-Apple, non-wasm targets: `fontconfig_freetype`. -//! * Linux & other non-Apple, non-wasm targets with the cargo feature +//! * Any non-wasm target (Apple included) with the cargo feature //! `font-backend-freetype`: `freetype` (FreeType rendering, HarfBuzz -//! shaping, no font discovery; Zig `-Dfont-backend=freetype`). +//! shaping, no font discovery; Zig `-Dfont-backend=freetype`). On Apple +//! it takes precedence over the `coretext-*` features. //! * wasm32: `web_canvas`. //! * Apple targets: `coretext` (default), or via cargo features //! `coretext-freetype`, `coretext-harfbuzz`, `coretext-noshape`. @@ -46,14 +47,15 @@ fn main() { let ct_hb = feat("CORETEXT_HARFBUZZ"); let ct_noshape = feat("CORETEXT_NOSHAPE"); + // Zig `-Dfont-backend=freetype` is valid on every non-wasm target, + // Darwin included (no discovery, so the embedded emoji fonts are used). let ft_only = feat("FONT_BACKEND_FREETYPE"); - if ft_only && (wasm || apple) { - println!( - "cargo:warning=feature `font-backend-freetype` only applies to non-Apple, \ - non-wasm targets; ignored" - ); + if ft_only && wasm { + println!("cargo:warning=feature `font-backend-freetype` does not apply to wasm32; ignored"); } - let ft_only = ft_only && !wasm && !apple; + let ft_only = ft_only && !wasm; + // CoreText is the backend on Apple targets unless FreeType-only is asked. + let coretext = apple && !ft_only; let on = |c: &str, v: bool| { if v { @@ -61,17 +63,17 @@ fn main() { } }; - let freetype = !wasm && (!apple || ct_ft); - let harfbuzz = !wasm && (!apple || ct_ft || ct_hb); + let freetype = !wasm && (!coretext || ct_ft); + let harfbuzz = !wasm && (!coretext || ct_ft || ct_hb); on("font_freetype", freetype); let fontconfig = !wasm && !apple && !ft_only; on("font_fontconfig", fontconfig); - on("font_discovery", fontconfig || apple); + on("font_discovery", fontconfig || coretext); on("font_harfbuzz", harfbuzz); - on("font_coretext", apple); - on("font_coretext_face", apple && !ct_ft); + on("font_coretext", coretext); + on("font_coretext_face", coretext && !ct_ft); // The shaper is CoreText only for the plain `coretext` backend; // `coretext_noshape` uses the no-op shaper. - on("font_coretext_shaper", apple && !harfbuzz && !ct_noshape); - on("font_noshape", apple && !harfbuzz && ct_noshape); + on("font_coretext_shaper", coretext && !harfbuzz && !ct_noshape); + on("font_noshape", coretext && !harfbuzz && ct_noshape); } diff --git a/crates/ghostty-font/src/backend.rs b/crates/ghostty-font/src/backend.rs index 8c2cb64..54ffbd5 100644 --- a/crates/ghostty-font/src/backend.rs +++ b/crates/ghostty-font/src/backend.rs @@ -13,9 +13,10 @@ //! (Zig: `-Dfont-backend=...`). "There is no scenario that is the default" //! for those; they're only used by people who want to self-compile Ghostty //! and prefer the freetype aesthetic. -//! * Non-Apple, non-wasm targets with the cargo feature +//! * Non-wasm targets (Apple included) with the cargo feature //! `font-backend-freetype`: [`Backend::Freetype`] (Zig -//! `-Dfont-backend=freetype`: no font discovery). +//! `-Dfont-backend=freetype`: no font discovery; takes precedence over the +//! `coretext-*` features). //! * wasm32: [`Backend::WebCanvas`]. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -104,11 +105,7 @@ pub struct Options { pub backend: Backend, } -#[cfg(all( - not(target_arch = "wasm32"), - not(target_vendor = "apple"), - feature = "font-backend-freetype" -))] +#[cfg(all(not(target_arch = "wasm32"), feature = "font-backend-freetype"))] pub const OPTIONS: Options = Options { backend: Backend::Freetype, }; @@ -122,13 +119,18 @@ pub const OPTIONS: Options = Options { backend: Backend::FontconfigFreetype, }; -#[cfg(all(target_vendor = "apple", feature = "coretext-freetype"))] +#[cfg(all( + target_vendor = "apple", + feature = "coretext-freetype", + not(feature = "font-backend-freetype") +))] pub const OPTIONS: Options = Options { backend: Backend::CoretextFreetype, }; #[cfg(all( target_vendor = "apple", + not(feature = "font-backend-freetype"), feature = "coretext-harfbuzz", not(feature = "coretext-freetype") ))] @@ -138,6 +140,7 @@ pub const OPTIONS: Options = Options { #[cfg(all( target_vendor = "apple", + not(feature = "font-backend-freetype"), feature = "coretext-noshape", not(feature = "coretext-freetype"), not(feature = "coretext-harfbuzz") @@ -148,6 +151,7 @@ pub const OPTIONS: Options = Options { #[cfg(all( target_vendor = "apple", + not(feature = "font-backend-freetype"), not(feature = "coretext-freetype"), not(feature = "coretext-harfbuzz"), not(feature = "coretext-noshape") @@ -165,6 +169,19 @@ pub const OPTIONS: Options = Options { mod tests { use super::*; + /// The `build.rs` cfgs must agree with the selected backend on every + /// target, including FreeType-only on Apple (Zig `Backend.has*`). + #[test] + fn selected_backend_matches_build_cfgs() { + let b = OPTIONS.backend; + assert_eq!(b.has_freetype(), cfg!(font_freetype)); + assert_eq!(b.has_fontconfig(), cfg!(font_fontconfig)); + assert_eq!(b.has_coretext(), cfg!(font_coretext)); + assert_eq!(b.has_harfbuzz(), cfg!(font_harfbuzz)); + assert_eq!(cfg!(font_discovery), b.has_fontconfig() || b.has_coretext()); + assert_eq!(crate::HAS_DISCOVERY, cfg!(font_discovery)); + } + #[test] fn web_canvas_capabilities() { let b = Backend::WebCanvas; @@ -236,11 +253,7 @@ mod tests { assert!(!b.has_coretext()); } - #[cfg(all( - not(target_arch = "wasm32"), - not(target_vendor = "apple"), - feature = "font-backend-freetype" - ))] + #[cfg(all(not(target_arch = "wasm32"), feature = "font-backend-freetype"))] #[test] fn freetype_only_backend_selected() { let b = OPTIONS.backend; diff --git a/crates/ghostty-font/src/face.rs b/crates/ghostty-font/src/face.rs index 191db5e..af1bcb0 100644 --- a/crates/ghostty-font/src/face.rs +++ b/crates/ghostty-font/src/face.rs @@ -31,6 +31,25 @@ pub use ghostty_config::types_b::FreetypeLoadFlags; #[cfg(target_arch = "wasm32")] pub use web_canvas::Face; +/// A retained `CTFontRef` of `face` for macOS quicklook (Zig +/// `ghostty_surface_quicklook_font`, which only answers for the plain +/// `.coretext` backend and returns null for every other one, FreeType-only +/// included). The caller owns the reference. +#[cfg(target_vendor = "apple")] +#[must_use] +pub fn quicklook_font_raw(face: &Face, scale: f64) -> Option<*mut std::ffi::c_void> { + #[cfg(font_coretext_face)] + if crate::backend::OPTIONS.backend == crate::Backend::Coretext { + return Some( + objc2_core_foundation::CFRetained::into_raw(face.quicklook_font(scale)) + .as_ptr() + .cast(), + ); + } + let _ = (face, scale); + None +} + /// If a DPI can't be calculated, this DPI is used. This is probably /// wrong on modern devices so it is highly recommended you get the DPI /// using whatever platform method you can. diff --git a/crates/ghostty-inspector/Cargo.toml b/crates/ghostty-inspector/Cargo.toml index 01c0625..ff06656 100644 --- a/crates/ghostty-inspector/Cargo.toml +++ b/crates/ghostty-inspector/Cargo.toml @@ -13,6 +13,13 @@ ghostty-terminal.workspace = true ghostty-vt.workspace = true log = "0.4" +# Zig builds Dear ImGui with `-DIMGUI_ENABLE_FREETYPE` (pkg/dcimgui), so the +# inspector font is rasterized with FreeType. The `freetype` feature needs a +# system FreeType (pkg-config); other targets keep stb_truetype (see +# `docs/rewrite/DEVIANT_FIXES.md`). +[target.'cfg(not(any(windows, target_vendor = "apple", target_arch = "wasm32")))'.dependencies] +dear-imgui-rs = { version = "0.19", features = ["freetype"] } + [lints.rust] unsafe_code = "deny" diff --git a/crates/ghostty-inspector/build.rs b/crates/ghostty-inspector/build.rs new file mode 100644 index 0000000..340d92f --- /dev/null +++ b/crates/ghostty-inspector/build.rs @@ -0,0 +1,14 @@ +//! Emits `cfg(ghostty_debug_build)` for the Cargo `dev` profile (Zig +//! `builtin.mode == .Debug`). `debug_assertions` is not equivalent: the +//! repository's `release` profile enables it, like Zig's `ReleaseSafe`. + +fn main() { + println!("cargo:rerun-if-changed=build.rs"); + println!("cargo:rerun-if-env-changed=PROFILE"); + println!("cargo:rustc-check-cfg=cfg(ghostty_debug_build)"); + let profile = std::env::var("PROFILE").unwrap_or_default(); + println!("cargo:rustc-env=GHOSTTY_BUILD_PROFILE={profile}"); + if profile == "debug" { + println!("cargo:rustc-cfg=ghostty_debug_build"); + } +} diff --git a/crates/ghostty-inspector/src/inspector.rs b/crates/ghostty-inspector/src/inspector.rs index 285370d..aba98f0 100644 --- a/crates/ghostty-inspector/src/inspector.rs +++ b/crates/ghostty-inspector/src/inspector.rs @@ -2,18 +2,19 @@ use std::sync::{Mutex, MutexGuard, PoisonError}; -use dear_imgui_rs::{ConfigFlags, Context, FontSource, StbTrueTypeFontData, Ui}; +use dear_imgui_rs::{ConfigFlags, Context, FontSource, Ui}; use ghostty_renderer::{overlay::Feature, state::InspectorHook}; use ghostty_terminal::terminal::Terminal; use crate::widgets; +/// The inspector font size in pixels. Zig hardcodes `16 * 2` ("This is +/// currently hardcoded to a 2x content scale", with a TODO to recalculate it +/// for different screen DPIs); the style is scaled separately by the apprt. +pub const FONT_SIZE: f32 = 16.0 * 2.0; + /// Setup the ImGui state. This requires an ImGui context. -/// -/// `scale_factor` is the content scale of the display; Zig hardcodes a 2x -/// scale here (with a TODO) and sizes the font as `16 * 2`, this uses -/// `16 * scale_factor` instead. -pub fn setup(ctx: &mut Context, scale_factor: f64) { +pub fn setup(ctx: &mut Context) { let io = ctx.io_mut(); // Enable docking, which we use heavily for the UI. Our colorspace is @@ -28,14 +29,28 @@ pub fn setup(ctx: &mut Context, scale_factor: f64) { log::warn!("failed to disable the imgui log file err={err}"); } - // Use our own embedded font - let font_size = (16.0 * scale_factor.max(1.0)) as f32; - match StbTrueTypeFontData::from_slice(ghostty_font::embedded::REGULAR) { - Ok(data) => { - ctx.font_atlas() - .add_font(&[FontSource::stb_truetype_with_size(data, font_size)]); + // Use our own embedded font. The atlas rasterizes it with FreeType (the + // default loader of the `freetype` build of Dear ImGui, as in Zig) where + // that build is available, otherwise with stb_truetype. + ctx.font_atlas().add_font(&[embedded_font_source()]); +} + +#[cfg(not(any(windows, target_vendor = "apple", target_arch = "wasm32")))] +#[allow(unsafe_code)] +fn embedded_font_source() -> FontSource<'static> { + // SAFETY: the embedded font is a complete, valid TrueType font that + // lives for the whole program; Dear ImGui copies it on add. + unsafe { FontSource::ttf_data_with_size(ghostty_font::embedded::REGULAR, FONT_SIZE) } +} + +#[cfg(any(windows, target_vendor = "apple", target_arch = "wasm32"))] +fn embedded_font_source() -> FontSource<'static> { + match dear_imgui_rs::StbTrueTypeFontData::from_slice(ghostty_font::embedded::REGULAR) { + Ok(data) => FontSource::stb_truetype_with_size(data, FONT_SIZE), + Err(err) => { + log::warn!("failed to load the embedded inspector font err={err}"); + FontSource::default_font_with_size(FONT_SIZE) } - Err(err) => log::warn!("failed to load the embedded inspector font err={err}"), } } @@ -110,6 +125,18 @@ impl InspectorHook for Shared { #[cfg(test)] mod tests { + /// Zig sizes the font as `16 * 2` and rasterizes with FreeType. + #[test] + fn font_size_and_rasterizer_match_zig() { + assert!((super::FONT_SIZE - 32.0).abs() < f32::EPSILON); + let freetype_target = cfg!(not(any( + windows, + target_vendor = "apple", + target_arch = "wasm32" + ))); + assert_eq!(dear_imgui_rs::HAS_FREETYPE, freetype_target); + } + use ghostty_input::{Action, Key, KeyEvent}; use ghostty_renderer::size::{CellSize, Padding, ScreenSize, Size}; use ghostty_terminal::terminal::Options; @@ -146,7 +173,7 @@ mod tests { #[test] fn draws_frames_headless() { let mut ctx = Context::create(); - setup(&mut ctx, 1.0); + setup(&mut ctx); ctx.io_mut().set_display_size([1000.0, 700.0]); ctx.io_mut().set_delta_time(1.0 / 60.0); ctx.font_atlas() diff --git a/crates/ghostty-inspector/src/widgets/surface.rs b/crates/ghostty-inspector/src/widgets/surface.rs index 20f4822..3fd65a1 100644 --- a/crates/ghostty-inspector/src/widgets/surface.rs +++ b/crates/ghostty-inspector/src/widgets/surface.rs @@ -13,6 +13,10 @@ use super::{key, renderer, terminal, termio}; use crate::dock::begin_window; /// This is discovered via the hardcoded string in the ImGui demo window. +/// True for the Cargo `dev` profile only (Zig `builtin.mode == .Debug`); +/// set by `build.rs` from `PROFILE`, deliberately not `debug_assertions`. +pub const DEBUG_BUILD: bool = cfg!(ghostty_debug_build); + const WINDOW_IMGUI_DEMO: &str = "Dear ImGui Demo"; const WINDOW_KEYBOARD: &str = "Keyboard"; const WINDOW_TERMINAL: &str = "Terminal"; @@ -133,8 +137,10 @@ impl Inspector { crate::dock::flush_splits(ui); // In debug we show the ImGui demo window so we can easily view - // available widgets and such. - if cfg!(debug_assertions) && self.show_demo_window { + // available widgets and such. Zig checks `builtin.mode == .Debug` + // (not `ReleaseSafe`), which is the Cargo `dev` profile; assertions + // enabled in a release profile do not count. + if DEBUG_BUILD && self.show_demo_window { ui.show_demo_window(&mut self.show_demo_window); } @@ -399,6 +405,14 @@ fn mouse_button_label(button: usize) -> &'static str { mod tests { use super::*; + #[test] + fn demo_window_gate_follows_the_dev_profile() { + // The `release` profile keeps debug assertions on (like Zig's + // ReleaseSafe) but must not show the demo. + let debug_build = DEBUG_BUILD; + assert_eq!(debug_build, env!("GHOSTTY_BUILD_PROFILE") == "debug"); + } + #[test] fn button_labels() { assert_eq!(mouse_button_label(0), "?"); diff --git a/crates/ghostty-inspector/src/widgets/termio.rs b/crates/ghostty-inspector/src/widgets/termio.rs index 09cce95..f334b5f 100644 --- a/crates/ghostty-inspector/src/widgets/termio.rs +++ b/crates/ghostty-inspector/src/widgets/termio.rs @@ -259,16 +259,20 @@ fn encode_action(md: &mut Vec<(String, String)>, action: &ParserAction) -> Strin } } -/// Mirrors `Parser.Action.format` for the actions that are not specially -/// encoded. +/// Mirrors Zig 0.15.2 `Parser.Action.format` for the actions that are not +/// specially encoded: `@typeName(Action)`, then `{ . = }` with +/// `u8` payloads as `0x{x}` and every other payload printed with +/// `printValue("any", .., depth 3)`: structs as `.{ .a = .., .b = .. }`, +/// slices as `{ a, b }` (`{ }` when empty), integers in decimal and `void` +/// as `void`. fn format_action(action: &ParserAction) -> String { let (tag, value) = match action { ParserAction::DcsHook(d) => ( "dcs_hook", format!( - "{{ intermediates = {:?}, params = {:?}, final = {} }}", - d.intermediates(), - d.params(), + ".{{ .intermediates = {}, .params = {}, .final = {} }}", + zig_any_slice(d.intermediates()), + zig_any_slice(d.params()), d.final_ ), ), @@ -280,7 +284,21 @@ fn format_action(action: &ParserAction) -> String { // Handled by the specific encoders. _ => ("unknown", String::new()), }; - format!("parser.Parser.Action{{ .{tag} = {value} }}") + format!("terminal.Parser.Action{{ .{tag} = {value} }}") +} + +/// A slice of integers as Zig's `{any}` prints it: `{ 1, 2 }`, or `{ }` +/// when empty. +fn zig_any_slice(items: &[T]) -> String { + let mut out = String::from("{ "); + for (i, v) in items.iter().enumerate() { + if i != 0 { + out.push_str(", "); + } + let _ = write!(out, "{v}"); + } + out.push_str(" }"); + out } fn encode_print(ch: u32) -> String { @@ -927,6 +945,35 @@ mod tests { assert!(filter("-").pass_filter("abc")); } + #[test] + fn dcs_and_apc_descriptions_match_zig_format() { + // Expected strings produced by Zig 0.15.2 `Parser.Action.format`. + let hook = ParserAction::DcsHook(ghostty_vt::parser::Dcs::new(b"$ ", &[1, 2, 300], b'q')); + assert_eq!( + format_action(&hook), + "terminal.Parser.Action{ .dcs_hook = .{ .intermediates = { 36, 32 }, \ + .params = { 1, 2, 300 }, .final = 113 } }" + ); + let empty = ParserAction::DcsHook(ghostty_vt::parser::Dcs::new(b"", &[], b'q')); + assert_eq!( + format_action(&empty), + "terminal.Parser.Action{ .dcs_hook = .{ .intermediates = { }, \ + .params = { }, .final = 113 } }" + ); + assert_eq!( + format_action(&ParserAction::DcsUnhook), + "terminal.Parser.Action{ .dcs_unhook = void }" + ); + assert_eq!( + format_action(&ParserAction::DcsPut(0x1b)), + "terminal.Parser.Action{ .dcs_put = 0x1b }" + ); + assert_eq!( + format_action(&ParserAction::ApcPut(0x41)), + "terminal.Parser.Action{ .apc_put = 0x41 }" + ); + } + #[test] fn encode_execute_names() { assert_eq!(encode_execute(0x07), "BEL (0x7)"); diff --git a/crates/ghostty-lib/src/inspector.rs b/crates/ghostty-lib/src/inspector.rs index cbafa16..bd35c60 100644 --- a/crates/ghostty-lib/src/inspector.rs +++ b/crates/ghostty-lib/src/inspector.rs @@ -54,8 +54,7 @@ impl Inspector { } // Setup our core inspector - let scale = f64::from(RtSurface::get_content_scale(surface.as_ref()).x); - ghostty_inspector::setup(&mut ctx, scale); + ghostty_inspector::setup(&mut ctx); if let Err(err) = core.activate_inspector() { log::error!("failed to activate inspector err={err}"); } diff --git a/docs/rewrite/DEVIANT_FIXES.md b/docs/rewrite/DEVIANT_FIXES.md index e90bfca..7a77d1a 100644 --- a/docs/rewrite/DEVIANT_FIXES.md +++ b/docs/rewrite/DEVIANT_FIXES.md @@ -98,6 +98,7 @@ Only entries with an observable behavioural difference are listed. | `+version` / build info | Real build metadata | Placeholder `build_info` values in places; `gtk-opengl-debug` follows `cfg!(debug_assertions)` (the `release` profile has them on, `production` off) | `notes/ghostty-cli.md`, `notes/ghostty-config.md` | 6286e02 | | OSC 52 base64 acceptance | Surface.zig:2149,2163 rules | Was different in both directions (Codex core audit); fixed to match Zig | `notes/review-core-codex.md` | 2f58f02 | | Benchmark / synthetic tooling extensions | `ghostty-bench` has no `--time` option and only the simplified terminal handler; `ghostty-synthetic` has the actions `ascii`, `osc`, `utf8` | `ghostty-bench --time` prints the measured step time to stderr; `ghostty-bench --handler=full` runs the full `stream_readonly` handler instead of the print-only one; `ghostty-synthetic +sgr` generates SGR-heavy output. Dev/benchmark tooling only; otherwise upstream behaviour is unchanged | `crates/ghostty-bench/src/main.rs`, `src/benches.rs`, `src/bin/ghostty-synthetic.rs`; `notes/review-rv16-codex.md` | 0c4f0ee | +| Inspector font rasterizer (Windows, Apple, wasm) | Dear ImGui is built with `IMGUI_ENABLE_FREETYPE` (`pkg/dcimgui`), so the 32 px inspector font is rasterized by FreeType | Linux and other Unix targets: FreeType (`dear-imgui-rs` feature `freetype`, system FreeType through pkg-config), 32 px. Windows, Apple and wasm32 keep Dear ImGui's stb_truetype loader at 32 px, because `dear-imgui-sys` can only locate FreeType through pkg-config/vcpkg and those targets cannot be built or run here. Glyph anti-aliasing differs slightly there. | `crates/ghostty-inspector/Cargo.toml`, `src/inspector.rs` (`embedded_font_source`) | Z20 (uncommitted) | ## Maintenance diff --git a/docs/rewrite/notes/final-Z20.md b/docs/rewrite/notes/final-Z20.md new file mode 100644 index 0000000..b228d4b --- /dev/null +++ b/docs/rewrite/notes/final-Z20.md @@ -0,0 +1,20 @@ +# Z20: RV17 P3 fixes (uncommitted) + +1. FreeType-only on Apple: `ghostty-font/build.rs` honours `font-backend-freetype` on every non-wasm + target (`coretext` cfgs off, so the embedded-emoji fallback applies, as in Zig `Discover == void`); + `backend.rs` OPTIONS and the `harfbuzz-sys/coretext` feature (now only for the coretext features, + because the SDK headers are needed otherwise). New test `selected_backend_matches_build_cfgs`. + `ghostty_surface_quicklook_font` now answers only for plain `.coretext` as in Zig + (`ghostty_font::face::quicklook_font_raw`). `scripts/macos-crosscheck.sh` takes + `MACOS_CHECK_FEATURES`; clippy passes on both Apple targets with and without the feature. +2. `cli/list_fonts.rs` test follows `ghostty_font::options.backend` / `HAS_DISCOVERY`. +3. Inspector: 32 px (`FONT_SIZE`), FreeType through `dear-imgui-rs/freetype` on non-Windows, non-Apple, + non-wasm targets (registered deviation elsewhere: stb_truetype). `setup` lost its scale parameter + (Zig has none). Visual check: new scenario `scripts/visual-parity.sh inspector` (ctrl+shift+i via + xdotool, production build vs /usr/bin/ghostty 1.3.1). The window position differs between runs, so the + tabs + table region was compared at the best-aligned offset (95,134): 0.031 % of pixels differ + (max channel delta 14); the list rows differ only by the hover highlight (pointer position). +4. DCS/APC descriptions follow Zig 0.15.2 `printValue(any)`; prefix is `terminal.Parser.Action` + (was `parser.Parser.Action`), checked by running Zig 0.15.2 (`{ 36, 32 }`, `{ }` for empty slices). +5. Demo window gate: `ghostty_debug_build` cfg from `PROFILE == "debug"` (`ghostty-inspector/build.rs`). +6. Metal buffer: `metal_layout::grown_buffer_bytes` + test; DEVIANT_FIXES row 34 (commit bab49ee). diff --git a/scripts/macos-crosscheck.sh b/scripts/macos-crosscheck.sh index 5431ee6..d16cbff 100755 --- a/scripts/macos-crosscheck.sh +++ b/scripts/macos-crosscheck.sh @@ -3,6 +3,8 @@ # Uses `zig cc` wrappers for the few C build scripts in the dependency # tree; real macOS builds need none of this. See docs/rewrite/notes/macos-M1.md. # Usage: scripts/macos-crosscheck.sh [check|clippy] [target] +# MACOS_CHECK_FEATURES="ghostty-font/font-backend-freetype" adds cargo features +# (for example the FreeType-only font backend on Apple). set -euo pipefail cd "$(dirname "$0")/.." MODE=${1:-clippy} @@ -54,8 +56,12 @@ CRATES=(-p ghostty-vt -p ghostty-unicode -p ghostty-datastruct -p ghostty-os -p -p ghostty-config -p ghostty-terminal -p ghostty-font -p ghostty-renderer -p ghostty-termio -p ghostty-core -p ghostty-apprt -p ghostty-inspector -p ghostty-lib -p ghostty-crash -p ghostty-bench -p ghostty-vt-capi) +FEATURES=() +if [ -n "${MACOS_CHECK_FEATURES:-}" ]; then + FEATURES=(--features "$MACOS_CHECK_FEATURES") +fi if [ "$MODE" = clippy ]; then - cargo clippy --target "$TARGET" "${CRATES[@]}" --lib --tests -- -D warnings + cargo clippy --target "$TARGET" "${CRATES[@]}" "${FEATURES[@]}" --lib --tests -- -D warnings else - cargo check --target "$TARGET" "${CRATES[@]}" --lib --tests + cargo check --target "$TARGET" "${CRATES[@]}" "${FEATURES[@]}" --lib --tests fi diff --git a/scripts/visual-parity.sh b/scripts/visual-parity.sh index f17fd0b..866f945 100755 --- a/scripts/visual-parity.sh +++ b/scripts/visual-parity.sh @@ -2,7 +2,8 @@ # Visual parity: capture original Ghostty and the Rust build in an isolated, # headless nested weston and compare pixel-wise. # -# scripts/visual-parity.sh [scenario...] (default: page page-bar tabs) +# scripts/visual-parity.sh [scenario...] (default: page page-bar tabs; +# also available: inspector = ctrl+shift+i via xdotool) # # env: RUST_BIN (default target/production/ghostty), REF_BIN (default # /usr/bin/ghostty), OUT (default target/visual-parity), W/H output size. @@ -22,7 +23,7 @@ WORK=$(mktemp -d "$OUT/work.XXXX") SOCK=vp$$ XWL= -case " $SCEN " in *" tabs "*) XWL=xwayland=true; mkdir -p /tmp/.X11-unix ;; esac +case " $SCEN " in *" tabs "*|*" inspector "*) XWL=xwayland=true; mkdir -p /tmp/.X11-unix ;; esac cat >"$WORK/weston.ini" <"$WORK/$label-$scen.log" 2>&1 & local pid=$! sleep 6 - xdotool_keys + xdotool_keys "$scen" sleep 3 shoot "$out" kill $pid 2>/dev/null; wait $pid 2>/dev/null @@ -106,6 +107,10 @@ run_one() { xwl_display() { grep -o "display :[0-9]*" "$WORK/weston.log" | tail -1 | sed 's/display //'; } xdotool_keys() { export DISPLAY=$(xwl_display) + if [ "${1:-}" = inspector ]; then + xdotool key --clearmodifiers ctrl+shift+i; sleep 3 + return + fi xdotool key --clearmodifiers ctrl+shift+t; sleep 1 xdotool key --clearmodifiers ctrl+shift+o; sleep 1 xdotool key --clearmodifiers ctrl+shift+e; sleep 1