gui-cs · harder · May 28, 2026 · May 26, 2026 · May 26, 2026 · May 26, 2026
@@ -280,6 +280,15 @@ public void LayoutAndDraw (bool forceRedraw = false)
 
             // Only force a complete redraw if needed (needsLayout or forceRedraw).
             // Otherwise, just redraw 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);
 
             Driver.Clip = new Region (clipRect);

@@ -15,9 +15,10 @@ namespace Terminal.Gui.ViewBase;
 ///         back-reference, making <see cref="AdornmentImpl"/> the single authoritative owner of Thickness.
 ///     </para>
 ///     <para>
-///         During the incremental migration, existing <see cref="Border"/> and <see cref="Padding"/> continue
-///         to extend <see cref="Adornment"/>. Only newly migrated adornments (starting with <c>MarginView</c>)
-///         extend <see cref="AdornmentView"/>.
+///         <see cref="MarginView"/>, <see cref="BorderView"/>, and <see cref="PaddingView"/> all extend
+///         <see cref="AdornmentView"/>. <see cref="AdornmentImpl"/> is the authoritative owner of
+///         <see cref="IAdornment.Thickness"/>; the corresponding <see cref="AdornmentView"/> hosts the
+///         render-layer behavior.
 ///     </para>
 /// </remarks>
 public class AdornmentView : View, IAdornmentView, IDesignable
@@ -151,8 +152,9 @@ protected override bool OnClearingViewport ()
             Adornment.Thickness.Draw (Driver, ViewportToScreen (Viewport), Diagnostics, ToString ());
         }
 
-        SetNeedsDraw ();
-
+        // Do NOT call SetNeedsDraw () here. The thickness has been drawn; calling
+        // SetNeedsDraw cascades to the parent's SubViewNeedsDraw mid-pass and adds
+        // churn that competes with the needsDrawSelf gate added for issue #5358.
         return true;
     }
 

@@ -133,9 +133,10 @@ internal void DoDrawAdornments (Region? originalClip)
         Padding.View?.SetNeedsDraw ();
         Margin.View?.SetNeedsDraw ();
 
-        // Ensure NeedsDraw is true for the rest of the draw pipeline (DoClearViewport, DoDrawText, etc.)
-        // When adornment Views are null (lightweight), their NeedsDraw doesn't contribute to the parent's
-        // NeedsDraw property. But if we're here, the parent IS drawing, so we must set NeedsDrawRect.
+        // Keep NeedsDraw true for DoRenderLineCanvas and ClearNeedsDraw. The self-content
+        // methods (DoClearViewport, DoDrawText, DoDrawContent) are now gated on the
+        // needsDrawSelf snapshot captured in Draw() *before* this escalation, so this no
+        // longer forces a full parent redraw when only a child was dirty (issue #5358).
         if (NeedsDrawRect == Rectangle.Empty)
         {
             NeedsDrawRect = Viewport;

@@ -91,8 +91,12 @@
 
         Region? originalClip = GetClip ();
 
-        // TODO: This can be further optimized by checking NeedsDraw below and only
-        // TODO: clearing, drawing text, drawing content, etc. if it is true.
+        // Capture whether THIS view's own content needs redrawing BEFORE DoDrawAdornments
+        // escalates NeedsDrawRect (see View.Drawing.Adornments.cs DoDrawAdornments).
+        // When only SubViewNeedsDraw is true, needsDrawSelf is false and we skip
+        // ClearViewport/DrawText/DrawContent so child-only invalidations stay narrow.
+        bool needsDrawSelf = NeedsDraw;
+
         if (NeedsDraw || SubViewNeedsDraw)
         {
             // ------------------------------------
@@ -120,10 +124,22 @@
             // SuperView's ClearViewport or peer SubViews' content.
             // This follows the same pattern as DrawAdornments(), which creates
             // per-adornment DrawContexts for the same reason.
-            _localDrawContext = new DrawContext ();
+            //
+            // Issue #5358 (review feedback item 2): only recreate _localDrawContext when
+            // we actually intend to redraw self-content this pass. On child-only passes
+            // (needsDrawSelf=false), we must preserve the prior context so DoDrawComplete
+            // doesn't overwrite CachedDrawnRegion with an empty region and break
+            // TransparentMouse hit-testing until the next full self-redraw.
+            if (needsDrawSelf)
+            {
+                _localDrawContext = new DrawContext ();
+            }
 
-            SetAttributeForRole (Enabled ? VisualRole.Normal : VisualRole.Disabled);
-            DoClearViewport (context);
+            if (needsDrawSelf)
+            {
+                SetAttributeForRole (Enabled ? VisualRole.Normal : VisualRole.Disabled);
+                DoClearViewport (context);
+            }
 
             // ------------------------------------
             // Draw the SubViews first (order matters: SubViews, Text, Content)
@@ -142,20 +158,23 @@
                 _lastClearedViewport = null;
             }
 
-            // ------------------------------------
-            // Draw the text — tracked in both shared (clip exclusion) and local (hit-testing) contexts
-            Trace.Draw (this.ToIdentifyingString (), "Text");
-            SetAttributeForRole (Enabled ? VisualRole.Normal : VisualRole.Disabled);
-            DoDrawText (_localDrawContext);
-
-            // ------------------------------------
-            // Draw the content — tracked in both shared (clip exclusion) and local (hit-testing) contexts
-            Trace.Draw (this.ToIdentifyingString (), "Content");
-            DoDrawContent (_localDrawContext);
-
-            // Merge this view's own draws into the shared context so the SuperView
-            // can track the aggregate for clip exclusion.
-            context.AddDrawnRegion (_localDrawContext.GetDrawnRegion ());
+            if (needsDrawSelf)
+            {
+                // ------------------------------------
+                // Draw the text — tracked in both shared (clip exclusion) and local (hit-testing) contexts
+                Trace.Draw (this.ToIdentifyingString (), "Text");
+                SetAttributeForRole (Enabled ? VisualRole.Normal : VisualRole.Disabled);
+                DoDrawText (_localDrawContext);
+
+                // ------------------------------------
+                // Draw the content — tracked in both shared (clip exclusion) and local (hit-testing) contexts
+                Trace.Draw (this.ToIdentifyingString (), "Content");
+                DoDrawContent (_localDrawContext);
+
+                // Merge this view's own draws into the shared context so the SuperView
+                // can track the aggregate for clip exclusion.
+                context.AddDrawnRegion (_localDrawContext.GetDrawnRegion ());
+            }
 
             // ------------------------------------
             // Draw adornment SubViews BEFORE rendering LineCanvas so their lines
@@ -221,11 +240,85 @@
             return;
         }
 
-        ClearViewport (context);
+        // Issue #5358: narrow the framework's clear to NeedsDrawRect when it's a true
+        // partial region AND this view is not itself scrolled. Narrowing in the public
+        // ClearViewport API would silently change the contract for direct callers
+        // (Code.OnClearingViewport, MarkdownCodeBlock, direct test calls) that expect
+        // a full fill of the viewport background — see review feedback items 1, 3.
+        //
+        // The "this view is not itself scrolled" guard sidesteps a separate coordinate-
+        // space inconsistency: SetNeedsDraw(Rectangle) cascades to subviews using
+        // frame-local coordinates (subtracts subview.Frame.X/Y), while the no-arg
+        // SetNeedsDraw passes Viewport (content-coord). For an unscrolled view those
+        // coincide; for a scrolled view they don't, and the narrowing math would shift
+        // the clear off-screen. Until that convention is normalized (out of scope here),
+        // only narrow when Viewport.Location is the origin.
+        if (CanNarrowClearToNeedsDrawRect (out Rectangle narrowedScreen))
+        {
+            Driver?.FillRect (narrowedScreen);
+            _lastClearedViewport = narrowedScreen;
+            SetNeedsDraw (NeedsDrawRect);
+        }
+        else
+        {
+            ClearViewport (context);
+        }
+
         OnClearedViewport ();
         ClearedViewport?.Invoke (this, new DrawEventArgs (Viewport, Viewport, null));
     }
 
+    /// <summary>
+    ///     Determines whether the framework's <see cref="DoClearViewport"/> can safely narrow
+    ///     the clear to just <see cref="NeedsDrawRect"/>. See <see cref="DoClearViewport"/> for
+    ///     the rationale.
+    /// </summary>
+    private bool CanNarrowClearToNeedsDrawRect (out Rectangle narrowedScreen)
+    {
+        narrowedScreen = Rectangle.Empty;
+
+        if (Driver is null)
+        {
+            return false;
+        }
+
+        if (NeedsDrawRect.IsEmpty)
+        {
+            return false;
+        }
+
+        Rectangle viewport = Viewport;
+
+        // Only narrow when this view is NOT itself scrolled — see DoClearViewport comment.
+        if (viewport.Location != Point.Empty)
+        {
+            return false;
+        }
+
+        // Only narrow when NeedsDrawRect is strictly smaller than the viewport. SetNeedsDraw()
+        // (no-arg) sets NeedsDrawRect to the current Viewport, meaning "everything is dirty";
+        // we don't want to narrow in that case.
+        if (NeedsDrawRect.Width >= viewport.Width && NeedsDrawRect.Height >= viewport.Height)
+        {
+            return false;
+        }
+
+        // ClearContentOnly: skip narrowing; the existing visible-content intersection is
+        // already a content-area optimization and combining the two correctly is non-trivial.
+        if (ViewportSettings.FastHasFlags (ViewportSettingsFlags.ClearContentOnly))
+        {
+            return false;
+        }
+
+        // NeedsDrawRect is in this view's coords; for an unscrolled view those equal viewport-
+        // local coords (origin (0,0) = top-left of visible area). Convert to screen.
+        Rectangle dirtyScreen = ViewportToScreen (NeedsDrawRect);
+        Rectangle toClear = ViewportToScreen (viewport with { Location = Point.Empty });
+        narrowedScreen = Rectangle.Intersect (toClear, dirtyScreen);
+
+        return !narrowedScreen.IsEmpty;
+    }
+
     /// <summary>
     ///     Called when the <see cref="Viewport"/> is to be cleared.
     /// </summary>

@@ -95,6 +95,7 @@ private bool SetFrame (in Rectangle frame)
             return false;
         }
 
+        Rectangle? oldFrame = _frame;
         var oldViewport = Rectangle.Empty;
 
         if (IsInitialized)
@@ -111,6 +112,17 @@ private bool SetFrame (in Rectangle frame)
         SetNeedsDraw ();
         SetNeedsLayout ();
 
+        // Issue #5358: when Frame shrinks or moves, the SuperView's old-frame area is now
+        // uncovered and must be cleared on the next draw. Invalidate the union of the old and
+        // new frames on the SuperView so its region-aware ClearViewport repaints just that area.
+        // This complements the same invalidation in View.Layout (which handles Frame changes
+        // that happen during a Pos/Dim-driven layout pass); SetFrame catches the direct-assignment
+        // path (e.g., view.Frame = newRect).
+        if (oldFrame is { } prev && SuperView is { })
+        {
+            SuperView.SetNeedsDraw (Rectangle.Union (prev, frame));
+        }
+
         // BUGBUG: When SetFrame is called from Frame_set, this event gets raised BEFORE OnResizeNeeded. Is that OK?
         OnFrameChanged (in frame);
         FrameChanged?.Invoke (this, new EventArgs<Rectangle> (in frame));
@@ -578,6 +590,12 @@ public bool Layout (Size contentSize)
             // recomputed during dependency resolution. Draw after layout only when this view was
             // directly invalidated for layout or its resolved frame actually changed.
             SetNeedsDraw ();
+
+            // NOTE (#5358, review feedback item 4): the SuperView invalidation for frame
+            // changes is handled in SetFrame, which is the single source of truth for Frame
+            // mutation. Both direct (view.Frame = ...) and layout-driven (SetRelativeLayout
+            // → SetFrame) paths go through it, so calling SuperView.SetNeedsDraw(union) here
+            // too would be redundant and would do the cascade work twice on the hot path.
         }
 
         return true;

@@ -85,11 +85,12 @@ public void SetNeedsDraw (Rectangle viewPortRelativeRegion)
         }
         else
         {
-            int x = Math.Min (Viewport.X, viewPortRelativeRegion.X);
-            int y = Math.Min (Viewport.Y, viewPortRelativeRegion.Y);
-            int w = Math.Max (Viewport.Width, viewPortRelativeRegion.Width);
-            int h = Math.Max (Viewport.Height, viewPortRelativeRegion.Height);
-            NeedsDrawRect = new Rectangle (x, y, w, h);
+            // Union NeedsDrawRect with the incoming region. The previous formula unioned
+            // against Viewport (a bug — it widened to nearly viewport-size on every call),
+            // which made NeedsDrawRect useless for narrowing draw work. Issue #5358
+            // requires an accurate dirty rect so the region-aware ClearViewport and
+            // SetNeedsDraw cascade can stay narrow.
+            NeedsDrawRect = Rectangle.Union (NeedsDrawRect, viewPortRelativeRegion);
         }
 
         // Do not set on Margin - it will be drawn in a separate pass.