Merge pull request #5464 from harder/copilot/issue-5434-adornment-layout-deltas

Fixes #5434. Track adornment thickness deltas so LayoutAndDraw can stop force-drawing

Author: harder

Terminal.Gui/App/ApplicationImpl.Screen.cs

@@ -278,18 +278,16 @@ public void LayoutAndDraw (bool forceRedraw = false)
             Rectangle clipRect = Screen with { X = 0, Y = 0 };
             Driver.Clip = new Region (clipRect);
 
-            // Only force a complete redraw if needed (needsLayout or forceRedraw).
-            // Otherwise, just redraw views that need it.
+            // Only force a complete redraw when explicitly requested.
+            // Otherwise, redraw only the views that need it.
             //
             // NOTE (#5358): passing force=true here calls SetNeedsDraw on the top runnable, which
             // cascades to all overlapping subviews via the existing SetNeedsDraw recursion. This
             // is the remaining draw-fan-out source documented by TabsFanOutIntegrationTests. The
             // proper fix requires tracking adornment thickness changes (in addition to Frame
             // changes) so the SuperView can be invalidated precisely instead of force-redrawing
-            // the whole tree. Dropping force-on-neededLayout without that tracking exposes
-            // stale-content bugs in the shrink/move and adornment-rebalance paths (covered by
-            // ShadowTests / BorderViewTests).
-            View.Draw (views.ToArray ().Cast<View> (), neededLayout || forceRedraw);
+            // the whole tree.
+            View.Draw (views.ToArray ().Cast<View> (), forceRedraw);
 
             Driver.Clip = new Region (clipRect);
 

Terminal.Gui/ViewBase/View.Adornments.cs

@@ -11,34 +11,54 @@ private void SetupAdornments ()
         {
             return;
         }
+
+        Thickness previousMarginThickness = Margin.Thickness;
+        Thickness previousBorderThickness = Border.Thickness;
+        Thickness previousPaddingThickness = Padding.Thickness;
+
         Margin.Parent = this;
         Border.Parent = this;
         Padding.Parent = this;
 
         // When any adornment's thickness changes, recompute frames and request layout + redraw.
         Margin.ThicknessChanged += (_, _) =>
                                    {
-                                       Margin.View?.SetNeedsLayout ();
-                                       SetAdornmentFrames ();
-                                       SetNeedsLayout ();
-                                       SetNeedsDraw ();
+                                       previousMarginThickness = HandleAdornmentThicknessChanged (Margin, previousMarginThickness);
                                    };
 
         Border.ThicknessChanged += (_, _) =>
                                    {
-                                       Border.View?.SetNeedsLayout ();
-                                       SetAdornmentFrames ();
-                                       SetNeedsLayout ();
-                                       SetNeedsDraw ();
+                                       previousBorderThickness = HandleAdornmentThicknessChanged (Border, previousBorderThickness);
                                    };
 
         Padding.ThicknessChanged += (_, _) =>
                                     {
-                                        Padding.View?.SetNeedsLayout ();
-                                        SetAdornmentFrames ();
-                                        SetNeedsLayout ();
-                                        SetNeedsDraw ();
+                                        previousPaddingThickness = HandleAdornmentThicknessChanged (Padding, previousPaddingThickness);
                                     };
+
+        Thickness HandleAdornmentThicknessChanged (IAdornment adornment, Thickness previousThickness)
+        {
+            Rectangle oldViewport = GetViewportForAdornmentThickness (adornment, previousThickness);
+            Rectangle oldViewportFrame = GetViewportFrameForAdornmentThickness (adornment, previousThickness);
+            Thickness currentThickness = adornment.Thickness;
+
+            (adornment.View as View)?.SetNeedsLayout ();
+            SetAdornmentFrames ();
+            SetNeedsLayout ();
+            SetNeedsDraw ();
+
+            if (oldViewportFrame != GetViewportFrameForAdornmentThickness (adornment, currentThickness))
+            {
+                InvalidateAdornmentThicknessChange ();
+            }
+
+            if (oldViewport != Viewport)
+            {
+                RaiseViewportChangedEvent (oldViewport);
+            }
+
+            return currentThickness;
+        }
     }
 
     private void BeginInitAdornments ()
@@ -345,4 +365,47 @@ internal void SetAdornmentFrames ()
         // Border and Padding dynamically update based on Margin's View's Frame changing
         Margin.View?.Frame = Frame with { Location = Point.Empty };
     }
+
+    private Rectangle GetViewportForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness)
+    {
+        Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness);
+
+        return new Rectangle (_viewportLocation, GetViewportSizeForThickness (thickness));
+    }
+
+    private Rectangle GetViewportFrameForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness)
+    {
+        Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness);
+
+        return new Rectangle (new Point (thickness.Left, thickness.Top), GetViewportSizeForThickness (thickness));
+    }
+
+    private Size GetViewportSizeForThickness (Thickness thickness) =>
+        new (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical));
+
+    private Thickness GetAdornmentsThicknessForAdornment (IAdornment changedAdornment, Thickness changedThickness)
+    {
+        Thickness thickness = Thickness.Empty;
+
+        thickness += ReferenceEquals (changedAdornment, Margin) ? changedThickness : Margin.Thickness;
+        thickness += ReferenceEquals (changedAdornment, Border) ? changedThickness : Border.Thickness;
+        thickness += ReferenceEquals (changedAdornment, Padding) ? changedThickness : Padding.Thickness;
+
+        return thickness;
+    }
+
+    private void InvalidateAdornmentThicknessChange ()
+    {
+        if (SuperView is null)
+        {
+            NeedsClearScreenNextIteration ();
+
+            return;
+        }
+
+        Rectangle invalidationViewport = Frame;
+        Point superScroll = SuperView.Viewport.Location;
+        invalidationViewport.Offset (-superScroll.X, -superScroll.Y);
+        SuperView.SetNeedsDraw (invalidationViewport);
+    }
 }

Tests/IntegrationTests/TabsFanOutIntegrationTests.cs

@@ -68,8 +68,8 @@ private static string MakeText (string prefix, int lines)
     }
 
     /// <summary>
-    ///     End-to-end check: a real <see cref="Key.PageDown"/> on the active tab still produces
-    ///     draw fan-out on inactive tabs, but layout work stays on the active tab.
+    ///     End-to-end check: a real <see cref="Key.PageDown"/> on the active tab keeps both
+    ///     layout and text drawing on the active tab.
     /// </summary>
     [Theory]
     [MemberData (nameof (GetAllDriverNames))]
@@ -169,21 +169,7 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa
         outputHelper.WriteLine ($"Sum inactive DrawingText = {inactiveTextDraws}");
         outputHelper.WriteLine ($"Sum inactive SubViewsLaidOut = {inactiveLayouts}");
 
-        // Issue #5358 fix narrows draw fan-out at the View.Draw pipeline level (verified by
-        // TabsFanOutDiagnosticTests at synthetic level). At integration level a separate
-        // cascade source remains: ApplicationImpl.LayoutAndDraw passes force=true to
-        // View.Draw whenever any view needed layout, which calls SetNeedsDraw on the top
-        // runnable, which cascades to overlapping subviews via the existing SetNeedsDraw
-        // recursion. Removing that force=true uncovers stale-content bugs in the shrink/move
-        // path (covered by existing ShadowTests and BorderViewTests) and is out of scope
-        // for #5358. Until that broader fix lands, inactive tab pages still receive
-        // NeedsDraw via the LayoutAndDraw force path. Layout fan-out is already fully
-        // eliminated by PR #5373.
-        Assert.True (
-                     inactiveTextDraws > 0,
-                     $"Documents the remaining draw fan-out via ApplicationImpl.LayoutAndDraw's force=true path: " +
-                     $"inactive_total DrawingText={inactiveTextDraws}. Flip to Assert.Equal(0, inactiveTextDraws) " +
-                     "after the broader LayoutAndDraw cascade is addressed (out of scope for #5358).");
+        Assert.Equal (0, inactiveTextDraws);
 
         Assert.Equal (0, inactiveLayouts);
     }