Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Application views do not refresh when the cursor is moved. #2860

Closed
kacperpikacz opened this issue Sep 18, 2023 · 31 comments · Fixed by #2848
Closed

Application views do not refresh when the cursor is moved. #2860

kacperpikacz opened this issue Sep 18, 2023 · 31 comments · Fixed by #2848

Comments

@kacperpikacz
Copy link

Describe the bug
Application views do not refresh when the cursor is moved. Affected versions are all above 1.9.0

Screenshots
bug

To Reproduce

using Terminal.Gui;

namespace Test
{
    public class Program
    {
        static readonly List<string> GlobalList = new() { "1" };
        static readonly ListView GlobalListView = new() { Width = Dim.Fill(), Height = Dim.Fill() };
        static object? _token = null;

        static void Main()
        {

            Application.Init();

            var startButton = new Button("Start");
            var stopButton = new Button("Stop") { Y = 1 };
            var container = new View() { X = Pos.Center(), Y = Pos.Center(), Width = 8, Height = 8, ColorScheme = Colors.Error };
            
            startButton.Clicked += Start;
            stopButton.Clicked += Stop;

            GlobalListView.SetSource(GlobalList);
            container.Add(GlobalListView);

            Application.Top.Add(container);
            Application.Top.Add(startButton, stopButton);
            Application.Run();

            return;
        }

        private static void Start()
        {
            _token = Application.MainLoop.AddTimeout(TimeSpan.FromMilliseconds(100), Add);
        }

        private static void Stop()
        {
            Application.MainLoop.RemoveTimeout(_token);
        }

        private static bool Add(MainLoop mainLoop)
        {
            GlobalList.Add(new Random().Next(100).ToString());
            GlobalListView.MoveDown();
            
            return true;
        }
    }
}


Expected behavior
Application views should refresh

Desktop
MacOS Ventura

@BDisp
Copy link
Collaborator

BDisp commented Sep 18, 2023

I tested it on macOS Big Sur hosted on Windows 11 with VirtualBox and it's working well. Unfortunately it isn't possible install macOS Ventura on a Windows 11 with VirtualBox and so I can't test it.

@kacperpikacz
Copy link
Author

Anyone else with Mac?

@BDisp
Copy link
Collaborator

BDisp commented Sep 19, 2023

@kacperpikacz can you run your code with the PR #2848, please. I think it is unrelated because is an issue that affect the SynchronizationContext but worth to try.

@kacperpikacz
Copy link
Author

@BDisp nothing changed after running code with #2848

@BDisp
Copy link
Collaborator

BDisp commented Sep 20, 2023

I already suspect that. The fix was for threading issue. I hope someone else with macOS Ventura can confirm that, because with Big Sur the timer is always refreshing the screen.

@TylerReid
Copy link

@BDisp I am on Ventura, and after applying your changes the UICatalog threading example works now. Before I had to move my mouse or do other inputs in order to get the async tasks to start, but now it works as expected.

@BDisp
Copy link
Collaborator

BDisp commented Sep 21, 2023

@BDisp I am on Ventura, and after applying your changes the UICatalog threading example works now. Before I had to move my mouse or do other inputs in order to get the async tasks to start, but now it works as expected.

Thanks for your answer, although this should be witted on the #306.
In this issue I ask, as you are on Ventura, if you can run the example code above and confirm if you have the same behavior as @kacperpikacz meant on this issue, please.

@TylerReid
Copy link

@BDisp Ah, I misunderstood. Your change did not fix this issue:

Screen.Recording.2023-09-21.at.12.48.30.PM.mov

@BDisp
Copy link
Collaborator

BDisp commented Sep 21, 2023

@TylerReid thanks very much. My fix was only for the threading issue and this one is different. It doesn't use no thread at all and if we continuously move the mouse, the console isn't refreshed. I don't have a mac, but only Big Sur installed on Oracle VM VirtualBox and this behavior doesn't happens. Due this limitation I can't find what is causing this behavior on Ventura. I already tried to install it on VirtualBox without success. It seems that now isn't possible to install it on a non-mac machine.

@BDisp
Copy link
Collaborator

BDisp commented Sep 21, 2023

@kacperpikacz and @TylerReid please run the Mouse scenario of the UICatalog and see the output for the mouse move event. Perhaps it's returning some other event. Thanks.

@TylerReid
Copy link

@BDisp I see a steady stream of ReportMousePosition events getting reported when I move in the same way.

@BDisp
Copy link
Collaborator

BDisp commented Sep 21, 2023

@BDisp I see a steady stream of ReportMousePosition events getting reported when I move in the same way.

And when the mouse is continuously moved do the report still refreshing or not?

@TylerReid
Copy link

Yes, the report list refreshes with no pauses

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 22, 2023

The problem is laying somewhere inside ProcessInput method from CursesDriver. In 1.9 there was no GetEscSeq method. Also when moving cursor slowly the problem is not present.

Commenting Curses.timeout (10) in ProcessInput will help but will not solve the problem completely.

Maybe #2375

v1.14

terminal.mov

v1.9

terminal.2.mov
using Terminal.Gui;

namespace Test
{
    public class Program
    {
        static readonly List<string> GlobalList = new() { "1" };
        static readonly ListView GlobalListView = new() { Width = Dim.Fill(), Height = Dim.Fill() };

        static Label CounterLabel = new();
        static Label BlinkingLabel = new();

        static int Counter = 0;

        static object? _listToken = null;
        static object? _blinkToken = null;
        static object? _countToken = null;

        static void Main()
        {
            Application.Init();

            var startButton = new Button("Start");
            var stopButton = new Button("Stop") { Y = 1 };
            var container = new View() { X = Pos.Center(), Y = Pos.Center(), Width = 8, Height = 8, ColorScheme = Colors.Error };

            CounterLabel = new Label("0") { X = container.X - 4, Y = container.Y - 6};
            BlinkingLabel = new Label("Blink") { X = CounterLabel.X, Y = CounterLabel.Y + 11 };

            startButton.Clicked += Start;
            stopButton.Clicked += Stop;

            GlobalListView.SetSource(GlobalList);
            container.Add(GlobalListView);

            Application.Top.Add(container, CounterLabel, BlinkingLabel);
            Application.Top.Add(startButton, stopButton);
            Application.Run();

            return;
        }

        private static void Start()
        {
            _listToken = Application.MainLoop.AddTimeout(TimeSpan.FromMilliseconds(100), Add);
            _blinkToken = Application.MainLoop.AddTimeout(TimeSpan.FromMilliseconds(1000), Blink);
            _countToken = Application.MainLoop.AddTimeout(TimeSpan.FromMilliseconds(1000), Count);
        }

        private static void Stop()
        {
            Application.MainLoop.RemoveTimeout(_listToken);
            Application.MainLoop.RemoveTimeout(_blinkToken);
            Application.MainLoop.RemoveTimeout(_countToken);
        }

        private static bool Add(MainLoop mainLoop)
        {
            GlobalList.Add(new Random().Next(100).ToString());
            GlobalListView.MoveDown();
            
            return true;
        }

        private static bool Blink(MainLoop mainLoop)
        {
            Application.MainLoop.Invoke(() =>
            {
                if (BlinkingLabel.Visible)
                {
                    BlinkingLabel.Visible = false;
                    System.Diagnostics.Debug.WriteLine(BlinkingLabel.Visible);
                }
                else
                {
                    BlinkingLabel.Visible = true;
                    System.Diagnostics.Debug.WriteLine(BlinkingLabel.Visible);
                }
                
            });

            return true;
        }

        private static bool Count(MainLoop mainLoop)
        {
            Application.MainLoop.Invoke(() =>
            {
                Counter++;
                CounterLabel.Text = Counter.ToString();
            });
            
            return true;
        }
    }
}

BDisp added a commit to BDisp/Terminal.Gui that referenced this issue Sep 22, 2023
@BDisp
Copy link
Collaborator

BDisp commented Sep 22, 2023

Commenting Curses.timeout (10) in ProcessInput will help but will not solve the problem completely.

This is necessary for waiting a while after a Esc key is detected to check if it was pressed alone or if it's part of a escape sequence, like Key.Home or mouse event, etc.

Well, I think the culprit is the poll method waiting infinitely if pollTimeout=-1 until some action is detected. I did the fix in the #2848.
I also did some change to your code:
Avoid initialize a view twice because if you use Debug.Assert with Responder.Instances to validate all instances it will fail. So, only declare the fields or initialize with all properties or initialize only with new(); and setting the properties in the Main().

        static Label CounterLabel;
        static Label BlinkingLabel; 

You don't need to use the return; in a void Main. Instead replace it with Application.Shutdown();

Also for accurating that the timeout action is running on the UI thread, change the Add method to call the Application.MainLoop.Invoke:

	private static bool Add(MainLoop mainLoop)
	{
		Application.MainLoop.Invoke(() => {
			GlobalList.Add(new Random().Next(100).ToString());
			GlobalListView.MoveDown();
		});

		return true;
	}

I appreciate you can run to check if the issue still persist or not. If it's fix I'll also do a commit to the v2 as well. Thanks.

@BDisp
Copy link
Collaborator

BDisp commented Sep 22, 2023

You also should avoid setting the view position by using the view.X or view.Y because they will only have the correct value after running and you are only playing with fixed values until achieve the correct values. The correct way to position them is using the Pos.X(View) and Pos.Y(View) which the are dynamically calculate on the LayoutSubviews method:

CounterLabel = new Label ("0") { X = Pos.X (container), Y = Pos.Y (container) - 2 };
BlinkingLabel = new Label ("Blink") { X = Pos.X (container), Y = Pos.Bottom (container) + 1 };

BDisp added a commit to BDisp/Terminal.Gui that referenced this issue Sep 22, 2023
@kacperpikacz
Copy link
Author

After running with #2848, I see no more problem present. Good job @BDisp

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 23, 2023

Sorry, I've used wrong source. The problem is still present plus problem with resizing appear.

terminal3.mov

@BDisp
Copy link
Collaborator

BDisp commented Sep 23, 2023

But are you sure you are running with the #2848? You'll have to debug that and try to find the cause because I can't. Loo at the IMainLoopDriver.EventsPending of the UnixMainLoop.cs. The fixes I did was for CursesDriver which freeze on the var n = poll (pollmap, (uint)pollmap.Length, pollTimeout);.

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 23, 2023

Yes, i can confirm that after running on #2848 the problem is still present + resizing terminal will cause complete freeze of the app.

After implementing ConsoleDriver and CursesDriver from 1.9 the problem is not present.

Maybe it has something with #2375

@BDisp
Copy link
Collaborator

BDisp commented Sep 23, 2023

Yes it's reproducing in macOS Monterey. The resizing terminal issue didn't happens before I had commit the latest changes?

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 23, 2023

No issue with resizing before.

And only pollTimeout = 0 will help when you exit the cursor from terminal. The app will continue to refresh the views.

I will also try to reproduce it on Intel Ventura, currently im on Silicon.

@BDisp
Copy link
Collaborator

BDisp commented Sep 23, 2023

Thanks, can you test the threading scenario, please?

@BDisp
Copy link
Collaborator

BDisp commented Sep 23, 2023

On resizing in the CursesDriver.cs on the ProcessInput method this var code = Curses.get_wch (out wch); is returning code = -1 which is equal to Curses.ERR and wch = 0. That why the ProcessWinChange method doesn't run. On Linux this doesn't happens at all. I have to do some workaround on this.

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 24, 2023

Threading scenario, when I don't move cursor the return will never happen. Tested on develop, no issue, with #2848

threading.mov

@kacperpikacz
Copy link
Author

kacperpikacz commented Sep 24, 2023

Latest commit, fixes issue with resizing. But still the views will not refresh when moving cursor fast.

void IMainLoopDriver.Setup (MainLoop mainLoop)
{
	this.mainLoop = mainLoop;
	pipe (wakeupPipes);
	AddWatch (wakeupPipes [0], Condition.PollIn, ml => {
+		read (wakeupPipes [1], ignore, (IntPtr)1);
		return true;
	});
}

public override void Refresh ()
{
	Curses.raw ();
	Curses.noecho ();
	Curses.refresh ();
+	ProcessWinChange ();
}

@kacperpikacz
Copy link
Author

Running on #2848. Inside GetEscSeq()method there is code = Curses.get_wch(out wch2) which returns -1.

code.mov

@Likarion
Copy link

Under SSH, the terminal mouse drop into the "SELECT " state and mouse click don't working. When install 1.9.0 all works ok.

@tig tig closed this as completed in #2848 Sep 29, 2023
@kacperpikacz
Copy link
Author

The bug is still not fixed completly.

@BDisp
Copy link
Collaborator

BDisp commented Oct 1, 2023

The bug is still not fixed completly.

@kacperpikacz you are right. There still some more issue. But the related with this issue is already fix. Instead of reopening this I'll use the #2865, which is more related with the resize issue. Thanks.

@kacperpikacz
Copy link
Author

Can someone take a look at this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants