From 78c76de8a6f92c1891534dc8495f1ae071007d6e Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Thu, 21 May 2026 09:19:32 -0500 Subject: [PATCH 01/14] Fixes #5356. Add tab fan-out layout/draw diagnostic tests Adds TabsFanOutDiagnosticTests with seven parallelizable tests that observe SubViewsLaidOut, DrawComplete, ClearedViewport, DrawingContent, and FrameChanged events per tab to measure layout/draw fan-out when an active TextView inside Tabs scrolls. Captures the current #4973 behavior so that future fixes can be verified and silent regressions caught: - Active tab vs. inactive tab layout work - Active tab vs. inactive tab draw work (DrawComplete, ClearedViewport, DrawingContent) - A comparable single-TextView baseline vs. tabbed scenario fan-out ratio - ViewportSettings.Transparent does not mask the diagnostic - Shadow margin does not mask active-tab activity - IOutput-level output (not Driver.Contents) is observed - Layout and draw counters reported separately so regressions can be localized to one pipeline The tests are instrumentation-only and do not change rendering or invalidation semantics. --- .../TabView/TabsFanOutDiagnosticTests.cs | 566 ++++++++++++++++++ 1 file changed, 566 insertions(+) create mode 100644 Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs new file mode 100644 index 0000000000..af1549dfd6 --- /dev/null +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -0,0 +1,566 @@ +using System.Text; +using UnitTests; + +namespace ViewsTests; + +// Claude - Opus 4.7 + +/// +/// Diagnostic tests for issue #5356 — 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. +/// +/// +/// 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 (View Root, Tabs Tabs, TextView [] TextViews, 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); + + TextView [] textViews = new TextView [TabCount]; + + for (var i = 0; i < TabCount; i++) + { + TextView tv = new () + { + Title = $"Tab{i + 1}", + Text = MakeText ($"Tab{i + 1} line", 50), + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + textViews [i] = tv; + tabs.Add (tv); + } + + ViewActivityCounters counters = new (); + counters.Track (tabs, "Tabs"); + + for (var i = 0; i < TabCount; i++) + { + counters.Track (textViews [i], $"TextView{i + 1}"); + } + + return (root, tabs, textViews, counters); + } + + private static (View Root, TextView TextView, ViewActivityCounters Counters) BuildSingleTextViewScenario (IDriver driver) + { + View root = new () + { + Driver = driver, + CanFocus = true, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + TextView tv = new () + { + Title = "Only", + Text = MakeText ("Only line", 50), + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + root.Add (tv); + + ViewActivityCounters counters = new (); + counters.Track (tv, "TextView"); + + return (root, tv, counters); + } + + /// + /// Issue #5356 / #4973 acceptance criterion: diagnostics can show whether inactive tabs + /// received layout work when the active tab scrolls. + /// + [Fact] + public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () + { + IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); + (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + TextView active = (TextView)tabs.Value!; + Assert.Same (textViews [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 (textViews [i]).SubViewsLaidOut; + } + + output.WriteLine ($"Active SubViewsLaidOut: {activeCounts.SubViewsLaidOut}"); + output.WriteLine ($"Sum of inactive SubViewsLaidOut: {inactiveLayouts}"); + + // CURRENT BEHAVIOR (issue #4973): inactive tabs receive the same layout count as active. + // After #4973 is fixed, flip this to `Assert.Equal (0, inactiveLayouts)`. + Assert.True ( + inactiveLayouts > 0, + $"Documents issue #4973: inactive tabs receive layout work when active tab scrolls. " + + $"Observed inactive_total={inactiveLayouts}, active={activeCounts.SubViewsLaidOut}. " + + "Flip to Assert.Equal(0, inactiveLayouts) after #4973 fix lands."); + + 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, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + TextView active = (TextView)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 (textViews [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 draw, clear, and content-draw work + // when the active tab scrolls. After #4973 is fixed, flip these to `Assert.Equal (0, ...)`. + 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."); + + Assert.True ( + inactiveClears > 0, + $"Documents issue #4973 clear fan-out: ClearViewport propagates via SetNeedsDraw. " + + $"inactive_total ClearedViewport={inactiveClears}. Flip after #4973 fix."); + + Assert.True ( + inactiveContentDraws > 0, + $"Documents issue #4973 content-draw fan-out: inactive_total DrawingContent={inactiveContentDraws}. " + + "Flip after #4973 fix."); + + root.Dispose (); + driver.Dispose (); + } + + /// + /// Issue #5356 acceptance criterion: a comparable fan-out metric between an equivalent + /// single-TextView scenario and a tabbed-TextView scenario. + /// + [Fact] + public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () + { + IDriver driverSingle = CreateTestDriver (DriverWidth, DriverHeight); + (View singleRoot, TextView singleTv, ViewActivityCounters singleCounters) = BuildSingleTextViewScenario (driverSingle); + + singleRoot.Layout (); + singleRoot.Draw (); + singleCounters.Reset (); + + for (var y = 1; y <= 5; y++) + { + singleTv.Viewport = singleTv.Viewport with { Y = y }; + singleRoot.Layout (); + singleRoot.Draw (); + } + + ViewActivityCounters.Counts singleCounts = singleCounters.Get (singleTv); + output.WriteLine (singleCounters.Report ("Single-TextView baseline (5 scrolls):")); + + IDriver driverTabbed = CreateTestDriver (DriverWidth, DriverHeight); + (View tabRoot, Tabs tabs, TextView [] textViews, ViewActivityCounters tabCounters) = BuildTabbedScenario (driverTabbed); + + tabRoot.Layout (); + tabRoot.Draw (); + + TextView active = (TextView)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 (textViews [i]).DrawComplete; + totalTabLayouts += tabCounters.Get (textViews [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-TextView baseline must record draw activity."); + Assert.True (tabActiveCounts.DrawComplete > 0, "Active tab in tabbed scenario must record draw activity."); + + // CURRENT BEHAVIOR (issue #4973): tabbed scenario produces N-times more total work than baseline. + // After #4973 is fixed, drawFanOut and layoutFanOut should approach 1.0 (within rounding). + Assert.True ( + drawFanOut > 1.0, + $"Documents issue #4973: tabbed draw fan-out ({drawFanOut:F2}x) exceeds single-TextView baseline. " + + "After fix, drawFanOut should approach 1.0."); + + Assert.True ( + layoutFanOut > 1.0, + $"Documents issue #4973: tabbed layout fan-out ({layoutFanOut:F2}x) exceeds single-TextView baseline. " + + "After fix, layoutFanOut should approach 1.0."); + + 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, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + textViews [1].ViewportSettings |= ViewportSettingsFlags.Transparent; + textViews [2].ViewportSettings |= ViewportSettingsFlags.Transparent; + + root.Layout (); + root.Draw (); + + TextView active = (TextView)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 (textViews [i]); + + Assert.True ( + c.DrawComplete >= 0, + $"Tab {i + 1} DrawComplete counter must be observable (got {c.DrawComplete}) even with Transparent."); + } + + 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, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + textViews [0].ShadowStyle = ShadowStyles.Opaque; + + root.Layout (); + root.Draw (); + + TextView active = (TextView)tabs.Value!; + Assert.Same (textViews [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, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + TextView active = (TextView)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, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + + root.Layout (); + root.Draw (); + + TextView active = (TextView)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 (textViews [i]).SubViewsLaidOut; + inactiveDraws += counters.Get (textViews [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 (); + } +} From 60b845908f201c6274e03d3c41bda7ae94048c07 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Thu, 21 May 2026 09:45:25 -0500 Subject: [PATCH 02/14] Swap TextView for Code; add integration test for fan-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TextView is being deprecated in favor of Code (read-only) and Editor (editable). The fan-out behavior lives in the View base class layout/draw pipeline, so any scrollable view inside Tabs reproduces it — swap to Code so the diagnostic survives the deprecation. Adjusts assertions to use only widget-agnostic signals (DrawComplete on each tab, ClearedViewport on the Tabs container). Code overrides OnClearingViewport/OnDrawingContent and returns true, which suppresses the ClearedViewport/DrawingContent events on the Code instances. The per-Code data is still reported but no longer asserted at that level. Adds Tests/IntegrationTests/TabsFanOutIntegrationTests.cs that drives the active tab via a real Key.PageDown through the input processor → command dispatch → main-loop LayoutAndDraw path (rather than mutating Viewport directly). Runs against all registered drivers. Confirms the fan-out is observable end-to-end: Driver: Active tab Viewport.Y after 3 PageDowns: 3 Tabs 3 3 3 Code1 (active) 3 3 0 Code2-4 (inact) 3 3 0 Sum inactive DrawComplete = 9 --- .../TabsFanOutIntegrationTests.cs | 179 ++++++++++++++++++ .../TabView/TabsFanOutDiagnosticTests.cs | 147 +++++++------- 2 files changed, 255 insertions(+), 71 deletions(-) create mode 100644 Tests/IntegrationTests/TabsFanOutIntegrationTests.cs diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs new file mode 100644 index 0000000000..308d30ab67 --- /dev/null +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -0,0 +1,179 @@ +using System.Text; +using AppTestHelpers; + +namespace IntegrationTests; + +// Claude - Opus 4.7 + +/// +/// 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 / #5356 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 fan-out check: a real on the active tab causes + /// layout/draw activity on inactive tabs. + /// + [Theory] + [MemberData (nameof (GetAllDriverNames))] + public void Integration_RealPageDown_OnActiveTab_FansOutToInactiveTabs (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 (issue #4973): inactive tabs receive draw and layout work when active scrolls, + // even through the real input → command → main-loop path. After #4973 lands, flip these to == 0. + Assert.True ( + inactiveDraws > 0, + $"Documents issue #4973 (integration-level): inactive_total DrawComplete={inactiveDraws}. " + + "Flip to Assert.Equal(0, inactiveDraws) after fix lands."); + + Assert.True ( + inactiveLayouts > 0, + $"Documents issue #4973 (integration-level): inactive_total SubViewsLaidOut={inactiveLayouts}. " + + "Flip to Assert.Equal(0, inactiveLayouts) after fix lands."); + } +} diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs index af1549dfd6..acca893ab7 100644 --- a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -22,6 +22,13 @@ namespace ViewsTests; /// 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 @@ -122,7 +129,18 @@ private static string MakeText (string prefix, int lines) return sb.ToString (); } - private static (View Root, Tabs Tabs, TextView [] TextViews, ViewActivityCounters Counters) BuildTabbedScenario (IDriver driver) + 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 () { @@ -135,20 +153,12 @@ private static (View Root, Tabs Tabs, TextView [] TextViews, ViewActivityCounter Tabs tabs = new () { Driver = driver, Width = Dim.Fill (), Height = Dim.Fill () }; root.Add (tabs); - TextView [] textViews = new TextView [TabCount]; + Code [] codes = new Code [TabCount]; for (var i = 0; i < TabCount; i++) { - TextView tv = new () - { - Title = $"Tab{i + 1}", - Text = MakeText ($"Tab{i + 1} line", 50), - Width = Dim.Fill (), - Height = Dim.Fill () - }; - - textViews [i] = tv; - tabs.Add (tv); + codes [i] = MakeCode ($"Tab{i + 1}", MakeText ($"Tab{i + 1} line", 50)); + tabs.Add (codes [i]); } ViewActivityCounters counters = new (); @@ -156,13 +166,13 @@ private static (View Root, Tabs Tabs, TextView [] TextViews, ViewActivityCounter for (var i = 0; i < TabCount; i++) { - counters.Track (textViews [i], $"TextView{i + 1}"); + counters.Track (codes [i], $"Code{i + 1}"); } - return (root, tabs, textViews, counters); + return (root, tabs, codes, counters); } - private static (View Root, TextView TextView, ViewActivityCounters Counters) BuildSingleTextViewScenario (IDriver driver) + private static (View Root, Code Code, ViewActivityCounters Counters) BuildSingleScenario (IDriver driver) { View root = new () { @@ -172,20 +182,13 @@ private static (View Root, TextView TextView, ViewActivityCounters Counters) Bui Height = Dim.Fill () }; - TextView tv = new () - { - Title = "Only", - Text = MakeText ("Only line", 50), - Width = Dim.Fill (), - Height = Dim.Fill () - }; - - root.Add (tv); + Code code = MakeCode ("Only", MakeText ("Only line", 50)); + root.Add (code); ViewActivityCounters counters = new (); - counters.Track (tv, "TextView"); + counters.Track (code, "Code"); - return (root, tv, counters); + return (root, code, counters); } /// @@ -196,13 +199,13 @@ private static (View Root, TextView TextView, ViewActivityCounters Counters) Bui public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; - Assert.Same (textViews [0], active); + Code active = (Code)tabs.Value!; + Assert.Same (codes [0], active); counters.Reset (); @@ -221,7 +224,7 @@ public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () for (var i = 1; i < TabCount; i++) { - inactiveLayouts += counters.Get (textViews [i]).SubViewsLaidOut; + inactiveLayouts += counters.Get (codes [i]).SubViewsLaidOut; } output.WriteLine ($"Active SubViewsLaidOut: {activeCounts.SubViewsLaidOut}"); @@ -247,12 +250,12 @@ public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; + Code active = (Code)tabs.Value!; counters.Reset (); @@ -274,7 +277,7 @@ public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () for (var i = 1; i < TabCount; i++) { - ViewActivityCounters.Counts c = counters.Get (textViews [i]); + ViewActivityCounters.Counts c = counters.Get (codes [i]); inactiveDraws += c.DrawComplete; inactiveClears += c.ClearedViewport; inactiveContentDraws += c.DrawingContent; @@ -285,22 +288,24 @@ public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () output.WriteLine ($"Sum of inactive ClearedViewport: {inactiveClears}"); output.WriteLine ($"Sum of inactive DrawingContent: {inactiveContentDraws}"); - // CURRENT BEHAVIOR (issue #4973): inactive tabs receive draw, clear, and content-draw work - // when the active tab scrolls. After #4973 is fixed, flip these to `Assert.Equal (0, ...)`. + // 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 ( - inactiveClears > 0, - $"Documents issue #4973 clear fan-out: ClearViewport propagates via SetNeedsDraw. " + - $"inactive_total ClearedViewport={inactiveClears}. Flip after #4973 fix."); - - Assert.True ( - inactiveContentDraws > 0, - $"Documents issue #4973 content-draw fan-out: inactive_total DrawingContent={inactiveContentDraws}. " + - "Flip after #4973 fix."); + 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 (); @@ -308,13 +313,13 @@ public void Diagnostic_ActiveTabScroll_DrawEvents_OnEachTab () /// /// Issue #5356 acceptance criterion: a comparable fan-out metric between an equivalent - /// single-TextView scenario and a tabbed-TextView scenario. + /// single-Code scenario and a tabbed-Code scenario. /// [Fact] - public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () + public void Diagnostic_TabbedFanOut_ComparedTo_SingleViewBaseline () { IDriver driverSingle = CreateTestDriver (DriverWidth, DriverHeight); - (View singleRoot, TextView singleTv, ViewActivityCounters singleCounters) = BuildSingleTextViewScenario (driverSingle); + (View singleRoot, Code single, ViewActivityCounters singleCounters) = BuildSingleScenario (driverSingle); singleRoot.Layout (); singleRoot.Draw (); @@ -322,21 +327,21 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () for (var y = 1; y <= 5; y++) { - singleTv.Viewport = singleTv.Viewport with { Y = y }; + single.Viewport = single.Viewport with { Y = y }; singleRoot.Layout (); singleRoot.Draw (); } - ViewActivityCounters.Counts singleCounts = singleCounters.Get (singleTv); - output.WriteLine (singleCounters.Report ("Single-TextView baseline (5 scrolls):")); + ViewActivityCounters.Counts singleCounts = singleCounters.Get (single); + output.WriteLine (singleCounters.Report ("Single-Code baseline (5 scrolls):")); IDriver driverTabbed = CreateTestDriver (DriverWidth, DriverHeight); - (View tabRoot, Tabs tabs, TextView [] textViews, ViewActivityCounters tabCounters) = BuildTabbedScenario (driverTabbed); + (View tabRoot, Tabs tabs, Code [] codes, ViewActivityCounters tabCounters) = BuildTabbedScenario (driverTabbed); tabRoot.Layout (); tabRoot.Draw (); - TextView active = (TextView)tabs.Value!; + Code active = (Code)tabs.Value!; tabCounters.Reset (); for (var y = 1; y <= 5; y++) @@ -354,8 +359,8 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () for (var i = 1; i < TabCount; i++) { - totalTabDraws += tabCounters.Get (textViews [i]).DrawComplete; - totalTabLayouts += tabCounters.Get (textViews [i]).SubViewsLaidOut; + totalTabDraws += tabCounters.Get (codes [i]).DrawComplete; + totalTabLayouts += tabCounters.Get (codes [i]).SubViewsLaidOut; } double drawFanOut = singleCounts.DrawComplete == 0 ? double.NaN : (double)totalTabDraws / singleCounts.DrawComplete; @@ -364,19 +369,19 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () 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-TextView baseline must record draw activity."); + 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 (issue #4973): tabbed scenario produces N-times more total work than baseline. // After #4973 is fixed, drawFanOut and layoutFanOut should approach 1.0 (within rounding). Assert.True ( drawFanOut > 1.0, - $"Documents issue #4973: tabbed draw fan-out ({drawFanOut:F2}x) exceeds single-TextView baseline. " + + $"Documents issue #4973: tabbed draw fan-out ({drawFanOut:F2}x) exceeds single-Code baseline. " + "After fix, drawFanOut should approach 1.0."); Assert.True ( layoutFanOut > 1.0, - $"Documents issue #4973: tabbed layout fan-out ({layoutFanOut:F2}x) exceeds single-TextView baseline. " + + $"Documents issue #4973: tabbed layout fan-out ({layoutFanOut:F2}x) exceeds single-Code baseline. " + "After fix, layoutFanOut should approach 1.0."); singleRoot.Dispose (); @@ -395,15 +400,15 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleTextViewBaseline () public void Diagnostic_TransparentInactiveTab_StillObservable () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); - textViews [1].ViewportSettings |= ViewportSettingsFlags.Transparent; - textViews [2].ViewportSettings |= ViewportSettingsFlags.Transparent; + codes [1].ViewportSettings |= ViewportSettingsFlags.Transparent; + codes [2].ViewportSettings |= ViewportSettingsFlags.Transparent; root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; + Code active = (Code)tabs.Value!; counters.Reset (); for (var y = 1; y <= 3; y++) @@ -420,7 +425,7 @@ public void Diagnostic_TransparentInactiveTab_StillObservable () for (var i = 1; i < TabCount; i++) { - ViewActivityCounters.Counts c = counters.Get (textViews [i]); + ViewActivityCounters.Counts c = counters.Get (codes [i]); Assert.True ( c.DrawComplete >= 0, @@ -439,15 +444,15 @@ public void Diagnostic_TransparentInactiveTab_StillObservable () public void Diagnostic_ShadowMargin_DoesNotMaskActiveTabActivity () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); - textViews [0].ShadowStyle = ShadowStyles.Opaque; + codes [0].ShadowStyle = ShadowStyles.Opaque; root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; - Assert.Same (textViews [0], active); + Code active = (Code)tabs.Value!; + Assert.Same (codes [0], active); counters.Reset (); @@ -477,12 +482,12 @@ public void Diagnostic_ShadowMargin_DoesNotMaskActiveTabActivity () public void Diagnostic_IOutputLayer_ObservesActiveTabContent () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; + Code active = (Code)tabs.Value!; IOutput output1 = driver.GetOutput (); IOutputBuffer buffer = driver.GetOutputBuffer (); @@ -523,12 +528,12 @@ public void Diagnostic_IOutputLayer_ObservesActiveTabContent () public void Diagnostic_LayoutAndDraw_ReportedSeparately () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); - (View root, Tabs tabs, TextView [] textViews, ViewActivityCounters counters) = BuildTabbedScenario (driver); + (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); root.Layout (); root.Draw (); - TextView active = (TextView)tabs.Value!; + Code active = (Code)tabs.Value!; counters.Reset (); active.Viewport = active.Viewport with { Y = 2 }; @@ -543,8 +548,8 @@ public void Diagnostic_LayoutAndDraw_ReportedSeparately () for (var i = 1; i < TabCount; i++) { - inactiveLayouts += counters.Get (textViews [i]).SubViewsLaidOut; - inactiveDraws += counters.Get (textViews [i]).DrawComplete; + inactiveLayouts += counters.Get (codes [i]).SubViewsLaidOut; + inactiveDraws += counters.Get (codes [i]).DrawComplete; } output.WriteLine ($"Layout: active={activeLayouts}, inactive_total={inactiveLayouts}"); From 7e3ccb6413dda8c129fa8bce7215609858333bfa Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Thu, 21 May 2026 15:48:52 -0500 Subject: [PATCH 03/14] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../Views/TabView/TabsFanOutDiagnosticTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs index acca893ab7..3eec4ad54f 100644 --- a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -427,9 +427,10 @@ 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} DrawComplete counter must be observable (got {c.DrawComplete}) even with Transparent."); + c.DrawComplete > 0, + $"Tab {i + 1} should currently still record DrawComplete even with Transparent (got {c.DrawComplete}). Update this expectation after #4973."); } root.Dispose (); From 09017a999fa7bc1ec109363270fe856f93b69f51 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 00:20:13 -0500 Subject: [PATCH 04/14] Add tests for #5357 ancestor-only layout propagation Pin the new SetNeedsLayout contract: changed view + own subtree + adornment subviews + ancestors are marked, but unaffected sibling subtrees stay clean. Three tests currently fail and will pass after the split lands. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Layout/SetNeedsLayoutPropagationTests.cs | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs new file mode 100644 index 0000000000..e627b1070e --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -0,0 +1,226 @@ +namespace ViewBaseTests.Layout; + +// Claude - Opus 4.7 + +/// +/// 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_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_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 (); + } +} From cd8c2f0c73bce7ca96f4ebcc18e4a828c6fb78d2 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 00:20:26 -0500 Subject: [PATCH 05/14] Fixes #5357. Split upward layout propagation from downward subtree invalidation SetNeedsLayout previously did downward subtree marking AND upward propagation via SuperView?.SetNeedsLayout, which re-entered the same downward cascade and re-marked every sibling subtree. In overlapping Tabs this meant activity in the active tab page dirtied every inactive page. Extract the downward subtree pass into MarkSubtreeNeedsLayout, the adornment subview helper into MarkAdornmentSubViewTrees, and the upward walk into MarkAncestorsNeedLayout. The ancestor walk marks each ancestor (and its adornment subview trees) without descending into the ancestor's regular SubViews tree. Sibling-dependent layout still works because TopologicalSort in LayoutSubViews already calls Layout on every child in dependency order regardless of NeedsLayout, so siblings that actually reference the changed view via Pos/Dim still recompute their frames. Co-Authored-By: Claude Opus 4.7 (1M context) --- Terminal.Gui/ViewBase/View.Layout.cs | 83 ++++++++++++++++++---------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index e6cb655723..b0a3cbb2b7 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -892,22 +892,28 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { - NeedsLayout = true; + MarkSubtreeNeedsLayout (); - if (Margin.View is { SubViews.Count: > 0 }) - { - Margin.View.SetNeedsLayout (); - } + TextFormatter.NeedsFormat = true; - if (Border.View is { SubViews.Count: > 0 }) + 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 +929,61 @@ 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 (Border.View is { SubViews.Count: > 0 }) + { + Border.View.MarkSubtreeNeedsLayout (); + } - if (SuperView is { NeedsLayout: false }) + if (Padding.View is { SubViews.Count: > 0 }) { - SuperView?.SetNeedsLayout (); + Padding.View.MarkSubtreeNeedsLayout (); } + } - if (this is not AdornmentView adornment) + // Walks up the ancestor chain marking each ancestor as needing layout, without descending + // into the ancestor's regular SubViews tree. Adornment subview trees on each ancestor are + // still marked so adornment content re-lays-out when its parent does. 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; + MarkAdornmentSubViewTrees (); + + 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 (); } } From 1fb219bfb3319dc5a8278fba89f8242acd941c6d Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 11:02:15 -0500 Subject: [PATCH 06/14] Fixes #5357. Avoid redraw fan-out from ancestor-only layout propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Layout.cs | 23 ++++++-- .../Layout/SetNeedsLayoutPropagationTests.cs | 52 +++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index b0a3cbb2b7..38808aba5f 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -558,15 +558,29 @@ internal static bool Layout (IEnumerable views, Size contentSize) /// If the view could not be laid out (typically because a dependencies was not ready). public bool Layout (Size contentSize) { + bool needsDrawAfterLayout = _needsDrawAfterLayout; + Rectangle originalFrame = Frame; + if (!SetRelativeLayout (contentSize)) { 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 (); + } + + if (!NeedsLayout) + { + _needsDrawAfterLayout = false; + } return true; } @@ -880,6 +894,8 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public bool NeedsLayout { get; internal set; } = true; + private bool _needsDrawAfterLayout = true; + /// /// Sets to return , indicating this View and all of it's subviews /// (including adornments) need to be laid out in the next Application iteration. @@ -892,6 +908,7 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { + _needsDrawAfterLayout = true; MarkSubtreeNeedsLayout (); TextFormatter.NeedsFormat = true; diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs index e627b1070e..c1b4e48e2b 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -185,6 +185,29 @@ public void SetNeedsLayout_From_AdornmentSubView_Reaches_AdornmentParent () 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 () { @@ -223,4 +246,33 @@ public void SetNeedsLayout_Tabs_Active_Scroll_Does_Not_Mark_Inactive_Pages () 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 (); + } } From 57c0f3e94145aaf2e624baaa6cef1bf715fa757a Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 00:20:13 -0500 Subject: [PATCH 07/14] Add tests for #5357 ancestor-only layout propagation Pin the new SetNeedsLayout contract: changed view + own subtree + adornment subviews + ancestors are marked, but unaffected sibling subtrees stay clean. Three tests currently fail and will pass after the split lands. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Layout/SetNeedsLayoutPropagationTests.cs | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs new file mode 100644 index 0000000000..e627b1070e --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -0,0 +1,226 @@ +namespace ViewBaseTests.Layout; + +// Claude - Opus 4.7 + +/// +/// 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_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_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 (); + } +} From 65154a49f88761d659140ecafb71531e1a589384 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 00:20:26 -0500 Subject: [PATCH 08/14] Fixes #5357. Split upward layout propagation from downward subtree invalidation SetNeedsLayout previously did downward subtree marking AND upward propagation via SuperView?.SetNeedsLayout, which re-entered the same downward cascade and re-marked every sibling subtree. In overlapping Tabs this meant activity in the active tab page dirtied every inactive page. Extract the downward subtree pass into MarkSubtreeNeedsLayout, the adornment subview helper into MarkAdornmentSubViewTrees, and the upward walk into MarkAncestorsNeedLayout. The ancestor walk marks each ancestor (and its adornment subview trees) without descending into the ancestor's regular SubViews tree. Sibling-dependent layout still works because TopologicalSort in LayoutSubViews already calls Layout on every child in dependency order regardless of NeedsLayout, so siblings that actually reference the changed view via Pos/Dim still recompute their frames. Co-Authored-By: Claude Opus 4.7 (1M context) --- Terminal.Gui/ViewBase/View.Layout.cs | 83 ++++++++++++++++++---------- 1 file changed, 53 insertions(+), 30 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index e6cb655723..b0a3cbb2b7 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -892,22 +892,28 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { - NeedsLayout = true; + MarkSubtreeNeedsLayout (); - if (Margin.View is { SubViews.Count: > 0 }) - { - Margin.View.SetNeedsLayout (); - } + TextFormatter.NeedsFormat = true; - if (Border.View is { SubViews.Count: > 0 }) + 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 +929,61 @@ 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 (Border.View is { SubViews.Count: > 0 }) + { + Border.View.MarkSubtreeNeedsLayout (); + } - if (SuperView is { NeedsLayout: false }) + if (Padding.View is { SubViews.Count: > 0 }) { - SuperView?.SetNeedsLayout (); + Padding.View.MarkSubtreeNeedsLayout (); } + } - if (this is not AdornmentView adornment) + // Walks up the ancestor chain marking each ancestor as needing layout, without descending + // into the ancestor's regular SubViews tree. Adornment subview trees on each ancestor are + // still marked so adornment content re-lays-out when its parent does. 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; + MarkAdornmentSubViewTrees (); + + 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 (); } } From 7be7ed0518bab5d069474312da38594d3abfc42c Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 11:02:15 -0500 Subject: [PATCH 09/14] Fixes #5357. Avoid redraw fan-out from ancestor-only layout propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Layout.cs | 23 ++++++-- .../Layout/SetNeedsLayoutPropagationTests.cs | 52 +++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index b0a3cbb2b7..38808aba5f 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -558,15 +558,29 @@ internal static bool Layout (IEnumerable views, Size contentSize) /// If the view could not be laid out (typically because a dependencies was not ready). public bool Layout (Size contentSize) { + bool needsDrawAfterLayout = _needsDrawAfterLayout; + Rectangle originalFrame = Frame; + if (!SetRelativeLayout (contentSize)) { 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 (); + } + + if (!NeedsLayout) + { + _needsDrawAfterLayout = false; + } return true; } @@ -880,6 +894,8 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public bool NeedsLayout { get; internal set; } = true; + private bool _needsDrawAfterLayout = true; + /// /// Sets to return , indicating this View and all of it's subviews /// (including adornments) need to be laid out in the next Application iteration. @@ -892,6 +908,7 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { + _needsDrawAfterLayout = true; MarkSubtreeNeedsLayout (); TextFormatter.NeedsFormat = true; diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs index e627b1070e..c1b4e48e2b 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -185,6 +185,29 @@ public void SetNeedsLayout_From_AdornmentSubView_Reaches_AdornmentParent () 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 () { @@ -223,4 +246,33 @@ public void SetNeedsLayout_Tabs_Active_Scroll_Does_Not_Mark_Inactive_Pages () 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 (); + } } From 5aa5d514ff855300f08d92c104de6fd7eaaa1447 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 May 2026 17:11:14 +0000 Subject: [PATCH 10/14] Fix possessive typo in SetNeedsLayout doc comment --- Terminal.Gui/ViewBase/View.Layout.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index 38808aba5f..2f890c105d 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -897,7 +897,7 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) private bool _needsDrawAfterLayout = true; /// - /// 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. /// /// From 2775087d6b1062946020a0f163167365c6a95406 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 May 2026 17:12:12 +0000 Subject: [PATCH 11/14] Fix doc comment grammar in Layout return description --- Terminal.Gui/ViewBase/View.Layout.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index 2f890c105d..58ce76d84b 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -555,7 +555,7 @@ 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; From 1489e750de6b5a8bfaae408caeb906dfdbdffed5 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Fri, 22 May 2026 12:27:59 -0500 Subject: [PATCH 12/14] Fixes #5357. Update fan-out tests for layout-only fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TabsFanOutIntegrationTests.cs | 17 ++++++------- .../TabView/TabsFanOutDiagnosticTests.cs | 25 +++++++------------ 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs index 308d30ab67..865b559bec 100644 --- a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -67,12 +67,12 @@ private static string MakeText (string prefix, int lines) } /// - /// End-to-end fan-out check: a real on the active tab causes - /// layout/draw activity on inactive tabs. + /// 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_FansOutToInactiveTabs (string driverName) + public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTabs (string driverName) { const int TabCount = 4; @@ -164,16 +164,15 @@ public void Integration_RealPageDown_OnActiveTab_FansOutToInactiveTabs (string d outputHelper.WriteLine ($"Sum inactive DrawComplete = {inactiveDraws}"); outputHelper.WriteLine ($"Sum inactive SubViewsLaidOut = {inactiveLayouts}"); - // CURRENT BEHAVIOR (issue #4973): inactive tabs receive draw and layout work when active scrolls, - // even through the real input → command → main-loop path. After #4973 lands, flip these to == 0. + // 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.True ( - inactiveLayouts > 0, - $"Documents issue #4973 (integration-level): inactive_total SubViewsLaidOut={inactiveLayouts}. " + - "Flip to Assert.Equal(0, inactiveLayouts) after fix lands."); + Assert.Equal ( + 0, + inactiveLayouts); } } diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs index 3eec4ad54f..51a7aec40e 100644 --- a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -192,11 +192,10 @@ private static (View Root, Code Code, ViewActivityCounters Counters) BuildSingle } /// - /// Issue #5356 / #4973 acceptance criterion: diagnostics can show whether inactive tabs - /// received layout work when the active tab scrolls. + /// Issue #5357 acceptance criterion: layout work stays on the active tab when the active tab scrolls. /// [Fact] - public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () + public void Diagnostic_ActiveTabScroll_LayoutEvents_StayOnActiveTab () { IDriver driver = CreateTestDriver (DriverWidth, DriverHeight); (View root, Tabs tabs, Code [] codes, ViewActivityCounters counters) = BuildTabbedScenario (driver); @@ -230,13 +229,9 @@ public void Diagnostic_ActiveTabScroll_LayoutEvents_OnEachTab () output.WriteLine ($"Active SubViewsLaidOut: {activeCounts.SubViewsLaidOut}"); output.WriteLine ($"Sum of inactive SubViewsLaidOut: {inactiveLayouts}"); - // CURRENT BEHAVIOR (issue #4973): inactive tabs receive the same layout count as active. - // After #4973 is fixed, flip this to `Assert.Equal (0, inactiveLayouts)`. - Assert.True ( - inactiveLayouts > 0, - $"Documents issue #4973: inactive tabs receive layout work when active tab scrolls. " + - $"Observed inactive_total={inactiveLayouts}, active={activeCounts.SubViewsLaidOut}. " + - "Flip to Assert.Equal(0, inactiveLayouts) after #4973 fix lands."); + Assert.Equal ( + 0, + inactiveLayouts); root.Dispose (); driver.Dispose (); @@ -372,17 +367,15 @@ public void Diagnostic_TabbedFanOut_ComparedTo_SingleViewBaseline () 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 (issue #4973): tabbed scenario produces N-times more total work than baseline. - // After #4973 is fixed, drawFanOut and layoutFanOut should approach 1.0 (within rounding). + // 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.True ( - layoutFanOut > 1.0, - $"Documents issue #4973: tabbed layout fan-out ({layoutFanOut:F2}x) exceeds single-Code baseline. " + - "After fix, layoutFanOut should approach 1.0."); + Assert.Equal ( + 1.0, + layoutFanOut); singleRoot.Dispose (); tabRoot.Dispose (); From b231e5a6ca999bd68fda95f6d45fad09678d2ae0 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 24 May 2026 21:46:35 -0600 Subject: [PATCH 13/14] =?UTF-8?q?Fix=20review=20comments:=20update=20AI=20?= =?UTF-8?q?headers=20to=20//=20Copilot=20and=20fix=20#5356=20=E2=86=92=20#?= =?UTF-8?q?5357=20references?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tests/IntegrationTests/TabsFanOutIntegrationTests.cs | 4 ++-- .../ViewBase/Layout/SetNeedsLayoutPropagationTests.cs | 2 +- .../Views/TabView/TabsFanOutDiagnosticTests.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs index 865b559bec..849094e050 100644 --- a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -3,13 +3,13 @@ namespace IntegrationTests; -// Claude - Opus 4.7 +// 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 / #5356 is observable end-to-end, not just under +/// verifies the fan-out from issue #4973 / #5357 is observable end-to-end, not just under /// synthetic / calls. /// /// diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs index c1b4e48e2b..6de2a5143d 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -1,6 +1,6 @@ namespace ViewBaseTests.Layout; -// Claude - Opus 4.7 +// Copilot /// /// Tests for the split between downward subtree invalidation and upward ancestor-only diff --git a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs index 51a7aec40e..5792502d1e 100644 --- a/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs +++ b/Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs @@ -3,10 +3,10 @@ namespace ViewsTests; -// Claude - Opus 4.7 +// Copilot /// -/// Diagnostic tests for issue #5356 — tabbed redraw/layout fan-out. +/// Diagnostic tests for issue #5357 — tabbed redraw/layout fan-out. /// /// /// From 00db60131f4bcf36df9fb6226731d8973ba442c3 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Mon, 25 May 2026 19:29:46 -0500 Subject: [PATCH 14/14] Fixes #5357. Narrow SetNeedsLayout propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Layout.cs | 34 ++++++++++++------- .../Layout/SetNeedsLayoutPropagationTests.cs | 26 ++++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index 58ce76d84b..e234499186 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -559,10 +559,13 @@ internal static bool Layout (IEnumerable views, Size contentSize) public bool Layout (Size contentSize) { bool needsDrawAfterLayout = _needsDrawAfterLayout; + _needsDrawAfterLayout = false; Rectangle originalFrame = Frame; if (!SetRelativeLayout (contentSize)) { + _needsDrawAfterLayout = needsDrawAfterLayout; + return false; } @@ -577,11 +580,6 @@ public bool Layout (Size contentSize) SetNeedsDraw (); } - if (!NeedsLayout) - { - _needsDrawAfterLayout = false; - } - return true; } @@ -694,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: @@ -895,6 +902,7 @@ 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 its subviews @@ -908,7 +916,11 @@ protected virtual void OnSubViewsLaidOut (LayoutEventArgs args) /// public void SetNeedsLayout () { - _needsDrawAfterLayout = true; + if (!_suppressNeedsDrawAfterLayout) + { + _needsDrawAfterLayout = true; + } + MarkSubtreeNeedsLayout (); TextFormatter.NeedsFormat = true; @@ -978,10 +990,9 @@ private void MarkAdornmentSubViewTrees () } // Walks up the ancestor chain marking each ancestor as needing layout, without descending - // into the ancestor's regular SubViews tree. Adornment subview trees on each ancestor are - // still marked so adornment content re-lays-out when its parent does. Stops at any ancestor - // already marked, mirroring the early-exit guards in the original recursive call. See - // issue #5357. + // 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) @@ -991,7 +1002,6 @@ private void MarkAncestorsNeedLayout () NeedsLayout = true; TextFormatter.NeedsFormat = true; - MarkAdornmentSubViewTrees (); if (SuperView is { NeedsLayout: false } superView) { diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs index 6de2a5143d..ecd52a9a6c 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs @@ -144,6 +144,32 @@ public void SetNeedsLayout_On_Cousin_Does_Not_Mark_Other_Branch () 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 () {