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

Custom cursor icon support #3039

Closed
wants to merge 9 commits into from

Conversation

valaphee
Copy link
Contributor

@valaphee valaphee commented Aug 22, 2023

  • 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

One possible solution for #3005, adds a way to set custom icons in winit using the existing Window::set_cursor_icon method.

At the moment this is only a draft, because not all major platforms are implemented/tested

  • iOS
  • X11
  • Wayland
  • MacOS
  • Web
  • Windows

I can't really test for Apple platforms because I don't own Apple devices.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that reusing Icon is a good idea, how does it map to the cursor icon used on the platforms? I'm interested wrt macOS/Web/Windows, since X11/Wayland use just argb/xrgb buffers for all of that.

Also, without hotspot it's all kind of useless.

@kchibisov
Copy link
Member

@valaphee
Copy link
Contributor Author

valaphee commented Aug 22, 2023

For web: you would have to use css and the "cursor" property, where you can specify a url, which can also be an base64 string, containing the png encoded cursor, but this is already how it works, its just a question of encoding the icon (rgba) to png and packing it into base64

For iOS: NSCursor has an instance method called initWithImage with NSImage and hotSpot, this would suggest Icon being of type NSImage, but cursor icon doesn't seem to implemented there? debug!("Window::set_cursor_icon ignored on iOS")

Good point, didn't thought about hotspot, would have to be platform-specific as for Windows and Web its part of the icon

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really feasible for Web like this.

Firstly PlatformIcon has to be split, because it's currently used for both cursor and window, in the case of Web window needs to continue using NoIcon but cursor now requires RgbaIcon.

Secondly adding the image dependency is overkill. I would argue that Web should support links before it starts supporting raw RGBA in the first place anyway. If we really want to support RGBA on Web then we should either use the png crate directly or use the WebAPI to do the conversion: ImageData -> ImageBitmap -> Canvas -> data URL.

Adding platform-specific formats shouldn't be a problem as they go through methods on Icon anyway.

@valaphee
Copy link
Contributor Author

I definitely agree with the image crate being overkill, and I'll split up Icon (as normal icons also don't need the hotspot property).

Using urls could definitely be an option, and for RGBA, I'll take a look into utilizing the web api.

@valaphee
Copy link
Contributor Author

Split-up Icon and CustomCursorIcon pub import, and switched to png crate, web also doesn't use RgbaImage and instead just the url repr.

src/icon.rs Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is the kinda stuff that goes into src/platform/web.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The platform-specific implementation at least for Windows is in the platform_impl module, and they are exposed in platform, by extending Icon

Basically just copied it from there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementations are usually in platform_impl indeed, but public platform-specific API is in platform.

@@ -36,6 +37,7 @@ pub(crate) use self::event_loop::{
pub use self::monitor::{MonitorHandle, VideoMode};
pub use self::window::{PlatformSpecificWindowBuilderAttributes, Window, WindowId};

pub(crate) use self::icon::WebIcon as PlatformCustomCursorIcon;
pub(crate) use self::keyboard::KeyEventExtra;
pub(crate) use crate::icon::NoIcon as PlatformIcon;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub(crate) use crate::icon::NoIcon as PlatformIcon;

@madsmtm madsmtm mentioned this pull request Nov 25, 2023
@madsmtm
Copy link
Member

madsmtm commented Dec 17, 2023

Superseded by #3218

@madsmtm madsmtm closed this Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants