Implemented deferred Margin drawing to enable shadow with minimal perf hit

Terminal.Gui/Application/Application.Run.cs

@@ -547,12 +547,38 @@ public static partial class Application // Run (Begin, Run, End, Stop)
             }
 
             tl.Draw ();
-            ExcludeFromClip (tl.FrameToScreen ());
         }
 
+        DrawMargins (TopLevels.Cast<View> ().ToList ());
+
         ClipToScreen ();
     }
 
+    // TODO: This is inefficent
+    private static bool DrawMargins (List<View> peers)
+    {
+        if (peers.Count == 0)
+        {
+            return false;
+        }
+        foreach (View view in peers)
+        {
+            if (view.Margin is { CachedClip: { }})
+            {
+                view.Margin.NeedsDraw = true;
+                Region? saved = Driver?.Clip;
+                Application.SetClip (view.Margin.CachedClip);
+                view.Margin.Draw ();
+                Application.SetClip (saved);
+            }
+            view.Margin.CachedClip = null;
+
+            DrawMargins (view.Subviews.ToList ());
+            view.NeedsDraw = false;
+        }
+
+        return true;
+    }
 
     /// <summary>This event is raised on each iteration of the main loop.</summary>
     /// <remarks>See also <see cref="Timeout"/></remarks>

Terminal.Gui/View/Adornment/Margin.cs

@@ -16,6 +16,11 @@ namespace Terminal.Gui;
 /// </remarks>
 public class Margin : Adornment
 {
+    private const int SHADOW_WIDTH = 1;
+    private const int SHADOW_HEIGHT = 1;
+    private const int PRESS_MOVE_HORIZONTAL = 1;
+    private const int PRESS_MOVE_VERTICAL = 0;
+
     /// <inheritdoc/>
     public Margin ()
     { /* Do nothing; A parameter-less constructor is required to support all views unit tests. */
@@ -35,6 +40,8 @@ public class Margin : Adornment
         CanFocus = false;
     }
 
+    public Region? CachedClip { get; set; }
+
     private bool _pressed;
 
     private ShadowView? _bottomShadow;
@@ -88,44 +95,12 @@ public class Margin : Adornment
         if (ShadowStyle != ShadowStyle.None)
         {
             // Don't clear where the shadow goes
-            screen = Rectangle.Inflate (screen, -1, -1);
+            screen = Rectangle.Inflate (screen, -SHADOW_WIDTH, -SHADOW_HEIGHT);
         }
 
-        // This just draws/clears the thickness, not the insides.
-        Thickness.Draw (screen, Diagnostics, ToString ());
-
         return true;
     }
 
-    ///// <inheritdoc />
-    //protected override bool OnDrawingContent ()
-    //{
-    //    Rectangle screen = FrameToScreen();
-    //    for (int r = 0; r < screen.Height; r++)
-    //    {
-    //        for (int c = 0; c < screen.Width; c++)
-    //        {
-    //            Driver?.Move (c, r);
-
-    //            if (Driver?.Contents is { } && c < Driver.Contents.GetLength (1) && r < Driver.Contents.GetLength (0))
-    //            {
-    //                Driver.AddRune (Driver.Contents [r, c].Rune);
-    //            }
-    //        }
-    //    }
-    //    return true;
-    //}
-
-    ///// <inheritdoc />
-    ////protected override bool OnDrawSubviews (Rectangle viewport) { return true; }
-
-    //protected override bool OnDrawComplete (Rectangle viewport)
-    //{
-    //    DoDrawSubviews (viewport);
-
-    //    return true;
-    //}
-
     /// <summary>
     ///     Sets whether the Margin includes a shadow effect. The shadow is drawn on the right and bottom sides of the
     ///     Margin.
@@ -149,22 +124,22 @@ public class Margin : Adornment
         if (ShadowStyle != ShadowStyle.None)
         {
             // Turn off shadow
-            Thickness = new (Thickness.Left, Thickness.Top, Thickness.Right - 1, Thickness.Bottom - 1);
+            Thickness = new (Thickness.Left, Thickness.Top, Thickness.Right - SHADOW_WIDTH, Thickness.Bottom - SHADOW_HEIGHT);
         }
 
         if (style != ShadowStyle.None)
         {
             // Turn on shadow
-            Thickness = new (Thickness.Left, Thickness.Top, Thickness.Right + 1, Thickness.Bottom + 1);
+            Thickness = new (Thickness.Left, Thickness.Top, Thickness.Right + SHADOW_WIDTH, Thickness.Bottom + SHADOW_HEIGHT);
         }
 
         if (style != ShadowStyle.None)
         {
             _rightShadow = new ()
             {
-                X = Pos.AnchorEnd (1),
+                X = Pos.AnchorEnd (SHADOW_WIDTH),
                 Y = 0,
-                Width = 1,
+                Width = SHADOW_WIDTH,
                 Height = Dim.Fill (),
                 ShadowStyle = style,
                 Orientation = Orientation.Vertical
@@ -173,11 +148,11 @@ public class Margin : Adornment
             _bottomShadow = new ()
             {
                 X = 0,
-                Y = Pos.AnchorEnd (1),
+                Y = Pos.AnchorEnd (SHADOW_HEIGHT),
                 Width = Dim.Fill (),
-                Height = 1,
+                Height = SHADOW_HEIGHT,
                 ShadowStyle = style,
-                Orientation = Orientation.Horizontal
+                Orientation = Orientation.Horizontal,
             };
             Add (_rightShadow, _bottomShadow);
         }
@@ -189,15 +164,9 @@ public class Margin : Adornment
     public override ShadowStyle ShadowStyle
     {
         get => base.ShadowStyle;
-        set
-        {
-            base.ShadowStyle = SetShadow (value);
-
-        }
+        set => base.ShadowStyle = SetShadow (value);
     }
 
-    private const int PRESS_MOVE_HORIZONTAL = 1;
-    private const int PRESS_MOVE_VERTICAL = 0;
 
     private void Margin_Highlight (object? sender, CancelEventArgs<HighlightStyle> e)
     {

Terminal.Gui/View/Adornment/ShadowView.cs

@@ -36,6 +36,13 @@ internal class ShadowView : View
         return base.GetNormalColor ();
     }
 
+    /// <inheritdoc />
+    /// <inheritdoc />
+    protected override bool OnDrawingText ()
+    {
+        return true;
+    }
+
     /// <inheritdoc />
     protected override bool OnClearingViewport ()
     {
@@ -116,13 +123,16 @@ internal class ShadowView : View
     {
         Rectangle screen = ViewportToScreen (Viewport);
 
-        for (int i = Math.Max(0, screen.X + 1); i < screen.X + screen.Width; i++)
+        for (int r = Math.Max (0, screen.Y); r < screen.Y + screen.Height; r++)
         {
-            Driver?.Move (i, screen.Y);
-
-            if (i < Driver?.Contents!.GetLength (1) && screen.Y < Driver?.Contents?.GetLength (0))
+            for (int c = Math.Max (0, screen.X + 1); c < screen.X + screen.Width; c++)
             {
-                Driver.AddRune (Driver.Contents [screen.Y, i].Rune);
+                Driver?.Move (c, r);
+
+                if (c < Driver?.Contents!.GetLength (1) && r < Driver?.Contents?.GetLength (0))
+                {
+                    Driver.AddRune (Driver.Contents [r, c].Rune);
+                }
             }
         }
     }
@@ -144,13 +154,16 @@ internal class ShadowView : View
         Rectangle screen = ViewportToScreen (Viewport);
 
         // Fill the rest of the rectangle
-        for (int i = Math.Max (0, screen.Y); i < screen.Y + viewport.Height; i++)
+        for (int c = Math.Max (0, screen.X); c < screen.X + screen.Width; c++)
         {
-            Driver?.Move (screen.X, i);
-
-            if (Driver?.Contents is { } && screen.X < Driver.Contents.GetLength (1) && i < Driver.Contents.GetLength (0))
+            for (int r = Math.Max (0, screen.Y); r < screen.Y + viewport.Height; r++)
             {
-                Driver.AddRune (Driver.Contents [i, screen.X].Rune);
+                Driver?.Move (c, r);
+
+                if (Driver?.Contents is { } && screen.X < Driver.Contents.GetLength (1) && r < Driver.Contents.GetLength (0))
+                {
+                    Driver.AddRune (Driver.Contents [r, c].Rune);
+                }
             }
         }
     }

Terminal.Gui/View/View.Drawing.cs

@@ -22,8 +22,13 @@ public partial class View // Drawing APIs
     /// </remarks>
     public void Draw ()
     {
+        if (!CanBeVisible (this))
+        {
+            return;
+        }
+
         Region? saved = Driver?.Clip;
-        if (CanBeVisible (this) && (NeedsDraw || SubViewNeedsDraw))
+        if (NeedsDraw || SubViewNeedsDraw)
         {
             saved = SetClipToFrame ();
             DoDrawAdornments ();
@@ -36,11 +41,16 @@ public partial class View // Drawing APIs
 
             saved = SetClipToViewport ();
 
+            // TODO: Simplify/optimize SetAttribute system.
             DoSetAttribute ();
             DoClearViewport ();
 
-            DoSetAttribute ();
-            DoDrawSubviews ();
+            if (SubViewNeedsDraw)
+            {
+                DoSetAttribute ();
+
+                DoDrawSubviews ();
+            }
 
             DoSetAttribute ();
             DoDrawText ();
@@ -69,35 +79,42 @@ public partial class View // Drawing APIs
             ClearNeedsDraw ();
         }
 
-        // We're done
+        // This causes the Margin to be drawn in a second pass
+        // TODO: Figure out how to make this more efficient
+        if (Margin is { } && Margin?.Thickness != Thickness.Empty)
+        {
+            Margin!.CachedClip = Application.Driver?.Clip?.Clone ();
+        }
+
+        // We're done drawing
         DoDrawComplete ();
+        // QUESTION: SHould this go before DoDrawComplete?
         Application.SetClip (saved);
 
+        // Exclude this view from the clip
         if (this is not Adornment && Driver?.Clip is { })
         {
-            Application.ExcludeFromClip (FrameToScreen ());
-        }
+            Rectangle borderFrame = FrameToScreen ();
 
+            if (Border is { })
+            {
+                borderFrame = Border.FrameToScreen ();
+            }
+
+            Application.ExcludeFromClip (borderFrame);
+        }
     }
 
     #region DrawAdornments
 
     private void DoDrawAdornmentSubViews ()
     {
-        // This causes the Adornment's subviews to be REDRAWN
-        // TODO: Figure out how to make this more efficient
-        if (Margin?.Subviews is { } && Margin.Thickness != Thickness.Empty)
-        {
-            foreach (View subview in Margin.Subviews)
-            {
-                subview.SetNeedsDraw ();
-            }
-
-            Region? saved = Margin?.SetClipToFrame ();
-            Margin?.DoDrawSubviews ();
-            Application.SetClip (saved);
-
-        }
+        //if (Margin?.Subviews is { } && Margin.Thickness != Thickness.Empty)
+        //{
+        //    //Region? saved = Margin?.SetClipToFrame ();
+        //    //Margin?.DoDrawSubviews ();
+        //    //Application.SetClip (saved);
+        //}
 
         if (Border?.Subviews is { } && Border.Thickness != Thickness.Empty)
         {
@@ -145,7 +162,7 @@ public partial class View // Drawing APIs
         // Those lines will be finally rendered in OnRenderLineCanvas
         if (Margin is { } && Margin.Thickness != Thickness.Empty)
         {
-            Margin?.Draw ();
+            //Margin?.Draw ();
         }
 
         if (Border is { } && Border.Thickness != Thickness.Empty)
@@ -165,7 +182,6 @@ public partial class View // Drawing APIs
     ///     <see cref="LineCanvas"/> of this view's subviews will be rendered. If <see cref="SuperViewRendersLineCanvas"/> is
     ///     false (the default), this method will cause the <see cref="LineCanvas"/> be prepared to be rendered.
     /// </summary>
-    /// <param name="clipRegion"></param>
     /// <returns><see langword="true"/> to stop further drawing of the Adornments.</returns>
     protected virtual bool OnDrawingAdornments () { return false; }
 
@@ -467,7 +483,6 @@ public partial class View // Drawing APIs
 #endif
             view.Draw ();
         }
-
     }
 
     #endregion DrawSubviews
@@ -489,7 +504,6 @@ public partial class View // Drawing APIs
     /// <summary>
     ///     Called when the <see cref="View.LineCanvas"/> is to be rendered. See <see cref="RenderLineCanvas"/>.
     /// </summary>
-    /// <param name="clipRegion"></param>
     /// <returns><see langword="true"/> to stop further drawing of <see cref="LineCanvas"/>.</returns>
     protected virtual bool OnRenderingLineCanvas () { return false; }
 
@@ -575,6 +589,7 @@ public partial class View // Drawing APIs
         DrawComplete?.Invoke (this, new (Viewport, Viewport));
 
         // Default implementation does nothing.
+
     }
 
     /// <summary>
@@ -591,6 +606,8 @@ public partial class View // Drawing APIs
 
     #region NeedsDraw
 
+    // TODO: Change NeedsDraw to use a Region instead of Rectangle
+
     // TODO: Make _needsDrawRect nullable instead of relying on Empty
     // TODO: If null, it means ?
     // TODO: If Empty, it means no need to redraw

Terminal.Gui/View/View.Hierarchy.cs

@@ -77,7 +77,6 @@ public partial class View // SuperView/SubView hierarchy management (SuperView,
 
         if (view.Enabled && !Enabled)
         {
-            view._oldEnabled = true;
             view.Enabled = false;
         }
 

Terminal.Gui/Views/SpinnerView/SpinnerStyle.cs

@@ -54,7 +54,7 @@ public abstract class SpinnerStyle
     /// </summary>
     /// <remarks>
     ///     This is the maximum speed the spinner will rotate at.  You still need to call
-    ///     <see cref="View.SetNeedsDraw"/> or <see cref="SpinnerView.AutoSpin"/> to advance/start animation.
+    ///     <see cref="View.SetNeedsDraw()"/> or <see cref="SpinnerView.AutoSpin"/> to advance/start animation.
     /// </remarks>
     public abstract int SpinDelay { get; }
 

UICatalog/Scenarios/AdvancedClipping.cs

@@ -20,37 +20,48 @@ public class AdvancedClipping : Scenario
             //BorderStyle = LineStyle.None
         };
 
-        app.DrawingText += (s, e) =>
+        app.DrawingContent += (s, e) =>
                            {
                                Application.Driver?.FillRect (app.ViewportToScreen (app.Viewport), CM.Glyphs.Dot);
-                               //app.SetSubViewNeedsDraw();
                                e.Cancel = true;
                            };
 
-        //var arrangementEditor = new ArrangementEditor()
-        //{
-        //    X = Pos.AnchorEnd (),
-        //    Y = 0,
-        //    AutoSelectViewToEdit = true,
-        //};
-        //app.Add (arrangementEditor);
+        var arrangementEditor = new ArrangementEditor ()
+        {
+            X = Pos.AnchorEnd (),
+            Y = 0,
+            AutoSelectViewToEdit = true,
+        };
+        app.Add (arrangementEditor);
 
         View tiledView1 = CreateTiledView (1, 0, 0);
 
-        ProgressBar tiledProgressBar = new ()
+        tiledView1.Width = 30;
+
+        ProgressBar tiledProgressBar1 = new ()
         {
             X = 0,
-            Y = Pos.AnchorEnd(),
+            Y = Pos.AnchorEnd (),
             Width = Dim.Fill (),
             Id = "tiledProgressBar",
             BidirectionalMarquee = true,
-            ProgressBarStyle = ProgressBarStyle.MarqueeBlocks
-           // BorderStyle = LineStyle.Rounded
         };
-        tiledView1.Add (tiledProgressBar);
+        tiledView1.Add (tiledProgressBar1);
 
         View tiledView2 = CreateTiledView (2, 4, 2);
 
+        ProgressBar tiledProgressBar2 = new ()
+        {
+            X = 0,
+            Y = Pos.AnchorEnd (),
+            Width = Dim.Fill (),
+            Id = "tiledProgressBar",
+            BidirectionalMarquee = true,
+            ProgressBarStyle = ProgressBarStyle.MarqueeBlocks
+            // BorderStyle = LineStyle.Rounded
+        };
+        tiledView2.Add (tiledProgressBar2);
+
         app.Add (tiledView1);
         app.Add (tiledView2);
 
@@ -84,7 +95,8 @@ public class AdvancedClipping : Scenario
 
         progressTimer.Elapsed += (s, e) =>
                                  {
-                                     tiledProgressBar.Pulse();
+                                     tiledProgressBar1.Pulse ();
+                                     tiledProgressBar2.Pulse ();
                                      Application.Wakeup ();
                                  };
 
@@ -131,12 +143,13 @@ public class AdvancedClipping : Scenario
             BorderStyle = LineStyle.Single,
             CanFocus = true, // Can't drag without this? BUGBUG
             TabStop = TabBehavior.TabStop,
-            Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable
+            Arrangement = ViewArrangement.Movable | ViewArrangement.Resizable,
+            ShadowStyle = ShadowStyle.Transparent
         };
         //tiled.Padding.Thickness = new (1);
         //tiled.Padding.Diagnostics =  ViewDiagnosticFlags.Thickness;
 
-        tiled.Margin.Thickness = new (1);
+        //tiled.Margin.Thickness = new (1);
 
         FrameView fv = new ()
         {
@@ -145,7 +158,7 @@ public class AdvancedClipping : Scenario
             Height = 3,
         };
         tiled.Add (fv);
-        
+
         return tiled;
     }
 

UICatalog/Scenarios/ShadowStyles.cs

@@ -43,7 +43,7 @@ public class ShadowStyles : Scenario
             ShadowStyle = ShadowStyle.Transparent,
         };
 
-        app.DrawingText += (s, e) =>
+        app.DrawingContent += (s, e) =>
                            {
                                Application.Driver?.FillRect (app.ViewportToScreen (app.Viewport), CM.Glyphs.Dot);
                                e.Cancel = true;