UI Texture 9 slice #11600

ManevilleF · 2024-01-29T11:50:15Z

Follow up to #10588
Closes #11749 (Supersedes #11756)

Enable Texture slicing for the following UI nodes:

ImageBundle
ButtonBundle

I also added a collection of fantazy-ui-borders from Kenney's 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

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
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
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

ManevilleF · 2024-01-29T11:57:46Z

@alice-i-cecile I think this should go to 0.13 as long as the sprite slices, the UI feature is probably even more useful than the sprite counterpart

github-actions · 2024-01-29T12:06:22Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

github-actions · 2024-01-29T12:07:34Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

github-actions · 2024-01-29T12:10:02Z

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

alice-i-cecile · 2024-01-29T15:14:02Z

I agree: the UI equivalent is heavily awaited and I'd like to be able to land both at once.

crates/bevy_sprite/src/texture_slice/slicer.rs

alice-i-cecile · 2024-01-29T15:16:10Z

crates/bevy_ui/src/texture_slice.rs

@@ -0,0 +1,177 @@
+// This module is mostly copied and pasted from `bevy_sprite::texture_slice`
+//
+// A more centralized solution should be investigated in the future


Strongly agree. The API duplication between 2D and UI, and the inconsistencies and delays we get is something that's been on my mind lately.

Tbh most of the code is not duplicated the main components and logic are reused, what is duplicated and adapted is the ComputedTextureSlices code, so only the cache elements with the sprite/ui specific logic

Anyway in the future we might want to implement slicing directly in the shader

examples/ui/ui_texture_slice.rs

ManevilleF · 2024-01-29T15:29:21Z

@alice-i-cecile What about the new textures ? Do you think keeping them makes sense or should I only keep the one in use ?

alice-i-cecile · 2024-01-29T15:39:02Z

I like the new textures a lot: I think they clearly demonstrate the value in a more appropriate way.

ManevilleF · 2024-01-29T15:55:42Z

I like the new textures a lot: I think they clearly demonstrate the value in a more appropriate way.

Great, but I added the whole pack in the assets folder, so we might want to remove the unused ones from the version control

alice-i-cecile · 2024-01-29T17:48:09Z

Yeah, let's strip it down to only the ones that we use. We can always add more later if we find a use in other examples.

crates/bevy_sprite/src/texture_slice/slicer.rs

odecay · 2024-02-06T21:40:04Z

Working on giving this a review, I noticed some left over unused system set ordering which seems like it was changed here #5103 (comment) in the Texture Atlas rework.

mockersf · 2024-02-06T21:48:32Z

could you also add the assets you're adding to https://github.com/bevyengine/bevy/blob/main/CREDITS.md with the direct link to Kenney's pack containing them and their license?

crates/bevy_sprite/src/lib.rs

ManevilleF · 2024-02-06T22:18:03Z

could you also add the assets you're adding to https://github.com/bevyengine/bevy/blob/main/CREDITS.md with the direct link to Kenney's pack containing them and their license?

Sure, currently the license file is in the asset sub folder, so in addition to the credits where should the license file be?

mockersf · 2024-02-06T22:27:28Z

it's a CC0 license so the license file is not strictly needed, it could be removed.

odecay

I see there is a note to support flipping, but it seems like there is an issue with the default orientation.

crates/bevy_ui/src/texture_slice.rs

ManevilleF · 2024-02-07T14:52:42Z

I see there is a note to support flipping, but it seems like there is an issue with the default orientation.

It was tricky but I fixed the flipped issue and added flipping support for sliced textures

ManevilleF · 2024-02-07T14:54:39Z

@alice-i-cecile Do you know why the CI fails on check-bans ? What can I do to fix this ?

mockersf · 2024-02-07T15:00:29Z

@alice-i-cecile Do you know why the CI fails on check-bans ? What can I do to fix this ?

you can't, and it's not blocking for merging (it doesn't have the "required" tag)

mockersf

I'm unhappy about the need for a compute_slices_on_asset_event system, and even worse that it was already added for sprites.

I think 9-slice enabled ui elements and sprites should not be part of the existing bundles but in new bundles, or a "add this component ImageScaleMode to any entity with an image and it will be sliced/scaled/tiled, otherwise it will be just scaled and nothing fancy will happen"

This has already been added for sprites, so approving here for feature parity

odecay

Nice, the flipping works as expected now and the default orientation is correct.
The ordering cleanup also looks 👍
LGTM

ManevilleF · 2024-02-07T18:35:09Z

I'm unhappy about the need for a compute_slices_on_asset_event system, and even worse that it was already added for sprites.

This is an optimization to avoid re computing the slices every frames, in a prepare or extract stage. It's not ideal but untile we are able to do slicing on the GPU I don't have a better solution

I think 9-slice enabled ui elements and sprites should not be part of the existing bundles but in new bundles, or a "add this component ImageScaleMode to any entity with an image and it will be sliced/scaled/tiled, otherwise it will be just scaled and nothing fancy will happen"

Since the sprite slicing was not released yet we can remove the scale mode from the sprite and UI bundles, note that in UI it was only added to bundles with a UI image

mockersf · 2024-02-07T20:17:13Z

I'm unhappy about the need for a compute_slices_on_asset_event system, and even worse that it was already added for sprites.

This is an optimization to avoid re computing the slices every frames, in a prepare or extract stage. It's not ideal but untile we are able to do slicing on the GPU I don't have a better solution

Yup, what I'm unhappy about is that any change to an image asset will go through all image (2d or UI) entities, even when you're not using 9 slice at all

I think 9-slice enabled ui elements and sprites should not be part of the existing bundles but in new bundles, or a "add this component ImageScaleMode to any entity with an image and it will be sliced/scaled/tiled, otherwise it will be just scaled and nothing fancy will happen"

Since the sprite slicing was not released yet we can remove the scale mode from the sprite and UI bundles, note that in UI it was only added to bundles with a UI image

I would like that. Do you think we should duplicate all bundles with an added ImageScaleMode component? Or have it as a free standing component, and show in the examples and doc "add this component and stuff happens"? @alice-i-cecile, do you have an opinion?

ManevilleF · 2024-02-07T20:37:29Z

Yup, what I'm unhappy about is that any change to an image asset will go through all image (2d or UI) entities, even when you're not using 9 slice at all

Honestly we can just remove those systems, the slices won't refresh if you change the image asset directly but users can be responsible to trigger change detection

I think 9-slice enabled ui elements and sprites should not be part of the existing bundles but in new bundles, or a "add this component ImageScaleMode to any entity with an image and it will be sliced/scaled/tiled, otherwise it will be just scaled and nothing fancy will happen"

Since the sprite slicing was not released yet we can remove the scale mode from the sprite and UI bundles, note that in UI it was only added to bundles with a UI image

I would like that. Do you think we should duplicate all bundles with an added ImageScaleMode component? Or have it as a free standing component, and show in the examples and doc "add this component and stuff happens"? @alice-i-cecile, do you have an opinion?

I feel we might be adding too many bundles that are basically the same as others with one extra component.
For example the sprite sheet bundle is strictly identical to the sprite bundle with one extra component. So maybe we could have some "basic" bundles and explain the possible additional behaviour through extra components? This would be fixed by optional components in bundles too I think

@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 Fixes #11944 ## Solution #11600 made an incorrect assumption on what `UiImageSize` does, removing its usage in slicing fixes the problem

# Objective Fixes bevyengine#11944 ## Solution bevyengine#11600 made an incorrect assumption on what `UiImageSize` does, removing its usage in slicing fixes the problem

# Objective Fixes #11944 ## Solution #11600 made an incorrect assumption on what `UiImageSize` does, removing its usage in slicing fixes the problem

# 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 <[email protected]> Co-authored-by: Pablo Reinhardt <[email protected]>

# 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 <[email protected]> Co-authored-by: Pablo Reinhardt <[email protected]>

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 <[email protected]> Co-authored-by: Pablo Reinhardt <[email protected]>

alice-i-cecile added this to the 0.13 milestone Jan 29, 2024

alice-i-cecile reviewed Jan 29, 2024

View reviewed changes

crates/bevy_sprite/src/texture_slice/slicer.rs Outdated Show resolved Hide resolved

alice-i-cecile reviewed Jan 29, 2024

View reviewed changes

examples/ui/ui_texture_slice.rs Show resolved Hide resolved

alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels Jan 29, 2024

robtfm self-requested a review January 29, 2024 23:50

ManevilleF requested a review from alice-i-cecile January 31, 2024 07:47

msvbg reviewed Feb 5, 2024

View reviewed changes

crates/bevy_sprite/src/texture_slice/slicer.rs Outdated Show resolved Hide resolved

ManevilleF commented Feb 5, 2024

View reviewed changes

crates/bevy_sprite/src/texture_slice/slicer.rs Outdated Show resolved Hide resolved

ManevilleF force-pushed the feat/ui_texture_slice branch from c4c8176 to d8b60e0 Compare February 5, 2024 08:17

ManevilleF commented Feb 5, 2024

View reviewed changes

crates/bevy_sprite/src/texture_slice/slicer.rs Outdated Show resolved Hide resolved

mockersf reviewed Feb 6, 2024

View reviewed changes

crates/bevy_sprite/src/lib.rs Show resolved Hide resolved

odecay reviewed Feb 6, 2024

View reviewed changes

crates/bevy_ui/src/texture_slice.rs Outdated Show resolved Hide resolved

ManevilleF added 2 commits February 7, 2024 14:57

UI Texture slice

a535528

cleanup example

17b8049

ManevilleF added 4 commits February 7, 2024 15:16

Updated CREDITS

8816e11

Fix bevyengine#11749

e33636f

Support flip for sprite and UI slice

134ed9a

Fixed flipping

75094e9

ManevilleF requested review from odecay and mockersf February 7, 2024 14:51

mockersf approved these changes Feb 7, 2024

View reviewed changes

odecay approved these changes Feb 7, 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 Feb 7, 2024

alice-i-cecile added this pull request to the merge queue Feb 7, 2024

Merged via the queue into bevyengine:main with commit ab16f5e Feb 7, 2024
27 of 28 checks passed

mockersf mentioned this pull request Feb 7, 2024

Removing ExtractAtlasNode from UIRender Extract Schedule #11756

Closed

ManevilleF mentioned this pull request Feb 8, 2024

Optional ImageScaleMode #11780

Merged

ManevilleF mentioned this pull request Feb 22, 2024

Fixed Ui Image slicing #12047

Merged

github-merge-queue bot pushed a commit that referenced this pull request Feb 23, 2024

Fixed Ui Image slicing (#12047)

fa17406

# Objective Fixes #11944 ## Solution #11600 made an incorrect assumption on what `UiImageSize` does, removing its usage in slicing fixes the problem

ManevilleF mentioned this pull request Feb 23, 2024

Slicing support for texture atlas #12059

Merged

mockersf pushed a commit that referenced this pull request Feb 27, 2024

Fixed Ui Image slicing (#12047)

0e75d00

# Objective Fixes #11944 ## Solution #11600 made an incorrect assumption on what `UiImageSize` does, removing its usage in slicing fixes the problem

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI Texture 9 slice #11600

UI Texture 9 slice #11600

ManevilleF commented Jan 29, 2024 •

edited

Loading

ManevilleF commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

alice-i-cecile Jan 29, 2024

ManevilleF Jan 29, 2024

ManevilleF Jan 29, 2024

ManevilleF commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

ManevilleF commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

odecay commented Feb 6, 2024

mockersf commented Feb 6, 2024 •

edited

Loading

ManevilleF commented Feb 6, 2024

mockersf commented Feb 6, 2024

odecay left a comment

ManevilleF commented Feb 7, 2024

ManevilleF commented Feb 7, 2024 •

edited

Loading

mockersf commented Feb 7, 2024

mockersf left a comment

odecay left a comment •

edited

Loading

ManevilleF commented Feb 7, 2024

mockersf commented Feb 7, 2024

ManevilleF commented Feb 7, 2024

UI Texture 9 slice #11600

UI Texture 9 slice #11600

Conversation

ManevilleF commented Jan 29, 2024 • edited Loading

Work done

Fixes

GPU Alternative

ManevilleF commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

github-actions bot commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

alice-i-cecile Jan 29, 2024

Choose a reason for hiding this comment

ManevilleF Jan 29, 2024

Choose a reason for hiding this comment

ManevilleF Jan 29, 2024

Choose a reason for hiding this comment

ManevilleF commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

ManevilleF commented Jan 29, 2024

alice-i-cecile commented Jan 29, 2024

odecay commented Feb 6, 2024

mockersf commented Feb 6, 2024 • edited Loading

ManevilleF commented Feb 6, 2024

mockersf commented Feb 6, 2024

odecay left a comment

Choose a reason for hiding this comment

ManevilleF commented Feb 7, 2024

ManevilleF commented Feb 7, 2024 • edited Loading

mockersf commented Feb 7, 2024

mockersf left a comment

Choose a reason for hiding this comment

odecay left a comment • edited Loading

Choose a reason for hiding this comment

ManevilleF commented Feb 7, 2024

mockersf commented Feb 7, 2024

ManevilleF commented Feb 7, 2024

ManevilleF commented Jan 29, 2024 •

edited

Loading

mockersf commented Feb 6, 2024 •

edited

Loading

ManevilleF commented Feb 7, 2024 •

edited

Loading

odecay left a comment •

edited

Loading