Fixes #4204. v2win and v2net aren't refreshing the Character Map correctly

[BDisp]

Fixes

• Fixes v2win and v2net aren't refreshing the Character Map correctly #4204

Proposed Changes/Todos

• Only needed to call Application.LayoutAndDrawImpl (true); in the MainThreadId

Pull Request checklist:

• I've named my PR in the form of "Fixes #issue. Terse description."
• My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• My code follows the Terminal.Gui library design guidelines
• I ran dotnet test before commit
• I have made corresponding changes to the API documentation (using /// style comments)
• My changes generate no new warnings
• I have checked my code and corrected any poor grammar or misspellings
• I conducted basic QA to assure all features are working

[tznind]

If i read this correct you are forcing an explicit redraw on every invoke user does?

This violates v2 design of redraw rate limiting.

There should only ever be 1 layout/redraw per iteration and it should come from the new loop I wrote.

Correct way to handle this would be for whoever wants redrawn to set their NeedsDraw flag instead.

[tznind]

Oh - I see this is actually going to do just the flags because of the override on the method...

Still i dont think every invoke should force this flag.

If user code invokes it should set its own flag and only if it really does want redrawn

[BDisp]

I think the main problem is the legacy drivers already set the console size in the Init method before call the Begin method and the V2 drivers only do that in the first iteration in the Begin method.

[tig]

I fixed this already. Maybe it was just in my active PR.

I'll look later.

[tig]

I fixed this already. Maybe it was just in my active PR.

I'll look later.

Nope. I mis-remembered. This bug still exists in #4126.

[BDisp]

@tznind has right about some bug in the redraw by not clearing correctly draw flags.

[tig]

I fixed this already. Maybe it was just in my active PR.
I'll look later.

Nope. I mis-remembered. This bug still exists in #4126.

Oops. Was running an older pull on this machine. I did already fix this!

This is the fix, in View.cs:

[tig]

I recommend this (at line 248 of View.cs):

        // Force a layout each time a View is initialized
        // See: https://github.com/gui-cs/Terminal.Gui/issues/3951
        // See: https://github.com/gui-cs/Terminal.Gui/issues/4204
        Layout ();

[tig]

Actually, please add these comments as well:

        // Force a layout each time a View is initialized
        // See: https://github.com/gui-cs/Terminal.Gui/issues/3951
        // See: https://github.com/gui-cs/Terminal.Gui/issues/4204
        Layout ();

        // Complex layout scenarios (e.g. DimAuto and PosAlign) may require multiple layouts to be performed.
        // Thus, we call SetNeedsLayout() to ensure that the layout is performed at least once.
        SetNeedsLayout ();

[BDisp]

Thanks for your fix. I really didn't saw that before. It really fix the v2 driver refresh but not the constantly redrawing in the legacy drivers as @tznind related. In the v2 drivers this doesn't happens because they only rely on NeedsDraw and NeedsLayout and the legacy drivers rely on NeedsLayout and SubViewNeedsDraw. So, the problem with the legacy drivers is that SubViewNeedsDraw is always true at the end. I'm still trying find the cause by adding Debug.Assert in some places to more easily see what going on.

[tig]

Thanks for your fix. I really didn't saw that before. It really fix the v2 driver refresh but not the constantly redrawing in the legacy drivers as @tznind related. In the v2 drivers this doesn't happens because they only rely on NeedsDraw and NeedsLayout and the legacy drivers rely on NeedsLayout and SubViewNeedsDraw. So, the problem with the legacy drivers is that SubViewNeedsDraw is always true at the end. I'm still trying find the cause by adding Debug.Assert in some places to more easily see what going on.

Great. But you agree the fix I posted above fixes THIS issue (#4204)? Right?

[BDisp]

Great. But you agree the fix I posted above fixes THIS issue (#4204)? Right?

Yes, absolutely.

[BDisp]

The CI pass on my laptop using Windows and Linux. So I think this is a race condition.

[BDisp]

The CI pass on my laptop using Windows and Linux. So I think this is a race condition.

Running again the unit tests it really failed. I merged into this the #4209 PR which fix the SyncrhonizationContextTests.

[tig]

@BDisp - Integration tests are still failing.

[BDisp]

@BDisp - Integration tests are still failing.

I run now without errors. I think that is a racing condition where a collection is being modified in the \IntegrationTests\FluentTests\MenuBarv2Tests.cs:line 454 when executing "Application.Top!.Add (menuBar);" which will modify a collection.

[BDisp]

I think FluentTests are running in parallel which is causing different results sometimes.

Now passed without any change. So, what I said above has some sense.