diff --git a/src/XTerm.NET.Tests/Graphics/KittyPlaceholderTests.cs b/src/XTerm.NET.Tests/Graphics/KittyPlaceholderTests.cs index 370391a..58f2373 100644 --- a/src/XTerm.NET.Tests/Graphics/KittyPlaceholderTests.cs +++ b/src/XTerm.NET.Tests/Graphics/KittyPlaceholderTests.cs @@ -331,24 +331,55 @@ public void Placeholders_do_nothing_when_kitty_is_switched_off() Assert.False(ImageAssertions.IsImageAt(terminal, 0, 0)); } - /// Placeholder cells are cells, so everything that happens to text happens to them. + /// + /// A placeholder tile is CONTENT: the picture is there because the cell holds the placeholder + /// character, so printing over the cell takes that tile with it and leaves the rest. + /// + /// + /// This test used to assert the opposite — that the picture survived the write — because the + /// tile was stored as a classic Kitty overlay. That reading leaves no way to ever remove a + /// tile: the protocol has no "delete the tile at this cell" command precisely because + /// overwriting the cell IS the deletion. It is how kitty itself behaves and what image.nvim + /// relies on when it redraws. Stored as an overlay, an application that drew a dialog across a + /// picture kept the picture on top of the dialog, permanently. + /// [Fact] - public void A_placeholder_picture_is_an_overlay_rather_than_content() + public void Printing_over_a_placeholder_cell_removes_that_tile() { var terminal = WithStoredImage(5); terminal.Write(SelectImageId(5) + Placeholder + Placeholder); terminal.Write($"{Esc}[1;1H{Esc}[39mX"); - // The character lands and the picture stays: a placeholder places a Kitty run, and a Kitty - // run is an overlay rather than content. + // The character lands and only its own tile goes; the neighbour keeps showing. Assert.Equal("X", Cell(terminal, 0, 0).Content); - Assert.True(ImageAssertions.IsImageAt(terminal, 0, 0)); + Assert.False(ImageAssertions.IsImageAt(terminal, 0, 0)); Assert.True(ImageAssertions.IsImageAt(terminal, 1, 0)); - // Erasing takes both. + // Erasing takes the rest. terminal.Write($"{Esc}[2J"); - Assert.False(ImageAssertions.IsImageAt(terminal, 0, 0)); Assert.False(ImageAssertions.IsImageAt(terminal, 1, 0)); } + + /// + /// The other direction of the split: writing one placeholder cell of a NEW picture over one + /// cell of an old one replaces exactly that tile. + /// + [Fact] + public void A_new_placeholder_over_an_old_tile_replaces_it() + { + var terminal = WithStoredImage(5); + terminal.Write($"{Esc}_Ga=t,i=7,f=32,s=4,v=6,q=2;{SolidRgba(4, 6, 30)}{St}"); + terminal.Write(SelectImageId(5) + Placeholder + Placeholder); + + var oldImage = ImageAssertions.ImageAt(terminal, 0, 0); + + terminal.Write($"{Esc}[1;1H" + SelectImageId(7) + Placeholder); + + var newImage = ImageAssertions.ImageAt(terminal, 0, 0); + Assert.NotNull(newImage); + Assert.False(ReferenceEquals(oldImage, newImage), "the tile should now show the new picture"); + Assert.True(ReferenceEquals(oldImage, ImageAssertions.ImageAt(terminal, 1, 0)), + "the neighbouring tile still belongs to the old picture"); + } } diff --git a/src/XTerm.NET.Tests/Graphics/KittyZIndexTests.cs b/src/XTerm.NET.Tests/Graphics/KittyZIndexTests.cs index d32dfd2..eda61f0 100644 --- a/src/XTerm.NET.Tests/Graphics/KittyZIndexTests.cs +++ b/src/XTerm.NET.Tests/Graphics/KittyZIndexTests.cs @@ -55,6 +55,23 @@ private static string Content(Terminal terminal, int col, int row) // ---- ordering between pictures ---------------------------------------------------------------- + /// + /// Printing over a classic placement leaves it whole — the z-index decides what shows. + /// Pinned here because placeholder tiles took the OPPOSITE behaviour (they are content and a + /// write removes the tile), and the split that implements that must not reach these. + /// + [Fact] + public void Text_printed_over_a_classic_placement_leaves_it_whole() + { + var terminal = WithTwoImages(); + PlaceAt(terminal, 1, 0, 0, z: 0); + + terminal.Write($"{Esc}[1;1HX"); + + Assert.Equal("X", Content(terminal, 0, 0)); + Assert.Single(ImageAssertions.StackAt(terminal, 0, 0)); + } + /// The stack at a cell is ordered by z, front first. [Fact] public void The_higher_z_picture_is_in_front() diff --git a/src/XTerm.NET/Buffer/BufferLine.cs b/src/XTerm.NET/Buffer/BufferLine.cs index b1b5d4e..e5b08f0 100644 --- a/src/XTerm.NET/Buffer/BufferLine.cs +++ b/src/XTerm.NET/Buffer/BufferLine.cs @@ -957,12 +957,14 @@ internal bool RemovePlacements(Func predicate) /// Splits any Sixel run covering around the text just written there. /// /// - /// Sixel semantics: printing replaces that part of the picture. With tiles in cells this + /// Content semantics: printing replaces that part of the picture. With tiles in cells this /// happened for free, because the write overwrote the cell; with runs it has to be done on /// purpose. The run becomes the fragments either side, each with its source rectangle narrowed - /// to match, so the rest of the picture survives a character landing in the middle of it. - /// Kitty runs are left alone — there the z-index decides what is on top, and text never - /// modifies a placement. + /// to match, so the rest of the picture survives a character landing in the middle of it. Sixel + /// is content, and so is a placeholder tile — it exists because its cell holds the placeholder + /// character, and overwriting the cell is the only way the protocol offers to remove it. + /// Classic Kitty runs are left alone — there the z-index decides what is on top, and text + /// never modifies a placement. /// Guarded on a null field at every call site, so a line without pictures — which is /// nearly every line — pays a single test. /// @@ -977,10 +979,10 @@ internal void SplitPlacementsAt(int column, bool includeOverlays = false) if (!placement.Covers(column)) continue; - // Printing only splits a Sixel, because only a Sixel is content. ERASING splits both: - // a cleared cell is blank, and a picture still showing through one would be a leak - // whichever protocol put it there. - if (!includeOverlays && placement.Kind != Graphics.PlacementKind.Sixel) + // Printing splits content — Sixel, and placeholder tiles — and leaves classic Kitty + // overlays alone. ERASING splits all of them: a cleared cell is blank, and a picture + // still showing through one would be a leak whichever protocol put it there. + if (!includeOverlays && placement.Kind == Graphics.PlacementKind.Kitty) continue; _placements.RemoveAt(i); @@ -1101,6 +1103,18 @@ public void CopyFrom(BufferLine line) _isWrapped = line._isWrapped; _lineAttribute = line._lineAttribute; + // The runs are the picture, exactly as in Clone above -- a copy that skipped them would + // silently lose it. Replaced rather than merged, like every field here: a recycled line + // that once held a picture must not keep showing it. The lists are copied but the images + // they name are shared; a stored picture is never mutated in place (a retransmission + // builds a new one), so a reader holding this copy is safe against later writes. + _placements = line._placements is null + ? null + : new List(line._placements); + _images = line._images is null + ? null + : new List(line._images); + // Assigned, not OR-ed: this REPLACES the line's contents rather than adding to them, so // the latch belongs to the incoming cells. A recycled scrollback line that once held a // wide character would otherwise carry that latch forever. diff --git a/src/XTerm.NET/Graphics/LinePlacement.cs b/src/XTerm.NET/Graphics/LinePlacement.cs index 58a2028..3aec39b 100644 --- a/src/XTerm.NET/Graphics/LinePlacement.cs +++ b/src/XTerm.NET/Graphics/LinePlacement.cs @@ -16,6 +16,20 @@ public enum PlacementKind /// the placement at all. /// Kitty = 1, + + /// + /// Kitty graphics shown by Unicode placeholder cells: the cell IS the picture, so a write into + /// it takes the tile with it, exactly as Sixel content behaves. + /// + /// + /// Distinct from because the two protocols disagree about what a write + /// means. A classic placement is an overlay and survives text; a placeholder tile exists + /// BECAUSE the cell holds the placeholder character, so a client erases a picture by + /// overwriting its cells and nothing else — there is no escape sequence for "remove the tile + /// at this cell". Left as an overlay, a dialog drawn across a picture kept the picture on top + /// of it, permanently. + /// + Placeholder = 2, } /// diff --git a/src/XTerm.NET/InputHandler.KittyGraphics.cs b/src/XTerm.NET/InputHandler.KittyGraphics.cs index 187ba61..5f79858 100644 --- a/src/XTerm.NET/InputHandler.KittyGraphics.cs +++ b/src/XTerm.NET/InputHandler.KittyGraphics.cs @@ -157,7 +157,8 @@ private bool WritePlaceholderCell(int row, int col, Graphics.TerminalImage image return false; // The cell keeps the placeholder character it was printed with; the picture is beside it - // rather than in it. Written before the run so SetCell's Sixel split cannot see it. + // rather than in it. Written before the run so SetCell's content split cannot see it -- + // and so that the split clears any stale tile another picture left at this cell. var cell = new BufferCell(" ", 1, _curAttr); line.SetCell(col, ref cell); @@ -169,7 +170,7 @@ private bool WritePlaceholderCell(int row, int col, Graphics.TerminalImage image new Graphics.LinePlacement( image.Id, col, 1, srcX: srcX, srcY: srcY, srcWidth: srcWidth, srcHeight: srcHeight, - kind: Graphics.PlacementKind.Kitty, + kind: Graphics.PlacementKind.Placeholder, serial: serial), image); return true;