From f64af093ab75c98cef4f309732463d7db03e61f2 Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Mon, 1 Jun 2026 11:41:21 -0500 Subject: [PATCH 1/2] Fixes #5434. Track adornment redraw deltas Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/App/ApplicationImpl.Screen.cs | 10 +-- Terminal.Gui/ViewBase/View.Adornments.cs | 86 ++++++++++++++++--- .../TabsFanOutIntegrationTests.cs | 20 +---- 3 files changed, 81 insertions(+), 35 deletions(-) diff --git a/Terminal.Gui/App/ApplicationImpl.Screen.cs b/Terminal.Gui/App/ApplicationImpl.Screen.cs index bec9509ef4..c0e2e8d7cd 100644 --- a/Terminal.Gui/App/ApplicationImpl.Screen.cs +++ b/Terminal.Gui/App/ApplicationImpl.Screen.cs @@ -278,18 +278,16 @@ public void LayoutAndDraw (bool forceRedraw = false) Rectangle clipRect = Screen with { X = 0, Y = 0 }; Driver.Clip = new Region (clipRect); - // Only force a complete redraw if needed (needsLayout or forceRedraw). - // Otherwise, just redraw views that need it. + // Only force a complete redraw when explicitly requested. + // Otherwise, redraw only the views that need it. // // NOTE (#5358): passing force=true here calls SetNeedsDraw on the top runnable, which // cascades to all overlapping subviews via the existing SetNeedsDraw recursion. This // is the remaining draw-fan-out source documented by TabsFanOutIntegrationTests. The // proper fix requires tracking adornment thickness changes (in addition to Frame // changes) so the SuperView can be invalidated precisely instead of force-redrawing - // the whole tree. Dropping force-on-neededLayout without that tracking exposes - // stale-content bugs in the shrink/move and adornment-rebalance paths (covered by - // ShadowTests / BorderViewTests). - View.Draw (views.ToArray ().Cast (), neededLayout || forceRedraw); + // the whole tree. + View.Draw (views.ToArray ().Cast (), forceRedraw); Driver.Clip = new Region (clipRect); diff --git a/Terminal.Gui/ViewBase/View.Adornments.cs b/Terminal.Gui/ViewBase/View.Adornments.cs index 4b81afc782..814d0f8420 100644 --- a/Terminal.Gui/ViewBase/View.Adornments.cs +++ b/Terminal.Gui/ViewBase/View.Adornments.cs @@ -11,6 +11,11 @@ private void SetupAdornments () { return; } + + Thickness previousMarginThickness = Margin.Thickness; + Thickness previousBorderThickness = Border.Thickness; + Thickness previousPaddingThickness = Padding.Thickness; + Margin.Parent = this; Border.Parent = this; Padding.Parent = this; @@ -18,27 +23,42 @@ private void SetupAdornments () // When any adornment's thickness changes, recompute frames and request layout + redraw. Margin.ThicknessChanged += (_, _) => { - Margin.View?.SetNeedsLayout (); - SetAdornmentFrames (); - SetNeedsLayout (); - SetNeedsDraw (); + previousMarginThickness = HandleAdornmentThicknessChanged (Margin, previousMarginThickness); }; Border.ThicknessChanged += (_, _) => { - Border.View?.SetNeedsLayout (); - SetAdornmentFrames (); - SetNeedsLayout (); - SetNeedsDraw (); + previousBorderThickness = HandleAdornmentThicknessChanged (Border, previousBorderThickness); }; Padding.ThicknessChanged += (_, _) => { - Padding.View?.SetNeedsLayout (); - SetAdornmentFrames (); - SetNeedsLayout (); - SetNeedsDraw (); + previousPaddingThickness = HandleAdornmentThicknessChanged (Padding, previousPaddingThickness); }; + + Thickness HandleAdornmentThicknessChanged (IAdornment adornment, Thickness previousThickness) + { + Rectangle oldViewport = GetViewportForAdornmentThickness (adornment, previousThickness); + Rectangle oldViewportFrame = GetViewportFrameForAdornmentThickness (adornment, previousThickness); + Thickness currentThickness = adornment.Thickness; + + (adornment.View as View)?.SetNeedsLayout (); + SetAdornmentFrames (); + SetNeedsLayout (); + SetNeedsDraw (); + + if (oldViewportFrame != GetViewportFrameForAdornmentThickness (adornment, currentThickness)) + { + InvalidateAdornmentThicknessChange (); + } + + if (oldViewport != Viewport) + { + RaiseViewportChangedEvent (oldViewport); + } + + return currentThickness; + } } private void BeginInitAdornments () @@ -345,4 +365,46 @@ internal void SetAdornmentFrames () // Border and Padding dynamically update based on Margin's View's Frame changing Margin.View?.Frame = Frame with { Location = Point.Empty }; } + + private Rectangle GetViewportForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness) + { + Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness); + + return new Rectangle (_viewportLocation, + new Size (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical))); + } + + private Rectangle GetViewportFrameForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness) + { + Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness); + + return new Rectangle (new Point (thickness.Left, thickness.Top), + new Size (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical))); + } + + private Thickness GetAdornmentsThicknessForAdornment (IAdornment changedAdornment, Thickness changedThickness) + { + Thickness thickness = Thickness.Empty; + + thickness += ReferenceEquals (changedAdornment, Margin) ? changedThickness : Margin.Thickness; + thickness += ReferenceEquals (changedAdornment, Border) ? changedThickness : Border.Thickness; + thickness += ReferenceEquals (changedAdornment, Padding) ? changedThickness : Padding.Thickness; + + return thickness; + } + + private void InvalidateAdornmentThicknessChange () + { + if (SuperView is null) + { + NeedsClearScreenNextIteration (); + + return; + } + + Rectangle invalidationViewport = Frame; + Point superScroll = SuperView.Viewport.Location; + invalidationViewport.Offset (-superScroll.X, -superScroll.Y); + SuperView.SetNeedsDraw (invalidationViewport); + } } diff --git a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs index de1d9175a1..e56002c3ed 100644 --- a/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs +++ b/Tests/IntegrationTests/TabsFanOutIntegrationTests.cs @@ -68,8 +68,8 @@ private static string MakeText (string prefix, int lines) } /// - /// 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. + /// End-to-end check: a real on the active tab keeps both + /// layout and text drawing on the active tab. /// [Theory] [MemberData (nameof (GetAllDriverNames))] @@ -169,21 +169,7 @@ public void Integration_RealPageDown_OnActiveTab_DoesNotFanOutLayoutToInactiveTa outputHelper.WriteLine ($"Sum inactive DrawingText = {inactiveTextDraws}"); outputHelper.WriteLine ($"Sum inactive SubViewsLaidOut = {inactiveLayouts}"); - // Issue #5358 fix narrows draw fan-out at the View.Draw pipeline level (verified by - // TabsFanOutDiagnosticTests at synthetic level). At integration level a separate - // cascade source remains: ApplicationImpl.LayoutAndDraw passes force=true to - // View.Draw whenever any view needed layout, which calls SetNeedsDraw on the top - // runnable, which cascades to overlapping subviews via the existing SetNeedsDraw - // recursion. Removing that force=true uncovers stale-content bugs in the shrink/move - // path (covered by existing ShadowTests and BorderViewTests) and is out of scope - // for #5358. Until that broader fix lands, inactive tab pages still receive - // NeedsDraw via the LayoutAndDraw force path. Layout fan-out is already fully - // eliminated by PR #5373. - Assert.True ( - inactiveTextDraws > 0, - $"Documents the remaining draw fan-out via ApplicationImpl.LayoutAndDraw's force=true path: " + - $"inactive_total DrawingText={inactiveTextDraws}. Flip to Assert.Equal(0, inactiveTextDraws) " + - "after the broader LayoutAndDraw cascade is addressed (out of scope for #5358)."); + Assert.Equal (0, inactiveTextDraws); Assert.Equal (0, inactiveLayouts); } From 61aaa713dac7528b14dddf80ea8d09bb3887d64d Mon Sep 17 00:00:00 2001 From: Kevin Harder Date: Mon, 1 Jun 2026 17:41:30 -0500 Subject: [PATCH 2/2] Clean up adornment thickness-delta helpers Fix indentation of the Margin.ThicknessChanged handler body and factor the duplicated viewport-size formula out of GetViewportForAdornmentThickness and GetViewportFrameForAdornmentThickness into a shared GetViewportSizeForThickness helper. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- Terminal.Gui/ViewBase/View.Adornments.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Adornments.cs b/Terminal.Gui/ViewBase/View.Adornments.cs index 814d0f8420..bea72f8d14 100644 --- a/Terminal.Gui/ViewBase/View.Adornments.cs +++ b/Terminal.Gui/ViewBase/View.Adornments.cs @@ -23,7 +23,7 @@ private void SetupAdornments () // When any adornment's thickness changes, recompute frames and request layout + redraw. Margin.ThicknessChanged += (_, _) => { - previousMarginThickness = HandleAdornmentThicknessChanged (Margin, previousMarginThickness); + previousMarginThickness = HandleAdornmentThicknessChanged (Margin, previousMarginThickness); }; Border.ThicknessChanged += (_, _) => @@ -370,18 +370,19 @@ private Rectangle GetViewportForAdornmentThickness (IAdornment changedAdornment, { Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness); - return new Rectangle (_viewportLocation, - new Size (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical))); + return new Rectangle (_viewportLocation, GetViewportSizeForThickness (thickness)); } private Rectangle GetViewportFrameForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness) { Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness); - return new Rectangle (new Point (thickness.Left, thickness.Top), - new Size (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical))); + return new Rectangle (new Point (thickness.Left, thickness.Top), GetViewportSizeForThickness (thickness)); } + private Size GetViewportSizeForThickness (Thickness thickness) => + new (Math.Max (0, Frame.Width - thickness.Horizontal), Math.Max (0, Frame.Height - thickness.Vertical)); + private Thickness GetAdornmentsThicknessForAdornment (IAdornment changedAdornment, Thickness changedThickness) { Thickness thickness = Thickness.Empty;