From aab90e1fbcb415f5ef59ff9b91cecfa8834e6286 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 17:13:58 -0700 Subject: [PATCH] Fixes #4463 - Adds `LineCanvas.GetRegion()` - 10x performance regression in `RenderLineCanvas` (#4464) * Initial plan * Revert xunit maxParallelThreads to default to fix performance issue Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix 10x performance regression in RenderLineCanvas Found via binary search of 51 commits that commit 027018368 introduced the slowdown by calling context.AddDrawnRectangle() for every cell in the LineCanvas (thousands of times per frame). Fixed by reporting the entire LineCanvas.Bounds once instead of per-cell. Test results: - Before: 1m 33s (10x slower than v2_release) - After: 9s (same as v2_release) Fixes the performance issue in AllViews_Draw_Does_Not_Layout test. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add comment clarifying LineCanvas coordinate system Addresses code review feedback about coordinate system conversion. LineCanvas.Bounds is already in screen-relative coordinates since cells are stored with screen positions (used with Driver.Move). Co-authored-by: tig <585482+tig@users.noreply.github.com> * Build non-rectangular Region from LineCanvas cells efficiently Instead of reporting entire LineCanvas.Bounds (which includes empty space), efficiently build a Region that encompasses only the actual line cells. Algorithm groups contiguous horizontal cells into spans per row, avoiding per-cell overhead while accurately representing the non-rectangular shape. Performance maintained: 10s (same as v2_release) All tests pass: 5695 drawing tests, 200 LineCanvas tests, 33 transparency tests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Address code review feedback - improve robustness - Remove redundant empty check (caller already verifies) - Add safety check for empty row groups to prevent IndexOutOfRangeException - Update documentation to clarify precondition Tests still pass with 9s performance. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Optimize BuildRegionFromLineCells - eliminate redundant sort Remove redundant OrderBy since X values are already sorted from the initial OrderBy().ThenBy() operation. Also remove unreachable empty check since GroupBy guarantees non-empty groups. Performance maintained at 9s. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Final code review fixes - improve comments and add defensive check - Clarify comment about sorting behavior - Add defensive empty check for edge cases - Performance maintained at 10s Co-authored-by: tig <585482+tig@users.noreply.github.com> * Move BuildRegionFromCells to LineCanvas class Moved the BuildRegionFromCells method from View.Drawing.cs to LineCanvas where it logically belongs as a public static method. Updated View.Drawing.cs to call Drawing.LineCanvas.BuildRegionFromCells. Performance maintained at 9s. All tests pass. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Update documentation for BuildRegionFromCells - Clarify that method handles empty cellMap gracefully - Improve comment explaining the defensive empty check - Performance maintained at 10s Co-authored-by: tig <585482+tig@users.noreply.github.com> * Rename BuildRegionFromCells to GetRegion Renamed the method as requested to have a more concise name. Performance maintained at 10s. All tests pass. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add GetCellMapWithRegion optimization for single-pass iteration Implemented optimized method that builds both cell map and Region in a single pass through the canvas bounds, eliminating the need for separate GetCellMap() and GetRegion() calls. Benefits: - Reduces from 2 iterations to 1 (GetCellMap then GetRegion -> single pass) - Eliminates sorting overhead (O(n log n) -> O(n)) - X values already sorted due to inner loop order - Builds Region incrementally per row as cells are discovered Performance maintained at 10s. All tests pass: - AllViews_Draw_Does_Not_Layout: 10s - 5695 drawing tests passed - 200 LineCanvas tests passed - 33 transparency tests passed Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add comprehensive unit tests for GetRegion and GetCellMapWithRegion Added 15 new unit tests covering: GetRegion tests: - Empty cell map handling - Single cell regions - Horizontal and vertical lines - L-shaped regions - Discontiguous cells creating separate spans - Intersecting lines GetCellMapWithRegion tests: - Empty canvas handling - Single horizontal/vertical lines - Intersecting lines with proper intersection handling - Complex shapes (boxes) - Consistency with separate GetCellMap + GetRegion calls - Negative coordinate handling - Exclusion region support All 215 LineCanvas tests pass. Performance maintained at 9s. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix code style violations per CONTRIBUTING.md - Replace var with explicit types (Dictionary, LineCanvas, Region, etc.) - Replace new TypeName() with target-typed new () where type is declared - Fix foreach loop to use explicit IGrouping type - Use StraightLine instead of var in foreach All style violations fixed per CONTRIBUTING.md coding conventions. Tests still pass: 215 LineCanvas tests, 9s performance maintained. Co-authored-by: tig <585482+tig@users.noreply.github.com> * Code cleanup. * Upgrades Transparent Scenario to demonstrate LineCanvas transparency. "dotnet" is drawn using LineCanvas. * Comment out debug assertions in View drawing logic The debug assertions in the `View` class, which validated the relationships between `Margin`, `Border`, `Padding`, and their parent objects, as well as their `SubViewNeedsDraw` and `NeedsDraw` states, have been commented out. These assertions were conditionally executed when the current object and its `SuperView` were not of type `Adornment`. The code has been retained as comments for potential future reference or debugging purposes, but the assertions are no longer active in the current implementation. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tig <585482+tig@users.noreply.github.com> Co-authored-by: Tig --- Examples/UICatalog/Scenarios/Transparent.cs | 91 ++++ Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs | 168 +++++++- Terminal.Gui/ViewBase/View.Drawing.cs | 55 +-- .../Drawing/Lines/LineCanvasTests.cs | 407 ++++++++++++++++-- .../UnitTestsParallelizable/xunit.runner.json | 2 +- 5 files changed, 656 insertions(+), 67 deletions(-) diff --git a/Examples/UICatalog/Scenarios/Transparent.cs b/Examples/UICatalog/Scenarios/Transparent.cs index 22dc866be..e1c94b325 100644 --- a/Examples/UICatalog/Scenarios/Transparent.cs +++ b/Examples/UICatalog/Scenarios/Transparent.cs @@ -219,11 +219,102 @@ public sealed class Transparent : Scenario return false; } + protected override bool OnRenderingLineCanvas () + { + // Draw "dotnet" using LineCanvas + Point screenPos = ViewportToScreen (new Point (7, 16)); + DrawDotnet (LineCanvas, screenPos.X, screenPos.Y, LineStyle.Single, GetAttributeForRole (VisualRole.Normal)); + + return false; + } + /// protected override bool OnClearingViewport () { return false; } /// protected override bool OnMouseEvent (MouseEventArgs mouseEvent) { return false; } + + + /// + /// Draws "dotnet" text using LineCanvas. The 'd' is 8 cells high. + /// + /// The LineCanvas to draw on + /// Starting X position + /// Starting Y position + /// Line style to use + /// Optional attribute for the lines + private void DrawDotnet (LineCanvas canvas, int x, int y, LineStyle style = LineStyle.Single, Attribute? attribute = null) + { + int currentX = x; + int letterHeight = 8; + int letterSpacing = 2; + + // Letter 'd' - lowercase, height 8 + // Vertical stem on right (goes up full 8 cells) + canvas.AddLine (new (currentX + 3, y), letterHeight, Orientation.Vertical, style, attribute); + // Top horizontal + canvas.AddLine (new (currentX, y + 3), 4, Orientation.Horizontal, style, attribute); + // Left vertical (only bottom 5 cells, leaving top 3 for ascender space) + canvas.AddLine (new (currentX, y + 3), 5, Orientation.Vertical, style, attribute); + // Bottom horizontal + canvas.AddLine (new (currentX, y + 7), 4, Orientation.Horizontal, style, attribute); + currentX += 4 + letterSpacing; + + // Letter 'o' - height 5 (x-height) + int oY = y + 3; // Align with x-height (leaving 3 cells for ascenders) + // Top + canvas.AddLine (new (currentX, oY), 4, Orientation.Horizontal, style, attribute); + // Left + canvas.AddLine (new (currentX, oY), 5, Orientation.Vertical, style, attribute); + // Right + canvas.AddLine (new (currentX + 3, oY), 5, Orientation.Vertical, style, attribute); + // Bottom + canvas.AddLine (new (currentX, oY + 4), 4, Orientation.Horizontal, style, attribute); + currentX += 4 + letterSpacing; + + // Letter 't' - height 7 (has ascender above x-height) + int tY = y + 1; // Starts 1 cell above x-height + // Vertical stem + canvas.AddLine (new (currentX + 1, tY), 7, Orientation.Vertical, style, attribute); + // Top cross bar (at x-height) + canvas.AddLine (new (currentX, tY + 2), 3, Orientation.Horizontal, style, attribute); + // Bottom horizontal (foot) + canvas.AddLine (new (currentX + 1, tY + 6), 2, Orientation.Horizontal, style, attribute); + currentX += 3 + letterSpacing; + + // Letter 'n' - height 5 (x-height) + int nY = y + 3; + // Left vertical + canvas.AddLine (new (currentX, nY), 5, Orientation.Vertical, style, attribute); + // Top horizontal + canvas.AddLine (new (currentX + 1, nY), 3, Orientation.Horizontal, style, attribute); + // Right vertical + canvas.AddLine (new (currentX + 3, nY), 5, Orientation.Vertical, style, attribute); + currentX += 4 + letterSpacing; + + // Letter 'e' - height 5 (x-height) + int eY = y + 3; + // Top + canvas.AddLine (new (currentX, eY), 4, Orientation.Horizontal, style, attribute); + // Left + canvas.AddLine (new (currentX, eY), 5, Orientation.Vertical, style, attribute); + // Right + canvas.AddLine (new (currentX + 3, eY), 3, Orientation.Vertical, style, attribute); + // Middle horizontal bar + canvas.AddLine (new (currentX, eY + 2), 4, Orientation.Horizontal, style, attribute); + // Bottom + canvas.AddLine (new (currentX, eY + 4), 4, Orientation.Horizontal, style, attribute); + currentX += 4 + letterSpacing; + + // Letter 't' - height 7 (has ascender above x-height) - second 't' + int t2Y = y + 1; + // Vertical stem + canvas.AddLine (new (currentX + 1, t2Y), 7, Orientation.Vertical, style, attribute); + // Top cross bar (at x-height) + canvas.AddLine (new (currentX, t2Y + 2), 3, Orientation.Horizontal, style, attribute); + // Bottom horizontal (foot) + canvas.AddLine (new (currentX + 1, t2Y + 6), 2, Orientation.Horizontal, style, attribute); + } } } diff --git a/Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs b/Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs index 2be985764..ed3e529ec 100644 --- a/Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs +++ b/Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs @@ -179,7 +179,7 @@ public class LineCanvas : IDisposable } } // Safe as long as the list is not modified while the span is in use. - ReadOnlySpan intersects = CollectionsMarshal.AsSpan(intersectionsBufferList); + ReadOnlySpan intersects = CollectionsMarshal.AsSpan (intersectionsBufferList); Cell? cell = GetCellForIntersects (intersects); // TODO: Can we skip the whole nested looping if _exclusionRegion is null? if (cell is { } && _exclusionRegion?.Contains (x, y) is null or false) @@ -192,6 +192,136 @@ public class LineCanvas : IDisposable return map; } + /// + /// Evaluates the lines and returns both the cell map and a Region encompassing the drawn cells. + /// This is more efficient than calling and separately + /// as it builds both in a single pass through the canvas bounds. + /// + /// A tuple containing the cell map and the Region of drawn cells + public (Dictionary CellMap, Region Region) GetCellMapWithRegion () + { + Dictionary map = new (); + Region region = new (); + + List intersectionsBufferList = []; + List rowXValues = []; + + // walk through each pixel of the bitmap, row by row + for (int y = Bounds.Y; y < Bounds.Y + Bounds.Height; y++) + { + rowXValues.Clear (); + + for (int x = Bounds.X; x < Bounds.X + Bounds.Width; x++) + { + intersectionsBufferList.Clear (); + foreach (StraightLine line in _lines) + { + if (line.Intersects (x, y) is { } intersect) + { + intersectionsBufferList.Add (intersect); + } + } + // Safe as long as the list is not modified while the span is in use. + ReadOnlySpan intersects = CollectionsMarshal.AsSpan (intersectionsBufferList); + Cell? cell = GetCellForIntersects (intersects); + + if (cell is { } && _exclusionRegion?.Contains (x, y) is null or false) + { + map.Add (new (x, y), cell); + rowXValues.Add (x); + } + } + + // Build Region spans for this completed row + if (rowXValues.Count <= 0) + { + continue; + } + + // X values are already sorted (inner loop iterates x in order) + int spanStart = rowXValues [0]; + int spanEnd = rowXValues [0]; + + for (int i = 1; i < rowXValues.Count; i++) + { + if (rowXValues [i] == spanEnd + 1) + { + // Continue the span + spanEnd = rowXValues [i]; + } + else + { + // End the current span and add it to the region + region.Combine (new Rectangle (spanStart, y, spanEnd - spanStart + 1, 1), RegionOp.Union); + spanStart = rowXValues [i]; + spanEnd = rowXValues [i]; + } + } + + // Add the final span for this row + region.Combine (new Rectangle (spanStart, y, spanEnd - spanStart + 1, 1), RegionOp.Union); + } + + return (map, region); + } + + /// + /// Efficiently builds a from line cells by grouping contiguous horizontal spans. + /// This avoids the performance overhead of adding each cell individually while accurately + /// representing the non-rectangular shape of the lines. + /// + /// Dictionary of points where line cells are drawn. If empty, returns an empty Region. + /// A Region encompassing all the line cells, or an empty Region if cellMap is empty + public static Region GetRegion (Dictionary cellMap) + { + // Group cells by row for efficient horizontal span detection + // Sort by Y then X so that within each row group, X values are in order + IEnumerable> rowGroups = cellMap.Keys + .OrderBy (p => p.Y) + .ThenBy (p => p.X) + .GroupBy (p => p.Y); + + Region region = new (); + + foreach (IGrouping row in rowGroups) + { + int y = row.Key; + // X values are sorted due to ThenBy above + List xValues = row.Select (p => p.X).ToList (); + + // Note: GroupBy on non-empty Keys guarantees non-empty groups, but check anyway for safety + if (xValues.Count == 0) + { + continue; + } + + // Merge contiguous x values into horizontal spans + int spanStart = xValues [0]; + int spanEnd = xValues [0]; + + for (int i = 1; i < xValues.Count; i++) + { + if (xValues [i] == spanEnd + 1) + { + // Continue the span + spanEnd = xValues [i]; + } + else + { + // End the current span and add it to the region + region.Combine (new Rectangle (spanStart, y, spanEnd - spanStart + 1, 1), RegionOp.Union); + spanStart = xValues [i]; + spanEnd = xValues [i]; + } + } + + // Add the final span for this row + region.Combine (new Rectangle (spanStart, y, spanEnd - spanStart + 1, 1), RegionOp.Union); + } + + return region; + } + // TODO: Unless there's an obvious use case for this API we should delete it in favor of the // simpler version that doesn't take an area. /// @@ -227,7 +357,7 @@ public class LineCanvas : IDisposable } } // Safe as long as the list is not modified while the span is in use. - ReadOnlySpan intersects = CollectionsMarshal.AsSpan(intersectionsBufferList); + ReadOnlySpan intersects = CollectionsMarshal.AsSpan (intersectionsBufferList); Rune? rune = GetRuneForIntersects (intersects); @@ -442,14 +572,14 @@ public class LineCanvas : IDisposable } // TODO: Remove these once we have all of the below ported to IntersectionRuneResolvers - bool useDouble = AnyLineStyles(intersects, [LineStyle.Double]); - bool useDashed = AnyLineStyles(intersects, [LineStyle.Dashed, LineStyle.RoundedDashed]); - bool useDotted = AnyLineStyles(intersects, [LineStyle.Dotted, LineStyle.RoundedDotted]); + bool useDouble = AnyLineStyles (intersects, [LineStyle.Double]); + bool useDashed = AnyLineStyles (intersects, [LineStyle.Dashed, LineStyle.RoundedDashed]); + bool useDotted = AnyLineStyles (intersects, [LineStyle.Dotted, LineStyle.RoundedDotted]); // horiz and vert lines same as Single for Rounded - bool useThick = AnyLineStyles(intersects, [LineStyle.Heavy]); - bool useThickDashed = AnyLineStyles(intersects, [LineStyle.HeavyDashed]); - bool useThickDotted = AnyLineStyles(intersects, [LineStyle.HeavyDotted]); + bool useThick = AnyLineStyles (intersects, [LineStyle.Heavy]); + bool useThickDashed = AnyLineStyles (intersects, [LineStyle.HeavyDashed]); + bool useThickDotted = AnyLineStyles (intersects, [LineStyle.HeavyDotted]); // TODO: Support ruler //var useRuler = intersects.Any (i => i.Line.Style == LineStyle.Ruler && i.Line.Length != 0); @@ -727,10 +857,10 @@ public class LineCanvas : IDisposable private static class CornerIntersections { // Names matching #region "Corner Conditions" IntersectionRuneType - internal static readonly IntersectionType[] UpperLeft = [IntersectionType.StartRight, IntersectionType.StartDown]; - internal static readonly IntersectionType[] UpperRight = [IntersectionType.StartLeft, IntersectionType.StartDown]; - internal static readonly IntersectionType[] LowerRight = [IntersectionType.StartUp, IntersectionType.StartLeft]; - internal static readonly IntersectionType[] LowerLeft = [IntersectionType.StartUp, IntersectionType.StartRight]; + internal static readonly IntersectionType [] UpperLeft = [IntersectionType.StartRight, IntersectionType.StartDown]; + internal static readonly IntersectionType [] UpperRight = [IntersectionType.StartLeft, IntersectionType.StartDown]; + internal static readonly IntersectionType [] LowerRight = [IntersectionType.StartUp, IntersectionType.StartLeft]; + internal static readonly IntersectionType [] LowerLeft = [IntersectionType.StartUp, IntersectionType.StartRight]; } private class BottomTeeIntersectionRuneResolver : IntersectionRuneResolver @@ -773,14 +903,18 @@ public class LineCanvas : IDisposable internal Rune _thickBoth; internal Rune _thickH; internal Rune _thickV; - protected IntersectionRuneResolver () { SetGlyphs (); } + + protected IntersectionRuneResolver () + { + SetGlyphs (); + } public Rune? GetRuneForIntersects (ReadOnlySpan intersects) { // Note that there aren't any glyphs for intersections of double lines with heavy lines - bool doubleHorizontal = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Horizontal, [LineStyle.Double]); - bool doubleVertical = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Vertical, [LineStyle.Double]); + bool doubleHorizontal = AnyWithOrientationAndAnyLineStyle (intersects, Orientation.Horizontal, [LineStyle.Double]); + bool doubleVertical = AnyWithOrientationAndAnyLineStyle (intersects, Orientation.Vertical, [LineStyle.Double]); if (doubleHorizontal) { @@ -792,9 +926,9 @@ public class LineCanvas : IDisposable return _doubleV; } - bool thickHorizontal = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Horizontal, + bool thickHorizontal = AnyWithOrientationAndAnyLineStyle (intersects, Orientation.Horizontal, [LineStyle.Heavy, LineStyle.HeavyDashed, LineStyle.HeavyDotted]); - bool thickVertical = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Vertical, + bool thickVertical = AnyWithOrientationAndAnyLineStyle (intersects, Orientation.Vertical, [LineStyle.Heavy, LineStyle.HeavyDashed, LineStyle.HeavyDotted]); if (thickHorizontal) diff --git a/Terminal.Gui/ViewBase/View.Drawing.cs b/Terminal.Gui/ViewBase/View.Drawing.cs index 871128e0a..a5e17da18 100644 --- a/Terminal.Gui/ViewBase/View.Drawing.cs +++ b/Terminal.Gui/ViewBase/View.Drawing.cs @@ -140,25 +140,25 @@ public partial class View // Drawing APIs ClearNeedsDraw (); - if (this is not Adornment && SuperView is not Adornment) - { - // Parent - Debug.Assert (Margin!.Parent == this); - Debug.Assert (Border!.Parent == this); - Debug.Assert (Padding!.Parent == this); + //if (this is not Adornment && SuperView is not Adornment) + //{ + // // Parent + // Debug.Assert (Margin!.Parent == this); + // Debug.Assert (Border!.Parent == this); + // Debug.Assert (Padding!.Parent == this); - // SubViewNeedsDraw is set to false by ClearNeedsDraw. - Debug.Assert (SubViewNeedsDraw == false); - Debug.Assert (Margin!.SubViewNeedsDraw == false); - Debug.Assert (Border!.SubViewNeedsDraw == false); - Debug.Assert (Padding!.SubViewNeedsDraw == false); + // // SubViewNeedsDraw is set to false by ClearNeedsDraw. + // Debug.Assert (SubViewNeedsDraw == false); + // Debug.Assert (Margin!.SubViewNeedsDraw == false); + // Debug.Assert (Border!.SubViewNeedsDraw == false); + // Debug.Assert (Padding!.SubViewNeedsDraw == false); - // NeedsDraw is set to false by ClearNeedsDraw. - Debug.Assert (NeedsDraw == false); - Debug.Assert (Margin!.NeedsDraw == false); - Debug.Assert (Border!.NeedsDraw == false); - Debug.Assert (Padding!.NeedsDraw == false); - } + // // NeedsDraw is set to false by ClearNeedsDraw. + // Debug.Assert (NeedsDraw == false); + // Debug.Assert (Margin!.NeedsDraw == false); + // Debug.Assert (Border!.NeedsDraw == false); + // Debug.Assert (Padding!.NeedsDraw == false); + //} } // ------------------------------------ @@ -226,7 +226,7 @@ public partial class View // Drawing APIs { // Set the clip to be just the thicknesses of the adornments // TODO: Put this union logic in a method on View? - Region? clipAdornments = Margin!.Thickness.AsRegion (Margin!.FrameToScreen ()); + Region clipAdornments = Margin!.Thickness.AsRegion (Margin!.FrameToScreen ()); clipAdornments.Combine (Border!.Thickness.AsRegion (Border!.FrameToScreen ()), RegionOp.Union); clipAdornments.Combine (Padding!.Thickness.AsRegion (Padding!.FrameToScreen ()), RegionOp.Union); clipAdornments.Combine (originalClip, RegionOp.Intersect); @@ -697,8 +697,8 @@ public partial class View // Drawing APIs /// to stop further drawing of . protected virtual bool OnRenderingLineCanvas () { return false; } - /// The canvas that any line drawing that is to be shared by SubViews of this view should add lines to. - /// adds border lines to this LineCanvas. + /// The canvas that any line drawing that is to be shared by subviews of this view should add lines to. + /// adds lines to this LineCanvas. public LineCanvas LineCanvas { get; } = new (); /// @@ -725,7 +725,10 @@ public partial class View // Drawing APIs if (!SuperViewRendersLineCanvas && LineCanvas.Bounds != Rectangle.Empty) { - foreach (KeyValuePair p in LineCanvas.GetCellMap ()) + // Get both cell map and Region in a single pass through the canvas + (Dictionary cellMap, Region lineRegion) = LineCanvas.GetCellMapWithRegion (); + + foreach (KeyValuePair p in cellMap) { // Get the entire map if (p.Value is { }) @@ -735,12 +738,16 @@ public partial class View // Drawing APIs // TODO: #2616 - Support combining sequences that don't normalize AddStr (p.Value.Value.Grapheme); - - // Add each drawn cell to the context - //context?.AddDrawnRectangle (new Rectangle (p.Key, new (1, 1)) ); } } + // Report the drawn region for transparency support + // Region was built during the GetCellMapWithRegion() call above + if (context is { } && cellMap.Count > 0) + { + context.AddDrawnRegion (lineRegion); + } + LineCanvas.Clear (); } } diff --git a/Tests/UnitTestsParallelizable/Drawing/Lines/LineCanvasTests.cs b/Tests/UnitTestsParallelizable/Drawing/Lines/LineCanvasTests.cs index 1feedd9f2..252a3c080 100644 --- a/Tests/UnitTestsParallelizable/Drawing/Lines/LineCanvasTests.cs +++ b/Tests/UnitTestsParallelizable/Drawing/Lines/LineCanvasTests.cs @@ -17,14 +17,14 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Empty_Canvas_ToString_Returns_EmptyString () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); Assert.Equal (string.Empty, canvas.ToString ()); } [Fact] public void Clear_Removes_All_Lines () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); canvas.AddLine (new (0, 0), 3, Orientation.Vertical, LineStyle.Single); @@ -38,7 +38,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Lines_Property_Returns_ReadOnly_Collection () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); Assert.Single (canvas.Lines); @@ -48,7 +48,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void AddLine_Adds_Line_To_Collection () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); Assert.Empty (canvas.Lines); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); @@ -94,7 +94,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase int expectedHeight ) { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (x, y), length, Orientation.Horizontal, LineStyle.Single); canvas.AddLine (new (x, y), length, Orientation.Vertical, LineStyle.Single); @@ -119,7 +119,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase int expectedHeight ) { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (x, y), length, Orientation.Horizontal, LineStyle.Single); Assert.Equal (new (expectedX, expectedY, expectedWidth, expectedHeight), canvas.Bounds); @@ -128,7 +128,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Specific_Coordinates () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (5, 5), 3, Orientation.Horizontal, LineStyle.Single); Assert.Equal (new (5, 5, 3, 1), canvas.Bounds); } @@ -136,14 +136,14 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Empty_Canvas_Returns_Empty_Rectangle () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); Assert.Equal (Rectangle.Empty, canvas.Bounds); } [Fact] public void Bounds_Single_Point_Zero_Length () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (5, 5), 0, Orientation.Horizontal, LineStyle.Single); Assert.Equal (new (5, 5, 1, 1), canvas.Bounds); @@ -152,7 +152,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Horizontal_Line () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (2, 3), 5, Orientation.Horizontal, LineStyle.Single); Assert.Equal (new (2, 3, 5, 1), canvas.Bounds); @@ -161,7 +161,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Vertical_Line () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (2, 3), 5, Orientation.Vertical, LineStyle.Single); Assert.Equal (new (2, 3, 1, 5), canvas.Bounds); @@ -170,7 +170,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Multiple_Lines_Returns_Union () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); canvas.AddLine (new (0, 0), 3, Orientation.Vertical, LineStyle.Single); @@ -180,7 +180,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Negative_Length_Line () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (5, 5), -3, Orientation.Horizontal, LineStyle.Single); // Line from (5,5) going left 3 positions: includes points 3, 4, 5 (width 3, X starts at 3) @@ -190,7 +190,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Bounds_Complex_Box () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // top canvas.AddLine (new (0, 0), 3, Orientation.Horizontal, LineStyle.Single); @@ -214,7 +214,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void ClearExclusions_Clears_Exclusion_Region () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); var region = new Region (new (0, 0, 2, 1)); @@ -229,7 +229,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Exclude_Removes_Points_From_Map () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (0, 0), 5, Orientation.Horizontal, LineStyle.Single); var region = new Region (new (0, 0, 2, 1)); @@ -260,7 +260,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Fill_Property_Defaults_To_Null () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); Assert.Null (canvas.Fill); } @@ -688,7 +688,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Theory] public void Length_0_Is_1_Long (int x, int y, Orientation orientation, string expected) { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // Add a line at 5, 5 that's has length of 1 canvas.AddLine (new (x, y), 1, orientation, LineStyle.Single); @@ -741,9 +741,10 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [InlineData (-1, 0, -2, Orientation.Vertical, "│\r\n│")] [InlineData (0, -1, -2, Orientation.Vertical, "│\r\n│")] [InlineData (-1, -1, -2, Orientation.Vertical, "│\r\n│")] - [Theory] public void Length_n_Is_n_Long (int x, int y, int length, Orientation orientation, string expected) + [Theory] + public void Length_n_Is_n_Long (int x, int y, int length, Orientation orientation, string expected) { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (new (x, y), length, orientation, LineStyle.Single); var result = canvas.ToString (); @@ -755,7 +756,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase { var offset = new Point (5, 5); - var canvas = new LineCanvas (); + LineCanvas canvas = new (); canvas.AddLine (offset, -3, Orientation.Horizontal, LineStyle.Single); var looksLike = "───"; @@ -820,7 +821,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void TestLineCanvas_LeaveMargin_Top1_Left1 () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // Upper box canvas.AddLine (Point.Empty, 2, Orientation.Horizontal, LineStyle.Single); @@ -927,7 +928,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Top_Left_From_TopRight_LeftUp () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // Upper box canvas.AddLine (Point.Empty, 2, Orientation.Horizontal, LineStyle.Single); @@ -943,7 +944,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Top_With_1Down () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // Top ─ canvas.AddLine (Point.Empty, 1, Orientation.Horizontal, LineStyle.Single); @@ -1328,7 +1329,7 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase [Fact] public void Window () { - var canvas = new LineCanvas (); + LineCanvas canvas = new (); // Frame canvas.AddLine (Point.Empty, 10, Orientation.Horizontal, LineStyle.Single); @@ -1507,4 +1508,360 @@ public class LineCanvasTests (ITestOutputHelper output) : FakeDriverBase return v; } + + #region GetRegion Tests + + [Fact] + public void GetRegion_EmptyCellMap_ReturnsEmptyRegion () + { + Dictionary cellMap = new (); + Region region = LineCanvas.GetRegion (cellMap); + + Assert.NotNull (region); + Assert.True (region.IsEmpty ()); + } + + [Fact] + public void GetRegion_SingleCell_ReturnsSingleRectangle () + { + Dictionary cellMap = new () + { + { new Point (5, 10), new Cell { Grapheme = "X" } } + }; + + Region region = LineCanvas.GetRegion (cellMap); + + Assert.NotNull (region); + Assert.False (region.IsEmpty ()); + Assert.True (region.Contains (5, 10)); + } + + [Fact] + public void GetRegion_HorizontalLine_CreatesHorizontalSpan () + { + Dictionary cellMap = new (); + // Horizontal line from (5, 10) to (9, 10) + for (int x = 5; x <= 9; x++) + { + cellMap.Add (new Point (x, 10), new Cell { Grapheme = "─" }); + } + + Region region = LineCanvas.GetRegion (cellMap); + + Assert.NotNull (region); + // All cells in the horizontal span should be in the region + for (int x = 5; x <= 9; x++) + { + Assert.True (region.Contains (x, 10), $"Expected ({x}, 10) to be in region"); + } + // Cells outside the span should not be in the region + Assert.False (region.Contains (4, 10)); + Assert.False (region.Contains (10, 10)); + Assert.False (region.Contains (7, 9)); + Assert.False (region.Contains (7, 11)); + } + + [Fact] + public void GetRegion_VerticalLine_CreatesMultipleHorizontalSpans () + { + Dictionary cellMap = new (); + // Vertical line from (5, 10) to (5, 14) + for (int y = 10; y <= 14; y++) + { + cellMap.Add (new Point (5, y), new Cell { Grapheme = "│" }); + } + + Region region = LineCanvas.GetRegion (cellMap); + + Assert.NotNull (region); + // All cells in the vertical line should be in the region + for (int y = 10; y <= 14; y++) + { + Assert.True (region.Contains (5, y), $"Expected (5, {y}) to be in region"); + } + // Cells outside should not be in the region + Assert.False (region.Contains (4, 12)); + Assert.False (region.Contains (6, 12)); + } + + [Fact] + public void GetRegion_LShape_CreatesCorrectSpans () + { + Dictionary cellMap = new (); + // L-shape: horizontal line from (0, 0) to (5, 0), then vertical to (5, 3) + for (int x = 0; x <= 5; x++) + { + cellMap.Add (new Point (x, 0), new Cell { Grapheme = "─" }); + } + for (int y = 1; y <= 3; y++) + { + cellMap.Add (new Point (5, y), new Cell { Grapheme = "│" }); + } + + Region region = LineCanvas.GetRegion (cellMap); + + // Horizontal part + for (int x = 0; x <= 5; x++) + { + Assert.True (region.Contains (x, 0), $"Expected ({x}, 0) to be in region"); + } + // Vertical part + for (int y = 1; y <= 3; y++) + { + Assert.True (region.Contains (5, y), $"Expected (5, {y}) to be in region"); + } + // Empty cells should not be in region + Assert.False (region.Contains (1, 1)); + Assert.False (region.Contains (4, 2)); + } + + [Fact] + public void GetRegion_DiscontiguousHorizontalCells_CreatesSeparateSpans () + { + Dictionary cellMap = new () + { + { new Point (0, 5), new Cell { Grapheme = "X" } }, + { new Point (1, 5), new Cell { Grapheme = "X" } }, + // Gap at (2, 5) + { new Point (3, 5), new Cell { Grapheme = "X" } }, + { new Point (4, 5), new Cell { Grapheme = "X" } } + }; + + Region region = LineCanvas.GetRegion (cellMap); + + Assert.True (region.Contains (0, 5)); + Assert.True (region.Contains (1, 5)); + Assert.False (region.Contains (2, 5)); // Gap + Assert.True (region.Contains (3, 5)); + Assert.True (region.Contains (4, 5)); + } + + [Fact] + public void GetRegion_IntersectingLines_CreatesCorrectRegion () + { + Dictionary cellMap = new (); + // Horizontal line + for (int x = 0; x <= 4; x++) + { + cellMap.Add (new Point (x, 2), new Cell { Grapheme = "─" }); + } + // Vertical line intersecting at (2, 2) + for (int y = 0; y <= 4; y++) + { + cellMap [new Point (2, y)] = new Cell { Grapheme = "┼" }; + } + + Region region = LineCanvas.GetRegion (cellMap); + + // Horizontal line + for (int x = 0; x <= 4; x++) + { + Assert.True (region.Contains (x, 2), $"Expected ({x}, 2) to be in region"); + } + // Vertical line + for (int y = 0; y <= 4; y++) + { + Assert.True (region.Contains (2, y), $"Expected (2, {y}) to be in region"); + } + } + + #endregion + + #region GetCellMapWithRegion Tests + + [Fact] + public void GetCellMapWithRegion_EmptyCanvas_ReturnsEmptyMapAndRegion () + { + LineCanvas canvas = new (); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.Empty (cellMap); + Assert.NotNull (region); + Assert.True (region.IsEmpty ()); + } + + [Fact] + public void GetCellMapWithRegion_SingleHorizontalLine_ReturnsCellMapAndRegion () + { + LineCanvas canvas = new (); + canvas.AddLine (new Point (5, 10), 5, Orientation.Horizontal, LineStyle.Single); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + Assert.NotNull (region); + Assert.False (region.IsEmpty ()); + + // Both cellMap and region should contain the same cells + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + } + + [Fact] + public void GetCellMapWithRegion_SingleVerticalLine_ReturnsCellMapAndRegion () + { + LineCanvas canvas = new (); + canvas.AddLine (new Point (5, 10), 5, Orientation.Vertical, LineStyle.Single); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + Assert.NotNull (region); + Assert.False (region.IsEmpty ()); + + // Both cellMap and region should contain the same cells + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + } + + [Fact] + public void GetCellMapWithRegion_IntersectingLines_CorrectlyHandlesIntersection () + { + LineCanvas canvas = new (); + // Create a cross pattern + canvas.AddLine (new Point (0, 2), 5, Orientation.Horizontal, LineStyle.Single); + canvas.AddLine (new Point (2, 0), 5, Orientation.Vertical, LineStyle.Single); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + Assert.NotNull (region); + + // Verify intersection point is in both + Assert.True (cellMap.ContainsKey (new Point (2, 2)), "Intersection should be in cellMap"); + Assert.True (region.Contains (2, 2), "Intersection should be in region"); + + // All cells should be in both structures + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + } + + [Fact] + public void GetCellMapWithRegion_ComplexShape_RegionMatchesCellMap () + { + LineCanvas canvas = new (); + // Create a box + canvas.AddLine (new Point (0, 0), 5, Orientation.Horizontal, LineStyle.Single); + canvas.AddLine (new Point (0, 3), 5, Orientation.Horizontal, LineStyle.Single); + canvas.AddLine (new Point (0, 0), 4, Orientation.Vertical, LineStyle.Single); + canvas.AddLine (new Point (4, 0), 4, Orientation.Vertical, LineStyle.Single); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + Assert.NotNull (region); + + // Every cell in the map should be in the region + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + + // Cells not in the map should not be in the region (interior of box) + Assert.False (cellMap.ContainsKey (new Point (2, 1))); + // Note: Region might contain interior if it's filled, so we just verify consistency + } + + [Fact] + public void GetCellMapWithRegion_ResultsMatchSeparateCalls () + { + LineCanvas canvas = new (); + // Create a complex pattern + canvas.AddLine (new Point (0, 0), 10, Orientation.Horizontal, LineStyle.Single); + canvas.AddLine (new Point (5, 0), 10, Orientation.Vertical, LineStyle.Single); + canvas.AddLine (new Point (0, 5), 10, Orientation.Horizontal, LineStyle.Double); + + // Get results from combined method + (Dictionary combinedCellMap, Region combinedRegion) = canvas.GetCellMapWithRegion (); + + // Get results from separate calls + Dictionary separateCellMap = canvas.GetCellMap (); + Region separateRegion = LineCanvas.GetRegion (separateCellMap); + + // Cell maps should be identical + Assert.Equal (separateCellMap.Count, combinedCellMap.Count); + foreach (KeyValuePair kvp in separateCellMap) + { + Assert.True (combinedCellMap.ContainsKey (kvp.Key), $"Combined map missing key {kvp.Key}"); + } + + // Regions should contain the same points + foreach (Point p in combinedCellMap.Keys) + { + Assert.True (combinedRegion.Contains (p.X, p.Y), $"Combined region missing ({p.X}, {p.Y})"); + Assert.True (separateRegion.Contains (p.X, p.Y), $"Separate region missing ({p.X}, {p.Y})"); + } + } + + [Fact] + public void GetCellMapWithRegion_NegativeCoordinates_HandlesCorrectly () + { + LineCanvas canvas = new (); + canvas.AddLine (new Point (-5, -5), 10, Orientation.Horizontal, LineStyle.Single); + canvas.AddLine (new Point (0, -5), 10, Orientation.Vertical, LineStyle.Single); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + Assert.NotNull (region); + + // Verify negative coordinates are handled + Assert.True (cellMap.Keys.Any (p => p.X < 0 || p.Y < 0), "Should have negative coordinates"); + + // All cells should be in region + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + } + + [Fact] + public void GetCellMapWithRegion_WithExclusion_RegionExcludesExcludedCells () + { + LineCanvas canvas = new (); + canvas.AddLine (new Point (0, 0), 10, Orientation.Horizontal, LineStyle.Single); + + // Exclude middle section + Region exclusionRegion = new (); + exclusionRegion.Combine (new Rectangle (3, 0, 4, 1), RegionOp.Union); + canvas.Exclude (exclusionRegion); + + (Dictionary cellMap, Region region) = canvas.GetCellMapWithRegion (); + + Assert.NotNull (cellMap); + Assert.NotEmpty (cellMap); + + // Excluded cells should not be in cellMap + for (int x = 3; x < 7; x++) + { + Assert.False (cellMap.ContainsKey (new Point (x, 0)), $"({x}, 0) should be excluded from cellMap"); + } + + // Region should match cellMap + foreach (Point p in cellMap.Keys) + { + Assert.True (region.Contains (p.X, p.Y), $"Expected ({p.X}, {p.Y}) to be in region"); + } + + // Excluded points should not be in region + for (int x = 3; x < 7; x++) + { + Assert.False (region.Contains (x, 0), $"({x}, 0) should be excluded from region"); + } + } + + #endregion } diff --git a/Tests/UnitTestsParallelizable/xunit.runner.json b/Tests/UnitTestsParallelizable/xunit.runner.json index bc6c60196..ba11279f6 100644 --- a/Tests/UnitTestsParallelizable/xunit.runner.json +++ b/Tests/UnitTestsParallelizable/xunit.runner.json @@ -3,5 +3,5 @@ "parallelizeTestCollections": true, "parallelizeAssembly": true, "stopOnFail": false, - "maxParallelThreads": "4x" + "maxParallelThreads": "default" } \ No newline at end of file