UI texture slice texture flipping reimplementation #15034

ickshonpe · 2024-09-03T19:40:18Z

Objective

Solution

Reimplement support for the flip_x and flip_y fields.
This doesn't flip the border geometry, I'm not really sure whether that is desirable or not.
Also fixes a bug that was causing the side and center slices to tile incorrectly.

Testing

cargo run --example ui_texture_slice_flip_and_tile

Showcase

With tiling need to use nearest filtering to avoid bleeding between the slices.

ickshonpe · 2024-09-03T19:43:55Z

Seems to work, just going to get something to eat then I'll make a few more test examples to be sure.

crates/bevy_ui/src/render/ui_texture_slice_pipeline.rs

…_flip_and_repeat`

ickshonpe · 2024-09-03T21:28:18Z

It seems to be alright but I'd be happiest if @ManevilleF could have a look before it's merged.

github-actions · 2024-09-03T21:53:16Z

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-09-03T23:55:26Z

FYI @janhohenheim, this is the other blocker on 0.14.2 :)

janhohenheim

Looks good except for one potential mistake that I'd like sorted out. The rest are just minor style nits.
Thanks for adding an example with the implementation, I really appreciate it :)

janhohenheim · 2024-09-04T00:36:40Z

assets/textures/fantasy_ui_borders/numbered_slices.png

FYI, you can shave off half of the size of this png with oxipng, but at this low filesize I'm fine with merging as-is. Something to keep in mind for bigger pngs :)

janhohenheim · 2024-09-04T00:39:12Z

crates/bevy_ui/src/render/ui_texture_slice_pipeline.rs

-            let tsx = target_size.x * (1. - border[0] - border[2]);
-            let tsy = target_size.y * (1. - border[1] - border[3]);
+            let isx = image_size.x * (slices[2] - slices[0]);
+            let isy = image_size.y * (slices[2] - slices[1]);


Don't know much about the nine-patch implementation, but the code before used 3 instead of 2 on this line.
I assume that's what you meant with your quote

Also fixes a bug that was causing the side and center slices to tile incorrectly.

?

Can't comment on the mathematical correctness of this change in general. It looks like it's doing something else than before mathematically? Could you comment on that?

In the old texture slicing implementation (which used a separate sprite for each slice) insets were used to define the slices, but I found it more natural to use normalized coordinates, offset from the the top-left corner.

So previously nine slicing a 100x100 image with 25x25 border corner pieces you had

border = [ 0.25 // left inset, 0.25 // top inset, 0.25 // right inset, 0.25 // bottom inset, ]

and then to calculate the normalized width of the horizontal side and center sections it would do:
1 - border[0] - border[2] = 1 - 0.25 - 0.25 = 0.5

whereas now it's

border = [ 0.25 // top-left x, 0.25 // top-left y, 0.75 // bottom-right x, 0.75 // bottom-right y, ]

and to get the normalized width it's
border[2] - border[0] = 0.75 - 0.25 = 0.5

Does that affect how users define the slices? Will it break current usage ?

No the user API should behave the same as before, the coordinates change is just for the internal calculations.

examples/ui/ui_texture_slice_flip_and_tile.rs

janhohenheim · 2024-09-04T00:44:19Z

examples/ui/ui_texture_slice_flip_and_tile.rs

+    let image = asset_server.load_with_settings(
+        "textures/fantasy_ui_borders/numbered_slices.png",
+        |settings: &mut ImageLoaderSettings| {
+            // Need to use nearest filtering to avoid bleeding between the slices with tiling


That's only because they're pixely, right? Would a more high-res png also require this?

It depends on the texture and the result the user wants. If, as in the ui_texture_slice example, the source texture is seamless and the sides are just mono-coloured horizontal and vertical lines then it can be stretched or tiled without any artifacts even with blending.

With the numbered texture from the example here, there is bleeding between each slice if you use ImageSampler::linear(). The same was true of the previous implementation using multiple sprites, it's not a regression at least.

examples/ui/ui_texture_slice_flip_and_tile.rs

janhohenheim · 2024-09-04T00:46:34Z

examples/ui/ui_texture_slice_flip_and_tile.rs

@@ -0,0 +1,75 @@
+//! This example illustrates how to how to flip and tile images with 9 Slicing in the UI


Some links or a basic explanation as to what 9-slicing even is would nice for beginners.

I think I'd rather make an issue and leave that for a separate PR. The new example is just adapated from the ui_texture_slice.rs example which didn't have much in the way of explanation either.

Co-authored-by: Jan Hohenheim <[email protected]>

…tion functions.

janhohenheim

Lovely changes, thanks! Left two very minor suggestions to apply, feel free to ignore :)

examples/ui/ui_texture_slice_flip_and_tile.rs

Co-authored-by: Jan Hohenheim <[email protected]>

# Objective Fixes #15032 ## Solution Reimplement support for the `flip_x` and `flip_y` fields. This doesn't flip the border geometry, I'm not really sure whether that is desirable or not. Also fixes a bug that was causing the side and center slices to tile incorrectly. ### Testing ``` cargo run --example ui_texture_slice_flip_and_tile ``` ## Showcase <img width="787" alt="nearest" src="https://github.com/user-attachments/assets/bc044bae-1748-42ba-92b5-0500c87264f6"> With tiling need to use nearest filtering to avoid bleeding between the slices. --------- Co-authored-by: Jan Hohenheim <[email protected]> Co-authored-by: Alice Cecile <[email protected]>

ickshonpe added 3 commits September 3, 2024 18:37

add image flipping

05440aa

flip texture atlas coords

5820eeb

fixed slice calculations

04abc06

ickshonpe changed the title ~~UI texture slice texture flipping~~ UI texture slice texture flipping reimplementation Sep 3, 2024

alice-i-cecile added this to the 0.14.2 milestone Sep 3, 2024

alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels Sep 3, 2024

alice-i-cecile reviewed Sep 3, 2024

View reviewed changes

crates/bevy_ui/src/render/ui_texture_slice_pipeline.rs Outdated Show resolved Hide resolved

alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Sep 3, 2024

ickshonpe added 3 commits September 3, 2024 20:45

Deletion.

51899db

Added numbered slice test texture and a new example `ui_texture_slice…

6d9251e

…_flip_and_repeat`

updated cargo and readme for new example

e70b73f

alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Sep 3, 2024

ickshonpe added 2 commits September 3, 2024 22:33

Cleaned up example and added a comment.

f0fe8c1

Removed unused stuff

15d83a1

build-templated-pages

60dc5ef

janhohenheim suggested changes Sep 4, 2024

View reviewed changes

ickshonpe and others added 5 commits September 4, 2024 07:28

Update examples/ui/ui_texture_slice_flip_and_tile.rs

3c41103

Co-authored-by: Jan Hohenheim <[email protected]>

Update examples/ui/ui_texture_slice_flip_and_tile.rs

91843dc

Co-authored-by: Jan Hohenheim <[email protected]>

use width and height in example

caf7406

Added some comments and improved variable names for the slice computa…

0ab4979

…tion functions.

updated comments

5fbd0c5

janhohenheim approved these changes Sep 4, 2024

View reviewed changes

examples/ui/ui_texture_slice_flip_and_tile.rs Outdated Show resolved Hide resolved

examples/ui/ui_texture_slice_flip_and_tile.rs Outdated Show resolved Hide resolved

Example nits

8ade0ca

Co-authored-by: Jan Hohenheim <[email protected]>

alice-i-cecile approved these changes Sep 4, 2024

View reviewed changes

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 Sep 4, 2024

alice-i-cecile enabled auto-merge September 4, 2024 19:21

alice-i-cecile added this pull request to the merge queue Sep 4, 2024

Merged via the queue into bevyengine:main with commit 8ac745a Sep 4, 2024
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI texture slice texture flipping reimplementation #15034

UI texture slice texture flipping reimplementation #15034

ickshonpe commented Sep 3, 2024 •

edited

Loading

ickshonpe commented Sep 3, 2024

ickshonpe commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

alice-i-cecile commented Sep 3, 2024

janhohenheim left a comment •

edited

Loading

janhohenheim Sep 4, 2024

janhohenheim Sep 4, 2024 •

edited

Loading

ickshonpe Sep 4, 2024

ManevilleF Sep 4, 2024

ickshonpe Sep 4, 2024

janhohenheim Sep 4, 2024

ickshonpe Sep 4, 2024

janhohenheim Sep 4, 2024 •

edited

Loading

ickshonpe Sep 4, 2024

janhohenheim left a comment

		@@ -0,0 +1,75 @@
		//! This example illustrates how to how to flip and tile images with 9 Slicing in the UI

UI texture slice texture flipping reimplementation #15034

UI texture slice texture flipping reimplementation #15034

Conversation

ickshonpe commented Sep 3, 2024 • edited Loading

Objective

Solution

Testing

Showcase

ickshonpe commented Sep 3, 2024

ickshonpe commented Sep 3, 2024

github-actions bot commented Sep 3, 2024

alice-i-cecile commented Sep 3, 2024

janhohenheim left a comment • edited Loading

Choose a reason for hiding this comment

janhohenheim Sep 4, 2024

Choose a reason for hiding this comment

janhohenheim Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

ickshonpe Sep 4, 2024

Choose a reason for hiding this comment

ManevilleF Sep 4, 2024

Choose a reason for hiding this comment

ickshonpe Sep 4, 2024

Choose a reason for hiding this comment

janhohenheim Sep 4, 2024

Choose a reason for hiding this comment

ickshonpe Sep 4, 2024

Choose a reason for hiding this comment

janhohenheim Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

ickshonpe Sep 4, 2024

Choose a reason for hiding this comment

janhohenheim left a comment

Choose a reason for hiding this comment

ickshonpe commented Sep 3, 2024 •

edited

Loading

janhohenheim left a comment •

edited

Loading

janhohenheim Sep 4, 2024 •

edited

Loading

janhohenheim Sep 4, 2024 •

edited

Loading