Fixes #5359. Normalize NeedsDrawRect to viewport-local coordinates

Terminal.Gui/ViewBase/View.Drawing.Adornments.cs

@@ -137,9 +137,12 @@ internal void DoDrawAdornments (Region? originalClip)
         // 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).
+        //
+        // Issue #5359: NeedsDrawRect is viewport-LOCAL, so set (0, 0, W, H) instead of
+        // Viewport (which carries the scroll offset and would corrupt the convention).
         if (NeedsDrawRect == Rectangle.Empty)
         {
-            NeedsDrawRect = Viewport;
+            NeedsDrawRect = new (Point.Empty, Viewport.Size);
         }
 
         if (OnDrawingAdornments ())

Terminal.Gui/ViewBase/View.Drawing.cs

@@ -173,7 +173,7 @@
 
                 // Merge this view's own draws into the shared context so the SuperView
                 // can track the aggregate for clip exclusion.
                 context.AddDrawnRegion (_localDrawContext.GetDrawnRegion ());
             }
 
             // ------------------------------------
@@ -241,18 +241,11 @@
         }
 
         // 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.
+        // partial region. 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 on PR #5431. Issue #5359 normalized NeedsDrawRect on viewport-local
+        // coordinates, so narrowing is now safe regardless of scroll state.
         if (CanNarrowClearToNeedsDrawRect (out Rectangle narrowedScreen))
         {
             Driver?.FillRect (narrowedScreen);
@@ -289,15 +282,9 @@
 
         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.
+        // (no-arg) sets NeedsDrawRect to (Point.Empty, Viewport.Size), meaning "everything is
+        // dirty"; we don't want to narrow in that case.
         if (NeedsDrawRect.Width >= viewport.Width && NeedsDrawRect.Height >= viewport.Height)
         {
             return false;
@@ -310,8 +297,8 @@
             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.
+        // NeedsDrawRect is viewport-LOCAL — (0, 0) is the top-left visible cell — so
+        // ViewportToScreen converts it correctly regardless of any scroll on this view.
         Rectangle dirtyScreen = ViewportToScreen (NeedsDrawRect);
         Rectangle toClear = ViewportToScreen (viewport with { Location = Point.Empty });
         narrowedScreen = Rectangle.Intersect (toClear, dirtyScreen);

Terminal.Gui/ViewBase/View.Layout.cs

@@ -117,9 +117,17 @@ private bool SetFrame (in Rectangle frame)
         // new frames on the SuperView so its region-aware ClearViewport repaints just that area.
         // SetFrame is the single source of truth for this invalidation for both direct Frame
         // assignment and layout-driven frame updates.
+        //
+        // Issue #5359: Frames are in SuperView CONTENT coords, but SetNeedsDraw expects
+        // SuperView VIEWPORT-LOCAL coords. Translate by subtracting the SuperView's scroll
+        // offset (Viewport.Location).
         if (oldFrame is { } prev && SuperView is { })
         {
-            SuperView.SetNeedsDraw (Rectangle.Union (prev, frame));
+            Rectangle invalidationContent = Rectangle.Union (prev, frame);
+            Point superScroll = SuperView.Viewport.Location;
+            Rectangle invalidationViewport = invalidationContent;
+            invalidationViewport.Offset (-superScroll.X, -superScroll.Y);
+            SuperView.SetNeedsDraw (invalidationViewport);
         }
 
         // BUGBUG: When SetFrame is called from Frame_set, this event gets raised BEFORE OnResizeNeeded. Is that OK?

Terminal.Gui/ViewBase/View.NeedsDraw.cs

@@ -2,14 +2,19 @@
 
 public partial class View
 {
-    // NOTE: NeedsDrawRect is not currently used to clip drawing to only the invalidated region.
-    //       It is only used within SetNeedsDraw to propagate redraw requests to subviews.
+    // NOTE: NeedsDrawRect is in viewport-LOCAL coordinates: (0, 0) is the top-left visible
+    //       cell of the View's Viewport (inside Padding, after any scroll). The rect is
+    //       independent of Viewport.Location, so scrolling and negative viewport locations do
+    //       not bleed into the dirty rect. See SetNeedsDraw(Rectangle) for the cascade
+    //       convention used to propagate dirty rects into SubViews.
     // NOTE: Consider changing NeedsDrawRect from Rectangle to Region for more precise invalidation
     //       NeedsDraw is already efficiently cached via NeedsDrawRect. It checks:
     //       1. NeedsDrawRect (cached by SetNeedsDraw/ClearNeedsDraw)
     //       2. Adornment NeedsDraw flags (each cached separately)
     /// <summary>
-    ///     INTERNAL: Gets the viewport-relative region that needs to be redrawn.
+    ///     INTERNAL: Gets the viewport-local region that needs to be redrawn. <c>(0, 0)</c> is the
+    ///     top-left of <see cref="Viewport"/>; the rect is independent of <see cref="Viewport"/>'s
+    ///     <see cref="Rectangle.Location"/> (scroll offset).
     /// </summary>
     internal Rectangle NeedsDrawRect { get; private set; } = Rectangle.Empty;
 
@@ -41,20 +46,33 @@ public void SetNeedsDraw ()
             return;
         }
 
-        SetNeedsDraw (viewport);
+        // Pass a viewport-LOCAL rect: (0, 0, W, H) covers the whole visible viewport regardless
+        // of scroll. Passing Viewport here would leak Viewport.Location (the scroll offset, which
+        // can also be negative under AllowNegativeX/Y) into NeedsDrawRect, breaking the
+        // viewport-local convention NeedsDrawRect is supposed to honor.
+        SetNeedsDraw (new (Point.Empty, viewport.Size));
     }
 
     /// <summary>Expands the area of this view needing to be redrawn to include <paramref name="viewPortRelativeRegion"/>.</summary>
     /// <remarks>
     ///     <para>
-    ///         The location of <paramref name="viewPortRelativeRegion"/> is relative to the View's <see cref="Viewport"/>.
+    ///         <paramref name="viewPortRelativeRegion"/> is in viewport-LOCAL coordinates:
+    ///         <c>(0, 0)</c> is the top-left visible cell of the View's <see cref="Viewport"/>.
+    ///         The rect does NOT include <see cref="Viewport"/>'s <see cref="Rectangle.Location"/>
+    ///         (scroll offset).
+    ///     </para>
+    ///     <para>
+    ///         The cascade to intersecting SubViews translates the region into each SubView's
+    ///         own viewport-local coordinates, accounting for the parent's scroll and the SubView's
+    ///         adornments. Viewport-local coordinates are scroll-independent, so the SubView's own
+    ///         <see cref="Viewport"/> <see cref="Rectangle.Location"/> does not affect the propagated rect.
     ///     </para>
     ///     <para>
     ///         If the view has not been initialized (<see cref="IsInitialized"/> is <see langword="false"/>), the area to be
     ///         redrawn will be the <paramref name="viewPortRelativeRegion"/>.
     ///     </para>
     /// </remarks>
-    /// <param name="viewPortRelativeRegion">The <see cref="Viewport"/>relative region that needs to be redrawn.</param>
+    /// <param name="viewPortRelativeRegion">The viewport-local region that needs to be redrawn.</param>
     public void SetNeedsDraw (Rectangle viewPortRelativeRegion)
     {
         // Invalidate the cached drawn region used for TransparentMouse hit-testing.
@@ -112,16 +130,53 @@ public void SetNeedsDraw (Rectangle viewPortRelativeRegion)
             adornment.Adornment?.Parent?.SetSubViewNeedsDrawDownHierarchy ();
         }
 
+        // Cascade the dirty region into intersecting SubViews. Coordinate conversion chain
+        // (issue #5359 — every step is needed for correctness under scroll/adornments):
+        //   1. viewPortRelativeRegion is in THIS view's viewport-local coords; translate to
+        //      this view's content coords by adding Viewport.Location (the scroll offset).
+        //   2. subview.Frame is in this view's content coords; intersect there.
+        //   3. Translate the intersection to subview-frame-local (subtract Frame.Location).
+        //   4. Translate to subview-VIEWPORT-local by subtracting the subview's adornment
+        //      offset (GetViewportOffsetFromFrame() — the combined Margin/Border/Padding inset).
+        //      Do NOT also subtract the subview's Viewport.Location:
+        //      viewport-local coords are scroll-INDEPENDENT — (0, 0) is always the top-left
+        //      visible cell regardless of which content cell the scroll maps there. The dirty
+        //      ON-SCREEN cells are what we propagate, not the content cells they're showing.
+        //   5. Clip to the subview's visible viewport bounds; anything outside is in adornment
+        //      territory. If nothing is left, fall back to a full subview invalidation so the
+        //      subview at least redraws itself safely (and flags its adornments).
+        Point thisScroll = Viewport.Location;
+        Rectangle contentRegion = viewPortRelativeRegion;
+        contentRegion.Offset (thisScroll.X, thisScroll.Y);
+
         foreach (View subview in InternalSubViews.Snapshot ())
         {
-            if (!subview.Frame.IntersectsWith (viewPortRelativeRegion))
+            if (!subview.Frame.IntersectsWith (contentRegion))
+            {
+                continue;
+            }
+
+            Rectangle subviewFrameRegion = Rectangle.Intersect (subview.Frame, contentRegion);
+            subviewFrameRegion.Offset (-subview.Frame.X, -subview.Frame.Y);
+
+            Point subviewAdornmentOffset = subview.GetViewportOffsetFromFrame ();
+            subviewFrameRegion.Offset (-subviewAdornmentOffset.X, -subviewAdornmentOffset.Y);
+
+            Rectangle subviewViewportBounds = new (Point.Empty, subview.Viewport.Size);
+            Rectangle subviewViewportRegion = Rectangle.Intersect (subviewViewportBounds, subviewFrameRegion);
+
+            if (subviewViewportRegion.IsEmpty)
             {
+                // Dirty region overlaps the subview's frame but only its adornment area. The
+                // subview's own viewport isn't dirty; a no-arg SetNeedsDraw is the safe
+                // fallback (also flags adornments so any border/padding the dirty region
+                // touched gets repainted).
+                subview.SetNeedsDraw ();
+
                 continue;
             }
-            Rectangle subviewRegion = Rectangle.Intersect (subview.Frame, viewPortRelativeRegion);
-            subviewRegion.X -= subview.Frame.X;
-            subviewRegion.Y -= subview.Frame.Y;
-            subview.SetNeedsDraw (subviewRegion);
+
+            subview.SetNeedsDraw (subviewViewportRegion);
         }
     }