Skip to content

Conversation

@mgi388
Copy link
Contributor

@mgi388 mgi388 commented Jan 3, 2025

Objective

  • Bevy 0.15 added support for custom cursor images in Add custom cursors #14284.
  • However, to do animated cursors using the initial support shipped in 0.15 means you'd have to animate the Handle<Image>: You can't use a TextureAtlas like you can with sprites and UI images.
  • For my use case, my cursors are spritesheets. To animate them, I'd have to break them down into multiple Image assets, but that seems less than ideal.

Solution

  • Allow users to specify a TextureAtlas field when creating a custom cursor image.
  • To create parity with Bevy's TextureAtlas support on Sprites and ImageNodes, this also allows users to specify rect, flip_x and flip_y. In fact, for my own use case, I need to flip_y.

Testing

  • I added unit tests for calculate_effective_rect and extract_and_transform_rgba_pixels.
  • I added a brand new example for custom cursor images. It has controls to toggle fields on and off. I opted to add a new example because the existing cursor example (window_settings) would be far too messy for showcasing these custom cursor features (I did start down that path but decided to stop and make a brand new example).
  • The new example uses a Kenny cursor icon sprite sheet. I included the licence even though it's not required (and it's CC0).
  • I decided to make the example just loop through all cursor icons for its animation even though it's not a realistic in-game animation sequence.
  • I ran the PNG through https://tinypng.com. Looks like it's about 35KB.
  • I'm open to adjusting the example spritesheet if required, but if it's fine as is, great.

Showcase

custom-cursor-001.mov

Migration Guide

The CustomCursor::Image enum variant has some new fields. Update your code to set them.

Before:

CustomCursor::Image {
    handle: asset_server.load("branding/icon.png"),
    hotspot: (128, 128),
}

After:

CustomCursor::Image {
    handle: asset_server.load("branding/icon.png"),
    texture_atlas: None,
    flip_x: false,
    flip_y: false,
    rect: None,
    hotspot: (128, 128),
}

References

@mgi388
Copy link
Contributor Author

mgi388 commented Jan 3, 2025

Another showcase video. This is a more realistic in-game animated cursor. The pointer animates every 5s (blink and you'll miss it):

Screen.Recording.2025-01-03.at.11.15.03.PM.mov

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Jan 3, 2025

The new example uses a Kenny cursor icon sprite sheet. I included the licence even though it's not required (and it's CC0).

First off: Thank you for attributing to Kenney. He's a cool person. :D

Second off: I don't see any assets added by this PR? All the files touched are code, except for examples/README.md and a Cargo.toml.

@IQuick143 IQuick143 added A-Windowing Platform-agnostic interface layer to run your app in X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jan 4, 2025
@mgi388 mgi388 force-pushed the custom-cursor-image-texture-atlas branch from c6ba500 to 2f77a85 Compare January 6, 2025 00:31
@mgi388
Copy link
Contributor Author

mgi388 commented Jan 6, 2025

@eero-lehtinen 👋 would you be interested in reviewing this PR for texture atlas support on custom cursor images?

Copy link
Contributor

@eero-lehtinen eero-lehtinen left a comment

Choose a reason for hiding this comment

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

Seems fine to me except for a couple of things.

@BenjaminBrienen BenjaminBrienen added S-Blocked This cannot move forward until something else changes and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 7, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 7, 2025
# Objective

- Allow other crates to use `TextureAtlas` and friends without needing
to depend on `bevy_sprite`.
- Specifically, this allows adding `TextureAtlas` support to custom
cursors in #17121 by allowing
`bevy_winit` to depend on `bevy_image` instead of `bevy_sprite` which is
a [non-starter].

[non-starter]:
#17121 (comment)

## Solution

- Move `TextureAtlas`, `TextureAtlasBuilder`, `TextureAtlasSources`,
`TextureAtlasLayout` and `DynamicTextureAtlasBuilder` into `bevy_image`.
- Add a new plugin to `bevy_image` named `TextureAtlasPlugin` which
allows us to register `TextureAtlas` and `TextureAtlasLayout` which was
previously done in `SpritePlugin`. Since `SpritePlugin` did the
registration previously, we just need to make it add
`TextureAtlasPlugin`.

## Testing

- CI builds it.
- I also ran multiple examples which hopefully covered any issues:

```
$ cargo run --example sprite
$ cargo run --example text
$ cargo run --example ui_texture_atlas
$ cargo run --example sprite_animation
$ cargo run --example sprite_sheet
$ cargo run --example sprite_picking
```

---

## Migration Guide

The following types have been moved from `bevy_sprite` to `bevy_image`:
`TextureAtlas`, `TextureAtlasBuilder`, `TextureAtlasSources`,
`TextureAtlasLayout` and `DynamicTextureAtlasBuilder`.

If you are using the `bevy` crate, and were importing these types
directly (e.g. before `use bevy::sprite::TextureAtlas`), be sure to
update your import paths (e.g. after `use bevy::image::TextureAtlas`)

If you are using the `bevy` prelude to import these types (e.g. `use
bevy::prelude::*`), you don't need to change anything.

If you are using the `bevy_sprite` subcrate, be sure to add `bevy_image`
as a dependency if you do not already have it, and be sure to update
your import paths.
@mgi388 mgi388 force-pushed the custom-cursor-image-texture-atlas branch from 00a36d4 to 569260c Compare January 8, 2025 02:33
@BenjaminBrienen BenjaminBrienen added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Blocked This cannot move forward until something else changes labels Jan 8, 2025
@mgi388
Copy link
Contributor Author

mgi388 commented Jan 8, 2025

Changes (also see latest commits):

  • Don't depend on bevy_sprite.
  • Follow fast path in the case there is no image transform to do.
  • Add custom_cursor.rs, CustomCursorPlugin and move most of the custom cursor code to it. This has the nice benefits of a) allowing us to encapsulate the registration of TextureAtlasPlugin and b) remove some #[cfg(feature = "custom_cursor")]. There's still room to move the CustomCursor type, but I wanted to reduce the diff in this PR so it's still in cursor.rs.

@BenjaminBrienen
Copy link
Contributor

I don't have time to do a full code review, but I'll run the examples when I get to my laptop

Copy link
Contributor

@BenjaminBrienen BenjaminBrienen left a comment

Choose a reason for hiding this comment

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

custom_cursor_image works. window_settings actually seems broken here and on main.

@mgi388
Copy link
Contributor Author

mgi388 commented Jan 8, 2025

custom_cursor_image works. window_settings actually seems broken here and on main.

@BenjaminBrienen how so? Seems to work for me. You need to left/right click to cycle the cursor icon if you didn't already do that.

@BenjaminBrienen
Copy link
Contributor

custom_cursor_image works. window_settings actually seems broken here and on main.

@BenjaminBrienen how so? Seems to work for me. You need to left/right click to cycle the cursor icon if you didn't already do that.

See #17227

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Jan 8, 2025
Copy link
Contributor

@eero-lehtinen eero-lehtinen left a comment

Choose a reason for hiding this comment

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

Nice tests, looks good.

@mgi388 mgi388 requested a review from eero-lehtinen January 10, 2025 21:28
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 10, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 14, 2025
Merged via the queue into bevyengine:main with commit 0756a19 Jan 14, 2025
32 checks passed
@mgi388 mgi388 deleted the custom-cursor-image-texture-atlas branch January 14, 2025 23:00
github-merge-queue bot pushed a commit that referenced this pull request Jan 15, 2025
# Objective

- Follow up work from
#17121 (comment) to
keep the `cursor.rs` file more manageable.

## Solution

- Move `CustomCursor` and make it compile.

## Testing

- Ran the example: `cargo run --example custom_cursor_image
--features=custom_cursor`
- CI
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
# Objective

- Allow other crates to use `TextureAtlas` and friends without needing
to depend on `bevy_sprite`.
- Specifically, this allows adding `TextureAtlas` support to custom
cursors in bevyengine#17121 by allowing
`bevy_winit` to depend on `bevy_image` instead of `bevy_sprite` which is
a [non-starter].

[non-starter]:
bevyengine#17121 (comment)

## Solution

- Move `TextureAtlas`, `TextureAtlasBuilder`, `TextureAtlasSources`,
`TextureAtlasLayout` and `DynamicTextureAtlasBuilder` into `bevy_image`.
- Add a new plugin to `bevy_image` named `TextureAtlasPlugin` which
allows us to register `TextureAtlas` and `TextureAtlasLayout` which was
previously done in `SpritePlugin`. Since `SpritePlugin` did the
registration previously, we just need to make it add
`TextureAtlasPlugin`.

## Testing

- CI builds it.
- I also ran multiple examples which hopefully covered any issues:

```
$ cargo run --example sprite
$ cargo run --example text
$ cargo run --example ui_texture_atlas
$ cargo run --example sprite_animation
$ cargo run --example sprite_sheet
$ cargo run --example sprite_picking
```

---

## Migration Guide

The following types have been moved from `bevy_sprite` to `bevy_image`:
`TextureAtlas`, `TextureAtlasBuilder`, `TextureAtlasSources`,
`TextureAtlasLayout` and `DynamicTextureAtlasBuilder`.

If you are using the `bevy` crate, and were importing these types
directly (e.g. before `use bevy::sprite::TextureAtlas`), be sure to
update your import paths (e.g. after `use bevy::image::TextureAtlas`)

If you are using the `bevy` prelude to import these types (e.g. `use
bevy::prelude::*`), you don't need to change anything.

If you are using the `bevy_sprite` subcrate, be sure to add `bevy_image`
as a dependency if you do not already have it, and be sure to update
your import paths.
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
# Objective

- Bevy 0.15 added support for custom cursor images in
bevyengine#14284.
- However, to do animated cursors using the initial support shipped in
0.15 means you'd have to animate the `Handle<Image>`: You can't use a
`TextureAtlas` like you can with sprites and UI images.
- For my use case, my cursors are spritesheets. To animate them, I'd
have to break them down into multiple `Image` assets, but that seems
less than ideal.


## Solution

- Allow users to specify a `TextureAtlas` field when creating a custom
cursor image.
- To create parity with Bevy's `TextureAtlas` support on `Sprite`s and
`ImageNode`s, this also allows users to specify `rect`, `flip_x` and
`flip_y`. In fact, for my own use case, I need to `flip_y`.

## Testing

- I added unit tests for `calculate_effective_rect` and
`extract_and_transform_rgba_pixels`.
- I added a brand new example for custom cursor images. It has controls
to toggle fields on and off. I opted to add a new example because the
existing cursor example (`window_settings`) would be far too messy for
showcasing these custom cursor features (I did start down that path but
decided to stop and make a brand new example).
- The new example uses a [Kenny cursor icon] sprite sheet. I included
the licence even though it's not required (and it's CC0).
- I decided to make the example just loop through all cursor icons for
its animation even though it's not a _realistic_ in-game animation
sequence.
- I ran the PNG through https://tinypng.com. Looks like it's about 35KB.
- I'm open to adjusting the example spritesheet if required, but if it's
fine as is, great.

[Kenny cursor icon]: https://kenney-assets.itch.io/crosshair-pack

---

## Showcase


https://github.com/user-attachments/assets/8f6be8d7-d1d4-42f9-b769-ef8532367749

## Migration Guide

The `CustomCursor::Image` enum variant has some new fields. Update your
code to set them.

Before:

```rust
CustomCursor::Image {
    handle: asset_server.load("branding/icon.png"),
    hotspot: (128, 128),
}
```

After:

```rust
CustomCursor::Image {
    handle: asset_server.load("branding/icon.png"),
    texture_atlas: None,
    flip_x: false,
    flip_y: false,
    rect: None,
    hotspot: (128, 128),
}
```

## References

- Feature request [originally raised in Discord].

[originally raised in Discord]:
https://discord.com/channels/691052431525675048/692572690833473578/1319836362219847681
mrchantey pushed a commit to mrchantey/bevy that referenced this pull request Feb 4, 2025
# Objective

- Follow up work from
bevyengine#17121 (comment) to
keep the `cursor.rs` file more manageable.

## Solution

- Move `CustomCursor` and make it compile.

## Testing

- Ran the example: `cargo run --example custom_cursor_image
--features=custom_cursor`
- CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Windowing Platform-agnostic interface layer to run your app in C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Contentious There are nontrivial implications that should be thought through

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants