Custom cursor images for all desktop platforms

[eero-lehtinen]

Issue: #3005

Hello.

There seems to be many PRs relating to this issue, but they don't include all platforms and for some reason lost steam. This PR again tries to make this feature happen, and does it for all desktop platforms (x11, wayland, macos, windows, web).

Use case

I think the best user of this feature and the reason I'm doing this is Bevy and game engines in general. There non laggy hardware cursors with custom images are very important. Game devs also like their PNGs so supporting platform native cursor files is not that important, but I guess could be added too.

UPDATE: API

Instead of the things below, we have chosen to make the simplest possible API, where we have a single function for setting the cursor, and it internally recreates whatever is needed to display the cursor every time.

API 1

At first I tried to make a very simple api (in branch custom-cursors) where the user "registers" cursors by calling a window method with a rgba vector and a u64 key. Then when the user want's to change the cursor, they would just use this key. This was simple and efficient because the window itself would handle making and deleting the cursors and switching between them.

This API didn't seem very rusty. I thought that there was a simpler api hiding in there so I tried to make another one, which is in this PR.

API 2

This PR contains a (simpler?) api that allows users to create their own platform specific cursors that are held by smart pointers and automatically deallocated on drop. This way cursors can be swapped by just setting a single smart pointer with no registration step. Update: I moved this implementation to custom-cursors2-old.

This works wonderfully with windows, macos and web, but linux is really annoying. First of all it seems that all linux cursors would need to support both x11 and wayland, because there is no way to know what kind of event loop they will be inserted into. Secondly there seems to be no way to use smart pointers with wayland, because it needs to have a reference to the wayland connection. So now on wayland we just use an rgba vector so the wayland buffer can be recreated every time the user want's to swap cursors. Linux really was a lot simpler and more efficient with API 1.

API Conclusion for Me

So in the end I think that API 1 was better because it's just more efficient and simpler on linux. We should probably make the CursorImage struct platform dependent so it could be extended by different platforms. Also maybe we could use some kind of better handle compared to just u64?

What do you guys think about these apis?

I chose this branch (API 2) to show my thought process, but we could also continue the other branch.

Scaling

Currently screen scaling isn't handled. Different platforms have different interactions.

• web: seems to upscale cursor automatically, kinda annoying
• x11: no scaling (meaning that if screen scale in increased, the cursor becomes smaller)
• wayland: no scaling. We own the buffer so upscaling is easy, but results in a blurry cursor
• windows: no scaling
• macos: my machine has no option for screen scaling so couldn't test :(

So in general we could solve this just by accepting an array of different sized images that could be used for different scales. Dunno what to do about the web and it's automatic scaling though.

Other Issues

• I guess memory gets leaked in API 1 for most platforms when the window is dropped and and only on wayland with API 2, but should be pretty easily fixable. Update: should be fixed.
• I'm not sure if I'm allowed to destroy wl_buffers in the way I currently do it because according to doc we would need to wait for some wl_buffer.release event, but I don't know much about that. Also seems to work without issues in practice.

Even if this PR doesn't make it, at least here I show how to create cursors for all platforms :).

Checklist

• Tested on all platforms changed
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[daxpedda]

@eero-lehtinen let me know if you need any help for the Web backend!

[eero-lehtinen]

@eero-lehtinen let me know if you need any help for the Web backend!

Thank you. I will continue working on this today and this week so I'll ask when things come up.

[daxpedda]

I think Web looks fine so far.

[daxpedda]

@eero-lehtinen would appreciate a rebase!

[eero-lehtinen]

@daxpedda Rebased

[kchibisov]

I appreciate your enthusiasm on this issue and thank you for the PR.

However I don't think I'll merge this approach in the current state, especially given how everything is more complex and inconsistent. But it's not like I'll do a release either even if you add those changes (unless someone really asks me to do that).

Like the API should be simple

pub fn set_cursor(Window, icon: CursorIcon { NamedCursorIcon, Custom }

And that's really all it should be, everything else is on the downstream when dealing with that. Yes it can do everything on its own just about fine. No special changes will be required for that on any platform no fighting with Wayland, because it already does that.

I'll also write Wayland part myself, since clearly you never wrote wayland stuff.

Besides, when writing a massive patches, please, do ask in #winit or on github issue, if we'd like to cope through review now, because otherwise your patch could stall until a good times.

It doesn't mean that the code will be discarded, because internally you're doing pretty much everything what should be done, it'll just be ported to a different API or you can port to the one I've suggested, where you just set cursor and don't do any registration, etc, because just setting it is more than enough (yes, client will just must reset it if they fill so).

[daxpedda]

The main problem seems to be how to handle multiple cursors without having to re-initialize them every time. Unfortunately to implement some Drop mechanic it seems to require a connection handler not only on both Linux backends. Web is pretty straightforward fortunately, but that is only one backend (the one I reviewed).

@eero-lehtinen I believe following what @kchibisov has said here, this PR should be refactored into just a simple function on Window that sets the cursor and that's it. Handling multiple cursors with proper Drop mechanic is a topic for a future PR.

---

Some thoughts on that future PR: I think probably the best thing we can do here is as we discussed above: creating the cursor would require EventLoopWindowTarget. The Drop mechanic could be implemented in two ways for backends that require it:

• The resulting cursor would hold a Weak to the internal connection and table and on Drop delete the cursor from there.
• Send an internal event that can schedule deleting the cursor, which would be more like garbage collection.

After we finish and merge the first PR I will open an issue where we can discuss the details of that and if it's desirable in the first place.

[kchibisov]

Handling multiple cursors with proper Drop mechanic is a topic for a future PR.

If you do that it'll work properly on Wayland, since wayland already draws custom cursors, they are just hidden, but it already does everything the same. The same with X11. So everything is properly dropped, etc in the way I said that because it's already how it works.

For the future I wouldn't abuse event loop for it, I'd just callback to client to provide a new shape when DPI changes, and clients should cache them somehow.

[kchibisov]

Just to make it clear.

Caching could be done internally by the backends, you don't need to do any of that. It also doesn't need to be efficient when it comes to dropping. The way to approach this PR would be:

1. Define custom cursor format which winit will accept
2. Have a simple method on a window to set this icon. Having it separate is fine if you want to make custom cursors into 0.29.x series.

Like doing anything more complex than that doesn't worth it. Also, for wayland stuff, just do regular sw rendering pipeline expected by it. Look at SCTK examples how the draw software buffers and just create a Shm pool. Like this https://github.com/Smithay/client-toolkit/blob/master/examples/simple_window.rs#L139 should be fine, and then share this pool with every window, like it's already done internally with SCTK.

You can look at pointer impl in it and on wayland-cursor crate.

[eero-lehtinen]

@kchibisov

I'll also write Wayland part myself, since clearly you never wrote wayland stuff.

It's true, I don't know much about wayland. I will do the best baseline implementation I can and you guys can edit it to be the way you like.

Besides, when writing a massive patches, please, do ask in #winit or on github issue, if we'd like to cope through review now, because otherwise your patch could stall until a good times.

Sorry. I just wanted a patch for myself and thought to post it here to see what you think. Anyway that didn't even work out because Bevy doesn't use 29 yet. I would have to port it to 28 to benefit from it right now.

It doesn't mean that the code will be discarded, because internally you're doing pretty much everything what should be done, it'll just be ported to a different API or you can port to the one I've suggested, where you just set cursor and don't do any registration, etc, because just setting it is more than enough (yes, client will just must reset it if they fill so).

I can port it to be that way. So basically always just a single function that always reallocates the cursor. My current solution tries to be more efficient but I guess creating a cursor even every frame won't be that taxing and will be simpler.

You can look at pointer impl in it and on wayland-cursor crate.

Thanks for the pointers. I think I was already looking at that when gobbling my solution together.

[kchibisov]

My current solution tries to be more efficient but I guess creating a cursor even every frame won't be that taxing and will be simpler.

You don't need to change cursor unless you change it every frame to something new, like when you're animating it. Once you set it, it's just being used and set back by winit when needed.

[eero-lehtinen]

You don't need to change cursor unless you change it every frame to something new, like when you're animating it. Once you set it, it's just being used and set back by winit when needed.

Some games might want to change it every frame or at least call the function to make sure if there is no way to query what the current cursor is.

[kchibisov]

Once you set the cursor it's winit job to keep it the same. Programs doing stupid things should get what they ask for.

[eero-lehtinen]

I don't think we disagree. The use case of changing custom cursors often exists and could be optimized for, but not in this pr. Edit: Or it is optimized currently, but I'll remove it as planned.

[kchibisov]

I mean, you surely can try to optimize, but you don't need to expose any of that to the user. Like you can't drop cursors easily on all platforms (you can't on Wayland, you either append or purge all of them).

[kchibisov]

@eero-lehtinen I've fixed some style issue, but it should be good.

[daxpedda]

@eero-lehtinen thank you for your patience in seeing this through! Great work!

[eero-lehtinen]

Thanks for the help!