Fixes #5357 - Avoid redraw fan-out from ancestor-only layout propagation by harder · Pull Request #5373 · gui-cs/Terminal.Gui

harder · 2026-05-22T16:48:43Z

Summary

Tighten the #5357 layout invalidation split so ancestor-only propagation does not force unchanged peers into redraw work.

Fixes Split upward layout propagation from downward subtree invalidation #5357

Changes

Split direct layout invalidation from ancestor-only propagation by tracking whether a view actually needs redraw after layout.
Keep redraw behavior for views that were directly invalidated or whose resolved frame changed.
Add regression coverage for inactive tab pages staying out of redraw work after layout.
Add regression coverage for the direct AdornmentView.SetNeedsLayout () path used by border arrangement code.

Testing

dotnet build --no-restore
dotnet test --project Tests/UnitTestsParallelizable --filter-class "*SetNeedsLayoutPropagationTests"
dotnet test --project Tests/UnitTestsParallelizable --filter-class "*LayoutTests"
dotnet test --project Tests/UnitTestsParallelizable --filter-class "*NeedsDrawTests"
dotnet test --project Tests/UnitTestsParallelizable --filter-class "*Tabs*"

To pull down this PR locally:

git remote add copilot git@github.com:harder/Terminal.Gui.git
git fetch copilot fix-5357-split-layout-propagation
git checkout copilot/fix-5357-split-layout-propagation

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 gui-cs#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.

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: <each> 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

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

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) <noreply@anthropic.com>

…ree 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) <noreply@anthropic.com>

…pagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…stics Fixes gui-cs#5356. Add tab fan-out layout/draw diagnostic tests

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) <noreply@anthropic.com>

…ree 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) <noreply@anthropic.com>

…pagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Refines View.SetNeedsLayout () propagation to avoid layout invalidation “fan-out” into unaffected sibling subtrees (notably in Tabs), while keeping redraw behavior for directly-invalidated views and views whose resolved Frame changes.

Changes:

Split layout invalidation into (1) downward subtree marking for the changed view and (2) ancestor-only upward marking that avoids re-dirtying sibling subtrees.
Gate “draw-after-layout” so ancestor-only propagation doesn’t automatically trigger redraw work for unaffected peers.
Add regression tests covering sibling/tab-page non-fan-out and adornment-related propagation paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
Terminal.Gui/ViewBase/View.Layout.cs	Implements split subtree vs. ancestor-only layout propagation and redraw-after-layout tracking.
Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs	Adds regression tests to lock in the new propagation/redraw behavior (including `Tabs` and adornment scenarios).

…n' into fix-5357-split-layout-propagation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs:243

This doc comment references issue #5356, but these tests are keyed to #5357 (and #4973). Update the issue number to keep the references consistent.

    /// <summary>
    ///     Issue #5356 / #4973 acceptance criterion: diagnostics can show whether inactive tabs
    ///     received draw work when the active tab scrolls.
    /// </summary>

Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs:312

This doc comment references issue #5356, but the surrounding tests and PR are for #5357. Update the issue number for consistency.

    /// <summary>
    ///     Issue #5356 acceptance criterion: a comparable fan-out metric between an equivalent
    ///     single-Code scenario and a tabbed-Code scenario.
    /// </summary>

harder · 2026-05-24T15:19:04Z

I ran the tab fan-out layout/draw diagnostic tests added in PR #5364 before and after this PR. Here are the results:

Layout-only unit diagnostic
Active layouts: 5 -> 5
Inactive layouts: 15 -> 0
Tabbed vs single-view baseline
Draw fan-out ratio: 4.00x -> 4.00x
Layout fan-out ratio: 4.00x -> 1.00x
Tabbed total layouts: 20 -> 5
Layout vs draw split diagnostic
Layout inactive total: 3 -> 0 (active stayed 1 -> 1)
Draw inactive total: 3 -> 3 (active stayed 1 -> 1)
Layout fan-out ratio: 3.00 -> 0.00
Draw fan-out ratio: 3.00 -> 3.00
Draw-only unit diagnostic
Active draws: 5 -> 5
Inactive draws: 15 -> 15
Integration diagnostic by driver
ansi inactive draws 9 -> 9; inactive layouts 9 -> 0
dotnet inactive draws 9 -> 9; inactive layouts 9 -> 0
windows inactive draws 9 -> 9; inactive layouts 9 -> 0

Summary

Layout fan-out is materially improved: inactive layout work dropped to zero in the targeted layout-only and integration diagnostics.
The tabbed-vs-single layout ratio dropped from 4.00x to 1.00x, which matches the intended post-fix baseline.
Draw fan-out is essentially unchanged in these diagnostics (still 4.00x in the baseline comparison and 15 inactive draws in the draw-only unit test).

tig

Concerns:

_needsDrawAfterLayout reset logic (warning) - The flag only resets when !NeedsLayout post-layout, but LayoutSubViews() can re-set NeedsLayout. This may cause stale draw invalidation on subsequent passes. Consider clearing unconditionally after capturing the local.
MarkAncestorsNeedLayout marks ancestor adornment trees (informational) - This partially reintroduces fan-out into adornment subview trees of uninvolved ancestors. Not a correctness issue but a missed optimization.
Trailing whitespace on lines 921 and 1001.

The core design is sound. Splitting upward propagation from downward cascade is the right fix for the O(N^2) behavior.

I think this is good to go. Your choice if you want to address the above minor issues found by a coding agent ;-)

→ gui-cs#5357 references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

Terminal.Gui/ViewBase/View.Layout.cs:950

Avoid an extra allocation in MarkSubtreeNeedsLayout: Snapshot() already returns an array, so ToList() is unnecessary before constructing the Stack.

        // TODO: Optimize this - see Setting_Thickness_Causes_Adornment_SubView_Layout
        // Use a stack to avoid recursion
        Stack<View> stack = new (InternalSubViews.Snapshot ().ToList ());

harder · 2026-05-26T17:31:59Z

I addressed the two minor issues raised by Tig in #5373 (review) and re-ran CoPilot review here.

tig · 2026-05-26T17:34:18Z

Excellent work Kevin. Thank you!

Side note: Configuration Editor using Editor became usable vs TextView. So the worst repro of the perf issues is now gone that I'm aware of....

DrawComplete fires unconditionally for clip-exclusion bookkeeping even when NeedsDraw is false, so it is the wrong signal for measuring whether a view actually did draw work. Add a DrawingText counter to both the diagnostic and integration tests — DrawingText is gated on NeedsDraw inside DoDrawText and is not intercepted by Code (which does intercept ClearedViewport / DrawingContent via OnClearingViewport / OnDrawingContent returning true). Use DrawingText to verify per-Code inactive-tab behavior, and use tabsCounts.ClearedViewport to verify the parent's own behavior. TabsFanOutDiagnosticTests (synthetic Layout/Draw path) now asserts: - inactive tabs have DrawingText == 0 - Tabs container has ClearedViewport == 0 - text-draw fan-out ratio == 1.0 (parity with single-Code baseline) TabsFanOutIntegrationTests (end-to-end via main loop) keeps the existing "documents remaining fan-out" assertion: a separate cascade source remains in ApplicationImpl.LayoutAndDraw (force=true when neededLayout, calling SetNeedsDraw on the top runnable which cascades to overlapping subviews). Removing that force=true uncovers stale-content bugs in the shrink/move path (already covered by ShadowTests / BorderViewTests) and is out of scope for #5358. Layout fan-out is fully eliminated by PR #5373. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

harder and others added 6 commits May 21, 2026 09:19

Potential fix for pull request finding

7e3ccb6

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fixes gui-cs#5357. Avoid redraw fan-out from ancestor-only layout pro…

1fb219b

…pagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

harder requested a review from tig as a code owner May 22, 2026 16:48

harder requested a review from Copilot May 22, 2026 16:48

Copilot started reviewing on behalf of harder May 22, 2026 16:49 View session

harder and others added 4 commits May 22, 2026 11:50

Merge pull request gui-cs#5364 from harder/fix-5356-tab-fanout-diagno…

d4df487

…stics Fixes gui-cs#5356. Add tab fan-out layout/draw diagnostic tests

Fixes gui-cs#5357. Avoid redraw fan-out from ancestor-only layout pro…

7be7ed0

…pagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread Terminal.Gui/ViewBase/View.Layout.cs Outdated

Comment thread Terminal.Gui/ViewBase/View.Layout.cs Outdated

Comment thread Tests/UnitTestsParallelizable/ViewBase/Layout/SetNeedsLayoutPropagationTests.cs Outdated

harder self-assigned this May 22, 2026

This was referenced May 22, 2026

Fix View.Layout XML doc grammar per review suggestion #5374

Closed

Fixes #issue. Correct SetNeedsLayout doc possessive #5375

Closed

[WIP] Fix code based on review comment #5376

Closed

harder and others added 4 commits May 22, 2026 12:14

Merge remote-tracking branch 'origin/fix-5357-split-layout-propagatio…

8b4f7b9

…n' into fix-5357-split-layout-propagation

Fix possessive typo in SetNeedsLayout doc comment

5aa5d51

Fix doc comment grammar in Layout return description

2775087

Fixes gui-cs#5357. Update fan-out tests for layout-only fix

1489e75

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

harder requested a review from Copilot May 22, 2026 17:37

Copilot started reviewing on behalf of harder May 22, 2026 17:38 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs

Comment thread Tests/IntegrationTests/TabsFanOutIntegrationTests.cs

tig approved these changes May 24, 2026

View reviewed changes

tig changed the title ~~Fixes #5357. Avoid redraw fan-out from ancestor-only layout propagation~~ Fixes #5357 - Avoid redraw fan-out from ancestor-only layout propagation May 25, 2026

tig and others added 2 commits May 24, 2026 21:46

Fix review comments: update AI headers to // Copilot and fix gui-cs#5356

b231e5a

→ gui-cs#5357 references Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fixes gui-cs#5357. Narrow SetNeedsLayout propagation

00db601

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

harder requested a review from Copilot May 26, 2026 00:31

Copilot started reviewing on behalf of harder May 26, 2026 00:31 View session

Copilot AI reviewed May 26, 2026

View reviewed changes

Comment thread Terminal.Gui/ViewBase/View.Layout.cs

Comment thread Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs

Comment thread Tests/UnitTestsParallelizable/Views/TabView/TabsFanOutDiagnosticTests.cs

harder merged commit 5d40f18 into gui-cs:develop May 26, 2026
13 checks passed

harder deleted the fix-5357-split-layout-propagation branch May 26, 2026 17:32

harder mentioned this pull request May 27, 2026

Fixes #5358. Narrow draw pipeline to skip parent on child-only redraws #5431

Merged

dependabot Bot mentioned this pull request Jun 5, 2026

Bump the non-major-updates group with 1 update Akeoott/ZenMonitor#27

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #5357 - Avoid redraw fan-out from ancestor-only layout propagation#5373

Fixes #5357 - Avoid redraw fan-out from ancestor-only layout propagation#5373
harder merged 16 commits into
gui-cs:developfrom
harder:fix-5357-split-layout-propagation

harder commented May 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

harder commented May 24, 2026

Uh oh!

tig left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harder commented May 26, 2026

Uh oh!

Uh oh!

tig commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

harder commented May 22, 2026

Summary

Changes

Testing

To pull down this PR locally:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

harder commented May 24, 2026

Summary

Uh oh!

tig left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harder commented May 26, 2026

Uh oh!

Uh oh!

tig commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants