From e8cd12daf48f1e3f6108a83fa3bcf10e02b4c246 Mon Sep 17 00:00:00 2001 From: mgi388 <135186256+mgi388@users.noreply.github.com> Date: Tue, 28 Jan 2025 16:49:46 +1100 Subject: [PATCH] Automatically transform cursor hotspot user asks to flip cursor image (#17540) # Objective - As discussed in https://github.com/bevyengine/bevy/issues/17276#issuecomment-2611203714, we should transform the cursor's hotspot if the user is asking for the image to be flipped. - This becomes more important when a `scale` transform option exists. It's harder for users to transform the hotspot themselves when using `scale` because they'd need to look up the image to get its dimensions. Instead, we let Bevy handle the hotspot transforms and make the `hotspot` field the "original/source" hotspot. - Refs #17276. ## Solution - When the image needs to be transformed, also transform the hotspot. If the image does not need to be transformed (i.e. fast path), no hotspot transformation is applied. ## Testing - Ran the example: `cargo run --example custom_cursor_image --features=custom_cursor`. - Add unit tests for the hotspot transform function. - I also ran the example I have in my `bevy_cursor_kit` crate, which I think is a good illustration of the reason for this PR. - In the following videos, there is an arrow pointing up. The button hover event fires as I move the mouse over it. - When I press `Y`, the cursor flips. - In the first video, on `bevy@main` **before** this PR, notice how the hotspot is wrong after flipping and no longer hovering the button. The arrow head and hotspot are no longer synced. - In the second video, on the branch of **this** PR, notice how the hotspot gets flipped as soon as I press `Y` and the cursor arrow head is in the correct position on the screen and still hovering the button. Speaking back to the objective listed at the start: The user originally defined the _source_ hotspot for the arrow. Later, they decide they want to flip the cursor vertically: It's nice that Bevy can automatically flip the _source_ hotspot for them at the same time it flips the _source_ image. First video (main): https://github.com/user-attachments/assets/1955048c-2f85-4951-bfd6-f0e7cfef0cf8 Second video (this PR): https://github.com/user-attachments/assets/73cb9095-ecb5-4bfd-af5b-9f772e92bd16 --- crates/bevy_winit/src/cursor.rs | 11 ++-- crates/bevy_winit/src/custom_cursor.rs | 76 ++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/crates/bevy_winit/src/cursor.rs b/crates/bevy_winit/src/cursor.rs index 2dbcb12d7ca81..6e601f24294d8 100644 --- a/crates/bevy_winit/src/cursor.rs +++ b/crates/bevy_winit/src/cursor.rs @@ -8,7 +8,7 @@ use crate::{ use crate::{ custom_cursor::{ calculate_effective_rect, extract_and_transform_rgba_pixels, extract_rgba_pixels, - CustomCursorPlugin, + transform_hotspot, CustomCursorPlugin, }, state::{CustomCursorCache, CustomCursorCacheKey}, WinitCustomCursor, @@ -124,10 +124,13 @@ fn update_cursors( let (rect, needs_sub_image) = calculate_effective_rect(&texture_atlases, image, texture_atlas, rect); - let maybe_rgba = if *flip_x || *flip_y || needs_sub_image { - extract_and_transform_rgba_pixels(image, *flip_x, *flip_y, rect) + let (maybe_rgba, hotspot) = if *flip_x || *flip_y || needs_sub_image { + ( + extract_and_transform_rgba_pixels(image, *flip_x, *flip_y, rect), + transform_hotspot(*hotspot, *flip_x, *flip_y, rect), + ) } else { - extract_rgba_pixels(image) + (extract_rgba_pixels(image), *hotspot) }; let Some(rgba) = maybe_rgba else { diff --git a/crates/bevy_winit/src/custom_cursor.rs b/crates/bevy_winit/src/custom_cursor.rs index cc5854e7ed342..fe6538b915e3b 100644 --- a/crates/bevy_winit/src/custom_cursor.rs +++ b/crates/bevy_winit/src/custom_cursor.rs @@ -17,8 +17,14 @@ pub struct CustomCursorImage { /// An optional texture atlas used to render the image. pub texture_atlas: Option, /// Whether the image should be flipped along its x-axis. + /// + /// If true, the cursor's `hotspot` automatically flips along with the + /// image. pub flip_x: bool, /// Whether the image should be flipped along its y-axis. + /// + /// If true, the cursor's `hotspot` automatically flips along with the + /// image. pub flip_y: bool, /// An optional rectangle representing the region of the image to render, /// instead of rendering the full image. This is an easy one-off alternative @@ -29,6 +35,10 @@ pub struct CustomCursorImage { pub rect: Option, /// X and Y coordinates of the hotspot in pixels. The hotspot must be within /// the image bounds. + /// + /// If you are flipping the image using `flip_x` or `flip_y`, you don't need + /// to adjust this field to account for the flip because it is adjusted + /// automatically. pub hotspot: (u16, u16), } @@ -184,6 +194,28 @@ pub(crate) fn extract_and_transform_rgba_pixels( Some(sub_image_data) } +/// Transforms the `hotspot` coordinates based on whether the image is flipped +/// or not. The `rect` is used to determine the image's dimensions. +pub(crate) fn transform_hotspot( + hotspot: (u16, u16), + flip_x: bool, + flip_y: bool, + rect: Rect, +) -> (u16, u16) { + let hotspot_x = hotspot.0 as f32; + let hotspot_y = hotspot.1 as f32; + let (width, height) = (rect.width(), rect.height()); + + let hotspot_x = if flip_x { width - hotspot_x } else { hotspot_x }; + let hotspot_y = if flip_y { + height - hotspot_y + } else { + hotspot_y + }; + + (hotspot_x as u16, hotspot_y as u16) +} + #[cfg(test)] mod tests { use bevy_app::App; @@ -542,4 +574,48 @@ mod tests { 0, 255, 255, 255, // Cyan ] ); + + #[test] + fn test_transform_hotspot_no_flip() { + let hotspot = (10, 20); + let rect = Rect { + min: Vec2::ZERO, + max: Vec2::new(100.0, 200.0), + }; + let transformed = transform_hotspot(hotspot, false, false, rect); + assert_eq!(transformed, (10, 20)); + } + + #[test] + fn test_transform_hotspot_flip_x() { + let hotspot = (10, 20); + let rect = Rect { + min: Vec2::ZERO, + max: Vec2::new(100.0, 200.0), + }; + let transformed = transform_hotspot(hotspot, true, false, rect); + assert_eq!(transformed, (90, 20)); + } + + #[test] + fn test_transform_hotspot_flip_y() { + let hotspot = (10, 20); + let rect = Rect { + min: Vec2::ZERO, + max: Vec2::new(100.0, 200.0), + }; + let transformed = transform_hotspot(hotspot, false, true, rect); + assert_eq!(transformed, (10, 180)); + } + + #[test] + fn test_transform_hotspot_flip_both() { + let hotspot = (10, 20); + let rect = Rect { + min: Vec2::ZERO, + max: Vec2::new(100.0, 200.0), + }; + let transformed = transform_hotspot(hotspot, true, true, rect); + assert_eq!(transformed, (90, 180)); + } }