diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index e6cb655723..e234499186 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -555,18 +555,30 @@ internal static bool Layout (IEnumerable views, Size contentSize) /// /// /// - /// If the view could not be laid out (typically because a dependencies was not ready). + /// If the view could not be laid out (typically because a dependency was not ready). public bool Layout (Size contentSize) { + bool needsDrawAfterLayout = _needsDrawAfterLayout; + _needsDrawAfterLayout = false; + Rectangle originalFrame = Frame; + if (!SetRelativeLayout (contentSize)) { + _needsDrawAfterLayout = needsDrawAfterLayout; + return false; } + + bool frameChanged = Frame != originalFrame; LayoutSubViews (); - // A layout was performed so a draw is needed - // NeedsLayout may still be true if a dependent View still needs layout after SubViewsLaidOut event - SetNeedsDraw (); + if (frameChanged || needsDrawAfterLayout) + { + // Ancestor-only layout propagation should not force redraw when a peer view is merely + // recomputed during dependency resolution. Draw after layout only when this view was + // directly invalidated for layout or its resolved frame actually changed. + SetNeedsDraw (); + } return true; } @@ -680,7 +692,16 @@ public bool SetRelativeLayout (Size superviewContentSize) { // Set the frame. Do NOT use `Frame = newFrame` as it overwrites X, Y, Width, and Height // SetFrame will set _frame, call SetsNeedsLayout, and raise OnViewportChanged/ViewportChanged - SetFrame (newFrame); + _suppressNeedsDrawAfterLayout = true; + + try + { + SetFrame (newFrame); + } + finally + { + _suppressNeedsDrawAfterLayout = false; + } // BUGBUG: We set the internal fields here to avoid recursion. However, this means that // BUGBUG: other logic in the property setters does not get executed. Specifically: @@ -880,8 +901,11 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public bool NeedsLayout { get; internal set; } = true; + private bool _needsDrawAfterLayout = true; + private bool _suppressNeedsDrawAfterLayout; + /// - /// Sets to return , indicating this View and all of it's subviews + /// Sets to return , indicating this View and all of its subviews /// (including adornments) need to be laid out in the next Application iteration. /// /// @@ -892,22 +916,33 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { - NeedsLayout = true; - - if (Margin.View is { SubViews.Count: > 0 }) + if (!_suppressNeedsDrawAfterLayout) { - Margin.View.SetNeedsLayout (); + _needsDrawAfterLayout = true; } - if (Border.View is { SubViews.Count: > 0 }) + MarkSubtreeNeedsLayout (); + + TextFormatter.NeedsFormat = true; + + if (SuperView is { NeedsLayout: false } superView) { - Border.View.SetNeedsLayout (); + superView.MarkAncestorsNeedLayout (); } - if (Padding.View is { SubViews.Count: > 0 }) + if (this is AdornmentView adornment && adornment.Adornment?.Parent is { NeedsLayout: false } adornmentParent) { - Padding.View.SetNeedsLayout (); + adornmentParent.MarkAncestorsNeedLayout (); } + } + + // Marks this view and every descendant in its own subtree (including adornment subview + // trees) as needing layout. Does NOT propagate to SuperView or Adornment.Parent. See + // and issue #5357. + private void MarkSubtreeNeedsLayout () + { + NeedsLayout = true; + MarkAdornmentSubViewTrees (); // TODO: Optimize this - see Setting_Thickness_Causes_Adornment_SubView_Layout // Use a stack to avoid recursion @@ -923,44 +958,59 @@ public void SetNeedsLayout () { continue; } - current.NeedsLayout = true; - if (current.Margin.View is { SubViews.Count: > 0 }) - { - current.Margin.View.SetNeedsLayout (); - } - - if (current.Border.View is { SubViews.Count: > 0 }) - { - current.Border.View.SetNeedsLayout (); - } - - if (current.Padding.View is { SubViews.Count: > 0 }) - { - current.Padding.View.SetNeedsLayout (); - } + current.NeedsLayout = true; + current.MarkAdornmentSubViewTrees (); foreach (View subview in current.SubViews) { stack.Push (subview); } } + } - TextFormatter.NeedsFormat = true; + // Marks this view's Margin/Border/Padding view trees as needing layout when they have + // subviews. Adornment subview trees live separately from the regular SubViews tree. + private void MarkAdornmentSubViewTrees () + { + if (Margin.View is { SubViews.Count: > 0 }) + { + Margin.View.MarkSubtreeNeedsLayout (); + } - if (SuperView is { NeedsLayout: false }) + if (Border.View is { SubViews.Count: > 0 }) { - SuperView?.SetNeedsLayout (); + Border.View.MarkSubtreeNeedsLayout (); } - if (this is not AdornmentView adornment) + if (Padding.View is { SubViews.Count: > 0 }) + { + Padding.View.MarkSubtreeNeedsLayout (); + } + } + + // Walks up the ancestor chain marking each ancestor as needing layout, without descending + // into the ancestor's regular SubViews tree or any uninvolved ancestor adornment subview + // trees. Stops at any ancestor already marked, mirroring the early-exit guards in the + // original recursive call. See issue #5357. + private void MarkAncestorsNeedLayout () + { + if (NeedsLayout) { return; } - if (adornment.Adornment?.Parent is { NeedsLayout: false }) + NeedsLayout = true; + TextFormatter.NeedsFormat = true; + + if (SuperView is { NeedsLayout: false } superView) + { + superView.MarkAncestorsNeedLayout (); + } + + if (this is AdornmentView adornment && adornment.Adornment?.Parent is { NeedsLayout: false } adornmentParent) { - adornment.Adornment.Parent?.SetNeedsLayout (); + adornmentParent.MarkAncestorsNeedLayout (); } } diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs new file mode 100644 index 0000000000..849094e050 --- /dev/null +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -0,0 +1,178 @@ +using System.Text; +using AppTestHelpers; + +namespace IntegrationTests; + +// Copilot + +/// +/// Integration counterpart to TabsFanOutDiagnosticTests. Drives the active tab via real +/// key injection through the driver's input processor → command dispatch → main-loop +/// LayoutAndDraw path, instead of mutating directly. This +/// verifies the fan-out from issue #4973 / #5357 is observable end-to-end, not just under +/// synthetic / calls. +/// +/// +/// Instrumentation-only. The per-tab counters are attached to event subscriptions on +/// , , and +/// ; no rendering or invalidation behavior is changed. +/// +public class TabsFanOutIntegrationTests (ITestOutputHelper outputHelper) : TestsAllDrivers +{ + private readonly TextWriter _out = new TestOutputWriter (outputHelper); + + /// + /// A that registers / + /// so / + /// drive vertical scrolling through the normal command pipeline. Used only by this test — + /// doesn't expose AddCommand publicly, so a subclass is the + /// simplest way to wire a real input → scroll path without modifying production code. + /// + private sealed class ScrollableCode : Code + { + public ScrollableCode () + { + AddCommand (Command.ScrollDown, () => ScrollVertical (1)); + AddCommand (Command.ScrollUp, () => ScrollVertical (-1)); + + KeyBindings.Add (Key.PageDown, Command.ScrollDown); + KeyBindings.Add (Key.PageUp, Command.ScrollUp); + } + } + + private sealed class Counters + { + public int SubViewsLaidOut; + public int DrawComplete; + public int ClearedViewport; + } + + private static string MakeText (string prefix, int lines) + { + StringBuilder sb = new (); + + for (var i = 1; i <= lines; i++) + { + sb.Append (prefix); + sb.Append (' '); + sb.Append (i); + + if (i < lines) + { + sb.Append ('\n'); + } + } + + return sb.ToString (); + } + + /// + /// End-to-end check: a real on the active tab still produces + /// draw fan-out on inactive tabs, but layout work stays on the active tab. + /// + [Theory] + [MemberData (nameof (GetAllDriverNames))] + public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTabs (string driverName) + { + const int TabCount = 4; + + Tabs tabs = new () { Width = Dim.Fill (), Height = Dim.Fill () }; + ScrollableCode [] codes = new ScrollableCode [TabCount]; + + for (var i = 0; i < TabCount; i++) + { + codes [i] = new ScrollableCode + { + Title = $"Tab{i + 1}", + Text = MakeText ($"Tab{i + 1} line", 80), + Language = null, + SyntaxHighlighter = null, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + tabs.Add (codes [i]); + } + + Counters [] perTab = new Counters [TabCount]; + Counters tabsContainer = new (); + + for (var i = 0; i < TabCount; i++) + { + int captured = i; + perTab [i] = new Counters (); + codes [i].SubViewsLaidOut += (_, _) => perTab [captured].SubViewsLaidOut++; + codes [i].DrawComplete += (_, _) => perTab [captured].DrawComplete++; + codes [i].ClearedViewport += (_, _) => perTab [captured].ClearedViewport++; + } + + tabs.SubViewsLaidOut += (_, _) => tabsContainer.SubViewsLaidOut++; + tabs.DrawComplete += (_, _) => tabsContainer.DrawComplete++; + tabs.ClearedViewport += (_, _) => tabsContainer.ClearedViewport++; + + ScrollableCode active = codes [0]; + + using AppTestHelper helper = With.A (60, 20, driverName, _out) + .Add (tabs) + .Focus (active) + .Then ( + _ => + { + for (var i = 0; i < TabCount; i++) + { + perTab [i].SubViewsLaidOut = 0; + perTab [i].DrawComplete = 0; + perTab [i].ClearedViewport = 0; + } + + tabsContainer.SubViewsLaidOut = 0; + tabsContainer.DrawComplete = 0; + tabsContainer.ClearedViewport = 0; + }) + .KeyDown (Key.PageDown) + .KeyDown (Key.PageDown) + .KeyDown (Key.PageDown); + + 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}"); + + 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}"); + } + + Assert.True ( + active.Viewport.Y > 0, + $"PageDown should have scrolled the active tab via real input → command path. Got Viewport.Y={active.Viewport.Y}."); + + Assert.True ( + perTab [0].DrawComplete > 0, + $"Active tab must draw in response to real PageDown, got DrawComplete={perTab [0].DrawComplete}."); + + int inactiveDraws = 0; + int inactiveLayouts = 0; + + for (var i = 1; i < TabCount; i++) + { + inactiveDraws += perTab [i].DrawComplete; + inactiveLayouts += perTab [i].SubViewsLaidOut; + } + + outputHelper.WriteLine ($"Sum inactive DrawComplete = {inactiveDraws}"); + 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. + Assert.True ( + inactiveDraws > 0, + $"Documents issue #4973 (integration-level): inactive_total DrawComplete={inactiveDraws}. " + + "Flip to Assert.Equal(0, inactiveDraws) after fix lands."); + + Assert.Equal ( + 0, + inactiveLayouts); + } +} diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs new file mode 100644 index 0000000000..ecd52a9a6c --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -0,0 +1,304 @@ +namespace ViewBaseTests.Layout; + +// Copilot + +/// +/// Tests for the split between downward subtree invalidation and upward ancestor-only +/// propagation in . Issue #5357. +/// +/// +/// Before #5357, calling on a deep SubView marked every +/// sibling subtree dirty because the upward recursion re-entered the downward cascade. These +/// tests pin the new contract: ancestors and the changed view's own subtree are marked, but +/// unaffected sibling subtrees stay clean. +/// +public class SetNeedsLayoutPropagationTests +{ + private static void ClearAllNeedsLayout (View root) + { + root.NeedsLayout = false; + + foreach (View sv in root.SubViews) + { + ClearAllNeedsLayout (sv); + } + + if (root.Margin.View is { }) + { + ClearAllNeedsLayout (root.Margin.View); + } + + if (root.Border.View is { }) + { + ClearAllNeedsLayout (root.Border.View); + } + + if (root.Padding.View is { }) + { + ClearAllNeedsLayout (root.Padding.View); + } + } + + [Fact] + public void SetNeedsLayout_OnSubView_Does_Not_Mark_Sibling_Subtree () + { + View root = new () { Id = "root" }; + + View sibling1 = new () { Id = "sibling1" }; + View sibling1Child = new () { Id = "sibling1Child" }; + sibling1.Add (sibling1Child); + + View sibling2 = new () { Id = "sibling2" }; + View sibling2Child = new () { Id = "sibling2Child" }; + sibling2.Add (sibling2Child); + + root.Add (sibling1, sibling2); + + ClearAllNeedsLayout (root); + + sibling1.SetNeedsLayout (); + + Assert.True (sibling1.NeedsLayout, "Changed view must be marked."); + Assert.True (sibling1Child.NeedsLayout, "Changed view's subtree must be marked."); + Assert.True (root.NeedsLayout, "Ancestor must be marked."); + + Assert.False (sibling2.NeedsLayout, "Sibling must not be re-marked."); + Assert.False (sibling2Child.NeedsLayout, "Sibling's subtree must not be re-marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_OnDeepSubView_Marks_All_Ancestors () + { + View root = new () { Id = "root" }; + View middle = new () { Id = "middle" }; + View deep = new () { Id = "deep" }; + + middle.Add (deep); + root.Add (middle); + + ClearAllNeedsLayout (root); + + deep.SetNeedsLayout (); + + Assert.True (deep.NeedsLayout); + Assert.True (middle.NeedsLayout); + Assert.True (root.NeedsLayout); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_OnSubView_Cascades_Through_Own_Subtree () + { + View root = new () { Id = "root" }; + View middle = new () { Id = "middle" }; + View deep1 = new () { Id = "deep1" }; + View deep2 = new () { Id = "deep2" }; + + middle.Add (deep1, deep2); + root.Add (middle); + + ClearAllNeedsLayout (root); + + middle.SetNeedsLayout (); + + Assert.True (middle.NeedsLayout); + Assert.True (deep1.NeedsLayout, "Changed view's whole subtree must be marked."); + Assert.True (deep2.NeedsLayout, "Changed view's whole subtree must be marked."); + Assert.True (root.NeedsLayout); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_On_Cousin_Does_Not_Mark_Other_Branch () + { + View root = new () { Id = "root" }; + + View branchA = new () { Id = "branchA" }; + View branchAChild = new () { Id = "branchAChild" }; + branchA.Add (branchAChild); + + View branchB = new () { Id = "branchB" }; + View branchBChild = new () { Id = "branchBChild" }; + View branchBGrand = new () { Id = "branchBGrand" }; + branchBChild.Add (branchBGrand); + branchB.Add (branchBChild); + + root.Add (branchA, branchB); + + ClearAllNeedsLayout (root); + + branchBGrand.SetNeedsLayout (); + + Assert.True (branchBGrand.NeedsLayout); + Assert.True (branchBChild.NeedsLayout); + Assert.True (branchB.NeedsLayout); + Assert.True (root.NeedsLayout); + + Assert.False (branchA.NeedsLayout, "Other branch root must not be re-marked."); + Assert.False (branchAChild.NeedsLayout, "Other branch's subtree must not be re-marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_OnSubView_Does_Not_Mark_Ancestor_Adornment_Subtree () + { + View root = new () { Id = "root" }; + View rootBorderSubView = new () { Id = "rootBorderSubView" }; + root.Border.GetOrCreateView ().Add (rootBorderSubView); + + View branch = new () { Id = "branch" }; + View branchChild = new () { Id = "branchChild" }; + branch.Add (branchChild); + root.Add (branch); + + ClearAllNeedsLayout (root); + + branchChild.SetNeedsLayout (); + + Assert.True (branchChild.NeedsLayout); + Assert.True (branch.NeedsLayout); + Assert.True (root.NeedsLayout); + + Assert.False (root.Border.View!.NeedsLayout, "Uninvolved ancestor adornment view must not be re-marked."); + Assert.False (rootBorderSubView.NeedsLayout, "Uninvolved ancestor adornment subtree must not be re-marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_Preserves_Adornment_SubView_Marking_On_Changed_View () + { + View root = new () { Id = "root" }; + View view = new () { Id = "view" }; + View borderSubView = new () { Id = "borderSubView" }; + view.Border.GetOrCreateView ().Add (borderSubView); + root.Add (view); + + ClearAllNeedsLayout (root); + + view.SetNeedsLayout (); + + Assert.True (view.NeedsLayout); + Assert.True (view.Border.View!.NeedsLayout, "Adornment view must be marked when its parent is the changed view."); + Assert.True (borderSubView.NeedsLayout, "Adornment subview must be marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_From_AdornmentSubView_Reaches_AdornmentParent () + { + View root = new () { Id = "root" }; + View view = new () { Id = "view" }; + View borderSubView = new () { Id = "borderSubView" }; + view.Border.GetOrCreateView ().Add (borderSubView); + root.Add (view); + + ClearAllNeedsLayout (root); + + borderSubView.SetNeedsLayout (); + + Assert.True (borderSubView.NeedsLayout); + Assert.True (view.Border.View!.NeedsLayout, "AdornmentView must be marked as ancestor."); + Assert.True (view.NeedsLayout, "AdornmentView.Parent must be marked."); + Assert.True (root.NeedsLayout, "Adornment parent's SuperView must be marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_On_AdornmentView_Directly_Reaches_AdornmentParent_Without_Marking_Siblings () + { + View root = new () { Id = "root" }; + View view = new () { Id = "view" }; + View sibling = new () { Id = "sibling" }; + View borderSubView = new () { Id = "borderSubView" }; + view.Border.GetOrCreateView ().Add (borderSubView); + root.Add (view, sibling); + + ClearAllNeedsLayout (root); + + view.Border.View!.SetNeedsLayout (); + + Assert.True (view.Border.View.NeedsLayout, "AdornmentView must be marked."); + Assert.True (borderSubView.NeedsLayout, "AdornmentView subtree must be marked."); + Assert.True (view.NeedsLayout, "Adornment parent must be marked."); + Assert.True (root.NeedsLayout, "Adornment parent's SuperView must be marked."); + Assert.False (sibling.NeedsLayout, "Sibling subtree must not be marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_Tabs_Active_Scroll_Does_Not_Mark_Inactive_Pages () + { + View root = new () { Id = "root", Width = 60, Height = 20 }; + Tabs tabs = new () { Width = Dim.Fill (), Height = Dim.Fill () }; + root.Add (tabs); + + View page1 = new () { Id = "page1", Title = "Tab1", Width = Dim.Fill (), Height = Dim.Fill () }; + View page1Child = new () { Id = "page1Child" }; + page1.Add (page1Child); + + View page2 = new () { Id = "page2", Title = "Tab2", Width = Dim.Fill (), Height = Dim.Fill () }; + View page2Child = new () { Id = "page2Child" }; + page2.Add (page2Child); + + View page3 = new () { Id = "page3", Title = "Tab3", Width = Dim.Fill (), Height = Dim.Fill () }; + View page3Child = new () { Id = "page3Child" }; + page3.Add (page3Child); + + tabs.Add (page1, page2, page3); + root.Layout (); + + ClearAllNeedsLayout (root); + + page1.SetNeedsLayout (); + + Assert.True (page1.NeedsLayout); + Assert.True (page1Child.NeedsLayout); + Assert.True (tabs.NeedsLayout); + Assert.True (root.NeedsLayout); + + Assert.False (page2.NeedsLayout, "Inactive tab page must not be re-marked."); + Assert.False (page2Child.NeedsLayout, "Inactive tab page subtree must not be re-marked."); + Assert.False (page3.NeedsLayout, "Inactive tab page must not be re-marked."); + Assert.False (page3Child.NeedsLayout, "Inactive tab page subtree must not be re-marked."); + + root.Dispose (); + } + + [Fact] + public void SetNeedsLayout_Tabs_Active_Scroll_Does_Not_Redraw_Inactive_Pages_After_Layout () + { + View root = new () { Id = "root", Width = 60, Height = 20 }; + Tabs tabs = new () { Width = Dim.Fill (), Height = Dim.Fill () }; + root.Add (tabs); + + View page1 = new () { Id = "page1", Title = "Tab1", Width = Dim.Fill (), Height = Dim.Fill () }; + View page1Child = new () { Id = "page1Child", Width = 10, Height = 1 }; + page1.Add (page1Child); + + View page2 = new () { Id = "page2", Title = "Tab2", Width = Dim.Fill (), Height = Dim.Fill () }; + View page2Child = new () { Id = "page2Child", Width = 10, Height = 1 }; + page2.Add (page2Child); + + tabs.Add (page1, page2); + root.Layout (); + root.ClearNeedsDraw (); + ClearAllNeedsLayout (root); + + page1.SetNeedsLayout (); + root.Layout (); + + Assert.False (page2.NeedsDraw, "Inactive tab page must not be redrawn."); + Assert.False (page2Child.NeedsDraw, "Inactive tab page subtree must not be redrawn."); + + root.Dispose (); + } +} diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs new file mode 100644 index 0000000000..5792502d1e --- /dev/null +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -0,0 +1,565 @@ +using System.Text; +using UnitTests; + +namespace ViewsTests; + +// Copilot + +/// +/// Diagnostic tests for issue #5357 — tabbed redraw/layout fan-out. +/// +/// +/// +/// These tests are intentionally instrumentation-only: they observe layout and draw activity +/// on each tab via , , and +/// events, but do not change rendering or invalidation +/// semantics. They lock in the current behavior so that future refactors of the layout/draw +/// pipeline (see issue #4973) can be measured and verified. +/// +/// +/// The diagnostics rely on counters/event traces — not on Driver.Contents alone — +/// so they remain meaningful in the presence of clipping, transparent viewports, +/// shadow margins, and adornment subviews. +/// +/// +/// is used as the scrollable tab content because the older +/// TextView is being deprecated. The fan-out behavior lives in the +/// layout/draw pipeline and is independent of which content widget is used, so any view +/// that scrolls inside an Overlapped-arrangement exercises the same +/// code paths. +/// +/// +/// Each test reports its measured counts through so the data +/// is visible in CI logs even when the test passes. When the fan-out problem is fixed, these +/// tests are expected to be updated to assert the new (lower) counts; the diagnostic helper +/// and the test scaffolding remain useful as a regression check. +/// +/// +public class TabsFanOutDiagnosticTests (ITestOutputHelper output) : TestDriverBase +{ + /// + /// Records per-view layout and draw activity for a set of tracked views. Used as the primary + /// diagnostic for tab fan-out — the counts here are deterministic and survive future changes + /// to clipping, shadow, transparency, and adornment-subview behavior. + /// + private sealed class ViewActivityCounters + { + private readonly Dictionary _counts = new (); + private readonly List<(View View, string Label)> _order = []; + + public void Track (View view, string label) + { + Counts counts = new (); + _counts [view] = counts; + _order.Add ((view, label)); + + view.SubViewsLaidOut += (_, _) => counts.SubViewsLaidOut++; + view.DrawComplete += (_, _) => counts.DrawComplete++; + view.ClearedViewport += (_, _) => counts.ClearedViewport++; + view.DrawingContent += (_, _) => counts.DrawingContent++; + view.FrameChanged += (_, _) => counts.FrameChanged++; + } + + public Counts Get (View view) => _counts [view]; + + public void Reset () + { + foreach (Counts counts in _counts.Values) + { + counts.Reset (); + } + } + + public string Report (string title) + { + StringBuilder sb = new (); + sb.AppendLine (title); + sb.AppendLine (" view laidOut drawComplete clearedViewport drawingContent 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}"); + } + + return sb.ToString (); + } + + public sealed class Counts + { + public int SubViewsLaidOut; + public int DrawComplete; + public int ClearedViewport; + public int DrawingContent; + public int FrameChanged; + + public void Reset () + { + SubViewsLaidOut = 0; + DrawComplete = 0; + ClearedViewport = 0; + DrawingContent = 0; + FrameChanged = 0; + } + } + } + + private const int TabCount = 4; + private const int DriverWidth = 60; + private const int DriverHeight = 20; + + private static string MakeText (string prefix, int lines) + { + StringBuilder sb = new (); + + for (var i = 1; i <= lines; i++) + { + sb.Append (prefix); + sb.Append (' '); + sb.Append (i); + + if (i < lines) + { + sb.Append ('\n'); + } + } + + return sb.ToString (); + } + + private static Code MakeCode (string title, string text) => + new () + { + Title = title, + Text = text, + Language = null, + SyntaxHighlighter = null, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + private static (View Root, Tabs Tabs, Code [] Codes, ViewActivityCounters Counters) BuildTabbedScenario (IDriver driver) + { + View root = new () + { + Driver = driver, + CanFocus = true, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + Tabs tabs = new () { Driver = driver, Width = Dim.Fill (), Height = Dim.Fill () }; + root.Add (tabs); + + Code [] codes = new Code [TabCount]; + + for (var i = 0; i < TabCount; i++) + { + codes [i] = MakeCode ($"Tab{i + 1}", MakeText ($"Tab{i + 1} line", 50)); + tabs.Add (codes [i]); + } + + ViewActivityCounters counters = new (); + counters.Track (tabs, "Tabs"); + + for (var i = 0; i < TabCount; i++) + { + counters.Track (codes [i], $"Code{i + 1}"); + } + + return (root, tabs, codes, counters); + } + + private static (View Root, Code Code, ViewActivityCounters Counters) BuildSingleScenario (IDriver driver) + { + View root = new () + { + Driver = driver, + CanFocus = true, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + Code code = MakeCode ("Only", MakeText ("Only line", 50)); + root.Add (code); + + ViewActivityCounters counters = new (); + counters.Track (code, "Code"); + + return (root, code, counters); + } + + /// + /// Issue #5357 acceptance criterion: layout work stays on the active tab when the active tab scrolls. + /// + [Fact] + public void Diagnostic_ActiveTabScroll_LayoutEvents_StayOnActiveTab () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + Assert.Same (codes [0], active); + + counters.Reset (); + + for (var y = 1; y <= 5; y++) + { + active.Viewport = active.Viewport with { Y = y }; + root.Layout (); + } + + output.WriteLine (counters.Report ("After 5 active-tab scrolls (layout-only):")); + + ViewActivityCounters.Counts activeCounts = counters.Get (active); + Assert.True (activeCounts.SubViewsLaidOut > 0, $"Active tab should receive layout activity, got {activeCounts.SubViewsLaidOut}"); + + int inactiveLayouts = 0; + + for (var i = 1; i < TabCount; i++) + { + inactiveLayouts += counters.Get (codes [i]).SubViewsLaidOut; + } + + output.WriteLine ($"Active SubViewsLaidOut: {activeCounts.SubViewsLaidOut}"); + output.WriteLine ($"Sum of inactive SubViewsLaidOut: {inactiveLayouts}"); + + Assert.Equal ( + 0, + inactiveLayouts); + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Issue #5356 / #4973 acceptance criterion: diagnostics can show whether inactive tabs + /// received draw work when the active tab scrolls. + /// + [Fact] + public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + + counters.Reset (); + + for (var y = 1; y <= 5; y++) + { + active.Viewport = active.Viewport with { Y = y }; + root.Layout (); + root.Draw (); + } + + output.WriteLine (counters.Report ("After 5 active-tab scrolls (layout + draw):")); + + ViewActivityCounters.Counts activeCounts = counters.Get (active); + Assert.True (activeCounts.DrawComplete > 0, $"Active tab must receive draw activity, got {activeCounts.DrawComplete}"); + + int inactiveDraws = 0; + int inactiveClears = 0; + int inactiveContentDraws = 0; + + for (var i = 1; i < TabCount; i++) + { + ViewActivityCounters.Counts c = counters.Get (codes [i]); + inactiveDraws += c.DrawComplete; + inactiveClears += c.ClearedViewport; + inactiveContentDraws += c.DrawingContent; + } + + 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. + 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."); + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Issue #5356 acceptance criterion: a comparable fan-out metric between an equivalent + /// single-Code scenario and a tabbed-Code scenario. + /// + [Fact] + public void Diagnostic_TabbedFanOut_ComparedTo_SingleViewBaseline () + { + IDriver driverSingle = CreateTestDriver (DriverWidth, DriverHeight); + (View singleRoot, Code single, ViewActivityCounters singleCounters) = BuildSingleScenario (driverSingle); + + singleRoot.Layout (); + singleRoot.Draw (); + singleCounters.Reset (); + + for (var y = 1; y <= 5; y++) + { + single.Viewport = single.Viewport with { Y = y }; + singleRoot.Layout (); + singleRoot.Draw (); + } + + ViewActivityCounters.Counts singleCounts = singleCounters.Get (single); + output.WriteLine (singleCounters.Report ("Single-Code baseline (5 scrolls):")); + + IDriver driverTabbed = CreateTestDriver (DriverWidth, DriverHeight); + (View tabRoot, Tabs tabs, Code [] codes, ViewActivityCounters tabCounters) = BuildTabbedScenario (driverTabbed); + + tabRoot.Layout (); + tabRoot.Draw (); + + Code active = (Code)tabs.Value!; + tabCounters.Reset (); + + for (var y = 1; y <= 5; y++) + { + active.Viewport = active.Viewport with { Y = y }; + tabRoot.Layout (); + tabRoot.Draw (); + } + + ViewActivityCounters.Counts tabActiveCounts = tabCounters.Get (active); + output.WriteLine (tabCounters.Report ("Tabbed scenario (5 scrolls of active tab):")); + + int totalTabDraws = tabActiveCounts.DrawComplete; + int totalTabLayouts = tabActiveCounts.SubViewsLaidOut; + + for (var i = 1; i < TabCount; i++) + { + totalTabDraws += tabCounters.Get (codes [i]).DrawComplete; + totalTabLayouts += tabCounters.Get (codes [i]).SubViewsLaidOut; + } + + double drawFanOut = singleCounts.DrawComplete == 0 ? double.NaN : (double)totalTabDraws / singleCounts.DrawComplete; + 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}"); + + 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."); + + // 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."); + + Assert.Equal ( + 1.0, + layoutFanOut); + + singleRoot.Dispose (); + tabRoot.Dispose (); + driverSingle.Dispose (); + driverTabbed.Dispose (); + } + + /// + /// Edge case: transparent inactive tabs must not silently bypass the diagnostic. + /// changes the clear/draw sequence in the inner + /// view's draw path but it does not eliminate firing on the + /// view itself. The diagnostic must still observe activity on transparent tabs. + /// + [Fact] + public void Diagnostic_TransparentInactiveTab_StillObservable () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + codes [1].ViewportSettings |= ViewportSettingsFlags.Transparent; + codes [2].ViewportSettings |= ViewportSettingsFlags.Transparent; + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + counters.Reset (); + + for (var y = 1; y <= 3; y++) + { + active.Viewport = active.Viewport with { Y = y }; + root.Layout (); + root.Draw (); + } + + output.WriteLine (counters.Report ("Transparent inactive tabs (3 active-tab scrolls):")); + + ViewActivityCounters.Counts activeCounts = counters.Get (active); + Assert.True (activeCounts.DrawComplete > 0, "Active tab still draws when peers are transparent."); + + for (var i = 1; i < TabCount; i++) + { + 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."); + } + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Edge case: shadow margins draw in a separate pass (). + /// The diagnostic must observe the active tab's draw activity even when its margin has a shadow. + /// + [Fact] + public void Diagnostic_ShadowMargin_DoesNotMaskActiveTabActivity () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + codes [0].ShadowStyle = ShadowStyles.Opaque; + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + Assert.Same (codes [0], active); + + counters.Reset (); + + for (var y = 1; y <= 3; y++) + { + active.Viewport = active.Viewport with { Y = y }; + root.Layout (); + root.Draw (); + } + + output.WriteLine (counters.Report ("Active tab with shadow margin (3 scrolls):")); + + ViewActivityCounters.Counts activeCounts = counters.Get (active); + Assert.True (activeCounts.DrawComplete > 0, $"Active tab with shadow must still record DrawComplete (got {activeCounts.DrawComplete})."); + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Acceptance criterion: assertions must not depend solely on Driver.Contents. + /// This test confirms the diagnostic can also reach the actual layer — + /// returns the ANSI bytes that would be written to the + /// terminal, which is the source of truth beyond the in-memory Driver.Contents grid. + /// + [Fact] + public void Diagnostic_IOutputLayer_ObservesActiveTabContent () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + + IOutput output1 = driver.GetOutput (); + IOutputBuffer buffer = driver.GetOutputBuffer (); + output1.Write (buffer); + string ansiAfterInitial = output1.GetLastOutput (); + + output.WriteLine ($"Initial GetLastOutput length: {ansiAfterInitial.Length}"); + Assert.False (string.IsNullOrEmpty (ansiAfterInitial), "IOutput must produce non-empty output for initial draw."); + + counters.Reset (); + + active.Viewport = active.Viewport with { Y = 10 }; + root.Layout (); + root.Draw (); + + output1.Write (buffer); + string ansiAfterScroll = output1.GetLastOutput (); + + output.WriteLine ($"After-scroll GetLastOutput length: {ansiAfterScroll.Length}"); + Assert.False (string.IsNullOrEmpty (ansiAfterScroll), "IOutput must produce non-empty output after a scroll."); + + ViewActivityCounters.Counts activeCounts = counters.Get (active); + Assert.True (activeCounts.DrawComplete > 0, "Active tab must draw for IOutput to receive content."); + + Assert.Contains ($"Tab{1} line", ansiAfterInitial); + + output.WriteLine (counters.Report ("IOutput-level check (1 scroll):")); + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Diagnostic reports layout and draw fan-out as separate metrics, so a regression can be + /// localized to one pipeline (layout vs draw) without conflating the two. + /// + [Fact] + public void Diagnostic_LayoutAndDraw_ReportedSeparately () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + Code active = (Code)tabs.Value!; + counters.Reset (); + + active.Viewport = active.Viewport with { Y = 2 }; + root.Layout (); + root.Draw (); + + int activeLayouts = counters.Get (active).SubViewsLaidOut; + int activeDraws = counters.Get (active).DrawComplete; + + var inactiveLayouts = 0; + var inactiveDraws = 0; + + for (var i = 1; i < TabCount; i++) + { + inactiveLayouts += counters.Get (codes [i]).SubViewsLaidOut; + inactiveDraws += counters.Get (codes [i]).DrawComplete; + } + + output.WriteLine ($"Layout: active={activeLayouts}, inactive_total={inactiveLayouts}"); + output.WriteLine ($"Draw: active={activeDraws}, inactive_total={inactiveDraws}"); + output.WriteLine (counters.Report ("Single-scroll layout-vs-draw breakdown:")); + + Assert.True (activeLayouts > 0, $"Active tab must register layout activity, got {activeLayouts}."); + Assert.True (activeDraws > 0, $"Active tab must register draw activity, got {activeDraws}."); + + // The two counters can move independently — RC2 in #4973 affects layout fan-out only, + // RC1 affects draw fan-out only. Reporting them separately lets a regression land in just + // one pipeline without being masked by the other. + output.WriteLine ($"layout_fanout_ratio = {(activeLayouts == 0 ? "n/a" : ((double)inactiveLayouts / activeLayouts).ToString ("F2"))}"); + output.WriteLine ($"draw_fanout_ratio = {(activeDraws == 0 ? "n/a" : ((double)inactiveDraws / activeDraws).ToString ("F2"))}"); + + root.Dispose (); + driver.Dispose (); + } +}