diff --git a/Terminal.Gui/App/ApplicationImpl.Screen.cs b/Terminal.Gui/App/ApplicationImpl.Screen.cs index 98f9c40ea8..bec9509ef4 100644 --- a/Terminal.Gui/App/ApplicationImpl.Screen.cs +++ b/Terminal.Gui/App/ApplicationImpl.Screen.cs @@ -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 (), neededLayout || forceRedraw); Driver.Clip = new Region (clipRect); diff --git a/Terminal.Gui/ViewBase/Adornment/AdornmentView.cs b/Terminal.Gui/ViewBase/Adornment/AdornmentView.cs index 61f1fdebc5..b4bef14208 100644 --- a/Terminal.Gui/ViewBase/Adornment/AdornmentView.cs +++ b/Terminal.Gui/ViewBase/Adornment/AdornmentView.cs @@ -15,9 +15,10 @@ namespace Terminal.Gui.ViewBase; /// back-reference, making the single authoritative owner of Thickness. /// /// -/// During the incremental migration, existing and continue -/// to extend . Only newly migrated adornments (starting with MarginView) -/// extend . +/// , , and all extend +/// . is the authoritative owner of +/// ; the corresponding hosts the +/// render-layer behavior. /// /// 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; } diff --git a/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs b/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs index 689c164f70..ebabfe0c2f 100644 --- a/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs +++ b/Terminal.Gui/ViewBase/View.Drawing.Adornments.cs @@ -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; diff --git a/Terminal.Gui/ViewBase/View.Drawing.cs b/Terminal.Gui/ViewBase/View.Drawing.cs index e17a052e73..3417168b93 100644 --- a/Terminal.Gui/ViewBase/View.Drawing.cs +++ b/Terminal.Gui/ViewBase/View.Drawing.cs @@ -91,8 +91,12 @@ public void Draw (DrawContext? context = null) 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 @@ public void Draw (DrawContext? context = null) // 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 @@ public void Draw (DrawContext? context = null) _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 @@ internal void DoClearViewport (DrawContext? context = null) 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)); } + /// + /// Determines whether the framework's can safely narrow + /// the clear to just . See for + /// the rationale. + /// + 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; + } + /// /// Called when the is to be cleared. /// diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index e234499186..0db07b69a4 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -95,6 +95,7 @@ private bool SetFrame (in Rectangle frame) return false; } + Rectangle? oldFrame = _frame; var oldViewport = Rectangle.Empty; if (IsInitialized) @@ -111,6 +112,16 @@ 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. + // SetFrame is the single source of truth for this invalidation for both direct Frame + // assignment and layout-driven frame updates. + 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 (in frame)); @@ -578,6 +589,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; diff --git a/Terminal.Gui/ViewBase/View.NeedsDraw.cs b/Terminal.Gui/ViewBase/View.NeedsDraw.cs index fc2e123b5d..5303d20106 100644 --- a/Terminal.Gui/ViewBase/View.NeedsDraw.cs +++ b/Terminal.Gui/ViewBase/View.NeedsDraw.cs @@ -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. diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs index 849094e050..de1d9175a1 100644 --- a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -45,6 +45,7 @@ private sealed class Counters public int SubViewsLaidOut; public int DrawComplete; public int ClearedViewport; + public int DrawingText; } private static string MakeText (string prefix, int lines) @@ -104,11 +105,13 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa codes [i].SubViewsLaidOut += (_, _) => perTab [captured].SubViewsLaidOut++; codes [i].DrawComplete += (_, _) => perTab [captured].DrawComplete++; codes [i].ClearedViewport += (_, _) => perTab [captured].ClearedViewport++; + codes [i].DrawingText += (_, _) => perTab [captured].DrawingText++; } tabs.SubViewsLaidOut += (_, _) => tabsContainer.SubViewsLaidOut++; tabs.DrawComplete += (_, _) => tabsContainer.DrawComplete++; tabs.ClearedViewport += (_, _) => tabsContainer.ClearedViewport++; + tabs.DrawingText += (_, _) => tabsContainer.DrawingText++; ScrollableCode active = codes [0]; @@ -123,11 +126,13 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa perTab [i].SubViewsLaidOut = 0; perTab [i].DrawComplete = 0; perTab [i].ClearedViewport = 0; + perTab [i].DrawingText = 0; } tabsContainer.SubViewsLaidOut = 0; tabsContainer.DrawComplete = 0; tabsContainer.ClearedViewport = 0; + tabsContainer.DrawingText = 0; }) .KeyDown (Key.PageDown) .KeyDown (Key.PageDown) @@ -136,12 +141,12 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa outputHelper.WriteLine ($"Driver: {driverName}"); outputHelper.WriteLine ($"Active tab viewport Y after 3 PageDowns: {active.Viewport.Y}"); outputHelper.WriteLine ("Per-tab counters (after 3 PageDowns on active tab):"); - outputHelper.WriteLine (" tab laidOut drawComplete clearedViewport"); - outputHelper.WriteLine ($" Tabs {tabsContainer.SubViewsLaidOut,7} {tabsContainer.DrawComplete,12} {tabsContainer.ClearedViewport,15}"); + outputHelper.WriteLine (" tab laidOut drawComplete clearedViewport drawingText"); + outputHelper.WriteLine ($" Tabs {tabsContainer.SubViewsLaidOut,7} {tabsContainer.DrawComplete,12} {tabsContainer.ClearedViewport,15} {tabsContainer.DrawingText,11}"); for (var i = 0; i < TabCount; i++) { - outputHelper.WriteLine ($" Code{i + 1,-6} {perTab [i].SubViewsLaidOut,7} {perTab [i].DrawComplete,12} {perTab [i].ClearedViewport,15}"); + outputHelper.WriteLine ($" Code{i + 1,-6} {perTab [i].SubViewsLaidOut,7} {perTab [i].DrawComplete,12} {perTab [i].ClearedViewport,15} {perTab [i].DrawingText,11}"); } Assert.True ( @@ -152,27 +157,34 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa perTab [0].DrawComplete > 0, $"Active tab must draw in response to real PageDown, got DrawComplete={perTab [0].DrawComplete}."); - int inactiveDraws = 0; + int inactiveTextDraws = 0; int inactiveLayouts = 0; for (var i = 1; i < TabCount; i++) { - inactiveDraws += perTab [i].DrawComplete; + inactiveTextDraws += perTab [i].DrawingText; inactiveLayouts += perTab [i].SubViewsLaidOut; } - outputHelper.WriteLine ($"Sum inactive DrawComplete = {inactiveDraws}"); + outputHelper.WriteLine ($"Sum inactive DrawingText = {inactiveTextDraws}"); outputHelper.WriteLine ($"Sum inactive SubViewsLaidOut = {inactiveLayouts}"); - // CURRENT BEHAVIOR: draw fan-out still exists through the real input → command → main-loop path, - // but layout fan-out should now be eliminated. + // 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 ( - inactiveDraws > 0, - $"Documents issue #4973 (integration-level): inactive_total DrawComplete={inactiveDraws}. " + - "Flip to Assert.Equal(0, inactiveDraws) after fix lands."); + 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, - inactiveLayouts); + Assert.Equal (0, inactiveLayouts); } } diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs index 70c36f8ead..3039b9ec94 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/NeedsDrawTests.cs @@ -239,6 +239,10 @@ public void NeedsDraw_False_After_Draw () [Fact] public void NeedsDrawRect_Is_Viewport_Relative () { + // Issue #5358: NeedsDrawRect is now a true accumulating union of all dirty regions + // since the last Draw() / ClearNeedsDraw(). Previously the broken union math clamped + // each call's result to the current Viewport size, which incidentally produced + // "current Viewport" semantics. This test asserts the accumulating union semantics. View superView = new () { Id = "superView", Width = 10, Height = 10 }; Assert.Equal (new Rectangle (0, 0, 10, 10), superView.Frame); Assert.Equal (new Rectangle (0, 0, 10, 10), superView.Viewport); @@ -262,37 +266,46 @@ public void NeedsDrawRect_Is_Viewport_Relative () view.Frame = new Rectangle (3, 3, 5, 5); Assert.Equal (new Rectangle (3, 3, 5, 5), view.Frame); Assert.Equal (new Rectangle (0, 0, 5, 5), view.Viewport); + // Union((0,0,2,3),(0,0,5,5)) = (0,0,5,5) Assert.Equal (new Rectangle (0, 0, 5, 5), view.NeedsDrawRect); 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 (0, 0, 6, 6), view.Viewport); + // Union((0,0,5,5),(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); // Shrink right/bottom 1 Assert.Equal (new Rectangle (3, 3, 5, 5), view.Frame); Assert.Equal (new Rectangle (0, 0, 5, 5), view.Viewport); - Assert.Equal (new Rectangle (0, 0, 5, 5), view.NeedsDrawRect); + // Union((0,0,6,6),(0,0,5,5)) = (0,0,6,6) — accumulates the prior larger area. + Assert.Equal (new Rectangle (0, 0, 6, 6), view.NeedsDrawRect); view.SetContentSize (new Size (10, 10)); Assert.Equal (new Rectangle (3, 3, 5, 5), view.Frame); Assert.Equal (new Rectangle (0, 0, 5, 5), view.Viewport); - Assert.Equal (new Rectangle (0, 0, 5, 5), view.NeedsDrawRect); + // SetContentSize only calls SetNeedsLayout, not SetNeedsDraw. NeedsDrawRect unchanged. + Assert.Equal (new Rectangle (0, 0, 6, 6), view.NeedsDrawRect); view.Viewport = new Rectangle (1, 1, 5, 5); // Scroll up/left 1 Assert.Equal (new Rectangle (3, 3, 5, 5), view.Frame); Assert.Equal (new Rectangle (1, 1, 5, 5), view.Viewport); - Assert.Equal (new Rectangle (0, 0, 5, 5), view.NeedsDrawRect); + // Scroll calls SetNeedsLayout (not SetNeedsDraw). NeedsDrawRect unchanged. + Assert.Equal (new Rectangle (0, 0, 6, 6), view.NeedsDrawRect); 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); - Assert.Equal (new Rectangle (1, 1, 6, 6), view.NeedsDrawRect); + // 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); 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); - Assert.Equal (new Rectangle (1, 1, 5, 5), view.NeedsDrawRect); + // Union((0,0,7,7),(1,1,5,5)) = (0,0,7,7). + Assert.Equal (new Rectangle (0, 0, 7, 7), view.NeedsDrawRect); } [Fact] @@ -498,9 +511,11 @@ public void SetNeedsDraw_MultipleRectangles_Expands () view.SetNeedsDraw (new Rectangle (5, 5, 10, 10)); view.SetNeedsDraw (new Rectangle (15, 15, 10, 10)); - // Should expand to cover the entire viewport when we have overlapping regions - // The current implementation expands to viewport size - var expected = new Rectangle (0, 0, 30, 30); + // Issue #5358: NeedsDrawRect is now a true union of all SetNeedsDraw regions, + // not a max-of-Viewport-and-region clamp. Previously the broken union math + // expanded to viewport size on any second call; that masked partial-dirty regions + // and made region-aware ClearViewport impossible. + var expected = new Rectangle (5, 5, 20, 20); Assert.Equal (expected, view.NeedsDrawRect); } diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs new file mode 100644 index 0000000000..78b68f4576 --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/RegionAwareClearViewportTests.cs @@ -0,0 +1,259 @@ +#nullable disable +using System.Text; +using UnitTests; + +namespace ViewBaseTests.Draw; + +// Claude - Opus 4.7 +/// +/// Issue #5358: the framework's DoClearViewport narrows the clear to NeedsDrawRect when an +/// explicit partial region has been set AND the view is not itself scrolled. The public +/// ClearViewport API always does a full clear (preserves the contract used by +/// Code.OnClearingViewport, MarkdownCodeBlock, direct test callers). View.SetFrame +/// invalidates the SuperView for the union of the old and new Frame when Frame changes, +/// so stale uncovered cells get cleared on the next pass. +/// +public class RegionAwareClearViewportTests : TestDriverBase +{ + /// + /// Public ClearViewport always does a full clear regardless of NeedsDrawRect state — + /// this is the backward-compatible contract used by Code.OnClearingViewport, + /// MarkdownCodeBlock.OnClearingViewport, and direct test callers like + /// ClearViewport_FillsViewportArea. + /// + [Fact] + public void PublicClearViewport_AlwaysClearsFullViewport () + { + 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.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + + driver.FillRect (driver.Screen, new Rune ('X')); + + // Even with a partial NeedsDrawRect, public ClearViewport must clear the full viewport. + view.ClearNeedsDraw (); + view.SetNeedsDraw (new Rectangle (1, 1, 3, 2)); + Assert.Equal (new Rectangle (1, 1, 3, 2), view.NeedsDrawRect); + + view.ClearViewport (); + + 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 (); + } + + /// + /// The framework's draw pipeline (Draw → DoClearViewport) narrows the clear to the + /// dirty region when the view has a partial NeedsDrawRect and is not itself scrolled. + /// Verifies by invoking Draw() and observing that cells outside the dirty region keep + /// their pre-fill 'X' value. + /// + [Fact] + public void FrameworkDraw_PartialNeedsDrawRect_ClearsOnlyDirtyRegion () + { + 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.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + view.Draw (); + + Assert.True (view.NeedsDrawRect.IsEmpty); + + // Re-set clip after Draw (DoDrawComplete excludes the view's frame from the clip, + // which would prevent the next FillRect from reaching cells inside the view's area). + driver.Clip = new Region (driver.Screen); + driver.FillRect (driver.Screen, new Rune ('X')); + Assert.Equal ("X", driver.Contents [5, 5].Grapheme); + + // Invalidate just a 3×2 region — strictly smaller than the 10×5 viewport. + view.SetNeedsDraw (new Rectangle (1, 1, 3, 2)); + Assert.Equal (new Rectangle (1, 1, 3, 2), view.NeedsDrawRect); + + // Drive the framework's DoClearViewport via the normal draw path. + view.Draw (); + + // The dirty region (viewport-local (1,1,3,2) → screen (6,6,3,2)) is cleared. + 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 inside the viewport remain 'X'. + Assert.Equal ("X", driver.Contents [5, 5].Grapheme); + Assert.Equal ("X", driver.Contents [9, 14].Grapheme); + + view.Dispose (); + driver.Dispose (); + } + + /// + /// SetNeedsDraw() (no-arg) sets NeedsDrawRect = Viewport. The framework must still + /// clear the full viewport in this case — narrowing only fires when the rect is + /// strictly smaller than the viewport. + /// + [Fact] + public void FrameworkDraw_FullViewportNeedsDrawRect_ClearsFullViewport () + { + 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.BeginInit (); + view.EndInit (); + view.LayoutSubViews (); + view.Draw (); + + Assert.True (view.NeedsDrawRect.IsEmpty); + + driver.Clip = new Region (driver.Screen); + driver.FillRect (driver.Screen, new Rune ('X')); + + // SetNeedsDraw() no-arg sets NeedsDrawRect to Viewport (full). + view.SetNeedsDraw (); + Assert.Equal (view.Viewport, view.NeedsDrawRect); + + view.Draw (); + + 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 (); + } + + /// + /// 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 (); + } + + /// + /// When a subview's Frame shrinks, View.Layout invalidates the SuperView for the union + /// of the old and new frames. This is the foundation for clearing stale uncovered cells + /// after geometry changes (without forcing the whole SuperView viewport to redraw). + /// + [Fact] + public void FrameShrink_InvalidatesSuperViewWithUnionOfOldAndNewFrames () + { + IDriver driver = CreateTestDriver (40, 20); + driver.Clip = new Region (driver.Screen); + + View superView = new () { Driver = driver, Width = 20, Height = 10 }; + View view = new () { X = 0, Y = 0, Width = 7, Height = 4 }; + superView.Add (view); + + superView.Layout (); + superView.Draw (); + + Assert.True (superView.NeedsDrawRect.IsEmpty); + Assert.True (view.NeedsDrawRect.IsEmpty); + + // Shrink the subview's frame. + view.Frame = new Rectangle (0, 0, 3, 2); + + // SuperView's NeedsDrawRect must include the OLD frame area so stale cells get cleared. + Assert.False (superView.NeedsDrawRect.IsEmpty); + Assert.True (superView.NeedsDrawRect.Contains (new Rectangle (0, 0, 7, 4))); + + superView.Dispose (); + driver.Dispose (); + } + + /// + /// Pure scroll (Viewport.X/Y change without Frame change) must NOT invalidate the + /// SuperView. Otherwise we re-introduce the per-scroll draw fan-out that issue #5358 + /// is fixing for overlapping tab content. + /// + [Fact] + public void Scroll_DoesNotInvalidateSuperView () + { + IDriver driver = CreateTestDriver (40, 20); + driver.Clip = new Region (driver.Screen); + + View superView = new () { Driver = driver, Width = 20, Height = 10 }; + View view = new () { X = 0, Y = 0, Width = 10, Height = 5 }; + view.SetContentSize (new Size (100, 100)); + superView.Add (view); + + superView.Layout (); + superView.Draw (); + + Assert.True (superView.NeedsDrawRect.IsEmpty); + + // Pure scroll — Viewport location changes, Frame does not. + view.Viewport = view.Viewport with { Y = 5 }; + + // SuperView must NOT have been invalidated by the scroll alone. + Assert.True (superView.NeedsDrawRect.IsEmpty); + + superView.Dispose (); + driver.Dispose (); + } +} diff --git a/Tests/UnitTestsParallelizable/ViewBase/Draw/SubViewOnlyRedrawTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Draw/SubViewOnlyRedrawTests.cs new file mode 100644 index 0000000000..d8a95351c7 --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Draw/SubViewOnlyRedrawTests.cs @@ -0,0 +1,241 @@ +using UnitTests; + +namespace ViewBaseTests.Draw; + +// Claude - Opus 4.7 +/// +/// Issue #5358: when a parent enters Draw() only because a child is dirty, +/// the parent must NOT clear its viewport, re-draw its text, or re-draw its +/// own content. Only adornments and subviews are allowed to run. +/// +public class SubViewOnlyRedrawTests : TestDriverBase +{ + /// + /// GIVEN a parent where only the child is dirty + /// WHEN Draw runs + /// THEN the parent's ClearingViewport / DrewText / DrawingContent events MUST NOT fire. + /// + [Fact] + public void ChildOnlyDirty_ParentDoesNotClearOrRedrawSelf () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () { Driver = driver, Width = 20, Height = 10, Text = "parent-text" }; + View child = new () { Width = 10, Height = 5, X = 1, Y = 1 }; + parent.Add (child); + + parent.Layout (); + parent.Draw (); + + var cleared = 0; + var drewText = 0; + var drewContent = 0; + parent.ClearedViewport += (_, _) => cleared++; + parent.DrewText += (_, _) => drewText++; + parent.DrawingContent += (_, _) => drewContent++; + + child.SetNeedsDraw (); + + parent.Draw (); + + Assert.Equal (0, cleared); + Assert.Equal (0, drewText); + Assert.Equal (0, drewContent); + + parent.Dispose (); + driver.Dispose (); + } + + /// + /// GIVEN a parent dirtied directly via SetNeedsDraw() + /// WHEN Draw runs + /// THEN the parent MUST clear and redraw its text/content (regression guard: + /// the new gate must not break the normal full-redraw path). + /// + [Fact] + public void ParentDirectlyDirty_StillClearsAndRedrawsSelf () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () { Driver = driver, Width = 20, Height = 10, Text = "parent-text" }; + View child = new () { Width = 10, Height = 5, X = 1, Y = 1 }; + parent.Add (child); + + parent.Layout (); + parent.Draw (); + + var cleared = 0; + var drewText = 0; + parent.ClearedViewport += (_, _) => cleared++; + parent.DrewText += (_, _) => drewText++; + + parent.SetNeedsDraw (); + + parent.Draw (); + + Assert.True (cleared > 0, $"Parent should clear when it itself is dirty (got {cleared})."); + Assert.True (drewText > 0, $"Parent should redraw text when it itself is dirty (got {drewText})."); + + parent.Dispose (); + driver.Dispose (); + } + + /// + /// GIVEN a transparent parent where only the child is dirty + /// WHEN Draw runs + /// THEN the existing Transparent early-return in DoClearViewport still wins, + /// and the child-only path stays correct (no regression to transparency). + /// + [Fact] + public void TransparentParent_ChildOnlyDirty_DoesNotClear () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () + { + Driver = driver, + Width = 20, + Height = 10, + ViewportSettings = ViewportSettingsFlags.Transparent + }; + View child = new () { Width = 10, Height = 5, X = 1, Y = 1 }; + parent.Add (child); + + parent.Layout (); + parent.Draw (); + + var cleared = 0; + parent.ClearedViewport += (_, _) => cleared++; + + child.SetNeedsDraw (); + parent.Draw (); + + Assert.Equal (0, cleared); + + parent.Dispose (); + driver.Dispose (); + } + + /// + /// GIVEN a parent with a Border adornment where only the child is dirty + /// WHEN Draw runs + /// THEN the parent's viewport is not cleared and the parent's own text/content + /// is not redrawn. (Adornments still run independently — they always redraw + /// when the parent is entered; that is by design.) + /// + [Fact] + public void ParentWithBorder_ChildOnlyDirty_ParentDoesNotClear () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () { Driver = driver, Width = 20, Height = 10 }; + parent.Border.Thickness = new Thickness (1); + + View child = new () { Width = 5, Height = 3, X = 2, Y = 2 }; + parent.Add (child); + + parent.Layout (); + parent.Draw (); + + var parentCleared = 0; + var parentDrewText = 0; + parent.ClearedViewport += (_, _) => parentCleared++; + parent.DrewText += (_, _) => parentDrewText++; + + child.SetNeedsDraw (); + parent.Draw (); + + Assert.Equal (0, parentCleared); + Assert.Equal (0, parentDrewText); + + parent.Dispose (); + driver.Dispose (); + } + + /// + /// GIVEN a parent and child rendered to a real driver / IOutput + /// WHEN only the child is dirtied and Draw runs + /// THEN IOutput must still produce non-empty output (the fix does not + /// accidentally suppress the child's render at the IOutput layer). + /// + [Fact] + public void ChildOnlyDirty_StillProducesOutputAtIOutputLayer () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () { Driver = driver, Width = 20, Height = 10 }; + View child = new () { Width = 10, Height = 5, X = 1, Y = 1, Text = "CHILD" }; + parent.Add (child); + + parent.Layout (); + parent.Draw (); + + IOutput output = driver.GetOutput (); + IOutputBuffer buffer = driver.GetOutputBuffer (); + + child.Text = "CHILDX"; + child.SetNeedsDraw (); + parent.Draw (); + + output.Write (buffer); + string ansi = output.GetLastOutput (); + + Assert.False (string.IsNullOrEmpty (ansi)); + + parent.Dispose (); + driver.Dispose (); + } + + /// + /// Review feedback item 2: a transparent parent with TransparentMouse must keep its + /// CachedDrawnRegion populated across child-only redraws. Before the fix, the + /// unconditional `_localDrawContext = new DrawContext()` reset combined with skipping + /// DoDrawText / DoDrawContent on child-only passes meant DoDrawComplete wrote an + /// empty region into CachedDrawnRegion, breaking mouse hit-testing until the next + /// full self-redraw. + /// + [Fact] + public void TransparentMouseParent_ChildOnlyDirty_PreservesCachedDrawnRegion () + { + IDriver driver = CreateTestDriver (40, 20); + + View parent = new () + { + Driver = driver, + Width = 20, + Height = 10, + ViewportSettings = ViewportSettingsFlags.Transparent | ViewportSettingsFlags.TransparentMouse + }; + View child = new () { Width = 10, Height = 5, X = 1, Y = 1 }; + parent.Add (child); + + // Force the parent to actually draw something so CachedDrawnRegion is populated + // (transparent views only cache cells they actually touched). + parent.DrawingContent += (_, e) => + { + parent.FillRect (new Rectangle (0, 0, 5, 3), new System.Text.Rune ('P')); + e.DrawContext?.AddDrawnRectangle (parent.ViewportToScreen (new Rectangle (0, 0, 5, 3))); + }; + + parent.Layout (); + parent.Draw (); + + Region? cachedAfterFullDraw = parent.CachedDrawnRegion; + Assert.NotNull (cachedAfterFullDraw); + Assert.False (cachedAfterFullDraw.GetBounds ().IsEmpty); + + // Child-only invalidation: only the child becomes dirty, parent stays clean. + child.SetNeedsDraw (); + Assert.False (parent.NeedsDraw); + + parent.Draw (); + + // After the child-only redraw, parent's cache must still reflect the previously-drawn + // region. If we naively wiped it, transparent-parent mouse hit-testing would break. + Assert.NotNull (parent.CachedDrawnRegion); + Assert.False (parent.CachedDrawnRegion.GetBounds ().IsEmpty); + + parent.Dispose (); + driver.Dispose (); + } +} diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs index 5792502d1e..b368361511 100644 --- a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -57,6 +57,7 @@ public void Track (View view, string label) view.DrawComplete += (_, _) => counts.DrawComplete++; view.ClearedViewport += (_, _) => counts.ClearedViewport++; view.DrawingContent += (_, _) => counts.DrawingContent++; + view.DrawingText += (_, _) => counts.DrawingText++; view.FrameChanged += (_, _) => counts.FrameChanged++; } @@ -74,14 +75,14 @@ public string Report (string title) { StringBuilder sb = new (); sb.AppendLine (title); - sb.AppendLine (" view laidOut drawComplete clearedViewport drawingContent frameChanged"); + sb.AppendLine (" view laidOut drawComplete clearedViewport drawingContent drawingText frameChanged"); foreach ((View view, string label) in _order) { Counts c = _counts [view]; sb.AppendLine ( - $" {label,-26} {c.SubViewsLaidOut,7} {c.DrawComplete,12} {c.ClearedViewport,15} {c.DrawingContent,14} {c.FrameChanged,12}"); + $" {label,-26} {c.SubViewsLaidOut,7} {c.DrawComplete,12} {c.ClearedViewport,15} {c.DrawingContent,14} {c.DrawingText,11} {c.FrameChanged,12}"); } return sb.ToString (); @@ -93,6 +94,7 @@ public sealed class Counts public int DrawComplete; public int ClearedViewport; public int DrawingContent; + public int DrawingText; public int FrameChanged; public void Reset () @@ -101,6 +103,7 @@ public void Reset () DrawComplete = 0; ClearedViewport = 0; DrawingContent = 0; + DrawingText = 0; FrameChanged = 0; } } @@ -269,6 +272,7 @@ public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () int inactiveDraws = 0; int inactiveClears = 0; int inactiveContentDraws = 0; + int inactiveTextDraws = 0; for (var i = 1; i < TabCount; i++) { @@ -276,31 +280,27 @@ public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () inactiveDraws += c.DrawComplete; inactiveClears += c.ClearedViewport; inactiveContentDraws += c.DrawingContent; + inactiveTextDraws += c.DrawingText; } output.WriteLine ($"Active DrawComplete: {activeCounts.DrawComplete}"); output.WriteLine ($"Sum of inactive DrawComplete: {inactiveDraws}"); output.WriteLine ($"Sum of inactive ClearedViewport: {inactiveClears}"); output.WriteLine ($"Sum of inactive DrawingContent: {inactiveContentDraws}"); - - // CURRENT BEHAVIOR (issue #4973): inactive tabs receive full draw passes when active scrolls. - // DrawComplete is the widget-agnostic signal — it always fires once per call to Draw(), - // regardless of whether the view overrides OnClearingViewport / OnDrawingContent (as Code does). - // After #4973 is fixed, flip this to `Assert.Equal (0, inactiveDraws)`. - Assert.True ( - inactiveDraws > 0, - $"Documents issue #4973 draw fan-out: inactive_total DrawComplete={inactiveDraws}, " + - $"active={activeCounts.DrawComplete}. Flip to Assert.Equal(0, inactiveDraws) after #4973 fix."); - - // ClearedViewport / DrawingContent on Code instances will be 0 because Code overrides those - // handlers and returns true (suppressing the events). The Tabs container still fires them, - // so they remain useful as a secondary diagnostic — recorded in the report above but not - // asserted at the per-tab level. + output.WriteLine ($"Sum of inactive DrawingText: {inactiveTextDraws}"); + + // Issue #5358 fix: inactive tabs' NeedsDraw is no longer set as a side-effect + // of the parent entering Draw(). DrawingText is the right metric here because + // it's gated on NeedsDraw inside DoDrawText — and Code does NOT override + // OnDrawingText (unlike OnClearingViewport / OnDrawingContent which Code + // intercepts, suppressing those events). DrawComplete fires unconditionally + // for clip-exclusion bookkeeping, so it is informational only. + Assert.Equal (0, inactiveTextDraws); + + // Issue #5358 fix: the Tabs container itself no longer clears its viewport + // when only a child tab is dirty. Before the fix this was > 0; after, == 0. ViewActivityCounters.Counts tabsCounts = counters.Get (tabs); - Assert.True ( - tabsCounts.ClearedViewport > 0, - $"Tabs container must register clear activity (got {tabsCounts.ClearedViewport}); " + - "this confirms the lower-level draw pipeline is being exercised."); + Assert.Equal (0, tabsCounts.ClearedViewport); root.Dispose (); driver.Dispose (); @@ -349,33 +349,30 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleViewBaseline () ViewActivityCounters.Counts tabActiveCounts = tabCounters.Get (active); output.WriteLine (tabCounters.Report ("Tabbed scenario (5 scrolls of active tab):")); - int totalTabDraws = tabActiveCounts.DrawComplete; + int totalTabTextDraws = tabActiveCounts.DrawingText; int totalTabLayouts = tabActiveCounts.SubViewsLaidOut; for (var i = 1; i < TabCount; i++) { - totalTabDraws += tabCounters.Get (codes [i]).DrawComplete; + totalTabTextDraws += tabCounters.Get (codes [i]).DrawingText; totalTabLayouts += tabCounters.Get (codes [i]).SubViewsLaidOut; } - double drawFanOut = singleCounts.DrawComplete == 0 ? double.NaN : (double)totalTabDraws / singleCounts.DrawComplete; + double textDrawFanOut = singleCounts.DrawingText == 0 ? double.NaN : (double)totalTabTextDraws / singleCounts.DrawingText; double layoutFanOut = singleCounts.SubViewsLaidOut == 0 ? double.NaN : (double)totalTabLayouts / singleCounts.SubViewsLaidOut; - output.WriteLine ($"Tabbed total draws / single draws = {totalTabDraws} / {singleCounts.DrawComplete} = {drawFanOut:F2}"); - output.WriteLine ($"Tabbed total layouts / single layouts = {totalTabLayouts} / {singleCounts.SubViewsLaidOut} = {layoutFanOut:F2}"); + output.WriteLine ($"Tabbed total text draws / single text draws = {totalTabTextDraws} / {singleCounts.DrawingText} = {textDrawFanOut:F2}"); + output.WriteLine ($"Tabbed total layouts / single layouts = {totalTabLayouts} / {singleCounts.SubViewsLaidOut} = {layoutFanOut:F2}"); - Assert.True (singleCounts.DrawComplete > 0, "Single-Code baseline must record draw activity."); - Assert.True (tabActiveCounts.DrawComplete > 0, "Active tab in tabbed scenario must record draw activity."); + Assert.True (singleCounts.DrawingText > 0, "Single-Code baseline must record text-draw activity (NeedsDraw-gated)."); + Assert.True (tabActiveCounts.DrawingText > 0, "Active tab in tabbed scenario must record text-draw activity."); - // CURRENT BEHAVIOR: draw fan-out still exists, but layout fan-out should now match baseline. - Assert.True ( - drawFanOut > 1.0, - $"Documents issue #4973: tabbed draw fan-out ({drawFanOut:F2}x) exceeds single-Code baseline. " + - "After fix, drawFanOut should approach 1.0."); + // Issue #5358 fix: tabbed text-draw fan-out is now at parity with the single-Code baseline. + // DrawingText is gated on NeedsDraw, so it only fires on tabs whose NeedsDraw was set — + // which is just the active tab after the fix. + Assert.Equal (1.0, textDrawFanOut); - Assert.Equal ( - 1.0, - layoutFanOut); + Assert.Equal (1.0, layoutFanOut); singleRoot.Dispose (); tabRoot.Dispose (); @@ -420,10 +417,9 @@ public void Diagnostic_TransparentInactiveTab_StillObservable () { ViewActivityCounters.Counts c = counters.Get (codes [i]); - // Copilot: Lock in current fan-out behavior. After #4973, this should likely become == 0. - Assert.True ( - c.DrawComplete > 0, - $"Tab {i + 1} should currently still record DrawComplete even with Transparent (got {c.DrawComplete}). Update this expectation after #4973."); + // Issue #5358 fix: transparent inactive peers also stay out of NeedsDraw-gated work. + // DrawingText is the right per-Code metric (Code intercepts ClearedViewport / DrawingContent). + Assert.Equal (0, c.DrawingText); } root.Dispose ();