-
Notifications
You must be signed in to change notification settings - Fork 791
Add support for setting custom bitmap cursors #10270
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
base: master
Are you sure you want to change the base?
Conversation
18350b2 to
80b3462
Compare
80b3462 to
1768401
Compare
| size: Cell<PhysicalSize>, | ||
| pub ime_requests: RefCell<Vec<InputMethodRequest>>, | ||
| pub mouse_cursor: Cell<i_slint_core::items::MouseCursor>, | ||
| pub mouse_cursor: RefCell<i_slint_core::items::MouseCursor>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not happy about this change (and the sibling one in headless) but it's the only way I got it to work, since MouseCursor is no longer Copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just for tests anyway so it's fine.
Maybe the tests would be a bit nicer if there was a pub fn mouse_cursor(&self)->MouseCursor { self.mouse_cursor.borrow().clone() } getter function.
(And in headless this isn't even used, so we could just remove it)
cf4124f to
d50c6ac
Compare
|
There is still some stuff in the TODO like documentation, but now that it works I would like some input on:
|
|
@redstrate There are quite a few people on vacation at the moment so replies/help might be slow till 2026 rolls around. |
Right now you can set the mouse cursor shape in a TouchArea to a pre-defined list of cursor shapes, but sometimes you have your own custom cursor set for actions that aren't in the CSS spec. This change introduces new API to set a mouse cursor to an Image, using the same property (mouse-cursor) and a changeable hotspot.
|
|
||
| // Custom cursor macro | ||
| TouchArea { | ||
| mouse-cursor: MouseCursor.custom(@image-url("cursor.png"), 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed something while testing that it won't allow things like:
// Custom cursor macro from image property
TouchArea {
in property <image> custom-cursor;
mouse-cursor: MouseCursor.custom(custom-cursor, 0, 0);
}Maybe this is just a limitation of macros?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ideally should be allowed. What was the error?
d50c6ac to
e4c102f
Compare
ogoffart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is great work so far.
I'm on vacation at the moment but I just had a look over this PR anyway.
One thing I wonder is what to do with regard to scale factor. Especially when rendering SVG, we should probably take it into account.
| /// A mouse cursor. | ||
| MouseCursor(i_slint_core::items::MouseCursor) = 15, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The i_slint_core::MouseCursor is still private API so it should have #[doc(hidden)]
| size: Cell<PhysicalSize>, | ||
| pub ime_requests: RefCell<Vec<InputMethodRequest>>, | ||
| pub mouse_cursor: Cell<i_slint_core::items::MouseCursor>, | ||
| pub mouse_cursor: RefCell<i_slint_core::items::MouseCursor>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just for tests anyway so it's fine.
Maybe the tests would be a bit nicer if there was a pub fn mouse_cursor(&self)->MouseCursor { self.mouse_cursor.borrow().clone() } getter function.
(And in headless this isn't even used, so we could just remove it)
|
|
||
| // Custom cursor macro | ||
| TouchArea { | ||
| mouse-cursor: MouseCursor.custom(@image-url("cursor.png"), 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ideally should be allowed. What was the error?
| TouchArea { | ||
| mouse-cursor: MouseCursor.custom(); | ||
| // > <error{Not enough arguments} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also try:
mouse-cursor: MouseCursor.custom; (not called)
mouse-cursor: custom(...) should work without the MouseCursor.
mouse-cursor: custom(@image-url("cursor.png", 0, 0, 0)); too many args.
| NeswResize, | ||
| /// Bidirectional resize north-west-south-east. | ||
| NwseResize, | ||
| /// Custom cursor from an `Image`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should document what the hotspot is and relative to what.
| #[non_exhaustive] | ||
| #[repr(C, u32)] | ||
| #[derive(Debug, Clone, PartialEq, Default)] | ||
| pub enum MouseCursor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when this was in the enums.rs macro, it got also automatic documentation in there: https://docs.slint.dev/latest/docs/slint/reference/gestures/toucharea/#mouse-cursor
Now, the documentation need to be written manually and kept in sync.
I wonder if it wouldn't be good to put that enum in i_slint_common, so it can be enumerated to generate the docs and avoid repetition in lookup.rs.
| NsResize, | ||
| NeswResize, | ||
| NwseResize, | ||
| CustomCursor(ImageReference, i32, i32), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be Expression so the image and hotspot can be dynamic.
| CustomCursor(ImageReference, i32, i32), | |
| CustomCursor{img: Expression, hotspot_x: Expression, hotspot_y: Expression}, |
| ) | ||
| } | ||
| } | ||
| Expression::MouseCursor(MouseCursor::Default) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we would used strnum macros to simplify this. (same in rust)
| MouseCursor::NwseResize => Some(winit::window::CursorIcon::NwseResize), | ||
| MouseCursor::CustomCursor { image, hotspot_x, hotspot_y } => { | ||
| if let Some(rgba8) = image.to_rgba8() { | ||
| let rgba_vec = rgba8.as_slice().to_vec(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why allocate a vector if we're going to allocate another vector in the next line?
Right now you can set the mouse cursor shape in a TouchArea to a
pre-defined list of cursor shapes, but sometimes you have your own
custom cursor set for actions that aren't in the CSS spec.
This change introduces new API to set a mouse cursor to an Image, using
the same property (mouse-cursor) and a changeable hotspot.
Example
TODO
Link to existing issue/discussion