Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

Cursor (shape/image) handling #136

Open
DJMcNab opened this issue Aug 6, 2023 · 6 comments
Open

Cursor (shape/image) handling #136

DJMcNab opened this issue Aug 6, 2023 · 6 comments

Comments

@DJMcNab
Copy link
Member

DJMcNab commented Aug 6, 2023

Our current API for cursor shape is:

glazier/src/window.rs

Lines 390 to 397 in b4c505a

/// Set the cursor icon.
pub fn set_cursor(&mut self, cursor: &Cursor) {
self.0.set_cursor(cursor)
}
pub fn make_cursor(&self, desc: &CursorDesc) -> Option<Cursor> {
self.0.make_cursor(desc)
}

This is probably fine, but it is a little bit unclear - it should be better documented. Reading CursorDesc and Cursor helps slightly:

glazier/src/mouse.rs

Lines 21 to 50 in b4c505a

//NOTE: this currently only contains cursors that are included by default on
//both Windows and macOS. We may want to provide polyfills for various additional cursors.
/// Mouse cursors.
#[derive(Clone, PartialEq, Eq)]
pub enum Cursor {
/// The default arrow cursor.
Arrow,
/// A vertical I-beam, for indicating insertion points in text.
IBeam,
Pointer,
Crosshair,
#[deprecated(note = "this will be removed in future because it is not available on windows")]
OpenHand,
NotAllowed,
ResizeLeftRight,
ResizeUpDown,
// The platform cursor should be small. Any image data that it uses should be shared (i.e.
// behind an `Arc` or using a platform API that does the sharing).
Custom(backend::window::CustomCursor),
}
/// A platform-independent description of a custom cursor.
#[derive(Clone)]
pub struct CursorDesc {
// #[allow(dead_code)] // Not yet used on all platforms.
// pub(crate) image: ImageBuf,
#[allow(dead_code)] // Not yet used on all platforms.
pub(crate) hot: Point,
}

So, make_cursor creates a long-lived handle to a buffer stored in Cursor::Custom, and set_cursor sets the cursor to that buffer, or one of the specified presets.

The first problem is that this doesn't actually allow accepting a custom image. This was lost when we migrated from druid-shell.
I don't think we want to bring in a full dependency on something like image for this, so I'd be tempted to just support 32-bit ARGB buffers in a Vec<u8> in idk, right then down order? Whatever order makes sense, anyway.
Maybe adding a dependency for this is fine?

The second issue is that the set of default cursors is very small. There is (now?) a web spec we can follow, so we should probably just use that set.

@waywardmonkeys
Copy link
Contributor

I was going to bring up https://github.com/rust-windowing/cursor-icon ... so this seems like a good time to do that.

@jneem
Copy link
Collaborator

jneem commented Aug 6, 2023

Hm, they don't seem to support custom cursors? Also, they have a lot more variants than we do, but I remember that our variants were specifically chosen to be the ones that are available on all our platforms. Has the cursor availability changed on windows or macos recently?

@DJMcNab
Copy link
Member Author

DJMcNab commented Aug 6, 2023

The interoperability crate not having custom cursors is fine - we can have an intermediate type StandardOrCustom (or whatever).
Then set_cursor can be generic over impl Into<StandardOrCustom>

In terms of being available on all platforms, that is a reasonable concern - however, I think it's still correct to use this set. This is the least surprising set we could use, and conversion code from this set to the best option on each platform's exist elsewhere. E.g.
https://github.com/rust-windowing/winit/blob/master/src/platform_impl/windows/util.rs#L167

Seems like an office hours discussion!

@xStrom
Copy link
Member

xStrom commented Sep 7, 2023

#146 revealed that Cursor doesn't have Send due to HCURSOR. Future improvements to the cursor code could strive for Cursor to have Send.

Alternatively there should be more robust testing that Cursor consistently doesn't have Send on all platforms, like we have for Window.

@DJMcNab
Copy link
Member Author

DJMcNab commented Oct 12, 2023

I reached a potential solution yesterday - instead of requiring something like the image crate, we can "just" give a list of supported image/byte layouts (like wgpu does for textures), and require clients to provide the texture of that kind

@xStrom
Copy link
Member

xStrom commented Oct 12, 2023

Seems doable and it would still allow for us to have additional helper methods that get enabled when an optional image feature or similar is enabled.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants