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

Changes to the window's title appear to unhide the mouse cursor #1419

Closed
yorickpeterse opened this issue Jan 31, 2024 · 15 comments · Fixed by #1821
Closed

Changes to the window's title appear to unhide the mouse cursor #1419

yorickpeterse opened this issue Jan 31, 2024 · 15 comments · Fixed by #1821
Labels
needs-confirmation A reproduction has been reported, but the bug hasn't been confirmed or reproduced by a maintainer.

Comments

@yorickpeterse
Copy link
Collaborator

When mouse hiding is enabled, it seems that a change in Ghostty's title unhides the mouse. This can be reproduced easily using the following NeoVim config:

set title
set titlestring=%f

nmap h <C-w>h
nmap l <C-w>l

file Foo
vne
file Bar

Save this in repro.vim, then run the following:

nvim -u repro.vim

Now press l and h to cycle between the two windows, and note how the cursor is changing rapidly between the hidden and visible state:

Screencast.from.2024-01-31.01-02-25.webm
@mitchellh mitchellh added apprt/gtk needs-confirmation A reproduction has been reported, but the bug hasn't been confirmed or reproduced by a maintainer. labels Jan 31, 2024
@yorickpeterse
Copy link
Collaborator Author

To better understand what's going on, I made an attempt at disabling mouse unhiding as follows:

diff --git a/src/Surface.zig b/src/Surface.zig
index eb0c178e..bdba99c9 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -2814,7 +2814,7 @@ fn hideMouse(self: *Surface) void {
 fn showMouse(self: *Surface) void {
     if (!self.mouse.hidden) return;
     self.mouse.hidden = false;
-    self.rt_surface.setMouseVisibility(true);
+    // self.rt_surface.setMouseVisibility(true);
 }
 
 /// Perform a binding action. A binding is a keybinding. This function

While this addresses the issue highlighted in the video, there are still certain key sequences that unhide the mouse. Most notably, pressing : seems to unhide it. This is a little odd given the only reference to setMouseVisibility(true) is in showMouse. My guess is that something else triggers GTK (in this case) to momentarily show the cursor again, bypassing the above logic and the Surface.mouse.hidden state.

@yorickpeterse
Copy link
Collaborator Author

Similarly, with the following patch the cursor still displays when pressing ::

diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig
index 5ba615f9..6ed66dce 100644
--- a/src/apprt/gtk/Surface.zig
+++ b/src/apprt/gtk/Surface.zig
@@ -814,7 +814,7 @@ pub fn setMouseVisibility(self: *Surface, visible: bool) void {
     // which means to just use the parent value.
 
     if (visible) {
-        c.gtk_widget_set_cursor(@ptrCast(self.gl_area), self.cursor);
+        // c.gtk_widget_set_cursor(@ptrCast(self.gl_area), self.cursor);
         return;
     }

This further suggests some other sort of GTK state change results in the cursor being displayed again.

@em-dash
Copy link
Collaborator

em-dash commented Jan 31, 2024

Can confirm I have the same issue on GTK ghostty (on arch linux, X11 plasma). The mouse cursor unhides when changing the window title, and : in neovim will cause the mouse to briefly become visible.

@yorickpeterse
Copy link
Collaborator Author

Looking at the logs when pressing :, they're as follows:

debug(surface): changing mouse shape: terminal.mouse_shape.MouseShape.default
debug(surface): changing mouse shape: terminal.mouse_shape.MouseShape.text
debug(surface): changing mouse shape: terminal.mouse_shape.MouseShape.default
debug(surface): changing mouse shape: terminal.mouse_shape.MouseShape.text

Based on this I applied the following patch:

diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig
index 5ba615f9..161cc7ab 100644
--- a/src/apprt/gtk/Surface.zig
+++ b/src/apprt/gtk/Surface.zig
@@ -800,7 +800,7 @@ pub fn setMouseShape(
     errdefer c.g_object_unref(cursor);
 
     // Set our new cursor
-    c.gtk_widget_set_cursor(@ptrCast(self.gl_area), cursor);
+    // c.gtk_widget_set_cursor(@ptrCast(self.gl_area), cursor);
 
     // Free our existing cursor
     if (self.cursor) |old| c.g_object_unref(old);

This fixes the issue of pressing :, though toggling between the windows still unhides the cursor. Based on this, it seems that gtk_widget_set_cursor shows the cursor again if it was previously hidden.

@yorickpeterse
Copy link
Collaborator Author

This might be a better fix for the : issue:

diff --git a/src/Surface.zig b/src/Surface.zig
index eb0c178e..719718f7 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -788,6 +788,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
         },
 
         .set_mouse_shape => |shape| {
+            if (self.mouse.hidden) return;
             log.debug("changing mouse shape: {}", .{shape});
             try self.rt_surface.setMouseShape(shape);
         },

@yorickpeterse
Copy link
Collaborator Author

Looking more at the Surface.zig, the flickering seems to possibly be due to cursorPosCallback: the very first thing it does is display the cursor, followed by a bunch of work that seems to hide it again. This presents a small window during which the cursor is visible before it's hidden again.

Assuming I'm not wrong here, I think the showMouse call needs to be delayed until the final state is calculated, such that there's no flickering. I'll see what I can cook up.

@yorickpeterse
Copy link
Collaborator Author

The other cause of flickering appears to be the call to gtk_window_set_title. If I apply the following diff, the flickering is gone:

diff --git a/src/Surface.zig b/src/Surface.zig
index eb0c178e..719718f7 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -788,6 +788,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
         },
 
         .set_mouse_shape => |shape| {
+            if (self.mouse.hidden) return;
             log.debug("changing mouse shape: {}", .{shape});
             try self.rt_surface.setMouseShape(shape);
         },
diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig
index 5ba615f9..e9fe4ca3 100644
--- a/src/apprt/gtk/Surface.zig
+++ b/src/apprt/gtk/Surface.zig
@@ -735,10 +735,10 @@ fn updateTitleLabels(self: *Surface) void {
     }
 
     // If we have a window and are focused, then we have to update the window title.
-    if (self.container.window()) |window| {
-        const widget = @as(*c.GtkWidget, @ptrCast(self.gl_area));
-        if (c.gtk_widget_is_focus(widget) == 1) c.gtk_window_set_title(window.window, title.ptr);
-    }
+    // if (self.container.window()) |window| {
+    //     const widget = @as(*c.GtkWidget, @ptrCast(self.gl_area));
+    //     if (c.gtk_widget_is_focus(widget) == 1) c.gtk_window_set_title(window.window, title.ptr);
+    // }
 }
 
 pub fn setTitle(self: *Surface, slice: [:0]const u8) !void {

This is weird though, as I don't see why changing the title would unhide the cursor.

@yorickpeterse
Copy link
Collaborator Author

A workaround for the time being is to:

  1. Apply the patch from Changes to the window's title appear to unhide the mouse cursor #1419 (comment)
  2. Use ghostty --title=NeoVim to set a fixed title

yorickpeterse added a commit to yorickpeterse/dotfiles that referenced this issue Jan 31, 2024
@yorickpeterse
Copy link
Collaborator Author

To add to the above comment: the proposed workaround seems to cover about 90% of the flickering, but in certain cases the cursor is still unhidden.

@mitchellh mitchellh added the bug label Feb 1, 2024
@yorickpeterse
Copy link
Collaborator Author

I think I figured out where the remaining flickering is coming from: keyCallback ends up calling cursorPosCallback when a modifier is pressed. This function in turn displays the mouse if it's hidden. Upon returning from cursorPosCallback, the mouse is hidden again if it was hidden before.

Based on the comments in the code, it seems to be meant for cases where the cursor is hovering over something that would be a URL, and pressing Control/Shift/etc is supposed to highlight the URL. This strikes me as odd, because if the cursor is hidden then one probably first wants/has to move the cursor to the URL in the first place (based on the assumption one isn't likely to remember where the cursor is).

Or to put it plainly: I think Ghostty shouldn't show the cursor just because a modifier is pressed, even if the cursor is over a URL (i.e. you moved it to a URL, then hid it by typing).

@yorickpeterse
Copy link
Collaborator Author

An alternative variation, perhaps better in line with the behaviour when the cursor isn't hidden, is to only show the cursor if a modifier is pressed AND the cursor is over a URL.

@yorickpeterse
Copy link
Collaborator Author

I'm currently using the following hack:

diff --git a/src/Surface.zig b/src/Surface.zig
index 6a4ae770..a6e71a39 100644
--- a/src/Surface.zig
+++ b/src/Surface.zig
@@ -793,6 +793,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
         },
 
         .set_mouse_shape => |shape| {
+            if (self.mouse.hidden) return;
             log.debug("changing mouse shape: {}", .{shape});
             try self.rt_surface.setMouseShape(shape);
         },
@@ -1416,19 +1417,16 @@ pub fn keyCallback(
     // This handles the scenario where URL highlighting should be
     // toggled for example.
     if (!self.mouse.mods.equal(event.mods)) mouse_mods: {
-        // Usually moving the cursor unhides the mouse so we need
-        // to hide it again if it was hidden.
-        const rehide = self.mouse.hidden;
-
         // Update our modifiers, this will update mouse mods too
         self.modsChanged(event.mods);
 
-        // We set this to null to force link reprocessing since
-        // mod changes can affect link highlighting.
-        self.mouse.link_point = null;
-        const pos = self.rt_surface.getCursorPos() catch break :mouse_mods;
-        self.cursorPosCallback(pos) catch {};
-        if (rehide) self.hideMouse();
+        if (!self.mouse.hidden) {
+            // We set this to null to force link reprocessing since
+            // mod changes can affect link highlighting.
+            self.mouse.link_point = null;
+            const pos = self.rt_surface.getCursorPos() catch break :mouse_mods;
+            self.cursorPosCallback(pos) catch {};
+        }
     }
 
     // Process the cursor state logic. This will update the cursor shape if

This seems to resolve all flickering for me, and doesn't show the mouse cursor when you press Control, it's hidden, and a URL is or shows up under the mouse cursor. The downside is that the URL is still underlined, as I couldn't figure out how to get it to not do that.

The above is very much a hack, but the basic idea behind it is that if the mouse cursor is hidden, we shouldn't handle link highlighting (which triggers the flickering), nor should the mouse shape change when it's hidden.

@mitchellh
Copy link
Contributor

I'm finally looking into this a bit. I'm unable to reproduce the flickering in the keyCallback => cursorPosCallback chain. I even introduced a 5 second sleep between hiding and showing the mouse and nothing happens. It seems like -- at least for me -- GTK is not processing mouse visibility changes until the next tick of the GTK event loop. That makes sense to me. Are you sure this is still happening for you? If so, I wonder if it's a Wayland vs. X thing.

I can reproduce the flickering mouse on window/tab change.

@mitchellh
Copy link
Contributor

Various fixes in #1821.

Note that the cursor showing again when gtk_window_set_title is called appears to be an upstream issue. I tried in many ways to workaround this problem but couldn't find any way to make it work. I tried the obvious thing of reseting the cursor after the title changes but that didn't work, I think do to event loop things...

@yorickpeterse
Copy link
Collaborator Author

@mitchellh With #1821 merged it indeed seems the cursor now stays hidden properly. Title changes do indeed still result in the cursor being shown. My workaround for that is to just configure a fixed title for the terminals, as having a dynamically adjusted title isn't that important to begin with. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-confirmation A reproduction has been reported, but the bug hasn't been confirmed or reproduced by a maintainer.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants