Implement HiDPI cursors on Wayland

[TheDaemoness]

NOTE FOR PEOPLE FROM THE FUTURE: The commits for this PR have been merged outside of Github.

Free time is a fickle thing, isn't it?

These commits implement properly-scaled cursors on Wayland compositors with HiDPI support, addressing #2647/#2669. It implements a reference-counting manager for cursor themes, and changes some of the Wayland-specific platform code to use the new manager to acquire themes with the required pixel sizes.

I tried to be as surgical as possible in my implementation, which lead to a couple of questionable design choices that I'd prefer to be upfront about:

• The semantics of the cursor variable in _GLFWcursorWayland have changed. NULL no longer means that buffer and its associated variables should be used instead. Instead, the scale variable stores the relevant information (see the attached comment). I've tried to find the spots where this would be an issue, but won't claim it's impossible that I've overlooked something.
• Actually loading a default cursor is delayed in most cases until setCursorImage gets called. Loading can fail, and setCursorImage handles this quietly, which in the worst case may result in a null pointer dereference. If you folks have a preference on what way this failure should be handled instead, please let me know. As a tangent, setting XCURSOR_THEME to an invalid value doesn't produce this issue for me (on sway).
• Managed cursor themes are pointed to by each window instance, not each monitor instance. This is probably the smallest problem of the bunch, especially given the possibilities for per-window custom cursors that this opens up.

[kovidgoyal]

I'll review when I have some of the fickle free time :) But right off the bat, what happens currently if loading a theme fails? To me the ideal behavior would be to fallback to the "hidpi" theme which IIRC is supposed to be always present, and if that also fails either to use a deault image or to abort with an error message. The last ight be problematic given that theme loading no longer happens at init time. Might be best to just store a default cursor image as a C header and use that as final fallback.

[TheDaemoness]

So in regards to actual theme loading, that's handled by wl_cursor_theme_load, which already has fallbacks for most situations that aren't allocation failures.

Sadly, the fallback for xcursor theme loading (EDIT: which would get used if the XCURSOR_THEME environment variable exists but is set to an invalid value) doesn't appear to be HiDPI. Moreover, there doesn't seem to be a good way to test if that theme is in use:

• wl_cursor_theme pointers are opaque.
• wl_cursor_theme_load doesn't set errno or have any way of reporting an error other than returning NULL.
• I can't find a function to extract the theme size from a wl_cursor_theme struct, much less confirm if it's the default fallback theme or not.

[kovidgoyal]

I have merged, but I have a couple of questions.

1. When scale is changed, you load a new cursor theme, but you dont invalidate the actual cursor. So how does the rendered cursor change?

2. in _glfwSetPlatforCursor when no cursor is specified, the default arrow cursor is set, however this is done with a stack based variable. If the cursor happens to be animated this will cause the timer to fire calling incrementCursorImage which in turn will do nothing but reset the timer to call itself again and again, without end.

[kovidgoyal]

Not to mention that the result of glfwloadcursor leaks in (2) above

[kovidgoyal]

Also in setCursorImage cursorWayland never has its scale set to be window->wl.scale

[kovidgoyal]

Not to mention that the result of glfwloadcursor leaks in (2) above

On further reading, it seems the result of wl_cursor_theme_get_cursor() does not need to be destroyed, so no leak there.

[TheDaemoness]

Closing the PR as the commits have been merged outside of it, but will still watch for discussion. Thank you!

As you've noted, wl_cursor_theme_get_cursor is effectively an accessor for data in wl_cursor_theme.

With respect to scale changes, there is a brief period where the cursor pointers might dangle after the scale gets changed. However, setCursorImage is called soon after a scale change, if not immediately. The cursor image does change when I change the scale of the display, I've not been able to confirm an invalid read that happens as a result (EDIT: assuming the scale variable issue is fixed), but it still might be a good idea to call setCursorImage in the same functions where checkScaleChange returns true anyway.

With respect to the behavior of _glfwPlatformSetCursor, the use of a stack-allocated _GLFWcursorWayland was existing behavior. It could probably be changed, though it's worth examining first how much code outside of wl_window.c relies on that particular behavior.

Also in setCursorImage cursorWayland never has its scale set to be window->wl.scale

I know I had that line at one point in the if (newCursor != NULL) statement, but must have deleted it by accident. Sorry!

[kovidgoyal]

I have committed code to fix both the stack cursor animation issue and to change the cursor image immediately on scale change. Please test.

[TheDaemoness]

The changed code appears to work! Cursors animate properly (including the arrow cursor in grabbed contexts), and scale changes work as before. ASAN builds don't report any new leaks related to the cursor code either.