Improvements to LineDrawing scenario (#2732)

* Improvements to LineDrawing scenario
- Add drag drawing of current line
- Add undo/redo
- LineCanvas is now more mutable with StraightLine now public and mutable

* Prevent redo after drawing

* Fix xmldoc and test

---------

Co-authored-by: Tig <[email protected]>

Terminal.Gui/Drawing/LineCanvas.cs

@@ -3,8 +3,6 @@ using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
-
-
 namespace Terminal.Gui {
 
 	/// <summary>
@@ -123,12 +121,30 @@ namespace Terminal.Gui {
 			_lines.Add (new StraightLine (start, length, orientation, style, attribute));
 		}
 
-		private void AddLine (StraightLine line)
+		/// <summary>
+		/// Adds a new line to the canvas
+		/// </summary>
+		/// <param name="line"></param>
+		public void AddLine (StraightLine line)
 		{
 			_cachedBounds = Rect.Empty;
 			_lines.Add (line);
 		}
 
+		/// <summary>
+		/// Removes the last line added to the canvas
+		/// </summary>
+		/// <returns></returns>
+		public StraightLine RemoveLastLine()
+		{
+			var l = _lines.LastOrDefault ();
+			if(l != null) {
+				_lines.Remove(l);
+			}
+
+			return l;
+		}
+
 		/// <summary>
 		/// Clears all lines from the LineCanvas.
 		/// </summary>
@@ -138,6 +154,15 @@ namespace Terminal.Gui {
 			_lines.Clear ();
 		}
 
+		/// <summary>
+		/// Clears any cached states from the canvas
+		/// Call this method if you make changes to lines
+		/// that have already been added.
+		/// </summary>
+		public void ClearCache ()
+		{
+			_cachedBounds = Rect.Empty;
+		}
 		private Rect _cachedBounds;
 
 		/// <summary>
@@ -703,256 +728,93 @@ namespace Terminal.Gui {
 				AddLine (line);
 			}
 		}
-
-		internal class IntersectionDefinition {
-			/// <summary>
-			/// The point at which the intersection happens
-			/// </summary>
-			internal Point Point { get; }
-
-			/// <summary>
-			/// Defines how <see cref="Line"/> position relates
-			/// to <see cref="Point"/>.
-			/// </summary>
-			internal IntersectionType Type { get; }
-
-			/// <summary>
-			/// The line that intersects <see cref="Point"/>
-			/// </summary>
-			internal StraightLine Line { get; }
-
-			internal IntersectionDefinition (Point point, IntersectionType type, StraightLine line)
-			{
-				Point = point;
-				Type = type;
-				Line = line;
-			}
-		}
-
+	}
+	internal class IntersectionDefinition {
 		/// <summary>
-		/// The type of Rune that we will use before considering
-		/// double width, curved borders etc
-		/// </summary>
-		internal enum IntersectionRuneType {
-			None,
-			Dot,
-			ULCorner,
-			URCorner,
-			LLCorner,
-			LRCorner,
-			TopTee,
-			BottomTee,
-			RightTee,
-			LeftTee,
-			Cross,
-			HLine,
-			VLine,
-		}
-
-		internal enum IntersectionType {
-			/// <summary>
-			/// There is no intersection
-			/// </summary>
-			None,
-
-			/// <summary>
-			///  A line passes directly over this point traveling along
-			///  the horizontal axis
-			/// </summary>
-			PassOverHorizontal,
-
-			/// <summary>
-			///  A line passes directly over this point traveling along
-			///  the vertical axis
-			/// </summary>
-			PassOverVertical,
-
-			/// <summary>
-			/// A line starts at this point and is traveling up
-			/// </summary>
-			StartUp,
-
-			/// <summary>
-			/// A line starts at this point and is traveling right
-			/// </summary>
-			StartRight,
+		/// The point at which the intersection happens
+		/// </summary>
+		internal Point Point { get; }
 
-			/// <summary>
-			/// A line starts at this point and is traveling down
-			/// </summary>
-			StartDown,
+		/// <summary>
+		/// Defines how <see cref="Line"/> position relates
+		/// to <see cref="Point"/>.
+		/// </summary>
+		internal IntersectionType Type { get; }
 
-			/// <summary>
-			/// A line starts at this point and is traveling left
-			/// </summary>
-			StartLeft,
+		/// <summary>
+		/// The line that intersects <see cref="Point"/>
+		/// </summary>
+		internal StraightLine Line { get; }
 
-			/// <summary>
-			/// A line exists at this point who has 0 length
-			/// </summary>
-			Dot
+		internal IntersectionDefinition (Point point, IntersectionType type, StraightLine line)
+		{
+			Point = point;
+			Type = type;
+			Line = line;
 		}
+	}
 
-		// TODO: Add events that notify when StraightLine changes to enable dynamic layout
-		internal class StraightLine {
-			public Point Start { get; }
-			public int Length { get; }
-			public Orientation Orientation { get; }
-			public LineStyle Style { get; }
-			public Attribute? Attribute { get; set; }
-
-			internal StraightLine (Point start, int length, Orientation orientation, LineStyle style, Attribute? attribute = default)
-			{
-				this.Start = start;
-				this.Length = length;
-				this.Orientation = orientation;
-				this.Style = style;
-				this.Attribute = attribute;
-			}
-
-			internal IntersectionDefinition Intersects (int x, int y)
-			{
-				switch (Orientation) {
-				case Orientation.Horizontal: return IntersectsHorizontally (x, y);
-				case Orientation.Vertical: return IntersectsVertically (x, y);
-				default: throw new ArgumentOutOfRangeException (nameof (Orientation));
-				}
-
-			}
-
-			private IntersectionDefinition IntersectsHorizontally (int x, int y)
-			{
-				if (Start.Y != y) {
-					return null;
-				} else {
-					if (StartsAt (x, y)) {
-
-						return new IntersectionDefinition (
-							Start,
-							GetTypeByLength (IntersectionType.StartLeft, IntersectionType.PassOverHorizontal, IntersectionType.StartRight),
-							this
-							);
-
-					}
-
-					if (EndsAt (x, y)) {
-
-						return new IntersectionDefinition (
-							Start,
-							Length < 0 ? IntersectionType.StartRight : IntersectionType.StartLeft,
-							this
-							);
-
-					} else {
-						var xmin = Math.Min (Start.X, Start.X + Length);
-						var xmax = Math.Max (Start.X, Start.X + Length);
-
-						if (xmin < x && xmax > x) {
-							return new IntersectionDefinition (
-							new Point (x, y),
-							IntersectionType.PassOverHorizontal,
-							this
-							);
-						}
-					}
-
-					return null;
-				}
-			}
-
-			private IntersectionDefinition IntersectsVertically (int x, int y)
-			{
-				if (Start.X != x) {
-					return null;
-				} else {
-					if (StartsAt (x, y)) {
-
-						return new IntersectionDefinition (
-							Start,
-							GetTypeByLength (IntersectionType.StartUp, IntersectionType.PassOverVertical, IntersectionType.StartDown),
-							this
-							);
-
-					}
-
-					if (EndsAt (x, y)) {
-
-						return new IntersectionDefinition (
-							Start,
-							Length < 0 ? IntersectionType.StartDown : IntersectionType.StartUp,
-							this
-							);
-
-					} else {
-						var ymin = Math.Min (Start.Y, Start.Y + Length);
-						var ymax = Math.Max (Start.Y, Start.Y + Length);
-
-						if (ymin < y && ymax > y) {
-							return new IntersectionDefinition (
-							new Point (x, y),
-							IntersectionType.PassOverVertical,
-							this
-							);
-						}
-					}
-
-					return null;
-				}
-			}
-
-			private IntersectionType GetTypeByLength (IntersectionType typeWhenNegative, IntersectionType typeWhenZero, IntersectionType typeWhenPositive)
-			{
-				if (Length == 0) {
-					return typeWhenZero;
-				}
-
-				return Length < 0 ? typeWhenNegative : typeWhenPositive;
-			}
+	/// <summary>
+	/// The type of Rune that we will use before considering
+	/// double width, curved borders etc
+	/// </summary>
+	internal enum IntersectionRuneType {
+		None,
+		Dot,
+		ULCorner,
+		URCorner,
+		LLCorner,
+		LRCorner,
+		TopTee,
+		BottomTee,
+		RightTee,
+		LeftTee,
+		Cross,
+		HLine,
+		VLine,
+	}
 
-			private bool EndsAt (int x, int y)
-			{
-				var sub = (Length == 0) ? 0 : (Length > 0) ? 1 : -1;
-				if (Orientation == Orientation.Horizontal) {
-					return Start.X + Length - sub == x && Start.Y == y;
-				}
+	internal enum IntersectionType {
+		/// <summary>
+		/// There is no intersection
+		/// </summary>
+		None,
 
-				return Start.X == x && Start.Y + Length - sub == y;
-			}
+		/// <summary>
+		///  A line passes directly over this point traveling along
+		///  the horizontal axis
+		/// </summary>
+		PassOverHorizontal,
 
-			private bool StartsAt (int x, int y)
-			{
-				return Start.X == x && Start.Y == y;
-			}
+		/// <summary>
+		///  A line passes directly over this point traveling along
+		///  the vertical axis
+		/// </summary>
+		PassOverVertical,
 
-			/// <summary>
-			/// Gets the rectangle that describes the bounds of the canvas. Location is the coordinates of the 
-			/// line that is furthest left/top and Size is defined by the line that extends the furthest
-			/// right/bottom.
-			/// </summary>
-			internal Rect Bounds {
-				get {
+		/// <summary>
+		/// A line starts at this point and is traveling up
+		/// </summary>
+		StartUp,
 
-					// 0 and 1/-1 Length means a size (width or height) of 1
-					var size = Math.Max (1, Math.Abs (Length));
+		/// <summary>
+		/// A line starts at this point and is traveling right
+		/// </summary>
+		StartRight,
 
-					// How much to offset x or y to get the start of the line
-					var offset = Math.Abs (Length < 0 ? Length + 1 : 0);
-					var x = Start.X - (Orientation == Orientation.Horizontal ? offset : 0);
-					var y = Start.Y - (Orientation == Orientation.Vertical ? offset : 0);
-					var width = Orientation == Orientation.Horizontal ? size : 1;
-					var height = Orientation == Orientation.Vertical ? size : 1;
+		/// <summary>
+		/// A line starts at this point and is traveling down
+		/// </summary>
+		StartDown,
 
-					return new Rect (x, y, width, height);
-				}
-			}
+		/// <summary>
+		/// A line starts at this point and is traveling left
+		/// </summary>
+		StartLeft,
 
-			/// <summary>
-			/// Formats the Line as a string in (Start.X,Start.Y,Length,Orientation) notation.
-			/// </summary>
-			public override string ToString ()
-			{
-				return $"({Start.X},{Start.Y},{Length},{Orientation})";
-			}
-		}
+		/// <summary>
+		/// A line exists at this point who has 0 length
+		/// </summary>
+		Dot
 	}
 }

Terminal.Gui/Drawing/StraightLine.cs

@@ -0,0 +1,196 @@
+using System;
+namespace Terminal.Gui {
+	// TODO: Add events that notify when StraightLine changes to enable dynamic layout
+	/// <summary>
+	/// A line between two points on a horizontal or vertical <see cref="Orientation"/>
+	/// and a given style/color.
+	/// </summary>
+	public class StraightLine {
+
+		/// <summary>
+		/// Gets or sets where the line begins.
+		/// </summary>
+		public Point Start { get; set; }
+
+		/// <summary>
+		/// Gets or sets the length of the line.
+		/// </summary>
+		public int Length { get; set; }
+
+		/// <summary>
+		/// Gets or sets the orientation (horizontal or vertical) of the line.
+		/// </summary>
+		public Orientation Orientation { get; set; }
+
+		/// <summary>
+		/// Gets or sets the line style of the line (e.g. dotted, double).
+		/// </summary>
+		public LineStyle Style { get; set; }
+
+		/// <summary>
+		/// Gets or sets the color of the line.
+		/// </summary>
+		public Attribute? Attribute { get; set; }
+
+		/// <summary>
+		/// Creates a new instance of the <see cref="StraightLine"/> class.
+		/// </summary>
+		/// <param name="start"></param>
+		/// <param name="length"></param>
+		/// <param name="orientation"></param>
+		/// <param name="style"></param>
+		/// <param name="attribute"></param>
+		public StraightLine (Point start, int length, Orientation orientation, LineStyle style, Attribute? attribute = default)
+		{
+			this.Start = start;
+			this.Length = length;
+			this.Orientation = orientation;
+			this.Style = style;
+			this.Attribute = attribute;
+		}
+
+		internal IntersectionDefinition Intersects (int x, int y)
+		{
+			switch (Orientation) {
+			case Orientation.Horizontal: return IntersectsHorizontally (x, y);
+			case Orientation.Vertical: return IntersectsVertically (x, y);
+			default: throw new ArgumentOutOfRangeException (nameof (Orientation));
+			}
+
+		}
+
+		private IntersectionDefinition IntersectsHorizontally (int x, int y)
+		{
+			if (Start.Y != y) {
+				return null;
+			} else {
+				if (StartsAt (x, y)) {
+
+					return new IntersectionDefinition (
+						Start,
+						GetTypeByLength (IntersectionType.StartLeft, IntersectionType.PassOverHorizontal, IntersectionType.StartRight),
+						this
+						);
+
+				}
+
+				if (EndsAt (x, y)) {
+
+					return new IntersectionDefinition (
+						Start,
+						Length < 0 ? IntersectionType.StartRight : IntersectionType.StartLeft,
+						this
+						);
+
+				} else {
+					var xmin = Math.Min (Start.X, Start.X + Length);
+					var xmax = Math.Max (Start.X, Start.X + Length);
+
+					if (xmin < x && xmax > x) {
+						return new IntersectionDefinition (
+						new Point (x, y),
+						IntersectionType.PassOverHorizontal,
+						this
+						);
+					}
+				}
+
+				return null;
+			}
+		}
+
+		private IntersectionDefinition IntersectsVertically (int x, int y)
+		{
+			if (Start.X != x) {
+				return null;
+			} else {
+				if (StartsAt (x, y)) {
+
+					return new IntersectionDefinition (
+						Start,
+						GetTypeByLength (IntersectionType.StartUp, IntersectionType.PassOverVertical, IntersectionType.StartDown),
+						this
+						);
+
+				}
+
+				if (EndsAt (x, y)) {
+
+					return new IntersectionDefinition (
+						Start,
+						Length < 0 ? IntersectionType.StartDown : IntersectionType.StartUp,
+						this
+						);
+
+				} else {
+					var ymin = Math.Min (Start.Y, Start.Y + Length);
+					var ymax = Math.Max (Start.Y, Start.Y + Length);
+
+					if (ymin < y && ymax > y) {
+						return new IntersectionDefinition (
+						new Point (x, y),
+						IntersectionType.PassOverVertical,
+						this
+						);
+					}
+				}
+
+				return null;
+			}
+		}
+
+		private IntersectionType GetTypeByLength (IntersectionType typeWhenNegative, IntersectionType typeWhenZero, IntersectionType typeWhenPositive)
+		{
+			if (Length == 0) {
+				return typeWhenZero;
+			}
+
+			return Length < 0 ? typeWhenNegative : typeWhenPositive;
+		}
+
+		private bool EndsAt (int x, int y)
+		{
+			var sub = (Length == 0) ? 0 : (Length > 0) ? 1 : -1;
+			if (Orientation == Orientation.Horizontal) {
+				return Start.X + Length - sub == x && Start.Y == y;
+			}
+
+			return Start.X == x && Start.Y + Length - sub == y;
+		}
+
+		private bool StartsAt (int x, int y)
+		{
+			return Start.X == x && Start.Y == y;
+		}
+
+		/// <summary>
+		/// Gets the rectangle that describes the bounds of the canvas. Location is the coordinates of the 
+		/// line that is furthest left/top and Size is defined by the line that extends the furthest
+		/// right/bottom.
+		/// </summary>
+		internal Rect Bounds {
+			get {
+
+				// 0 and 1/-1 Length means a size (width or height) of 1
+				var size = Math.Max (1, Math.Abs (Length));
+
+				// How much to offset x or y to get the start of the line
+				var offset = Math.Abs (Length < 0 ? Length + 1 : 0);
+				var x = Start.X - (Orientation == Orientation.Horizontal ? offset : 0);
+				var y = Start.Y - (Orientation == Orientation.Vertical ? offset : 0);
+				var width = Orientation == Orientation.Horizontal ? size : 1;
+				var height = Orientation == Orientation.Vertical ? size : 1;
+
+				return new Rect (x, y, width, height);
+			}
+		}
+
+		/// <summary>
+		/// Formats the Line as a string in (Start.X,Start.Y,Length,Orientation) notation.
+		/// </summary>
+		public override string ToString ()
+		{
+			return $"({Start.X},{Start.Y},{Length},{Orientation})";
+		}
+	}
+}

UICatalog/Scenarios/LineDrawing.cs

@@ -1,8 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Reflection.Metadata.Ecma335;
-using System.Text;
 using Terminal.Gui;
 using Attribute = Terminal.Gui.Attribute;
 
@@ -24,7 +22,7 @@ namespace UICatalog.Scenarios {
 
 			var tools = new ToolsView () {
 				Title = "Tools",
-				X = Pos.Right(canvas) - 20,
+				X = Pos.Right (canvas) - 20,
 				Y = 2
 			};
 
@@ -34,6 +32,8 @@ namespace UICatalog.Scenarios {
 
 			Win.Add (canvas);
 			Win.Add (tools);
+
+			Win.KeyPress += (s,e) => { e.Handled = canvas.ProcessKey (e.KeyEvent); };
 		}
 
 		class ToolsView : Window {
@@ -55,7 +55,7 @@ namespace UICatalog.Scenarios {
 			private void ToolsView_Initialized (object sender, EventArgs e)
 			{
 				LayoutSubviews ();
-				Width = Math.Max (_colorPicker.Frame.Width, _stylePicker.Frame.Width) + GetFramesThickness().Horizontal;
+				Width = Math.Max (_colorPicker.Frame.Width, _stylePicker.Frame.Width) + GetFramesThickness ().Horizontal;
 				Height = _colorPicker.Frame.Height + _stylePicker.Frame.Height + _addLayerBtn.Frame.Height + GetFramesThickness ().Vertical;
 				SuperView.LayoutSubviews ();
 			}
@@ -97,7 +97,7 @@ namespace UICatalog.Scenarios {
 			List<LineCanvas> _layers = new List<LineCanvas> ();
 			LineCanvas _currentLayer;
 			Color _currentColor = Color.White;
-			Point? _currentLineStart = null;
+			StraightLine? _currentLine = null;
 
 			public LineStyle LineStyle { get; set; }
 
@@ -106,18 +106,41 @@ namespace UICatalog.Scenarios {
 				AddLayer ();
 			}
 
+			Stack<StraightLine> undoHistory = new ();
+
+			public override bool ProcessKey (KeyEvent e)
+			{
+				if (e.Key == (Key.Z | Key.CtrlMask)) {
+					var pop = _currentLayer.RemoveLastLine ();
+					if(pop != null) {
+						undoHistory.Push (pop);
+						SetNeedsDisplay ();
+						return true;
+					}
+				}
+
+				if (e.Key == (Key.Y | Key.CtrlMask)) {
+					if (undoHistory.Any()) {
+						var pop = undoHistory.Pop ();
+						_currentLayer.AddLine(pop);
+						SetNeedsDisplay ();
+						return true;
+					}
+				}
+
+				return base.ProcessKey (e);
+			}
 			internal void AddLayer ()
 			{
 				_currentLayer = new LineCanvas ();
 				_layers.Add (_currentLayer);
 			}
 
-			public override void OnDrawContent (Rect contentArea)
+			public override void OnDrawContentComplete (Rect contentArea)
 			{
-				base.OnDrawContent (contentArea);
-
+				base.OnDrawContentComplete (contentArea);
 				foreach (var canvas in _layers) {
-					
+
 					foreach (var c in canvas.GetCellMap ()) {
 						Driver.SetAttribute (c.Value.Attribute?.Value ?? ColorScheme.Normal);
 						this.AddRune (c.Key.X, c.Key.Y, c.Value.Rune.Value);
@@ -128,14 +151,15 @@ namespace UICatalog.Scenarios {
 			public override bool OnMouseEvent (MouseEvent mouseEvent)
 			{
 				if (mouseEvent.Flags.HasFlag (MouseFlags.Button1Pressed)) {
-					if (_currentLineStart == null) {
-						_currentLineStart = new Point (mouseEvent.X - GetBoundsOffset().X, mouseEvent.Y - GetBoundsOffset ().X);
-					}
-				} else {
-					if (_currentLineStart != null) {
-
-						var start = _currentLineStart.Value;
-						var end = new Point (mouseEvent.X - GetBoundsOffset ().X, mouseEvent.Y - GetBoundsOffset ().X);
+					if (_currentLine == null) {
+
+						_currentLine = new StraightLine (
+							new Point (mouseEvent.X - GetBoundsOffset ().X, mouseEvent.Y - GetBoundsOffset ().X),
+							0, Orientation.Vertical, LineStyle, new Attribute (_currentColor, GetNormalColor ().Background));
+						_currentLayer.AddLine (_currentLine);
+					} else {
+						var start = _currentLine.Start;
+						var end = new Point (mouseEvent.X - GetBoundsOffset ().X, mouseEvent.Y - GetBoundsOffset ().Y);
 						var orientation = Orientation.Vertical;
 						var length = end.Y - start.Y;
 
@@ -150,15 +174,15 @@ namespace UICatalog.Scenarios {
 						} else {
 							length--;
 						}
-
-						_currentLayer.AddLine (
-							start,
-							length,
-							orientation,
-							LineStyle,
-							new Attribute (_currentColor, GetNormalColor().Background));
-
-						_currentLineStart = null;
+						_currentLine.Length = length;
+						_currentLine.Orientation = orientation;
+						_currentLayer.ClearCache ();
+						SetNeedsDisplay ();
+					}
+				} else {
+					if (_currentLine != null) {
+						_currentLine = null;
+						undoHistory.Clear ();
 						SetNeedsDisplay ();
 					}
 				}

UnitTests/Drawing/StraightLineTests.cs

@@ -81,7 +81,7 @@ namespace Terminal.Gui.DrawingTests {
 		[Theory, SetupFakeDriver]
 		public void Bounds (Orientation orientation, int x, int y, int length, int expectedX, int expectedY, int expectedWidth, int expectedHeight)
 		{
-			var sl = new LineCanvas.StraightLine (new Point (x, y), length, orientation, LineStyle.Single);
+			var sl = new StraightLine (new Point (x, y), length, orientation, LineStyle.Single);
 
 			Assert.Equal (new Rect (expectedX, expectedY, expectedWidth, expectedHeight), sl.Bounds);
 		}