Skip to content

Commit

Permalink
Fixed more clipping bugs and unit tests. All tests pass!
Browse files Browse the repository at this point in the history
  • Loading branch information
@tig
tig committed Oct 30, 2024
1 parent 0e6a2bc commit d835b56
Show file tree
Hide file tree
Showing 17 changed files with 404 additions and 383 deletions.
24 changes: 19 additions & 5 deletions Terminal.Gui/ConsoleDrivers/ConsoleDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ internal set
public void AddRune (Rune rune)
{
int runeWidth = -1;
bool validLocation = IsValidLocation (Col, Row);
bool validLocation = IsValidLocation (rune, Col, Row);

if (Contents is null)
{
Expand Down Expand Up @@ -231,12 +231,17 @@ public void AddRune (Rune rune)
}
else if (runeWidth == 2)
{
if (Col == clipRect.Right - 1)
if (!Clip.Contains (Col + 1, Row))
{
// We're at the right edge of the clip, so we can't display a wide character.
// TODO: Figure out if it is better to show a replacement character or ' '
Contents [Row, Col].Rune = Rune.ReplacementChar;
}
else if (!Clip.Contains (Col, Row))
{
// Our 1st column is outside the clip, so we can't display a wide character.
Contents [Row, Col+1].Rune = Rune.ReplacementChar;
}
else
{
Contents [Row, Col].Rune = rune;
Expand Down Expand Up @@ -392,7 +397,7 @@ public void FillRect (Rectangle rect, Rune rune = default)
{
for (int c = rect.X; c < rect.X + rect.Width; c++)
{
if (!IsValidLocation (c, r))
if (!IsValidLocation (rune, c, r))
{
continue;
}
Expand Down Expand Up @@ -433,15 +438,24 @@ public void FillRect (Rectangle rect, Rune rune = default)
public virtual bool IsRuneSupported (Rune rune) { return Rune.IsValid (rune.Value); }

/// <summary>Tests whether the specified coordinate are valid for drawing.</summary>
/// <param name="rune"></param>
/// <param name="col">The column.</param>
/// <param name="row">The row.</param>
/// <returns>
/// <see langword="false"/> if the coordinate is outside the screen bounds or outside of <see cref="Clip"/>.
/// <see langword="true"/> otherwise.
/// </returns>
public bool IsValidLocation (int col, int row)
public bool IsValidLocation (Rune rune, int col, int row)
{
return col >= 0 && row >= 0 && col < Cols && row < Rows && Clip!.Contains (col, row);
if (rune.GetColumns () < 2)
{
return col >= 0 && row >= 0 && col < Cols && row < Rows && Clip!.Contains (col, row);
}
else
{

return Clip!.Contains (col, row) || Clip!.Contains (col + 1, row);
}
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion Terminal.Gui/ConsoleDrivers/CursesDriver/CursesDriver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public override void Move (int col, int row)
return;
}

if (IsValidLocation (col, row))
if (IsValidLocation (default, col, row))
{
Curses.move (row, col);
}
Expand Down
8 changes: 8 additions & 0 deletions Terminal.Gui/Views/ScrollView.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ protected override bool OnDrawingContent ()

if (!string.IsNullOrEmpty (_contentView.Text) || _contentView.Subviews.Count > 0)
{
Region? saved = SetClipToFrame();

Check warning on line 384 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_release 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 384 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_and_test_debug (ubuntu-latest) 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 384 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_and_test_debug (macos-latest) 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
_contentView.Draw ();
Application.SetClip (saved);
}

DrawScrollBars ();
Expand Down Expand Up @@ -582,18 +584,24 @@ private void DrawScrollBars ()
{
if (ShowVerticalScrollIndicator)
{
Region? saved = Application.ClipToScreen ();

Check warning on line 587 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_release 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

Check warning on line 587 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_and_test_debug (macos-latest) 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
_vertical.Draw ();
Application.SetClip (saved);
}

if (ShowHorizontalScrollIndicator)
{
Region? saved = Application.ClipToScreen ();

Check warning on line 594 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_and_test_debug (macos-latest) 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
_horizontal.Draw ();
Application.SetClip (saved);
}

if (ShowVerticalScrollIndicator && ShowHorizontalScrollIndicator)
{
SetContentBottomRightCornerVisibility ();
Region? saved = Application.ClipToScreen ();

Check warning on line 602 in Terminal.Gui/Views/ScrollView.cs

View workflow job for this annotation

GitHub Actions / build_and_test_debug (macos-latest) 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
_contentBottomRightCorner.Draw ();
Application.SetClip (saved);
}
}
}
Expand Down
62 changes: 31 additions & 31 deletions UnitTests/ConsoleDrivers/ClipRegionTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,16 @@ public void Clip_Set_To_Empty_AllInvalid (Type driverType)
driver.Clip = new (Rectangle.Empty);

// negative
Assert.False (driver.IsValidLocation (4, 5));
Assert.False (driver.IsValidLocation (5, 4));
Assert.False (driver.IsValidLocation (10, 9));
Assert.False (driver.IsValidLocation (9, 10));
Assert.False (driver.IsValidLocation (-1, 0));
Assert.False (driver.IsValidLocation (0, -1));
Assert.False (driver.IsValidLocation (-1, -1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows));
Assert.False (driver.IsValidLocation (default, 4, 5));
Assert.False (driver.IsValidLocation (default, 5, 4));
Assert.False (driver.IsValidLocation (default, 10, 9));
Assert.False (driver.IsValidLocation (default, 9, 10));
Assert.False (driver.IsValidLocation (default, -1, 0));
Assert.False (driver.IsValidLocation (default, 0, -1));
Assert.False (driver.IsValidLocation (default, -1, -1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows));

Application.Shutdown ();
}
Expand All @@ -98,36 +98,36 @@ public void IsValidLocation (Type driverType)
Application.Driver!.Cols = 10;

// positive
Assert.True (driver.IsValidLocation (0, 0));
Assert.True (driver.IsValidLocation (1, 1));
Assert.True (driver.IsValidLocation (driver.Cols - 1, driver.Rows - 1));
Assert.True (driver.IsValidLocation (default, 0, 0));
Assert.True (driver.IsValidLocation (default, 1, 1));
Assert.True (driver.IsValidLocation (default, driver.Cols - 1, driver.Rows - 1));

// negative
Assert.False (driver.IsValidLocation (-1, 0));
Assert.False (driver.IsValidLocation (0, -1));
Assert.False (driver.IsValidLocation (-1, -1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows));
Assert.False (driver.IsValidLocation (default, -1, 0));
Assert.False (driver.IsValidLocation (default, 0, -1));
Assert.False (driver.IsValidLocation (default, -1, -1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows));

// Define a clip rectangle
driver.Clip = new(new Rectangle(5, 5, 5, 5));

// positive
Assert.True (driver.IsValidLocation (5, 5));
Assert.True (driver.IsValidLocation (9, 9));
Assert.True (driver.IsValidLocation (default, 5, 5));
Assert.True (driver.IsValidLocation (default, 9, 9));

// negative
Assert.False (driver.IsValidLocation (4, 5));
Assert.False (driver.IsValidLocation (5, 4));
Assert.False (driver.IsValidLocation (10, 9));
Assert.False (driver.IsValidLocation (9, 10));
Assert.False (driver.IsValidLocation (-1, 0));
Assert.False (driver.IsValidLocation (0, -1));
Assert.False (driver.IsValidLocation (-1, -1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (driver.Cols, driver.Rows));
Assert.False (driver.IsValidLocation (default, 4, 5));
Assert.False (driver.IsValidLocation (default, 5, 4));
Assert.False (driver.IsValidLocation (default, 10, 9));
Assert.False (driver.IsValidLocation (default, 9, 10));
Assert.False (driver.IsValidLocation (default, -1, 0));
Assert.False (driver.IsValidLocation (default, 0, -1));
Assert.False (driver.IsValidLocation (default, -1, -1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows - 1));
Assert.False (driver.IsValidLocation (default, driver.Cols, driver.Rows));

Application.Shutdown ();
}
Expand Down
90 changes: 49 additions & 41 deletions UnitTests/View/Draw/DrawTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,64 +292,72 @@ public void CJK_Compatibility_Ideographs_ConsoleWidth_ColumnWidth_Equal_Two ()
top.Dispose ();
}

// TODO: Refactor this test to not depend on TextView etc... Make it as primitive as possible
// TODO: Simplify this test to just use AddRune directly
[Fact]
[AutoInitShutdown]
[SetupFakeDriver]
[Trait ("Category", "Unicode")]
public void Clipping_AddRune_Left_Or_Right_Replace_Previous_Or_Next_Wide_Rune_With_Space ()
public void Clipping_Wide_Runes ()
{
var tv = new TextView
((FakeDriver)Application.Driver!).SetBufferSize (30, 1);

var top = new View ()
{
Id = "top",
Width = Dim.Fill (),
Height = Dim.Fill (),
};
var frameView = new View ()
{
Id = "frameView",
Width = Dim.Fill (),
Height = Dim.Fill (),
Text = """
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
これは広いルーンラインです。
"""
""",
};
var win = new Window { Width = Dim.Fill (), Height = Dim.Fill () };
win.Add (tv);
var top = new Toplevel ();
top.Add (win);
frameView.Border.LineStyle = LineStyle.Single;
frameView.Border.Thickness = new (1, 0, 0, 0);

var view = new View { Text = "ワイドルーン。", Height = Dim.Fill (), Width = Dim.Fill () };
top.Add (frameView);
Application.ClipToScreen ();
top.Layout ();
top.Draw ();

// Don't have unit tests use things that aren't absolutely critical for the test, like Dialog
var dg = new Window { X = 2, Y = 2, Width = 14, Height = 3 };
dg.Add (view);
RunState rsTop = Application.Begin (top);
RunState rsDiag = Application.Begin (dg);
((FakeDriver)Application.Driver!).SetBufferSize (30, 10);
string expectedOutput = """
│これは広いルーンラインです。
""";

const string expectedOutput = """
TestHelpers.AssertDriverContentsWithFrameAre (expectedOutput, _output);

┌────────────────────────────┐
│これは広いルーンラインです。│
│�┌────────────┐�ラインです。│
│�│ワイドルーン│�ラインです。│
│�└────────────┘�ラインです。│
│これは広いルーンラインです。│
│これは広いルーンラインです。│
│これは広いルーンラインです。│
│これは広いルーンラインです。│
└────────────────────────────┘
""";
var view = new View
{
Text = "0123456789",
//Text = "ワイドルー。",
X = 2,
Height = Dim.Auto (),
Width = Dim.Auto (),
BorderStyle = LineStyle.Single
};
view.Border.Thickness = new (1, 0, 1, 0);

Rectangle pos = TestHelpers.AssertDriverContentsWithFrameAre (expectedOutput, _output);
Assert.Equal (new (0, 0, 30, 10), pos);
top.Add (view);
top.Layout ();
Application.ClipToScreen ();
top.Draw ();
// 012345678901234567890123456789012345678
// 012 34 56 78 90 12 34 56 78 90 12 34 56 78
// │こ れ は 広 い ル ー ン ラ イ ン で す 。
// 01 2345678901234 56 78 90 12 34 56
// │� |0123456989│� ン ラ イ ン で す 。
expectedOutput = """
│�│0123456789│�ンラインです。
""";

Application.End (rsDiag);
dg.Dispose ();
Application.End (rsTop);
top.Dispose ();
TestHelpers.AssertDriverContentsWithFrameAre (expectedOutput, _output);
}

// TODO: Add more AddRune tests to cover all the cases where wide runes are clipped

[Fact]
[AutoInitShutdown]
[Trait ("Category", "Output")]
Expand Down
Loading

0 comments on commit d835b56

Please sign in to comment.