Sprite slicing and tiling #10588

ManevilleF · 2023-11-16T13:02:07Z

Replaces #5213

Objective

Implement sprite tiling and 9 slice scaling for bevy_sprite.
Allowing slice scaling and texture tiling.

Basic scaling vs 9 slice scaling:

Slicing example:

Tiling example:

Solution

SpriteBundlue now has a scale_mode component storing a SpriteScaleMode enum with three variants:
- Stretched (default)
- Tiled to have sprites tile horizontally and/or vertically
- Sliced allowing 9 slicing the texture and optionally tile some sections with a Textureslicer.
bevy_sprite has two extra systems to compute a ComputedTextureSlices if necessary,:
- One system react to changes on Sprite, Handle<Image> or SpriteScaleMode
- The other listens to AssetEvent<Image> to compute slices on sprites when the texture is ready or changed
I updated the bevy_sprite extraction stage to extract potentially multiple textures instead of one, depending on the presence of ComputedTextureSlices
I added two examples showcasing the slicing and tiling feature.

The addition of ComputedTextureSlices as a cache is to avoid querying the image data, to retrieve its dimensions, every frame in a extract or prepare stage. Also it reacts to changes so we can have stuff like this (tiling example):

Screen.Recording.2023-11-16.at.13.59.25.mp4

To discuss

There is an other option, to consider slice/tiling as part of the asset, using the new asset preprocessing but I have no clue on how to do it.

Also, instead of retrieving the Image dimensions, we could use the same system as the sprite sheet and have the user give the image dimensions directly (grid). But I think it's less user friendly

crates/bevy_sprite/src/render/mod.rs

crates/bevy_sprite/src/sprite.rs

alice-i-cecile · 2023-11-16T17:38:09Z

crates/bevy_sprite/src/sprite.rs

@@ -23,6 +25,26 @@ pub struct Sprite {
    pub anchor: Anchor,
 }

+#[derive(Component, Debug, Default, Clone, Reflect)]
+#[reflect(Component, Default)]
+pub enum SpriteScaleMode {


IMO we should call this ImageScaleMode, since we're intending to support non-sprite images eventually.

Right ! But then where should it be put ? Still in bevy_sprite ? Is there a common crate for textures shared by bevy_ui as well ?

bevy_render feels like the correct home, and is a shared dependency.

bevy_sprite is also a dependency of bevy_ui. and I'm not sure bevy_render is the right place, it feels like this abstraction is too high level for this core crate

Yeah @superdump and @robftm can decide :) Here is fine for now though.

pablo-lua

Was reading the changes, and some types here and there don't reflect what they could even if they could, not sure if there is a reason for it. Well, liked this changes very much, hope that goes on :)

alice-i-cecile · 2024-01-04T18:29:19Z

@pablo-lua can you expand on that comment? Which types didn't make sense to you?

pablo-lua · 2024-01-04T18:39:55Z

This was pretty much all I found, I was not sure if this was a problem, but now I'm pointing which types can probably reflect

brandon-reinhart

Edit: The demo could be improved, but I decided to switch to approve.

I have no technical criticisms. This looks good to me and would offer immediate value to users.

I would mark this "Approve" if the labels on the demo didn't overlap themselves.

Two nits I didn't mention is that I wish Bevy PR authors would put an empty line between blocks more frequently (aids in code readability) and all comments that were sentences ended with a period. This is just personal pedantry.

crates/bevy_sprite/src/bundle.rs

brandon-reinhart · 2024-01-08T00:39:01Z

crates/bevy_sprite/src/sprite.rs

+    Sliced(TextureSlicer),
+    /// The texture will be repeated if stretched beyond `stretched_value`
+    Tiled {
+        /// Should the image repeat horizontally


change to:

/// The image will repeat horizontally.
...
/// The image will repeat vertically.

brandon-reinhart · 2024-01-08T00:40:21Z

crates/bevy_sprite/src/texture_slice/border_rect.rs

+/// Struct defining a [`Sprite`](crate::Sprite) border with padding values
+#[derive(Default, Copy, Clone, PartialEq, Debug, Reflect)]
+pub struct BorderRect {
+    /// Pixel padding to the left


These comments probably aren't necessary (like the UiRect fields).

brandon-reinhart · 2024-01-08T00:52:31Z

examples/2d/sprite_slice.rs

+        transform: Transform::from_xyz(-400.0, 0.0, 0.0),
+        ..default()
+    });
+    commands.spawn(Text2dBundle {


This text overlaps. Drop the font size or stagger the Y of the labels.

alice-i-cecile · 2024-01-13T22:23:10Z

I suspect #11326 may have fixed up the label arrangement issue: can you merge in main while you're fixing CI?

ickshonpe

It seems to work really well, the examples are really slick (apart from the text overlap problems).
I think moving to a shader based implementation would make more sense eventually but this seems good for now.

examples/2d/sprite_slice.rs

ickshonpe · 2024-01-13T23:40:17Z

crates/bevy_sprite/src/sprite.rs

+/// Controls how the image is altered when scaled.
+#[derive(Component, Debug, Default, Clone, Reflect)]
+#[reflect(Component, Default)]
+pub enum ImageScaleMode {


I'm not sure about the name ImageScaleMode but it's not that much a problem I guess, I can't think of anything better.

ImageScalingMode or ImageScalingBehavior is a marginal improvement I think.

Maybe instead of scale, because the other variants aren't really scalings, something like ImageExtendMode or ImageExtensionMode

ickshonpe · 2024-01-13T23:43:58Z

crates/bevy_sprite/src/render/mod.rs

+                    .extract_sprites(transform, entity, sprite, handle)
+                    .map(|e| (commands.spawn_empty().id(), e)),


We could use a custom shader here that draws all the slices in one pass, then only need one entity id is needed.

With a shader based solution, the whole texture_slice module wouldn't be needed even. All it would need is just the ImageScaleMode component, extraction function, some pipeline specialisation stuff, and a fragment shader.

ickshonpe · 2024-01-13T23:48:47Z

crates/bevy_sprite/src/render/mod.rs

+            extracted_sprites.sprites.extend(
+                slices
+                    .extract_sprites(transform, entity, sprite, handle)
+                    .map(|e| (commands.spawn_empty().id(), e)),


Also it isn't very difficult either to modify batching to group sprites together with the same entity id when there isn't any need to sort the group internally. I thought I even wrote a PR somewhere that implemented it for use with Text2d.

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

pablo-lua · 2024-01-04T18:36:35Z

crates/bevy_sprite/src/texture_slice/border_rect.rs

+use bevy_reflect::Reflect;
+
+/// Struct defining a [`Sprite`](crate::Sprite) border with padding values
+#[derive(Default, Copy, Clone, PartialEq, Debug, Reflect)]


Couldn't this type reflect Default and PartialEq?

#[reflect(Default, PartialEq)]

pablo-lua · 2024-01-04T18:37:50Z

crates/bevy_sprite/src/texture_slice/slicer.rs

+
+/// Defines how a texture slice scales when resized
+#[derive(Debug, Copy, Clone, Default, Reflect)]
+pub enum SliceScaleMode {


The same with this Type, could probably reflect Default.

examples/2d/sprite_slice.rs

ManevilleF · 2024-01-15T11:39:10Z

I think moving to a shader based implementation would make more sense eventually but this seems good for now.

Definetely, this should be GPU based, but I don't know how to do that, merging this would provide an API for slicing/tiling but in the future the internals should move from the CPU to a shader

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

@odecay

> Follow up to #10588 > Closes #11749 (Supersedes #11756) Enable Texture slicing for the following UI nodes: - `ImageBundle` - `ButtonBundle` <img width="739" alt="Screenshot 2024-01-29 at 13 57 43" src="https://github.com/bevyengine/bevy/assets/26703856/37675681-74eb-4689-ab42-024310cf3134"> I also added a collection of `fantazy-ui-borders` from [Kenney's](www.kenney.nl) assets, with the appropriate license (CC). If it's a problem I can use the same textures as the `sprite_slice` example # Work done Added the `ImageScaleMode` component to the targetted bundles, most of the logic is directly reused from `bevy_sprite`. The only additional internal component is the UI specific `ComputedSlices`, which does the same thing as its spritee equivalent but adapted to UI code. Again the slicing is not compatible with `TextureAtlas`, it's something I need to tackle more deeply in the future # Fixes * [x] I noticed that `TextureSlicer::compute_slices` could infinitely loop if the border was larger that the image half extents, now an error is triggered and the texture will fallback to being stretched * [x] I noticed that when using small textures with very small *tiling* options we could generate hundred of thousands of slices. Now I set a minimum size of 1 pixel per slice, which is already ridiculously small, and a warning will be sent at runtime when slice count goes above 1000 * [x] Sprite slicing with `flip_x` or `flip_y` would give incorrect results, correct flipping is now supported to both sprites and ui image nodes thanks to @odecay observation # GPU Alternative I create a separate branch attempting to implementing 9 slicing and tiling directly through the `ui.wgsl` fragment shader. It works but requires sending more data to the GPU: - slice border - tiling factors And more importantly, the actual quad *scale* which is hard to put in the shader with the current code, so that would be for a later iteration

@mockersf

> Follow up to #11600 and #10588 @mockersf expressed some [valid concerns](#11600 (comment)) about the current system this PR attempts to fix: The `ComputedTextureSlices` reacts to asset change in both `bevy_sprite` and `bevy_ui`, meaning that if the `ImageScaleMode` is inserted by default in the bundles, we will iterate through most 2d items every time an asset is updated. # Solution - `ImageScaleMode` only has two variants: `Sliced` and `Tiled`. I removed the `Stretched` default - `ImageScaleMode` is no longer part of any bundle, but the relevant bundles explain that this additional component can be inserted This way, the *absence* of `ImageScaleMode` means the image will be stretched, and its *presence* will include the entity to the various slicing systems Optional components in bundles would make this more straigthfoward # Additional work Should I add new bundles with the `ImageScaleMode` component ?

# Objective Follow up to #11600 and #10588 #11944 made clear that some people want to use slicing with texture atlases ## Changelog * Added support for `TextureAtlas` slicing and tiling. `SpriteSheetBundle` and `AtlasImageBundle` can now use `ImageScaleMode` * Added new `ui_texture_atlas_slice` example using a texture sheet <img width="798" alt="Screenshot 2024-02-23 at 11 58 35" src="https://github.com/bevyengine/bevy/assets/26703856/47a8b764-127c-4a06-893f-181703777501"> --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>

# Objective Follow up to bevyengine#11600 and bevyengine#10588 bevyengine#11944 made clear that some people want to use slicing with texture atlases ## Changelog * Added support for `TextureAtlas` slicing and tiling. `SpriteSheetBundle` and `AtlasImageBundle` can now use `ImageScaleMode` * Added new `ui_texture_atlas_slice` example using a texture sheet <img width="798" alt="Screenshot 2024-02-23 at 11 58 35" src="https://github.com/bevyengine/bevy/assets/26703856/47a8b764-127c-4a06-893f-181703777501"> --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>

Follow up to bevyengine#11600 and bevyengine#10588 bevyengine#11944 made clear that some people want to use slicing with texture atlases * Added support for `TextureAtlas` slicing and tiling. `SpriteSheetBundle` and `AtlasImageBundle` can now use `ImageScaleMode` * Added new `ui_texture_atlas_slice` example using a texture sheet <img width="798" alt="Screenshot 2024-02-23 at 11 58 35" src="https://github.com/bevyengine/bevy/assets/26703856/47a8b764-127c-4a06-893f-181703777501"> --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>

Sprite slicing and tiling

cc9ae89

ManevilleF mentioned this pull request Nov 16, 2023

Sprite slice scaling and tiling #5213

Closed

11 tasks

alice-i-cecile requested review from mockersf and ickshonpe November 16, 2023 13:05

alice-i-cecile added this to the 0.13 milestone Nov 16, 2023

alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Nov 16, 2023

Fix clippy in example

5e27898

alice-i-cecile reviewed Nov 16, 2023

View reviewed changes

crates/bevy_sprite/src/render/mod.rs Show resolved Hide resolved

ManevilleF added 2 commits November 16, 2023 14:15

must_use attrs

c520369

rustdoc fix

d857b32

alice-i-cecile reviewed Nov 16, 2023

View reviewed changes

crates/bevy_sprite/src/sprite.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Nov 16, 2023

View reviewed changes

ManevilleF added 2 commits November 17, 2023 10:09

PR suggestions

1f55f8c

fixed rustdoc

3589a08

alice-i-cecile approved these changes Nov 19, 2023

View reviewed changes

pablo-lua reviewed Jan 4, 2024

View reviewed changes

brandon-reinhart suggested changes Jan 8, 2024

View reviewed changes

brandon-reinhart approved these changes Jan 12, 2024

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 12, 2024

alice-i-cecile added 2 commits January 12, 2024 16:05

Comment nits

272d43b

Merge branch 'main' into feat/sprite_9_slice

6d124d7

ickshonpe approved these changes Jan 14, 2024

View reviewed changes

New text formatting API

42b29cc

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

pablo-lua reviewed Jan 14, 2024

View reviewed changes

ickshonpe reviewed Jan 14, 2024

View reviewed changes

examples/2d/sprite_slice.rs Outdated Show resolved Hide resolved

alice-i-cecile and others added 2 commits January 15, 2024 10:20

Relatively position labels

3c98c03

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

Cargo fmt

3b89390

alice-i-cecile enabled auto-merge January 15, 2024 15:23

alice-i-cecile added this pull request to the merge queue Jan 15, 2024

Merged via the queue into bevyengine:main with commit 01139b3 Jan 15, 2024
26 checks passed

ManevilleF mentioned this pull request Jan 29, 2024

UI Texture 9 slice #11600

Merged

3 tasks

nicopap mentioned this pull request Jan 29, 2024

0.13 announcement post bevyengine/bevy-website#891

Merged

ManevilleF mentioned this pull request Feb 8, 2024

Optional ImageScaleMode #11780

Merged

ManevilleF mentioned this pull request Feb 23, 2024

Slicing support for texture atlas #12059

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sprite slicing and tiling #10588

Sprite slicing and tiling #10588

ManevilleF commented Nov 16, 2023 •

edited

Loading

alice-i-cecile Nov 16, 2023

ManevilleF Nov 17, 2023 •

edited

Loading

alice-i-cecile Nov 17, 2023

ManevilleF Nov 18, 2023

alice-i-cecile Nov 19, 2023

pablo-lua left a comment

alice-i-cecile commented Jan 4, 2024

pablo-lua commented Jan 4, 2024

brandon-reinhart left a comment •

edited

Loading

brandon-reinhart Jan 8, 2024

brandon-reinhart Jan 8, 2024

brandon-reinhart Jan 8, 2024

alice-i-cecile commented Jan 13, 2024

ickshonpe left a comment •

edited

Loading

ickshonpe Jan 13, 2024 •

edited

Loading

alice-i-cecile Jan 14, 2024

ickshonpe Jan 14, 2024

ickshonpe Jan 13, 2024 •

edited

Loading

ickshonpe Jan 13, 2024

pablo-lua Jan 4, 2024

pablo-lua Jan 4, 2024

ManevilleF commented Jan 15, 2024

		.extract_sprites(transform, entity, sprite, handle)
		.map(\|e\| (commands.spawn_empty().id(), e)),

Sprite slicing and tiling #10588

Sprite slicing and tiling #10588

Conversation

ManevilleF commented Nov 16, 2023 • edited Loading

Objective

Solution

Related

To discuss

Choose a reason for hiding this comment

ManevilleF Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablo-lua left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jan 4, 2024

pablo-lua commented Jan 4, 2024

brandon-reinhart left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile commented Jan 13, 2024

ickshonpe left a comment • edited Loading

Choose a reason for hiding this comment

ickshonpe Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ickshonpe Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ManevilleF commented Jan 15, 2024

ManevilleF commented Nov 16, 2023 •

edited

Loading

ManevilleF Nov 17, 2023 •

edited

Loading

brandon-reinhart left a comment •

edited

Loading

ickshonpe left a comment •

edited

Loading

ickshonpe Jan 13, 2024 •

edited

Loading

ickshonpe Jan 13, 2024 •

edited

Loading