Git_Mirror/Terminal.Gui
1
0
Fork 0
mirror of https://github.com/gui-cs/Terminal.Gui.git synced 2025-12-26 15:57:56 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fixes #4258 - Glyphs drawn at mid-point of wide glyphs don't get drawn with clipping (#4462)

Browse Source 
* Enhanced `View.Drawing.cs` with improved comments, a new
`DoDrawComplete` method for clip region updates, and
clarified terminology. Added detailed remarks for the
`OnDrawComplete` method and `DrawComplete` event.

Refactored `ViewDrawingClippingTests` to simplify driver
setup, use target-typed `new`, and add a new test for wide
glyph clipping with bordered subviews. Improved handling of
edge cases like empty viewports and nested clips.

Added `WideGlyphs.DrawFlow.md` and
`ViewDrawingClippingTests.DrawFlow.md` to document the draw
flow, clipping behavior, and coordinate systems for both the
scenario and the test.

Commented out redundant `Driver.Clip` initialization in
`ApplicationImpl`. Added a `BUGBUG` comment in `Border` to
highlight missing redraw logic for `LineStyle` changes.

* Uncomment Driver.Clip initialization in Screen redraw

* Fixed it!

* Fixes #4258 - Correct wide glyph and border rendering

Refactored `OutputBufferImpl.AddStr` to improve handling of wide glyphs:
- Wide glyphs now modify only the first column they occupy, leaving the second column untouched.
- Removed redundant code that set replacement characters and marked cells as not dirty.
- Synchronized cursor updates (`Col` and `Row`) with the buffer lock to prevent race conditions.
- Modularized logic with helper methods for better readability and maintainability.

Updated `WideGlyphs.cs`:
- Removed dashed `BorderStyle` and added border thickness and subview for `arrangeableViewAtEven`.
- Removed unused `superView` initialization.

Enhanced tests:
- Added unit tests to verify correct rendering of borders and content at odd columns overlapping wide glyphs.
- Updated existing tests to reflect the new behavior of wide glyph handling.
- Introduced `DriverAssert.AssertDriverOutputIs` to validate raw ANSI output.

Improved documentation:
- Expanded problem description and root cause analysis in `WideGlyphBorderBugFix.md`.
- Detailed the fix and its impact, ensuring proper layering of content at any column position.

General cleanup:
- Removed unused imports and redundant code.
- Improved code readability and maintainability.

* Code cleanup

* Update Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingClippingTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update Terminal.Gui/Drivers/OutputBufferImpl.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingClippingTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingClippingTests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fixed test slowness problem

* Simplified

* Rmoved temp .md files

* Refactor I/O handling and improve testability

Refactored `InputProcessor` and `Output` access by replacing direct property usage with `GetInputProcessor()` and `GetOutput()` methods to enhance encapsulation. Introduced `GetLastOutput()` and `GetLastBuffer()` methods for better debugging and testability.

Centralized `StringBuilder` usage in `OutputBase` implementations to ensure consistency. Improved exception handling with clearer messages. Updated tests to align with the refactored structure and added a new test for wide glyph handling.

Enhanced ANSI sequence handling and simplified cursor visibility logic to prevent flickering. Standardized method naming for consistency. Cleaned up redundant code and improved documentation for better developer clarity.

* Refactored `NetOutput`, `FakeOutput`, `UnixOutput`, and `WindowsOutput` classes to support access to `Output` and added a `IDriver.GetOutput` to acess the `IOutput`. `IOutput` now has a `GetLastOutput` method.

Simplified `DriverAssert` logic and enhanced `DriverTests` with a new test for wide glyph clipping across drivers.

Performed general cleanup, including removal of unused code, improved formatting, and adoption of modern C# practices. Added `using System.Diagnostics` in `OutputBufferImpl` for debugging.

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Tig
2025-12-08 12:28:32 -07:00
committed by GitHub
parent 5da7e59aa2
commit f548059a27
20 changed files with 1046 additions and 324 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
Tests/UnitTestsParallelizable/Drivers/AddRuneTests.cs
View File

@@ -3,14 +3,10 @@ using System.Text;
using UnitTests;
using Xunit.Abstractions;
// Alias Console to MockConsole so we don't accidentally use Console
namespace DriverTests;
public class AddRuneTests (ITestOutputHelper output) : FakeDriverBase
{
private readonly ITestOutputHelper _output = output;
[Fact]
public void AddRune ()
{
@@ -179,4 +175,36 @@ public class AddRuneTests (ITestOutputHelper output) : FakeDriverBase
driver.Dispose ();
}
[Fact]
public void AddStr_Glyph_On_Second_Cell_Of_Wide_Glyph_Outputs_Correctly ()
{
IDriver? driver = CreateFakeDriver ();
driver.SetScreenSize (6, 3);
driver!.Clip = new (driver.Screen);
driver.Move (1, 0);
driver.AddStr ("┌");
driver.Move (2, 0);
driver.AddStr ("─");
driver.Move (3, 0);
driver.AddStr ("┐");
driver.Clip.Exclude (new Region (new (1, 0, 3, 1)));
driver.Move (0, 0);
driver.AddStr ("🍎🍎🍎🍎");
DriverAssert.AssertDriverContentsAre (
"""
<EFBFBD>┌─┐🍎
""",
output,
driver);
driver.Refresh ();
DriverAssert.AssertDriverOutputIs (@"\x1b[38;2;0;0;0m\x1b[48;2;0;0;0m<30>┌─┐🍎\x1b[38;2;255;255;255m\x1b[48;2;0;0;0m",
output, driver);
}
}

1
Tests/UnitTestsParallelizable/Drivers/ClipRegionTests.cs
View File

@@ -1,4 +1,5 @@
#nullable enable
using System.Text;
using UnitTests;
using Xunit.Abstractions;

45
Tests/UnitTestsParallelizable/Drivers/DriverTests.cs
View File

@@ -92,6 +92,51 @@ public class DriverTests (ITestOutputHelper output) : FakeDriverBase
app.Dispose ();
}
// Tests fix for https://github.com/gui-cs/Terminal.Gui/issues/4258
[Theory]
[InlineData ("fake")]
[InlineData ("windows")]
[InlineData ("dotnet")]
[InlineData ("unix")]
public void All_Drivers_When_Clipped_AddStr_Glyph_On_Second_Cell_Of_Wide_Glyph_Outputs_Correctly (string driverName)
{
IApplication? app = Application.Create ();
app.Init (driverName);
IDriver driver = app.Driver!;
// Need to force "windows" driver to override legacy console mode for this test
driver.IsLegacyConsole = false;
driver.Force16Colors = false;
driver.SetScreenSize (6, 3);
driver!.Clip = new (driver.Screen);
driver.Move (1, 0);
driver.AddStr ("┌");
driver.Move (2, 0);
driver.AddStr ("─");
driver.Move (3, 0);
driver.AddStr ("┐");
driver.Clip.Exclude (new Region (new (1, 0, 3, 1)));
driver.Move (0, 0);
driver.AddStr ("🍎🍎🍎🍎");
DriverAssert.AssertDriverContentsAre (
"""
<EFBFBD>┌─┐🍎
""",
output,
driver);
driver.Refresh ();
DriverAssert.AssertDriverOutputIs (@"\x1b[38;2;0;0;0m\x1b[48;2;0;0;0m<30>┌─┐🍎\x1b[38;2;255;255;255m\x1b[48;2;0;0;0m",
output, driver);
}
}
public class TestTop : Runnable

99
Tests/UnitTestsParallelizable/Drivers/OutputBaseTests.cs
View File

@@ -1,6 +1,4 @@
#nullable enable
namespace DriverTests;
namespace DriverTests;
public class OutputBaseTests
{
@@ -9,7 +7,7 @@ public class OutputBaseTests
{
// Arrange
var output = new FakeOutput ();
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (1, 1);
// Act
@@ -32,21 +30,21 @@ public class OutputBaseTests
// Create DriverImpl and associate it with the FakeOutput to test Sixel output
IDriver driver = new DriverImpl (
new FakeInputProcessor (null!),
new OutputBufferImpl (),
output,
new (new AnsiResponseParser ()),
new SizeMonitorImpl (output));
new FakeInputProcessor (null!),
new OutputBufferImpl (),
output,
new (new AnsiResponseParser ()),
new SizeMonitorImpl (output));
driver.Force16Colors = force16Colors;
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (1, 1);
// Use a known RGB color and attribute
var fg = new Color (1, 2, 3);
var bg = new Color (4, 5, 6);
buffer.CurrentAttribute = new Attribute (fg, bg);
buffer.CurrentAttribute = new (fg, bg);
buffer.AddStr ("X");
// Act
@@ -59,7 +57,7 @@ public class OutputBaseTests
}
else if (!isLegacyConsole && force16Colors)
{
var expected16 = EscSeqUtils.CSI_SetForegroundColor (fg.GetAnsiColorCode ());
string expected16 = EscSeqUtils.CSI_SetForegroundColor (fg.GetAnsiColorCode ());
Assert.Contains (expected16, ansi);
}
else
@@ -78,7 +76,7 @@ public class OutputBaseTests
{
// Arrange
var output = new FakeOutput ();
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (2, 1);
// Mark two characters as dirty by writing them into the buffer
@@ -92,7 +90,7 @@ public class OutputBaseTests
output.Write (buffer); // calls OutputBase.Write via FakeOutput
// Assert: content was written to the fake output and dirty flags cleared
Assert.Contains ("AB", output.Output);
Assert.Contains ("AB", output.GetLastOutput ());
Assert.False (buffer.Contents! [0, 0].IsDirty);
Assert.False (buffer.Contents! [0, 1].IsDirty);
}
@@ -105,7 +103,7 @@ public class OutputBaseTests
// Arrange
// FakeOutput exposes this because it's in test scope
var output = new FakeOutput { IsLegacyConsole = isLegacyConsole };
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (3, 1);
// Write 'A' at col 0 and 'C' at col 2; leave col 1 untouched (not dirty)
@@ -122,15 +120,15 @@ public class OutputBaseTests
output.Write (buffer);
// Assert: both characters were written (use Contains to avoid CI side effects)
Assert.Contains ("A", output.Output);
Assert.Contains ("C", output.Output);
Assert.Contains ("A", output.GetLastOutput ());
Assert.Contains ("C", output.GetLastOutput ());
// Dirty flags cleared for the written cells
Assert.False (buffer.Contents! [0, 0].IsDirty);
Assert.False (buffer.Contents! [0, 2].IsDirty);
// Verify SetCursorPositionImpl was invoked by WriteToConsole (cursor set to a written column)
Assert.Equal (new Point (0, 0), output.GetCursorPosition ());
Assert.Equal (new (0, 0), output.GetCursorPosition ());
// Now write 'X' at col 0 to verify subsequent writes also work
buffer.Move (0, 0);
@@ -143,15 +141,15 @@ public class OutputBaseTests
output.Write (buffer);
// Assert: both characters were written (use Contains to avoid CI side effects)
Assert.Contains ("A", output.Output);
Assert.Contains ("C", output.Output);
Assert.Contains ("A", output.GetLastOutput ());
Assert.Contains ("C", output.GetLastOutput ());
// Dirty flags cleared for the written cells
Assert.False (buffer.Contents! [0, 0].IsDirty);
Assert.False (buffer.Contents! [0, 2].IsDirty);
// Verify SetCursorPositionImpl was invoked by WriteToConsole (cursor set to a written column)
Assert.Equal (new Point (2, 0), output.GetCursorPosition ());
Assert.Equal (new (2, 0), output.GetCursorPosition ());
}
[Theory]
@@ -162,44 +160,57 @@ public class OutputBaseTests
// Arrange
// FakeOutput exposes this because it's in test scope
var output = new FakeOutput { IsLegacyConsole = isLegacyConsole };
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (3, 1);
// Write '🦮' at col 0 and 'A' at col 3; leave col 1 untouched (not dirty)
// Write '🦮' at col 0 and 'A' at col 2
buffer.Move (0, 0);
buffer.AddStr ("🦮A");
// Confirm some dirtiness before to write
// After the fix for https://github.com/gui-cs/Terminal.Gui/issues/4258:
// Writing a wide glyph at column 0 no longer sets column 1 to IsDirty = false.
// Column 1 retains whatever state it had (in this case, it was initialized as dirty
// by ClearContents, but may have been cleared by a previous Write call).
//
// What we care about is that wide glyphs work correctly and don't prevent
// other content from being drawn at odd columns.
Assert.True (buffer.Contents! [0, 0].IsDirty);
Assert.False (buffer.Contents! [0, 1].IsDirty);
// Column 1 state depends on whether it was cleared by a previous Write - don't assert
Assert.True (buffer.Contents! [0, 2].IsDirty);
// Act
output.Write (buffer);
Assert.Contains ("🦮", output.Output);
Assert.Contains ("A", output.Output);
Assert.Contains ("🦮", output.GetLastOutput ());
Assert.Contains ("A", output.GetLastOutput ());
// Dirty flags cleared for the written cells
// Column 0 was written (wide glyph)
Assert.False (buffer.Contents! [0, 0].IsDirty);
Assert.False (buffer.Contents! [0, 1].IsDirty);
// Column 1 was skipped by OutputBase.Write because column 0 had a wide glyph
// So its dirty flag remains true (it was initialized as dirty by ClearContents)
Assert.True (buffer.Contents! [0, 1].IsDirty);
// Column 2 was written ('A')
Assert.False (buffer.Contents! [0, 2].IsDirty);
Assert.Equal (new (0, 0), output.GetCursorPosition ());
// Now write 'X' at col 1 which replaces with the replacement character the col 0
// Now write 'X' at col 1 which invalidates the wide glyph at col 0
buffer.Move (1, 0);
buffer.AddStr ("X");
// Confirm dirtiness state before to write
Assert.True (buffer.Contents! [0, 0].IsDirty);
Assert.True (buffer.Contents! [0, 1].IsDirty);
Assert.True (buffer.Contents! [0, 2].IsDirty);
Assert.True (buffer.Contents! [0, 0].IsDirty); // Invalidated by writing at col 1
Assert.True (buffer.Contents! [0, 1].IsDirty); // Just written
Assert.True (buffer.Contents! [0, 2].IsDirty); // Marked dirty by writing at col 1
output.Write (buffer);
Assert.Contains ("<22>", output.Output);
Assert.Contains ("X", output.Output);
Assert.Contains ("<22>", output.GetLastOutput ());
Assert.Contains ("X", output.GetLastOutput ());
// Dirty flags cleared for the written cells
Assert.False (buffer.Contents! [0, 0].IsDirty);
@@ -217,7 +228,7 @@ public class OutputBaseTests
{
// Arrange
var output = new FakeOutput ();
IOutputBuffer buffer = output.LastBuffer!;
IOutputBuffer buffer = output.GetLastBuffer ()!;
buffer.SetSize (1, 1);
// Ensure the buffer has some content so Write traverses rows
@@ -227,16 +238,16 @@ public class OutputBaseTests
var s = new SixelToRender
{
SixelData = "SIXEL-DATA",
ScreenPosition = new Point (4, 2)
ScreenPosition = new (4, 2)
};
// Create DriverImpl and associate it with the FakeOutput to test Sixel output
IDriver driver = new DriverImpl (
new FakeInputProcessor (null!),
new OutputBufferImpl (),
output,
new (new AnsiResponseParser ()),
new SizeMonitorImpl (output));
new FakeInputProcessor (null!),
new OutputBufferImpl (),
output,
new (new AnsiResponseParser ()),
new SizeMonitorImpl (output));
// Add the Sixel to the driver
driver.GetSixels ().Enqueue (s);
@@ -250,7 +261,7 @@ public class OutputBaseTests
if (!isLegacyConsole)
{
// Assert: Sixel data was emitted (use Contains to avoid equality/side-effects)
Assert.Contains ("SIXEL-DATA", output.Output);
Assert.Contains ("SIXEL-DATA", output.GetLastOutput ());
// Cursor was moved to Sixel position
Assert.Equal (s.ScreenPosition, output.GetCursorPosition ());
@@ -258,7 +269,7 @@ public class OutputBaseTests
else
{
// Assert: Sixel data was NOT emitted
Assert.DoesNotContain ("SIXEL-DATA", output.Output);
Assert.DoesNotContain ("SIXEL-DATA", output.GetLastOutput ());
// Cursor was NOT moved to Sixel position
Assert.NotEqual (s.ScreenPosition, output.GetCursorPosition ());
@@ -271,4 +282,4 @@ public class OutputBaseTests
app.Dispose ();
}
}
}