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

Non-arrow cursors are offset from the hotspot in Wayland #1706

Closed
warisb opened this issue May 28, 2020 · 6 comments
Closed

Non-arrow cursors are offset from the hotspot in Wayland #1706

warisb opened this issue May 28, 2020 · 6 comments
Assignees
Labels
bug Bug reports and bugfix pull requests verified Reproduced or otherwise verified bugs Wayland
Milestone

Comments

@warisb
Copy link
Contributor

warisb commented May 28, 2020

Not sure if there is a bug already filed on this, but it is a bit hard to notice.

OS and version: Ubuntu 20.04 or 18.04 running "Ubuntu on Wayland" desktop environment
Release or commit: libglfw3-wayland/focal,now 3.3.2-1 amd64
Error messages: There are no error messages or unusual events, but the minimal app mentioned below can easily show the problem.

cursorswitch

Basically, when switching from standard arrow to I-beam, the I-beam does not seem to be aligned with the arrow's hotspot. The app allows changing the cursor shapes using the number keys.

#include <GLFW/glfw3.h>
#include <iostream>

using namespace std;

bool s_quit = false;

void keyCallback(GLFWwindow* window, int key, int scancode, int action, int mods)
{
    if (key == GLFW_KEY_Q && action == GLFW_PRESS)
    {
        s_quit = true;
    }
    if (key == GLFW_KEY_1)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_ARROW_CURSOR);
        glfwSetCursor(window, cursor);
    }
    if (key == GLFW_KEY_2)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_IBEAM_CURSOR);
        glfwSetCursor(window, cursor);
    }
    if (key == GLFW_KEY_3)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_CROSSHAIR_CURSOR);
        glfwSetCursor(window, cursor);
    }
    if (key == GLFW_KEY_4)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_HAND_CURSOR);
        glfwSetCursor(window, cursor);
    }
    if (key == GLFW_KEY_5)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_HRESIZE_CURSOR);
        glfwSetCursor(window, cursor);
    }
    if (key == GLFW_KEY_6)
    {
        auto cursor = glfwCreateStandardCursor(GLFW_VRESIZE_CURSOR);
        glfwSetCursor(window, cursor);
    }
}

void cursorPosCallback(GLFWwindow* window, double xpos, double ypos)
{
    cout << "Cursor position is: " << xpos << ", " << ypos << endl;
}


int main()
{
    glfwInit();
    glfwWindowHint(GLFW_VISIBLE, GLFW_TRUE);
    GLFWwindow* window = glfwCreateWindow(1920, 1080, "My Title", glfwGetPrimaryMonitor(), nullptr);
    if (!window)
    {
        cerr << "Can't create window!" << endl;
        glfwTerminate();
        return 1;
    }

    glfwSetKeyCallback(window, keyCallback);
    glfwSetCursorPosCallback(window, cursorPosCallback);

    while (!s_quit)
    {
        glfwWaitEvents();
    }

    glfwDestroyWindow(window);
    glfwTerminate();

    return 0;
}

Steps to Reproduce

  1. Compile the program above and link against the wayland version of libglfw3
  2. Run the program, observe the cursor
  3. Press 2 to change the cursor to an I-beam, observe its hotspot.
  4. Press 1 to change back to the arrow, observe its hotspot.

Expected behaviour
The I-beam's hotspot should align with the arrow's tip

Actual behaviour
The I-beam is offset from the arrow's tip. This seems to happen with other shapes as well. This does NOT happen in X11, so it seems to be a Wayland issue.

@linkmauve linkmauve added bug Bug reports and bugfix pull requests Wayland labels Jun 12, 2020
@elmindreda elmindreda added the verified Reproduced or otherwise verified bugs label Jun 24, 2020
@elmindreda
Copy link
Member

Successfully reproduced on Weston. We seem to be passing along sane hotspot coordinates, though.

@warisb
Copy link
Contributor Author

warisb commented Jun 24, 2020

It seems to me like the hotspot remains unchanged, but the bitmap (the cursor image) is offset. Is there a way to verify that?

@elmindreda
Copy link
Member

Based on some careful cursor wiggling, it seemed that at least in Weston like the reported cursor position didn't change when we changed to a different cursor, but the compositor treated all cursors as if their hotspots were 0,0 regardless of what we were passing in. I need to read a bit more to be sure GLFW is not doing something wrong.

@warisb
Copy link
Contributor Author

warisb commented Feb 4, 2021

I have been in contact with the wayland mailing list and based on their suggestion, I have reasons to believe the problem is somewhere within the GLFW codebase.

There is a program in weston called clickdot which traces the cursor position. A weston developer suggested that I try to modify the program to use an alternative cursor, and sure enough, the problem is not reproducible there.

Original message from the developer:

If you do this change in clickdot:
--- a/clients/clickdot.c
+++ b/clients/clickdot.c
@@ -240,7 +240,8 @@ motion_handler(struct widget *widget,
cursor_timeout_reset(clickdot);
clickdot->cursor_timeout_input = input;

  •   return CURSOR_BLANK;
    
  •   return CURSOR_BOTTOM;
    

}
does the hotspot move correctly to the middle bottom of the cursor
image? You can tell by looking at where the line gets drawn (it's a
simple drawing app).
If clickdot works ok, but your GLFW app does not, then there is still
some difference in the protocol sequence.
If clickdot has wrong hotspot, then it's something else. What backend
did you try Weston with? If DRM, how about X11 or Wayland backends?

This seems to be the case for us

If clickdot works ok, but your GLFW app does not, then there is still
some difference in the protocol sequence.

I will try to dig some more to see if I can find the root cause, but at least we know we should focus on GLFW.

@warisb
Copy link
Contributor Author

warisb commented Feb 4, 2021

One more interesting response from the weston developer:

The one difference I see compared to your trace is that weston demos first commit the surface and then set it as the pointer cursor.

which suggests some order-of-operation problem. However, I tried swapping the order:

--- a/src/wl_window.c
+++ b/src/wl_window.c
@@ -796,15 +796,16 @@ static void setCursorImage(_GLFWwindow* window,
         cursorWayland->yhot = image->hotspot_y;
     }
 
-    wl_pointer_set_cursor(_glfw.wl.pointer, _glfw.wl.serial,
-                          surface,
-                          cursorWayland->xhot / scale,
-                          cursorWayland->yhot / scale);
     wl_surface_set_buffer_scale(surface, scale);
     wl_surface_attach(surface, buffer, 0, 0);
     wl_surface_damage(surface, 0, 0,
                       cursorWayland->width, cursorWayland->height);
     wl_surface_commit(surface);
+
+    wl_pointer_set_cursor(_glfw.wl.pointer, _glfw.wl.serial,
+                          surface,
+                          cursorWayland->xhot / scale,
+                          cursorWayland->yhot / scale);
 }

with no success.

warisb added a commit to warisb/glfw that referenced this issue May 12, 2021
The wayland protocol spec
https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer

States that `set_cursor` must be called with the serial number of the
`enter` event. However, GLFW is passing in the serial number of the latest
received event, which does not meet the protocol spec.

As a result, `set_cursor` calls were simply ignored by the compositor.

This fix complies with the protocol more closely by specifically caching the
`enter` event serial, and using it for all `set_cursor` calls.

Fixes glfw#1706
warisb added a commit to warisb/glfw that referenced this issue May 12, 2021
The wayland protocol spec
https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer

States that `set_cursor` must be called with the serial number of the
`enter` event. However, GLFW is passing in the serial number of the latest
received event, which does not meet the protocol spec.

As a result, `set_cursor` calls were simply ignored by the compositor.

This fix complies with the protocol more closely by specifically caching the
`enter` event serial, and using it for all `set_cursor` calls.

Fixes glfw#1706
@warisb
Copy link
Contributor Author

warisb commented May 12, 2021

@elmindreda FYI I found the problem and put the fix up at #1899

@elmindreda elmindreda self-assigned this Aug 31, 2021
elmindreda pushed a commit that referenced this issue Oct 13, 2021
The Wayland protocol spec[1] states that set_cursor must be called
with the serial number of the enter event.  However, GLFW is passing in
the serial number of the latest received event, which does not meet the
protocol spec.

[1] https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer

As a result, set_cursor calls were simply ignored by the compositor.

This fix complies with the protocol more closely by specifically caching
the enter event serial, and using it for all set_cursor calls.

Fixes #1706
Closes #1899

(cherry picked from commit e7758c5)
@elmindreda elmindreda modified the milestones: 3.4, 3.3.5 Dec 1, 2021
swarnimarun pushed a commit to swarnimarun/glfw-meson that referenced this issue Jul 9, 2022
The Wayland protocol spec[1] states that set_cursor must be called
with the serial number of the enter event.  However, GLFW is passing in
the serial number of the latest received event, which does not meet the
protocol spec.

[1] https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer

As a result, set_cursor calls were simply ignored by the compositor.

This fix complies with the protocol more closely by specifically caching
the enter event serial, and using it for all set_cursor calls.

Fixes glfw#1706
Closes glfw#1899

(cherry picked from commit e7758c5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports and bugfix pull requests verified Reproduced or otherwise verified bugs Wayland
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants