Fixes #3761, #2886, #3780, #3485, #3622, #3413, #2995 - Draw and Layout performance/correctness #3798
Conversation
|
@BDisp, @tznind, @dodexahedron or anyone else who'd like to comment... As I've worked this PR, I've learned a bunch of things, and gotten clarity on other issues that have bugged me for a long time. Before I go further, I want to share my thoughts in writing. This will solidify my thinking and enable y'all to challenge and debate it. Some of the changes I've made, and want to continue to make are significant. While they don't fundamentally change the TG programming model, they do require changes to how devs will develop against it. Some will make porting from v1 more challenging. Layout vs DrawFirst, TG has always been confused about the relationship between Layout and Draw. This confusion got worse and worse over time. The addition of The codebase is full of convoluted code that attempts to ensure things get laid out then drawn. Property We have to fix this at a fundamental level or the library will continue to get bogged down. Assertions:
Asynchronous Layout vs Explicit/SynchronousWay back, the only place that called The names of these methods leads to confusion as well. Assertion:
Great, but what about cases where a dev DOES want immediate layout? Should it be implicit (as it is today) or explicit? Option 1
View v = new ();
Assert.Equal (0, v.Frame.Height);
v.Height = 1;
Assert.Equal (1, v.Frame.Height);This will not fail: View v = new ();
Assert.Equal (0, v.Frame.Height);
v.Height = 1;
Assert.Equal (0, v.Frame.Height);
v.Layout ();
Assert.Equal (1, v.Frame.Height);Option 2
The downside of this is that devs may be confused: View v = new ();
Assert.Equal (0, v.Frame.Height);
v.Height = 1;
Assert.False (v.IsLayoutNeeded());
Assert.Equal (1, v.Frame.Height); // works because `1` is `Dim.Absoulte(1)`
v.Width = Dim.Percent(50);
Assert.True (v.IsLayoutNeeded());
Assert.Equal (0, v.Frame.Width); // still 0The reality is the above code is really only valid in UNIT TESTS. No dev will ever do this IRL. IOW, if we don't call In this PR I started with Option 1. It felt pure. But it means I have to touch 100s of Unit tests that simply are using absolute values and add calls to .... More later. This is enough for now... |
|
As I remember the layout was always done before drawn. When the drawn was called alone and |
|
Awesome!
This will definitely help with our flicker issue and bring some structure
to layout - which has been a bit unstructured and opaque.
Regarding redraw - i think we can also eliminate redraws of identical
screen content. I did this in the sixel branch to work around the constant
refreshes and it seemed to work. Basically on screen redraw you record the
final string sent to console and if it's the same you bail. This could be
a 'last line of defence' against this kind of thing.
…On Sat, 19 Oct 2024, 00:29 BDisp, ***@***.***> wrote:
As I remember the layout was always done before drawn. When the drawn was
called alone and IsLayoutNeeded() == true the layout was being performed
first. I also agree with the option 2. The #3761
<#3761> really needs to be
fixed. Good work.
—
Reply to this email directly, view it on GitHub
<#3798 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHO3C5DRFANN3IZSQDEWGCTZ4GKVBAVCNFSM6AAAAABQAGJGOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRTGM3DOOBUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Made Adornment support being a subview for testing purposes.
Fix SingleBackgroundWorker scenario.
…Gui into v2_3761_2886-Draw-and-Layout-Perf
|
Huge progress on auto-join borders and lines. I've been upgrading AllViewsTester as a hard-core test case...
Things that are not quite right. I plan on fixing these in later PRs.
|
tig
changed the title
|
@tznind, @BDisp, @dodexahedron - This is as ready as it's gonna get. Please read the first post to see all that I've done. I'd like to get this merged tomorrow. |
There was a problem hiding this comment.
Only remining the TabView but I think it will be fixed with the #3808.
Please can you change the file TabMouseEventArgs.cs as below? Thanks!
using System.ComponentModel;
namespace Terminal.Gui;
/// <summary>Describes a mouse event over a specific <see cref="Tab"/> in a <see cref="TabView"/>.</summary>
public class TabMouseEventArgs : HandledEventArgs
{
/// <summary>Creates a new instance of the <see cref="TabMouseEventArgs"/> class.</summary>
/// <param name="tab"><see cref="Tab"/> that the mouse was over when the event occurred.</param>
/// <param name="mouseEvent">The mouse activity being reported</param>
public TabMouseEventArgs (Tab tab, MouseEventArgs mouseEvent)
{
Tab = tab;
MouseEvent = mouseEvent;
}
/// <summary>
/// Gets the actual mouse event. Use <see cref="HandledEventArgs.Handled"/> to cancel this event and perform custom
/// behavior (e.g. show a context menu).
/// </summary>
public MouseEventArgs MouseEvent { get; }
/// <summary>Gets the <see cref="Tab"/> (if any) that the mouse was over when the <see cref="MouseEvent"/> occurred.</summary>
/// <remarks>This will be null if the click is after last tab or before first.</remarks>
public Tab Tab { get; }
}
Fixes
SetNeedsLayout-> HavePos/Dimuse events #3485ConsoleDriver#3622Clipbe aRegioninstead ofRectangle#3413Borderis not always working #2995Related
Proposed Changes/Todos
View.LayoutSubviewsetc... to make more testable and clearer to use. IntroduceView.Layout()as the primary API that builds onView.SetRelativeLayoutandView.LayoutSubviews. Move from depending onIsInitializedtoNeedsLayoutas the signal that layout has been performed.View.Drawto be more granular and use proper event patterns.View.SetAttribute; makeDriver.SetAttributeinternalClipbe aRegioninstead ofRectangle#3413Enabledto enable disabled views to stay disabled even if superview changes; use an event instead of hacky logic.Driver.Move/AddStrLineCanvasand auto-join of lines/BorderKnown Issues that will be fixed later
TabViewworks, but has horrible performance and strange redraw. Unit tests are disabled. This will be fixed with RefactorsTabViewusing modern v2 capabilties #3808ScrollViewis non-functional. This will be fixed with Fixes #2489. Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #3498Borderstarts actually using subviews for the border lines and title. Abandoned - Fixes #3407.BorderSubviews #3409SuperViewRendersLineCanvasset to redraw anytime any of them changes. This negates the perf work I've done whenSuperViewRendersLineCanvasis enabled.Pull Request checklist:
CTRL-K-Dto automatically reformat your files before committing.dotnet testbefore commit///style comments)