From ce7fc04100bb9cc3f93537e86e7a24f449eb68b8 Mon Sep 17 00:00:00 2001 From: Tig Date: Thu, 13 Mar 2025 18:16:53 +0100 Subject: [PATCH] Fixes #3984 - `Margin` w/out shadow should not force draw (#3985) * shortcut tests * Generic demos * Optimize Margin to not defer draw if there's no shadow --- Terminal.Gui/View/Adornment/Margin.cs | 2 +- Terminal.Gui/View/View.Drawing.cs | 38 +++++++++++++-------- Terminal.Gui/Views/GraphView/Annotations.cs | 2 +- Terminal.Gui/Views/Menu/Menu.cs | 2 +- Terminal.Gui/Views/TileView.cs | 2 +- UICatalog/Scenarios/Generic.cs | 18 ++++++++-- 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/Terminal.Gui/View/Adornment/Margin.cs b/Terminal.Gui/View/Adornment/Margin.cs index ac1977028..9d5dc44e8 100644 --- a/Terminal.Gui/View/Adornment/Margin.cs +++ b/Terminal.Gui/View/Adornment/Margin.cs @@ -48,7 +48,7 @@ public class Margin : Adornment internal void CacheClip () { - if (Thickness != Thickness.Empty) + if (Thickness != Thickness.Empty && ShadowStyle != ShadowStyle.None) { // PERFORMANCE: How expensive are these clones? _cachedClip = GetClip ()?.Clone (); diff --git a/Terminal.Gui/View/View.Drawing.cs b/Terminal.Gui/View/View.Drawing.cs index 9d5521c02..09954c701 100644 --- a/Terminal.Gui/View/View.Drawing.cs +++ b/Terminal.Gui/View/View.Drawing.cs @@ -27,6 +27,7 @@ public partial class View // Drawing APIs view.Draw (context); } + // Draw the margins (those whith Shadows) last to ensure they are drawn on top of the content. Margin.DrawMargins (viewsArray); } @@ -57,9 +58,9 @@ public partial class View // Drawing APIs { // ------------------------------------ // Draw the Border and Padding. - // Note Margin is special-cased and drawn in a separate pass to support + // Note Margin with a Shadow is special-cased and drawn in a separate pass to support // transparent shadows. - DoDrawBorderAndPadding (originalClip); + DoDrawAdornments (originalClip); SetClip (originalClip); // ------------------------------------ @@ -106,7 +107,7 @@ public partial class View // Drawing APIs // ------------------------------------ // Re-draw the border and padding subviews // HACK: This is a hack to ensure that the border and padding subviews are drawn after the line canvas. - DoDrawBorderAndPaddingSubViews (); + DoDrawAdornmentsSubViews (); // ------------------------------------ // Advance the diagnostics draw indicator @@ -116,8 +117,8 @@ public partial class View // Drawing APIs } // ------------------------------------ - // This causes the Margin to be drawn in a second pass - // PERFORMANCE: If there is a Margin, it will be redrawn each iteration of the main loop. + // This causes the Margin to be drawn in a second pass if it has a ShadowStyle + // PERFORMANCE: If there is a Margin w/ Shadow, it will be redrawn each iteration of the main loop. Margin?.CacheClip (); // ------------------------------------ @@ -131,8 +132,11 @@ public partial class View // Drawing APIs #region DrawAdornments - private void DoDrawBorderAndPaddingSubViews () + private void DoDrawAdornmentsSubViews () { + + // NOTE: We do not support subviews of Margin? + if (Border?.SubViews is { } && Border.Thickness != Thickness.Empty) { // PERFORMANCE: Get the check for DrawIndicator out of this somehow. @@ -164,7 +168,7 @@ public partial class View // Drawing APIs } } - private void DoDrawBorderAndPadding (Region? originalClip) + private void DoDrawAdornments (Region? originalClip) { if (this is Adornment) { @@ -194,27 +198,28 @@ public partial class View // Drawing APIs // A SubView may add to the LineCanvas. This ensures any Adornment LineCanvas updates happen. Border?.SetNeedsDraw (); Padding?.SetNeedsDraw (); + Margin?.SetNeedsDraw (); } - if (OnDrawingBorderAndPadding ()) + if (OnDrawingAdornments ()) { return; } // TODO: add event. - DrawBorderAndPadding (); + DrawAdornments (); } /// - /// Causes and to be drawn. + /// Causes , , and to be drawn. /// /// /// - /// is drawn in a separate pass. + /// is drawn in a separate pass if is set. /// /// - public void DrawBorderAndPadding () + public void DrawAdornments () { // We do not attempt to draw Margin. It is drawn in a separate pass. @@ -230,6 +235,11 @@ public partial class View // Drawing APIs Padding?.Draw (); } + + if (Margin is { } && Margin.Thickness != Thickness.Empty && Margin.ShadowStyle == ShadowStyle.None) + { + Margin?.Draw (); + } } private void ClearFrame () @@ -255,7 +265,7 @@ public partial class View // Drawing APIs /// false (the default), this method will cause the be prepared to be rendered. /// /// to stop further drawing of the Adornments. - protected virtual bool OnDrawingBorderAndPadding () { return false; } + protected virtual bool OnDrawingAdornments () { return false; } #endregion DrawAdornments @@ -635,7 +645,7 @@ public partial class View // Drawing APIs /// /// Gets or sets whether this View will use it's SuperView's for rendering any /// lines. If the rendering of any borders drawn by this Frame will be done by its parent's - /// SuperView. If (the default) this View's method will + /// SuperView. If (the default) this View's method will /// be /// called to render the borders. /// diff --git a/Terminal.Gui/Views/GraphView/Annotations.cs b/Terminal.Gui/Views/GraphView/Annotations.cs index 02b67c974..7d41ae4f9 100644 --- a/Terminal.Gui/Views/GraphView/Annotations.cs +++ b/Terminal.Gui/Views/GraphView/Annotations.cs @@ -150,7 +150,7 @@ public class LegendAnnotation : View, IAnnotation if (BorderStyle != LineStyle.None) { - DrawBorderAndPadding (); + DrawAdornments (); RenderLineCanvas (); } diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs index 1ea62b3ab..3f1e406fb 100644 --- a/Terminal.Gui/Views/Menu/Menu.cs +++ b/Terminal.Gui/Views/Menu/Menu.cs @@ -831,7 +831,7 @@ internal sealed class Menu : View return; } - DrawBorderAndPadding (); + DrawAdornments (); RenderLineCanvas (); // BUGBUG: Views should not change the clip. Doing so is an indcation of poor design or a bug in the framework. diff --git a/Terminal.Gui/Views/TileView.cs b/Terminal.Gui/Views/TileView.cs index f3aed9c2a..398474f48 100644 --- a/Terminal.Gui/Views/TileView.cs +++ b/Terminal.Gui/Views/TileView.cs @@ -173,7 +173,7 @@ public class TileView : View /// Overridden so no Frames get drawn /// - protected override bool OnDrawingBorderAndPadding () { return true; } + protected override bool OnDrawingAdornments () { return true; } /// protected override bool OnRenderingLineCanvas () { return false; } diff --git a/UICatalog/Scenarios/Generic.cs b/UICatalog/Scenarios/Generic.cs index 3fc926ded..33ca9e73a 100644 --- a/UICatalog/Scenarios/Generic.cs +++ b/UICatalog/Scenarios/Generic.cs @@ -18,7 +18,21 @@ public sealed class Generic : Scenario Title = GetQuitKeyAndName (), }; - var button = new Button { Id = "button", X = Pos.Center (), Y = 1, Text = "_Press me!" }; + FrameView frame = new () + { + Height = Dim.Fill (), + Width = Dim.Fill (), + Title = "Frame" + }; + appWindow.Add (frame); + + var button = new Shortcut () + { + Id = "button", + X = Pos.Center (), + Y = 1, + Text = "_Press me!" + }; button.Accepting += (s, e) => { @@ -27,7 +41,7 @@ public sealed class Generic : Scenario MessageBox.ErrorQuery ("Error", "You pressed the button!", "_Ok"); }; - appWindow.Add (button); + frame.Add (button); // Run - Start the application. Application.Run (appWindow);