Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions Terminal.Gui/App/ApplicationImpl.Screen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<View> (), neededLayout || forceRedraw);
// the whole tree.
View.Draw (views.ToArray ().Cast<View> (), forceRedraw);

Driver.Clip = new Region (clipRect);

Expand Down
87 changes: 75 additions & 12 deletions Terminal.Gui/ViewBase/View.Adornments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,54 @@ 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;

// 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 ()
Expand Down Expand Up @@ -345,4 +365,47 @@ 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, GetViewportSizeForThickness (thickness));
}

private Rectangle GetViewportFrameForAdornmentThickness (IAdornment changedAdornment, Thickness changedThickness)
{
Thickness thickness = GetAdornmentsThicknessForAdornment (changedAdornment, changedThickness);

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;

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);
}
}
20 changes: 3 additions & 17 deletions Tests/IntegrationTests/TabsFanOutIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ private static string MakeText (string prefix, int lines)
}

/// <summary>
/// End-to-end check: a real <see cref="Key.PageDown"/> 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 <see cref="Key.PageDown"/> on the active tab keeps both
/// layout and text drawing on the active tab.
/// </summary>
[Theory]
[MemberData (nameof (GetAllDriverNames))]
Expand Down Expand Up @@ -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);
}
Expand Down
Loading