Add custom cursor icon support on Windows #1617

Osspial · 2020-07-04T04:54:16Z

Tested on all platforms changed
Compilation warnings were addressed
cargo fmt has been run on this branch
cargo doc builds successfully
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

kchibisov · 2020-07-04T20:05:12Z

The PR mentions windows, however you're touching other backends and exposing a new API to some sort. So what's the buffer pixel on Windows(RGBA/ARGB)? On Wayland for example the buffer is in ARGB(the most used one), so it could mean that for custom cursor loading I should change it a bit(convert ARGB to RGBA). Also, maybe we should check what cursor buffer format is on majority platforms and just use it? I'd be interested myself in ARGB, but I guess the most used should be picked?

I also can add wayland implementation for such thing, I guess, but it's a bit obscure. And has some edges like scale factor and what user should provide. I'd assume that all HiDPI handling is up to user here, and it should just ensure that buffer is scaled properly.

Osspial · 2020-07-04T20:21:11Z

I needed to touch the other platforms because this PR makes CursorIcon non-Copy and non-Hash, and adds a new variant that the other platforms weren't handling. Right now, I'm having the other platforms treat custom cursors as the default cursor, which seems like the least intrusive way of ignoring the Custom variant.

The public API surface area for this is fairly small - we're just adding a Custom(Icon) variant to CursorIcon, and adding a new_with_hot_spot method to Icon. Icon's pixel buffer was already publicly formatted as RGBA, and this PR doesn't change that. Each individual backend is expected to convert that to the pixel format it needs, and on Windows, that's BGRA.

I also can add wayland implementation for such thing, I guess, but it's a bit obscure. And has some edges like scale factor and what user should provide. I'd assume that all HiDPI handling is up to user here, and it should just ensure that buffer is scaled properly.

@kchibisov That'd be great, thanks! On Windows, the OS doesn't do any automatic DPI scaling for the custom cursor, so the user would have to manually upload an appropriately-scaled version of the cursor when the scale factor changes. I'm personally okay with that approach, but it should definitely be documented.

kchibisov · 2020-07-09T22:04:53Z

note, I'll send Wayland impl later(as separate PR), since it'll actually require some rework of Wayland backend before doing so. Just saying so won't end up waiting on something.

…don't make sense This is the API design strategy recommended by the official Rust book: https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html#guidelines-for-error-handling

Osspial · 2020-07-10T02:33:30Z

I've thought some more about how icon scaling works, and I've changed my mind somewhat. I still don't think that Winit should do any implicit icon scaling beyond what the OS does, but it should provide an API for attaching multiple images at different scales for a single logical Icon. This is for a couple of reasons:

Windows ICO files and icon resources include multiple icon sizes in a single file, and if an Icon struct only contains an icon at a single scale (as it does today), users have to manually reload those icons when the scale changes. That's super counterintuitive.
The OS can use multiple window icon sizes at the same time. The current model doesn't support that usecase.
Windows changes the cursor size when the cursor changes monitor, not when the underlying window changes the scale factor. Expecting users to monitor the exact position of the cursor in a multiple-monitor setup to determine the cursor size is unreasonable, especially since not all backends give access to that information.

I'll update this PR to expose that API and implement it on Windows.

kchibisov · 2020-07-10T02:57:08Z

I'll just add that on Wayland client draws the cursor itself with the size and scale it wants. winit shouldn't care about size part on it, since sctk does that already for it, so only scale. Since scaling is done by clients, they should just set buffer scale and attach the proper buffer size (just physical size thing).

So multiple icons is softly required thing on Wayland (you can in theory draw with some big scale, and rely on compositor downscaling).

Windows changes the cursor size when the cursor changes monitor, not when the underlying window changes the scale factor. Expecting users to monitor the exact position of the cursor in a multiple-monitor setup to determine the cursor size is unreasonable, especially since not all backends give access to that information.

Same for Wayland, but since cursor is managed by users, and it's just a normal surface, you get scale changes events for it separately, and so can react and pick proper cursor for that scale, so not a big issue. So if winit wants some consistency across the platforms the cursor DPI scaling should be entirely done by winit, and user should just provide some set of images to pick from.

The OS can use multiple window icon sizes at the same time. The current model doesn't support that usecase.

Not sure how it's related to cursor related things?

Osspial · 2020-07-13T03:02:40Z

Not sure how it's related to cursor related things?

The current API uses the same type for custom window icons and custom cursor icons, so adding support in the public API for this helps improve both the cursor implementation and the window icon implementation.

src/icon.rs

CHANGELOG.md

src/icon.rs

kchibisov · 2020-07-14T01:16:50Z

src/icon.rs

+pub struct RgbaIcon<I: Deref<Target = [u8]>> {
+    pub(crate) rgba: I,
+    pub(crate) size: PhysicalSize<u32>,
+    pub(crate) hot_spot: PhysicalPosition<u32>,


I don't really like that we have a cursor and normal icons under the same RgbaIcon, in general, we can name it like RgbaBuffer, since the intent to have RgbaBuffer and have Icon and a new type for cursor icons, which will use that RgbaBuffer under the hood?

It's just strange that we have a field that shouldn't be used in any ways in certain scenarios, but in other cases it's fine to use them. Such issues should be solved on type system level.

That's a decent point. I've updated the API to split the custom window type and custom cursor type into two parts, though it's not documented yet. Could you take a look at it?

src/icon.rs

src/platform_impl/linux/x11/util/cursor.rs

Co-authored-by: Kirill Chibisov <[email protected]>

anlumo · 2020-08-20T23:09:48Z

This change removes the Send trait from winit::window::CursorIcon for Windows, which is kinda a big deal. Is there a way to get it back?

    = help: within `winit::platform_impl::platform::icon::RaiiIcon`, the trait `std::marker::Send` is not implemented for `*mut winapi::shared::windef::HICON__`
    = note: required because it appears within the type `winit::platform_impl::platform::icon::RaiiIcon`
    = note: required because of the requirements on the impl of `std::marker::Sync` for `std::sync::Arc<winit::platform_impl::platform::icon::RaiiIcon>`
    = note: required because it appears within the type `winit::platform_impl::platform::icon::CustomCursorIcon`
    = note: required because it appears within the type `winit::icon::CustomCursorIcon`
    = note: required because it appears within the type `winit::window::CursorIcon`

Osspial · 2020-09-16T23:20:15Z

This change removes the Send trait from winit::window::CursorIcon for Windows, which is kinda a big deal. Is there a way to get it back?

Yep! That was just an oversight - the implementation was written with Send and Sync in mind, but I forgot to mark the relevant types with those traits. That's fixed now.

daxpedda · 2023-08-23T13:46:20Z

Closing due to inactivity.

Tracked in #3005.

Osspial changed the title ~~Set cursor custom icon~~ Add custom cursor icon support on Windows Jul 4, 2020

Osspial added 15 commits July 9, 2020 20:38

Add ability to set custom cursor icon on Windows

ad3a1e0

Move to cross-platform API

0af0a12

Remove BadIcon in favor of io::Error and panicking when input values …

9c438b6

…don't make sense This is the API design strategy recommended by the official Rust book: https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html#guidelines-for-error-handling

Use PhysicalSize in icon creation fns

90b276c

Add cursor hotspot support

87b42d4

Try to address compilation errors off of Windows

1a8c250

Make compile on Linux, and Arc the pixel data on Linux

2bcb41a

Make the examples compile again

4506d5d

Fix linux wayland-only builds

72f4f23

Try to fix macos build

ab0931d

Fix web build

4e9b7cc

Format

23d398f

Don't print binary image data for RgbaIcon debug impl

962d44f

Add changelog entry

cade872

docs update

54646ae

Osspial force-pushed the set_cursor_custom_icon branch from 1923108 to 54646ae Compare July 10, 2020 00:41

Osspial added 4 commits July 12, 2020 14:52

First pass at scalable icon support

1fffadf

Support automatic icon scaling on Windows

3414511

Format

b7b8752

Try to fix macos and linux builds

8cef810

dhardy mentioned this pull request Jul 13, 2020

winit::MouseCursor broken ? need to customize the cursor. #1623

Open

kchibisov requested changes Jul 14, 2020

View reviewed changes

Update src/icon.rs

3c61b2a

Co-authored-by: Kirill Chibisov <[email protected]>

Osspial and others added 10 commits July 30, 2020 23:36

Update CHANGELOG.md

b402c2d

Co-authored-by: Kirill Chibisov <[email protected]>

Update src/icon.rs

17d6f2e

Co-authored-by: Kirill Chibisov <[email protected]>

Update src/icon.rs

da43221

Co-authored-by: Kirill Chibisov <[email protected]>

Update src/icon.rs

5409114

Co-authored-by: Kirill Chibisov <[email protected]>

Improve code quality of X11 custom cursor ignore

9ee8a78

Redesign cursor API and implement on Windows

4bea102

Make this compile on Linux

b21a92e

Try to fix macos build

186c03b

Attempt again

a030c6c

Format

058fcd7

raphlinus mentioned this pull request Sep 6, 2020

Custom cursor linebender/druid#1183

Merged

Implement both send and sync for RaiiIcon

9b7ea98

maroider added the DS - windows label May 7, 2021

rofferom mentioned this pull request Dec 28, 2021

Add RGBA cursor icon support on Windows/macOS/Linux #2117

Closed

8 tasks

madsmtm mentioned this pull request Aug 5, 2023

Custom cursor icons #3005

Closed

daxpedda closed this Aug 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom cursor icon support on Windows #1617

Add custom cursor icon support on Windows #1617

Osspial commented Jul 4, 2020 •

edited

Loading

kchibisov commented Jul 4, 2020 •

edited

Loading

Osspial commented Jul 4, 2020

kchibisov commented Jul 9, 2020 •

edited

Loading

Osspial commented Jul 10, 2020

kchibisov commented Jul 10, 2020 •

edited

Loading

Osspial commented Jul 13, 2020

kchibisov Jul 14, 2020

Osspial Aug 4, 2020

anlumo commented Aug 20, 2020

Osspial commented Sep 16, 2020

daxpedda commented Aug 23, 2023

Add custom cursor icon support on Windows #1617

Add custom cursor icon support on Windows #1617

Conversation

Osspial commented Jul 4, 2020 • edited Loading

kchibisov commented Jul 4, 2020 • edited Loading

Osspial commented Jul 4, 2020

kchibisov commented Jul 9, 2020 • edited Loading

Osspial commented Jul 10, 2020

kchibisov commented Jul 10, 2020 • edited Loading

Osspial commented Jul 13, 2020

kchibisov Jul 14, 2020

Choose a reason for hiding this comment

Osspial Aug 4, 2020

Choose a reason for hiding this comment

anlumo commented Aug 20, 2020

Osspial commented Sep 16, 2020

daxpedda commented Aug 23, 2023

Osspial commented Jul 4, 2020 •

edited

Loading

kchibisov commented Jul 4, 2020 •

edited

Loading

kchibisov commented Jul 9, 2020 •

edited

Loading

kchibisov commented Jul 10, 2020 •

edited

Loading