diff --git a/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs b/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs
index ebabfe0c2f..179ed4c54c 100644
--- a/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs
+++ b/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 ())
diff --git a/Terminal.Gui/ViewBase/View.Drawing.cs b/Terminal.Gui/ViewBase/View.Drawing.cs
index 3417168b93..6eda6390dc 100644
--- a/Terminal.Gui/ViewBase/View.Drawing.cs
+++ b/Terminal.Gui/ViewBase/View.Drawing.cs
@@ -241,18 +241,11 @@ internal void DoClearViewport (DrawContext? context = null)
}
// 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 @@ private bool CanNarrowClearToNeedsDrawRect (out Rectangle narrowedScreen)
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 @@ private bool CanNarrowClearToNeedsDrawRect (out Rectangle narrowedScreen)
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);
diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs
index 0db07b69a4..a740587f30 100644
--- a/Terminal.Gui/ViewBase/View.Layout.cs
+++ b/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?
diff --git a/Terminal.Gui/ViewBase/View.NeedsDraw.cs b/Terminal.Gui/ViewBase/View.NeedsDraw.cs
index 5303d20106..993da4da9a 100644
--- a/Terminal.Gui/ViewBase/View.NeedsDraw.cs
+++ b/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)
///
- /// INTERNAL: Gets the viewport-relative region that needs to be redrawn.
+ /// INTERNAL: Gets the viewport-local region that needs to be redrawn. (0, 0) is the
+ /// top-left of ; the rect is independent of 's
+ /// (scroll offset).
///
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));
}
/// Expands the area of this view needing to be redrawn to include .
///
///
- /// The location of is relative to the View's .
+ /// is in viewport-LOCAL coordinates:
+ /// (0, 0) is the top-left visible cell of the View's .
+ /// The rect does NOT include 's
+ /// (scroll offset).
+ ///
+ ///
+ /// 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
+ /// does not affect the propagated rect.
///
///
/// If the view has not been initialized ( is ), the area to be
/// redrawn will be the .
///
///
- /// The relative region that needs to be redrawn.
+ /// The viewport-local region that needs to be redrawn.
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);
}
}
diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawCoordTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawCoordTests.cs
new file mode 100644
index 0000000000..cf4334d4f3
--- /dev/null
+++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawCoordTests.cs
@@ -0,0 +1,411 @@
+#nullable disable
+using System.Text;
+using UnitTests;
+
+namespace ViewBaseTests.Draw;
+
+// Claude - Opus 4.7
+///
+/// Issue #5359: NeedsDrawRect and the viewPortRelativeRegion parameter of SetNeedsDraw are in
+/// viewport-LOCAL coordinates — (0, 0) is the top-left visible cell of the View's Viewport,
+/// independent of Viewport.Location (scroll offset, possibly negative).
+///
+/// These tests pin down the convention across:
+/// * SetNeedsDraw() no-arg on a scrolled or negative-viewport-location view
+/// * Subview cascade when parent is scrolled
+/// * Subview cascade when subview has adornments
+/// * Subview cascade when subview is itself scrolled
+/// * Subview cascade when dirty region only overlaps subview's adornments
+/// * Framework ClearViewport narrowing now working on scrolled views
+/// * Zero-size viewport edge case
+///
+public class NeedsDrawCoordTests : TestDriverBase
+{
+ ///
+ /// SetNeedsDraw() no-arg on a scrolled view stores a viewport-LOCAL rect — (0, 0, W, H),
+ /// NOT the current Viewport (which carries the scroll offset).
+ ///
+ [Fact]
+ public void SetNeedsDraw_NoArg_OnScrolledView_StoresViewportLocalRect ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View view = new () { Driver = driver, X = 0, Y = 0, Width = 10, Height = 8 };
+ view.SetContentSize (new Size (100, 100));
+ view.BeginInit ();
+ view.EndInit ();
+ view.LayoutSubViews ();
+ view.Draw ();
+
+ // Scroll so Viewport.Location is non-zero.
+ view.Viewport = view.Viewport with { Location = new Point (5, 3) };
+ Assert.Equal (new Point (5, 3), view.Viewport.Location);
+
+ view.ClearNeedsDraw ();
+ view.SetNeedsDraw ();
+
+ // NeedsDrawRect must be viewport-LOCAL: (0, 0, W, H).
+ Assert.Equal (new Rectangle (0, 0, 10, 8), view.NeedsDrawRect);
+
+ view.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// SetNeedsDraw() no-arg on a view with a negative Viewport.Location (allowed via
+ /// AllowNegativeX/Y) must still produce a viewport-local rect — negative locations must
+ /// not leak into NeedsDrawRect.
+ ///
+ [Fact]
+ public void SetNeedsDraw_NoArg_OnNegativeViewportLocation_StoresViewportLocalRect ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View view = new ()
+ {
+ Driver = driver,
+ X = 0,
+ Y = 0,
+ Width = 10,
+ Height = 5,
+ ViewportSettings = ViewportSettingsFlags.AllowNegativeLocation
+ | ViewportSettingsFlags.AllowLocationGreaterThanContentSize
+ };
+ view.SetContentSize (new Size (100, 100));
+ view.BeginInit ();
+ view.EndInit ();
+ view.LayoutSubViews ();
+ view.Draw ();
+
+ view.Viewport = view.Viewport with { Location = new Point (-3, -2) };
+ Assert.Equal (new Point (-3, -2), view.Viewport.Location);
+
+ view.ClearNeedsDraw ();
+ view.SetNeedsDraw ();
+
+ Assert.Equal (new Rectangle (0, 0, 10, 5), view.NeedsDrawRect);
+
+ view.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// When the parent is scrolled and a content-area region is invalidated, the cascade
+ /// translates the parent's viewport-local region into the subview's viewport-local
+ /// coordinates correctly.
+ ///
+ /// Setup: parent (20×10), scrolled to (5, 0). Subview at Frame (3, 1, 10, 6) in
+ /// parent's CONTENT coords — that means in the parent's viewport-local frame the
+ /// subview's visible portion starts at viewport-local X = (3 - 5) = -2. Invalidate a
+ /// parent viewport-local rect of (1, 2, 5, 2). That maps to parent CONTENT
+ /// (6, 2, 5, 2), which overlaps the subview at content (6, 2, 5, 2) ∩ Frame
+ /// (3, 1, 10, 6) = (6, 2, 5, 2). In subview-frame-local: (3, 1, 5, 2). The subview
+ /// has no adornments and isn't scrolled, so subview-viewport-local equals
+ /// subview-frame-local.
+ ///
+ [Fact]
+ public void SetNeedsDraw_ScrolledParent_CascadesViewportLocalRectToSubview ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View parent = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+ parent.SetContentSize (new Size (40, 20));
+
+ View subview = new () { X = 3, Y = 1, Width = 10, Height = 6 };
+ parent.Add (subview);
+
+ parent.BeginInit ();
+ parent.EndInit ();
+ parent.LayoutSubViews ();
+ parent.Draw ();
+
+ // Scroll parent.
+ parent.Viewport = parent.Viewport with { Location = new Point (5, 0) };
+ parent.Draw ();
+
+ parent.ClearNeedsDraw ();
+
+ // Invalidate a parent viewport-local 5×2 rect at viewport (1, 2).
+ parent.SetNeedsDraw (new Rectangle (1, 2, 5, 2));
+
+ Assert.Equal (new Rectangle (3, 1, 5, 2), subview.NeedsDrawRect);
+
+ parent.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// When a subview has padding (adornment), the cascade lands the dirty region in the
+ /// subview's viewport-local coords (inside the padding), NOT subview-frame-local.
+ ///
+ /// Setup: parent (20×10), subview at Frame (2, 2, 12, 6) with Padding.Thickness = 1.
+ /// Subview Viewport size = (10, 4). Invalidate parent viewport-local (3, 3, 10, 4).
+ /// That intersects subview.Frame at content (3, 3, 10, 4) ∩ (2, 2, 12, 6)
+ /// = (3, 3, 10, 4). Subview-frame-local: (1, 1, 10, 4). Subtract padding offset
+ /// (1, 1): (0, 0, 10, 4). Clip to subview viewport (0, 0, 10, 4): (0, 0, 10, 4).
+ ///
+ [Fact]
+ public void SetNeedsDraw_SubViewWithPadding_CascadesViewportLocalRectIntoSubviewViewport ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View parent = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+
+ View subview = new () { X = 2, Y = 2, Width = 12, Height = 6 };
+ subview.Padding.Thickness = new Thickness (1);
+ parent.Add (subview);
+
+ parent.BeginInit ();
+ parent.EndInit ();
+ parent.LayoutSubViews ();
+ parent.Draw ();
+
+ Assert.Equal (new Size (10, 4), subview.Viewport.Size);
+
+ parent.ClearNeedsDraw ();
+
+ parent.SetNeedsDraw (new Rectangle (3, 3, 10, 4));
+
+ // Subview's NeedsDrawRect is in subview-VIEWPORT-local — adornment offset is removed.
+ Assert.Equal (new Rectangle (0, 0, 10, 4), subview.NeedsDrawRect);
+
+ parent.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// When the subview itself is scrolled, the cascade lands the dirty rect at the
+ /// correct VIEWPORT-LOCAL position — viewport-local coords are scroll-INDEPENDENT
+ /// ((0, 0) is always the top-left visible cell regardless of which content cell the
+ /// scroll maps there). We propagate the dirty ON-SCREEN cells, not the content cells
+ /// they happen to be showing.
+ ///
+ /// Setup: parent (20×10), subview Frame (0, 0, 10, 5) with content (100, 100) and
+ /// subview is scrolled to (4, 1). Parent invalidation at viewport-local (2, 2, 4, 2).
+ /// Subview-frame-local: (2, 2, 4, 2). No adornment offset, no scroll subtraction:
+ /// subview-viewport-local: (2, 2, 4, 2). Clip to (0, 0, 10, 5): (2, 2, 4, 2).
+ ///
+ [Fact]
+ public void SetNeedsDraw_ScrolledSubview_CascadesIntoSubviewViewportAtCorrectOnScreenPosition ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View parent = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+
+ View subview = new () { X = 0, Y = 0, Width = 10, Height = 5 };
+ subview.SetContentSize (new Size (100, 100));
+ parent.Add (subview);
+
+ parent.BeginInit ();
+ parent.EndInit ();
+ parent.LayoutSubViews ();
+ parent.Draw ();
+
+ subview.Viewport = subview.Viewport with { Location = new Point (4, 1) };
+ parent.Draw ();
+ parent.ClearNeedsDraw ();
+
+ parent.SetNeedsDraw (new Rectangle (2, 2, 4, 2));
+
+ Assert.Equal (new Rectangle (2, 2, 4, 2), subview.NeedsDrawRect);
+
+ parent.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// When the dirty region only overlaps the subview's adornment area (not its viewport),
+ /// the cascade falls back to a no-arg SetNeedsDraw on the subview — the subview's
+ /// NeedsDrawRect becomes the full viewport AND its adornments are flagged for redraw.
+ ///
+ /// Setup: subview Frame (5, 5, 10, 6) with Padding.Thickness = 1; visible viewport
+ /// is (0, 0, 8, 4) in subview-local. Invalidate parent (5, 5, 1, 1) — only the
+ /// top-left padding cell of the subview. Subview-frame-local: (0, 0, 1, 1). Minus
+ /// padding offset (1, 1): (-1, -1, 1, 1). Clip with viewport bounds (0, 0, 8, 4):
+ /// empty. Cascade falls back to full subview invalidation.
+ ///
+ [Fact]
+ public void SetNeedsDraw_DirtyRegionInSubviewAdornmentOnly_FallsBackToFullSubviewRedraw ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View parent = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+
+ View subview = new () { X = 5, Y = 5, Width = 10, Height = 6 };
+ subview.Padding.Thickness = new Thickness (1);
+ subview.Padding.GetOrCreateView ();
+ parent.Add (subview);
+
+ parent.BeginInit ();
+ parent.EndInit ();
+ parent.LayoutSubViews ();
+ parent.Draw ();
+
+ Assert.Equal (new Size (8, 4), subview.Viewport.Size);
+
+ parent.ClearNeedsDraw ();
+
+ parent.SetNeedsDraw (new Rectangle (5, 5, 1, 1));
+
+ // Fallback: subview's NeedsDrawRect ends up as the full viewport, and adornments
+ // are flagged for redraw.
+ Assert.Equal (new Rectangle (0, 0, 8, 4), subview.NeedsDrawRect);
+ Assert.True (subview.Padding.View?.NeedsDraw);
+
+ parent.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// Issue #5359 follow-up to PR #5431: with the coord-system normalized, the framework's
+ /// region-aware ClearViewport now narrows correctly on scrolled views. The temporary
+ /// "Viewport.Location == Point.Empty" guard from PR #5431 is removed.
+ ///
+ [Fact]
+ public void FrameworkNarrowing_NowWorks_OnScrolledView ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+ driver.Clip = new Region (driver.Screen);
+
+ View view = new () { Driver = driver, X = 5, Y = 5, Width = 10, Height = 5 };
+ view.SetContentSize (new Size (100, 100));
+ view.BeginInit ();
+ view.EndInit ();
+ view.LayoutSubViews ();
+ view.Draw ();
+
+ // Scroll so Viewport.Location != (0, 0).
+ view.Viewport = view.Viewport with { Y = 5 };
+ Assert.Equal (new Point (0, 5), view.Viewport.Location);
+
+ view.Draw ();
+ driver.Clip = new Region (driver.Screen);
+ driver.FillRect (driver.Screen, new Rune ('X'));
+
+ // Partial dirty rect in viewport-local coords.
+ view.ClearNeedsDraw ();
+ view.SetNeedsDraw (new Rectangle (1, 1, 3, 2));
+
+ view.Draw ();
+
+ // Only the 3×2 dirty region in viewport-local should be cleared. viewport-local (1, 1)
+ // → screen (5+1, 5+1) = (6, 6) (the view's screen position is (5, 5), no adornments).
+ for (var y = 6; y < 8; y++)
+ {
+ for (var x = 6; x < 9; x++)
+ {
+ Assert.Equal (" ", driver.Contents [y, x].Grapheme);
+ }
+ }
+
+ // Cells outside the dirty region (but inside the viewport) keep their 'X'.
+ Assert.Equal ("X", driver.Contents [5, 5].Grapheme);
+ Assert.Equal ("X", driver.Contents [9, 14].Grapheme);
+
+ view.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// SetNeedsDraw() no-arg on a view with a zero-size viewport is a no-op (early-return
+ /// guards: NeedsDrawRect already set + viewport empty).
+ ///
+ [Fact]
+ public void SetNeedsDraw_ZeroSizeViewport_IsNoOp ()
+ {
+ View view = new () { Width = 0, Height = 0 };
+ view.BeginInit ();
+ view.EndInit ();
+
+ view.SetNeedsDraw ();
+
+ Assert.False (view.NeedsDraw);
+ Assert.Equal (Rectangle.Empty, view.NeedsDrawRect);
+
+ view.Dispose ();
+ }
+
+ ///
+ /// When a subview's Frame changes and the SuperView is itself scrolled, SetFrame's
+ /// SuperView invalidation must translate the union(old, new) rect from SuperView
+ /// CONTENT coords to SuperView VIEWPORT-LOCAL coords. Without the translation, the
+ /// invalidation lands at the wrong on-screen position.
+ ///
+ /// Setup: superView (20×10) scrolled by (5, 3). subview at Frame (10, 8, 7, 4) in
+ /// superView's content coords. Shrink subview Frame to (10, 8, 3, 2). SetFrame
+ /// invalidates with union(old, new) = (10, 8, 7, 4) (content coords). Translation
+ /// to superView viewport-local: subtract (5, 3) → (5, 5, 7, 4). superView's
+ /// NeedsDrawRect must contain this rect (it may be larger due to union with previously
+ /// accumulated dirty regions, but must contain (5, 5, 7, 4)).
+ ///
+ [Fact]
+ public void SetFrame_ScrolledSuperView_TranslatesInvalidationToViewportLocal ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View superView = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+ superView.SetContentSize (new Size (100, 100));
+
+ View subview = new () { X = 10, Y = 8, Width = 7, Height = 4 };
+ superView.Add (subview);
+
+ superView.BeginInit ();
+ superView.EndInit ();
+ superView.LayoutSubViews ();
+ superView.Draw ();
+
+ superView.Viewport = superView.Viewport with { Location = new Point (5, 3) };
+ Assert.Equal (new Point (5, 3), superView.Viewport.Location);
+
+ superView.Draw ();
+ superView.ClearNeedsDraw ();
+ Assert.Equal (Rectangle.Empty, superView.NeedsDrawRect);
+
+ // Shrink subview's Frame. SetFrame invalidates SuperView with union(old, new) =
+ // (10, 8, 7, 4) in superView CONTENT coords; the translation subtracts the scroll
+ // (5, 3) to produce viewport-local (5, 5, 7, 4).
+ subview.Frame = new Rectangle (10, 8, 3, 2);
+
+ Assert.True (superView.NeedsDrawRect.Contains (new Rectangle (5, 5, 7, 4)),
+ $"Expected superView.NeedsDrawRect to contain viewport-local (5, 5, 7, 4); got {superView.NeedsDrawRect}.");
+
+ // Sanity: the rect must NOT have left the scroll offset baked in. If translation
+ // were skipped, NeedsDrawRect would contain content-coord (10, 8, ...) which has
+ // X >= 10.
+ Assert.True (superView.NeedsDrawRect.X < 10,
+ $"NeedsDrawRect.X = {superView.NeedsDrawRect.X}; if scroll wasn't subtracted it would be >= 10 (content-coord X of the subview).");
+
+ superView.Dispose ();
+ driver.Dispose ();
+ }
+
+ ///
+ /// Repeated small invalidations on a scrolled view still produce a precise accumulating
+ /// union (AC1 from #5359 — already enabled by #5431's union fix, but worth pinning to
+ /// verify the new coord convention doesn't regress accumulation).
+ ///
+ [Fact]
+ public void SetNeedsDraw_RepeatedSmallInvalidationsOnScrolledView_AccumulateAsViewportLocalUnion ()
+ {
+ IDriver driver = CreateTestDriver (40, 20);
+
+ View view = new () { Driver = driver, X = 0, Y = 0, Width = 20, Height = 10 };
+ view.SetContentSize (new Size (100, 100));
+ view.BeginInit ();
+ view.EndInit ();
+ view.LayoutSubViews ();
+ view.Draw ();
+
+ view.Viewport = view.Viewport with { Location = new Point (4, 2) };
+ view.ClearNeedsDraw ();
+
+ view.SetNeedsDraw (new Rectangle (1, 1, 3, 2));
+ view.SetNeedsDraw (new Rectangle (10, 5, 4, 3));
+
+ // Union((1,1,3,2),(10,5,4,3)) = (1,1,13,7).
+ Assert.Equal (new Rectangle (1, 1, 13, 7), view.NeedsDrawRect);
+
+ view.Dispose ();
+ driver.Dispose ();
+ }
+}
diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs
index 3039b9ec94..a5b800a9d9 100644
--- a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs
+++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs
@@ -296,16 +296,16 @@ public void NeedsDrawRect_Is_Viewport_Relative ()
view.Frame = new Rectangle (3, 3, 6, 6); // Grow right/bottom 1
Assert.Equal (new Rectangle (3, 3, 6, 6), view.Frame);
Assert.Equal (new Rectangle (1, 1, 6, 6), view.Viewport);
- // Union((0,0,6,6),(1,1,6,6)) — Viewport now has scroll offset (1,1).
- // SetFrame's SetNeedsDraw passes the current Viewport = (1,1,6,6).
- // Rectangle.Union((0,0,6,6),(1,1,6,6)) = (0,0,7,7).
- Assert.Equal (new Rectangle (0, 0, 7, 7), view.NeedsDrawRect);
+ // Issue #5359: NeedsDrawRect is viewport-LOCAL — independent of Viewport.Location
+ // (the scroll offset). SetFrame's SetNeedsDraw passes (0, 0, 6, 6), not (1, 1, 6, 6).
+ // Union((0,0,6,6),(0,0,6,6)) = (0,0,6,6).
+ Assert.Equal (new Rectangle (0, 0, 6, 6), view.NeedsDrawRect);
view.Frame = new Rectangle (3, 3, 5, 5);
Assert.Equal (new Rectangle (3, 3, 5, 5), view.Frame);
Assert.Equal (new Rectangle (1, 1, 5, 5), view.Viewport);
- // Union((0,0,7,7),(1,1,5,5)) = (0,0,7,7).
- Assert.Equal (new Rectangle (0, 0, 7, 7), view.NeedsDrawRect);
+ // Union((0,0,6,6),(0,0,5,5)) = (0,0,6,6).
+ Assert.Equal (new Rectangle (0, 0, 6, 6), view.NeedsDrawRect);
}
[Fact]
diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs
index 78b68f4576..e34b30346a 100644
--- a/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs
+++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs
@@ -147,52 +147,11 @@ public void FrameworkDraw_FullViewportNeedsDrawRect_ClearsFullViewport ()
driver.Dispose ();
}
- ///
- /// Review feedback item 1: narrowing must NOT fire when the view is itself scrolled,
- /// because SetNeedsDraw(Rectangle) cascades to subviews using frame-local coordinates
- /// while the no-arg version passes content-coord Viewport. Until that convention is
- /// normalized, the framework falls back to a full clear for scrolled views.
- ///
- [Fact]
- public void FrameworkDraw_ScrolledView_FallsBackToFullClear ()
- {
- IDriver driver = CreateTestDriver (40, 20);
- driver.Clip = new Region (driver.Screen);
-
- View view = new () { Driver = driver, X = 5, Y = 5, Width = 10, Height = 5 };
- view.SetContentSize (new Size (100, 100));
- view.BeginInit ();
- view.EndInit ();
- view.LayoutSubViews ();
- view.Draw ();
-
- // Scroll so Viewport.Location is non-empty.
- view.Viewport = view.Viewport with { Y = 5 };
- Assert.Equal (new Point (0, 5), view.Viewport.Location);
-
- view.Draw ();
- driver.Clip = new Region (driver.Screen);
- driver.FillRect (driver.Screen, new Rune ('X'));
-
- // Set a partial dirty rect (which would narrow if the view weren't scrolled).
- view.ClearNeedsDraw ();
- view.SetNeedsDraw (new Rectangle (1, 6, 3, 2));
-
- view.Draw ();
-
- // Full viewport should be cleared (narrowing must not fire on scrolled view).
- Rectangle screen = view.ViewportToScreen (view.Viewport with { Location = new Point (0, 0) });
- for (int y = screen.Y; y < screen.Y + screen.Height; y++)
- {
- for (int x = screen.X; x < screen.X + screen.Width; x++)
- {
- Assert.Equal (" ", driver.Contents [y, x].Grapheme);
- }
- }
-
- view.Dispose ();
- driver.Dispose ();
- }
+ // Note: a prior FrameworkDraw_ScrolledView_FallsBackToFullClear test was removed in #5359.
+ // It documented the temporary "Viewport.Location == Point.Empty" guard added in #5431 as a
+ // workaround for the cascade coord-system inconsistency. Issue #5359 normalized the
+ // convention and removed that guard. Narrowing on scrolled views is now correct and is
+ // covered by NeedsDrawCoordTests.FrameworkNarrowing_NowWorks_OnScrolledView.
///
/// When a subview's Frame shrinks, View.Layout invalidates the SuperView for the union