From 4d10798eb201ef68ac23d5b401fe9c986c076968 Mon Sep 17 00:00:00 2001 From: "a-team-app[bot]" <334837322+a-team-app[bot]@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:34:10 +1300 Subject: [PATCH] About draws its artwork with ImageView: kitty graphics in kitty and Ghostty About now draws through Terminal.Gui 2.5's ImageView: kitty graphics where TG detects kitty or Ghostty outside tmux, sixel where our probe finds it, and the ASCII art otherwise. Our probe's sixel answer and cell size are handed to the driver on every open, and inside tmux the kitty graphics TG detects from a leaked KITTY_WINDOW_ID are turned off. The host keeps one ImageView across opens so its sixel encoding is reused. F12 Diagnostics gains an Image protocol row naming the one About uses. Closes #537 Co-Authored-By: Claude Opus 5.5 (1M context) --- .tui/TuiCode.workspaces.json | 5 + AGENTS.md | 9 +- src/TuiCode.Workbench/About/AboutView.cs | 95 +++++----- src/TuiCode.Workbench/About/ImageProtocols.cs | 44 +++++ src/TuiCode.Workbench/About/SixelProbe.cs | 5 - .../Diagnostics/DiagnosticsView.cs | 18 +- src/TuiCode.Workbench/WorkbenchHost.cs | 26 ++- tests/TuiCode.Tests/AboutImageHostTests.cs | 162 ++++++++++++++++++ tests/TuiCode.Tests/AboutViewTests.cs | 3 +- tests/TuiCode.Tests/WorkbenchHostTests.cs | 6 +- 10 files changed, 306 insertions(+), 67 deletions(-) create mode 100644 src/TuiCode.Workbench/About/ImageProtocols.cs create mode 100644 tests/TuiCode.Tests/AboutImageHostTests.cs diff --git a/.tui/TuiCode.workspaces.json b/.tui/TuiCode.workspaces.json index 35eb4ef4..f7a29a92 100644 --- a/.tui/TuiCode.workspaces.json +++ b/.tui/TuiCode.workspaces.json @@ -1,4 +1,9 @@ [ + { + "Folder": "/Users/jamescrosswell/code/TuiCode/about-kitty", + "Files": [], + "Active": null + }, { "Folder": "/Users/jamescrosswell/code/TuiCode/tabstrip-no-default-key", "Files": [ diff --git a/AGENTS.md b/AGENTS.md index 80c00a44..ed6e8af5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -237,13 +237,14 @@ What follows is how TuiCode implements it today. ## About dialog (#107) -- `tui` opens `AboutView`. If the terminal supports sixel (WezTerm, iTerm2, tmux 3.6, Windows Terminal, foot…) it shows the artwork as an image. Otherwise (kitty, Ghostty, Terminal.app — TG 2.1.0 has no kitty graphics) it shows ASCII art. +- `tui` opens `AboutView`, which draws the artwork with TG's `ImageView` (#537): through kitty graphics in kitty and Ghostty, through sixel in WezTerm, iTerm2, tmux 3.6, Windows Terminal, foot…, and as ASCII art where neither works (Alacritty, Terminal.app). We don't use `ImageView`'s coloured-cell fallback: at the dialog's size it doesn't read as the logo. F12 Diagnostics' **Image protocol** row says which one About uses. +- `ImageProtocols.Choose` decides. Kitty graphics comes from TG's own detection (`IDriver.KittyGraphicsSupport`, which goes by `KITTY_WINDOW_ID` / `TERM_PROGRAM`), except inside tmux (`TMUX`), which needs passthrough for it. Sixel comes from our probe. `ImageProtocols.Configure` then hands the driver our answer and cell size (`SetSixelSupport`, `SetKittyGraphicsSupport`) on every open, so TG's own sixel detection, which gets both wrong below, doesn't decide; in tmux it also turns off the kitty graphics TG detected from a leaked `KITTY_WINDOW_ID`. - `SixelProbe.Detect` sends DA1 once at startup (`WorkbenchHost` ctor), so the dialog knows up front and shows a spinner, not ASCII art that gets replaced. `SixelProbe.MeasureCell` runs each time About opens, because the cell's pixel size changes when the window moves between a Retina and a non-Retina screen. We don't use TG's `SixelSupportDetector`: it asks `CSI 16 t` first and iTerm2 never answers, which costs TG's 1 s abandon timeout, and its fallback (window pixels ÷ cells) counts the title bar and margins. `MeasureCell` sends iTerm2's `OSC 1337 ; ReportCellSize` and `CSI 16 t` together and takes the first answer. `Detect` also parses tmux's DA1 reply (`…;4c`), which TG's check misses. - iTerm2 reports sizes in points everywhere, its `CSI 14 t` reply included. Only `ReportCellSize` carries the scale (`2.0` on Retina), so `ParseIterm2CellSize` multiplies by it (clamped to 1–4; absent in older iTerm2s, meaning 1). Sixels are drawn in device pixels, so ignoring it drew the About image at a quarter of its area (#333). - F12 Diagnostics runs `MeasureCell` too each time it opens and shows the size with the reply it came from (`CellMeasurement.Source`), or that none came and 10 × 20 was assumed (#335). -- The cell size has to be exact: iTerm2 blanks every row an image touches, so an image that ends mid-row leaves a dark band (`AboutImage.Fit` rounds down to whole rows and `Cover` trims the top/bottom to match). -- Encoding runs in the background and is cached per pixel size, so reopening shows the image immediately. -- TG re-emits queued sixels on every output write and only rewrites cells whose contents changed, so `AboutView.Dispose` dequeues its sixel and sets `ClearScreenNextIteration`. Otherwise the image stays on screen after close. +- The cell size has to be exact: iTerm2 blanks every row an image touches, so an image that ends mid-row leaves a dark band. `AboutImage.Fit` rounds down to whole rows and `Cover` trims the top/bottom to match, so `ImageView` gets an image exactly the size of its viewport and draws it unscaled. The driver takes a whole-pixel cell size, so `Configure` rounds ours. +- The host keeps one `ImageView` (`AboutView.CreateImage`) across opens, since it caches its sixel encoding, and the scaled image is cached per pixel size, so reopening shows the image straight away. `AboutView.Dispose` takes it out before disposing the rest. Its pan and zoom keys and mouse bindings are off: About is a picture, not a viewer. +- TG deletes a raster image once its view leaves the hierarchy, but `AboutView.Dispose` still sets `ClearScreenNextIteration` after drawing one, for terminals that keep a sixel until its cells are written again. - The artwork is `assets/about.png`, baked into the embedded `About/about.rgb.z` (raw RGB, zlib) by `dotnet run scripts/update-about-image.cs`. We have no image decoder at runtime and don't want one in the AOT binary. ## File icons (#119) diff --git a/src/TuiCode.Workbench/About/AboutView.cs b/src/TuiCode.Workbench/About/AboutView.cs index faf4d098..d6dbd740 100644 --- a/src/TuiCode.Workbench/About/AboutView.cs +++ b/src/TuiCode.Workbench/About/AboutView.cs @@ -1,6 +1,4 @@ -using Point = System.Drawing.Point; using Size = System.Drawing.Size; -using Terminal.Gui.Drivers; using TuiCode.Abstractions; using TuiCode.Workbench.Services; @@ -31,20 +29,21 @@ public sealed class AboutView : Window private const int ChromeHeight = 8; private readonly AboutArt _art; - private static readonly System.Collections.Concurrent.ConcurrentDictionary EncodedImages = new(); + private readonly ImageView _image; + private static readonly System.Collections.Concurrent.ConcurrentDictionary ScaledImages = new(); - private IApplication? _app; - private SixelToRender? _sixel; private bool _disposed; public IKeybindingService Scope => _scopeKeybindings; internal bool IsLoading => _art.Mode == ArtMode.Loading; internal bool ShowsAsciiArt => _art.Mode == ArtMode.Ascii; + internal bool ShowsImage => _art.Mode == ArtMode.Image; public event EventHandler? Closed; - public AboutView(string version) + /// Kept by the caller across opens, so it doesn't encode the image again. + public AboutView(string version, ImageView image) { Title = "About"; BorderStyle = LineStyle.Single; @@ -62,6 +61,12 @@ public AboutView(string version) Height = Art.Length, }; + _image = image; + _image.X = 1; + _image.Y = 1; + _image.Width = Dim.Fill(1); + _image.Visible = false; + var details = new Label { X = Pos.Center(), @@ -77,7 +82,7 @@ public AboutView(string version) Text = "Esc · Enter close", }; - Add(art, details, footer); + Add(art, _image, details, footer); _scopeCommands = new CommandService(); _scopeKeybindings = new KeybindingService(_scopeCommands); @@ -86,60 +91,63 @@ public AboutView(string version) _scopeKeybindings.Bind("Enter", CommandIds.AboutClose); } - /// - /// Shows the artwork as a sixel image if allows, else the ASCII art. Null means - /// detection is still running. Call once the view is added. - /// - internal void Present(SixelSupport? support) + /// An for : a picture, without pan and zoom. + public static ImageView CreateImage() { - if (_disposed || App is not { Driver: { } driver } app) return; - - if (support is null) + var image = new ImageView { - StartLoading(app); - return; - } + CanFocus = false, + UseBackgroundRendering = true, + MaxSixelPaletteColors = 256, + }; + image.MouseBindings.Clear(); + return image; + } - if (!support.IsSupported) - { - _art.Mode = ArtMode.Ascii; - _art.SetNeedsDraw(); - return; - } + internal void ShowAsciiArt() + { + _image.Visible = false; + _art.Mode = ArtMode.Ascii; + _art.SetNeedsDraw(); + } + + /// Shows the artwork through whichever protocol the driver has been given, to a cell. + internal void ShowImage(Size cellPixels) + { + if (_disposed || App is not { } app) return; var columns = Art.Max(line => line.Length); - var (pixels, rows) = AboutImage.Fit(AboutImage.ReadSize(), support.CellPixels, columns); + var (pixels, rows) = AboutImage.Fit(AboutImage.ReadSize(), cellPixels, columns); _art.Height = rows; + _image.Height = rows; Height = rows + ChromeHeight; - if (EncodedImages.TryGetValue(pixels, out var cached)) + if (ScaledImages.TryGetValue(pixels, out var cached)) { - ShowSixel(app, driver, cached); + Show(cached); return; } - StartLoading(app); + ShowLoading(); Task.Run(() => { - var data = EncodedImages.GetOrAdd(pixels, size => new SixelEncoder().EncodeSixel(AboutImage.Cover(AboutImage.Load(), size))); - app.Invoke(() => ShowSixel(app, driver, data)); + var image = ScaledImages.GetOrAdd(pixels, size => AboutImage.Cover(AboutImage.Load(), size)); + app.Invoke(() => Show(image)); }); } - private void ShowSixel(IApplication app, IDriver driver, string data) + private void Show(Color[,] image) { if (_disposed) return; - _app = app; - _sixel = new SixelToRender { Id = "about", SixelData = data }; - _art.Sixel = _sixel; + if (!ReferenceEquals(_image.Image, image)) _image.Image = image; + _image.Visible = true; _art.Mode = ArtMode.Image; - driver.GetOutput().GetSixels().Enqueue(_sixel); _art.SetNeedsDraw(); } - private void StartLoading(IApplication app) + internal void ShowLoading() { - if (_art.Mode == ArtMode.Loading) return; + if (_disposed || App is not { } app || _art.Mode == ArtMode.Loading) return; _art.Mode = ArtMode.Loading; _art.SetNeedsDraw(); app.AddTimeout(TimeSpan.FromMilliseconds(80), () => @@ -153,14 +161,11 @@ private void StartLoading(IApplication app) protected override void Dispose(bool disposing) { - if (disposing && !_disposed && _sixel is not null && _app?.Driver is { } driver) + if (disposing && !_disposed) { - var sixels = driver.GetOutput().GetSixels(); - var others = sixels.Where(s => !ReferenceEquals(s, _sixel)).ToList(); - sixels.Clear(); - foreach (var other in others) sixels.Enqueue(other); - // TG only rewrites cells whose contents changed, which would leave the image on screen. - _app.ClearScreenNextIteration = true; + Remove(_image); + // Some terminals keep a sixel until the cells under it are written again. + if (_art.Mode == ArtMode.Image && App is { } app) app.ClearScreenNextIteration = true; } _disposed = true; base.Dispose(disposing); @@ -175,7 +180,6 @@ private sealed class AboutArt : View public ArtMode Mode { get; set; } public int SpinnerFrame { get; set; } - public SixelToRender? Sixel { get; set; } protected override bool OnDrawingContent(DrawContext? context) { @@ -183,7 +187,6 @@ protected override bool OnDrawingContent(DrawContext? context) switch (Mode) { case ArtMode.Image: - Sixel!.ScreenPosition = ViewportToScreen(Point.Empty); break; case ArtMode.Loading: Move(Viewport.Width / 2, Viewport.Height / 2); diff --git a/src/TuiCode.Workbench/About/ImageProtocols.cs b/src/TuiCode.Workbench/About/ImageProtocols.cs new file mode 100644 index 00000000..781b06ec --- /dev/null +++ b/src/TuiCode.Workbench/About/ImageProtocols.cs @@ -0,0 +1,44 @@ +using Terminal.Gui.Drivers; +using Size = System.Drawing.Size; +using SizeF = System.Drawing.SizeF; + +namespace TuiCode.Workbench.About; + +internal enum ImageProtocol { None, Sixel, Kitty } + +internal static class ImageProtocols +{ + /// Kitty graphics where Terminal.Gui found it outside tmux, else sixel where our probe found it. Null while that's unknown. + public static ImageProtocol? Choose(bool kittyDetected, bool inTmux, bool? sixel) => + kittyDetected && !inTmux ? ImageProtocol.Kitty + : sixel switch + { + true => ImageProtocol.Sixel, + false => ImageProtocol.None, + null => null, + }; + + public static string Describe(ImageProtocol? protocol) => protocol switch + { + ImageProtocol.Kitty => "kitty graphics", + ImageProtocol.Sixel => "sixel", + ImageProtocol.None => "none (ASCII art)", + _ => "asking the terminal…", + }; + + /// Tells the driver what our probe found, so Terminal.Gui's own detection doesn't decide. Returns the cell size it was given. + public static Size Configure(IDriver driver, ImageProtocol protocol, SizeF cellPixels) + { + var cell = new Size((int)Math.Round(cellPixels.Width), (int)Math.Round(cellPixels.Height)); + if (protocol == ImageProtocol.Kitty) + { + driver.SetKittyGraphicsSupport(new KittyGraphicsSupportResult { IsSupported = true, Resolution = cell }); + return cell; + } + + if (driver.KittyGraphicsSupport is { IsSupported: true }) + driver.SetKittyGraphicsSupport(new KittyGraphicsSupportResult { IsSupported = false, Resolution = cell }); + driver.SetSixelSupport(new SixelSupportResult { IsSupported = protocol == ImageProtocol.Sixel, Resolution = cell }); + return cell; + } +} diff --git a/src/TuiCode.Workbench/About/SixelProbe.cs b/src/TuiCode.Workbench/About/SixelProbe.cs index 26b97d89..85d2734c 100644 --- a/src/TuiCode.Workbench/About/SixelProbe.cs +++ b/src/TuiCode.Workbench/About/SixelProbe.cs @@ -5,11 +5,6 @@ namespace TuiCode.Workbench.About; -internal sealed record SixelSupport(bool IsSupported, SizeF CellPixels) -{ - public static readonly SixelSupport Unsupported = new(false, SizeF.Empty); -} - internal enum CellSizeSource { Iterm2Report, CellResolutionReply, Assumed } internal sealed record CellMeasurement(SizeF Pixels, CellSizeSource Source, float Scale = 1); diff --git a/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs b/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs index e3da5737..cbf47155 100644 --- a/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs +++ b/src/TuiCode.Workbench/Diagnostics/DiagnosticsView.cs @@ -19,13 +19,14 @@ public sealed class DiagnosticsView : Window private readonly Label _keyRune; private readonly Label _cursorColour; private readonly Label _cellSize; + private readonly Label _imageProtocol; private readonly string? _askedCursorColour; public IKeybindingService Scope => _scopeKeybindings; public event EventHandler? Closed; - public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? cursorColour = null) + public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? cursorColour = null, string imageProtocol = "") { _askedCursorColour = cursorColour; Title = "Diagnostics"; @@ -39,7 +40,8 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? var kittyStatusText = string.Join("\n", kittyStatusLines); var cursorColourY = 2 + kittyStatusLines.Length; var cellSizeY = cursorColourY + 1; - var keyHeadingY = cellSizeY + 2; + var imageProtocolY = cellSizeY + 1; + var keyHeadingY = imageProtocolY + 2; var nameY = keyHeadingY + 1; var hintY = nameY + 6; Height = hintY + 6; @@ -86,6 +88,15 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? Text = "asking the terminal…", }; + var imageProtocolHeading = Heading("Image protocol", imageProtocolY); + _imageProtocol = new Label + { + X = FieldX, + Y = imageProtocolY, + Width = GetFieldWidth(), + Text = imageProtocol, + }; + var keyHeading = Heading("Last key", keyHeadingY); var nameLabel = Heading(" Name", nameY); var hexLabel = Heading(" Hex", nameY + 1); @@ -114,6 +125,7 @@ public DiagnosticsView(string driverName, string kittyNegotiationStatus, string? Add(driverHeading, driverNameLabel, kittyHeading, kittyStatusLabel, cursorColourHeading, _cursorColour, cellSizeHeading, _cellSize, + imageProtocolHeading, _imageProtocol, keyHeading, nameLabel, hexLabel, baseLabel, runeLabel, _keyName, _keyHex, _keyBase, _keyRune, hint, footer); @@ -178,6 +190,8 @@ public void ShowTerminalCursorColour(string? reported) public string CellSizeText => _cellSize.Text; + public string ImageProtocolText => _imageProtocol.Text; + internal void ShowCellSize(CellMeasurement cell) => _cellSize.Text = DescribeCellSize(cell); private static string DescribeCellSize(CellMeasurement cell) diff --git a/src/TuiCode.Workbench/WorkbenchHost.cs b/src/TuiCode.Workbench/WorkbenchHost.cs index 8a5daded..3302e14b 100644 --- a/src/TuiCode.Workbench/WorkbenchHost.cs +++ b/src/TuiCode.Workbench/WorkbenchHost.cs @@ -99,6 +99,7 @@ public sealed class WorkbenchHost : IDisposable private DiagnosticsView? _activeDiagnostics; private string? _cursorColour; private AboutView? _activeAbout; + private readonly ImageView _aboutImage = AboutView.CreateImage(); private DocumentInfoView? _activeDocumentInfo; private BlameView? _activeBlame; private bool? _sixelSupported; @@ -2210,7 +2211,7 @@ private void OpenAbout() { if (_activeAbout is not null) return; - var view = new AboutView(AppVersion.Current); + var view = new AboutView(AppVersion.Current, _aboutImage); view.Closed += (_, _) => CloseAbout(view); _activeAbout = view; _workbench.Add(view); @@ -2222,20 +2223,32 @@ private void OpenAbout() private void PresentAbout(AboutView view) { - if (_sixelSupported is not true || _app.Driver is not { } driver) + if (_app.Driver is not { } driver || AboutImageProtocol() is not { } protocol) { - view.Present(_sixelSupported is null ? null : SixelSupport.Unsupported); + view.ShowLoading(); + return; + } + + if (protocol == ImageProtocol.None) + { + view.ShowAsciiArt(); return; } - view.Present(null); + view.ShowLoading(); // Measured on every open: the window may have moved to a screen with another scale since the last one. SixelProbe.MeasureCell(driver, cell => _app.Invoke(() => { - if (ReferenceEquals(_activeAbout, view)) view.Present(new SixelSupport(true, cell.Pixels)); + if (ReferenceEquals(_activeAbout, view)) view.ShowImage(ImageProtocols.Configure(driver, protocol, cell.Pixels)); })); } + internal ImageProtocol? AboutImageProtocol() => + _app.Driver is { } driver + ? ImageProtocols.Choose(driver.KittyGraphicsSupport is { IsSupported: true }, + !string.IsNullOrEmpty(_environment.GetEnvironmentVariable("TMUX")), _sixelSupported) + : ImageProtocol.None; + private void CloseAbout(AboutView view) { if (!ReferenceEquals(_activeAbout, view)) return; @@ -3557,7 +3570,7 @@ private void OpenDiagnostics() var driverName = _app.Driver?.GetName() ?? "Unknown"; var kittyNegotiationStatus = GetKittyNegotiationStatus(); - var view = new DiagnosticsView(driverName, kittyNegotiationStatus, _cursorColour); + var view = new DiagnosticsView(driverName, kittyNegotiationStatus, _cursorColour, ImageProtocols.Describe(AboutImageProtocol())); view.Closed += (_, _) => CloseDiagnostics(view); _activeDiagnostics = view; _workbench.Add(view); @@ -3645,6 +3658,7 @@ public void Dispose() _find.Dispose(); _terminalCursors.Dispose(); _workbench.Dispose(); + _aboutImage.Dispose(); _app.Dispose(); // Tell WezTerm and kitty to stop remapping keys; matches the startup activation. // Emitted post-Dispose so it reaches the live terminal after TG restores it. diff --git a/tests/TuiCode.Tests/AboutImageHostTests.cs b/tests/TuiCode.Tests/AboutImageHostTests.cs new file mode 100644 index 00000000..51a97769 --- /dev/null +++ b/tests/TuiCode.Tests/AboutImageHostTests.cs @@ -0,0 +1,162 @@ +using Terminal.Gui.Drawing; +using Terminal.Gui.Drivers; +using Terminal.Gui.Views; +using TuiCode.Abstractions; +using TuiCode.Workbench; +using TuiCode.Workbench.About; +using TuiCode.Workbench.Diagnostics; +using TuiCode.Workbench.Parts; +using TuiCode.Explorer; +using TuiCode.Workbench.Services; +using Size = System.Drawing.Size; +using SizeF = System.Drawing.SizeF; + +namespace TuiCode.Tests; + +public class ImageProtocolsTests +{ + [Theory] + [InlineData(true, false, false, "Kitty")] + [InlineData(true, false, null, "Kitty")] + [InlineData(true, false, true, "Kitty")] + [InlineData(false, false, true, "Sixel")] + [InlineData(false, false, false, "None")] + [InlineData(true, true, true, "Sixel")] + [InlineData(true, true, false, "None")] + [InlineData(false, false, null, null)] + [InlineData(true, true, null, null)] + public void Choose_prefers_kitty_graphics_outside_tmux_then_sixel_then_the_ascii_art( + bool kitty, bool inTmux, bool? sixel, string? expected) => + Assert.Equal(expected, ImageProtocols.Choose(kitty, inTmux, sixel)?.ToString()); + + [Theory] + [InlineData("Kitty", "kitty graphics")] + [InlineData("Sixel", "sixel")] + [InlineData("None", "none (ASCII art)")] + public void Describe_names_how_About_draws_the_artwork(string protocol, string expected) => + Assert.Equal(expected, ImageProtocols.Describe(Enum.Parse(protocol))); + + [Fact] + public void Diagnostics_shows_the_image_protocol_on_its_own_row() + { + using var view = new DiagnosticsView("ansi", "No", imageProtocol: "kitty graphics"); + view.Layout(new Size(200, 100)); + + var labels = view.SubViews.OfType