texture atlas top & left padding#23056
Conversation
| @@ -104,8 +104,20 @@ impl<'a> TextureAtlasBuilder<'a> { | |||
| /// Sets the amount of padding in pixels to add between the textures in the texture atlas. | |||
| /// | |||
| /// The `x` value provide will be added to the right edge, while the `y` value will be added to the bottom edge. | |||
| /// | |||
| /// calling this function will also set the initial padding (on the top and left edge). | |||
There was a problem hiding this comment.
| /// calling this function will also set the initial padding (on the top and left edge). | |
| /// calling this function will also set the initial padding (`x` for the left edge and `y` for the top) |
| @@ -104,8 +104,20 @@ impl<'a> TextureAtlasBuilder<'a> { | |||
| /// Sets the amount of padding in pixels to add between the textures in the texture atlas. | |||
| /// | |||
| /// The `x` value provide will be added to the right edge, while the `y` value will be added to the bottom edge. | |||
| /// | |||
| /// calling this function will also set the initial padding (on the top and left edge). | |||
| /// call [`initial_padding`] to set that value only | |||
There was a problem hiding this comment.
| /// call [`initial_padding`] to set that value only | |
| /// call [`initial_padding`](TextureAtlasBuilder::initial_padding) to set the left and top padding values only |
| self | ||
| } | ||
|
|
||
| /// Sets the amount of padding in pixels to add on the top and left edge of the texture atlas. |
There was a problem hiding this comment.
| /// Sets the amount of padding in pixels to add on the top and left edge of the texture atlas. | |
| /// Sets the amount of padding in pixels to add on the left and top edge of the texture atlas. |
just to make it consistent that the “first” aka x number is for the left edge
| @@ -114,13 +126,14 @@ impl<'a> TextureAtlasBuilder<'a> { | |||
| texture: &Image, | |||
| packed_location: &PackedLocation, | |||
| padding: UVec2, | |||
| init_padding: UVec2, | |||
| ) -> TextureAtlasBuilderResult<()> { | |||
| let rect_width = (packed_location.width() - padding.x) as usize; | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
No, it still pack the rects with padding only added on the bottom and right. Rectangle Pack is passed a single TargetBin with its size equal to the atlas target size minus initial_padding. Then the returned packed rects are displaced right and down by intial_padding to leave the top left padded region clear.
There was a problem hiding this comment.
I added an atlas test thing to testbed_2d (#23074) and everything looks good:
The default texture atlas packing behaviour is changed, so we need a migration guide. Just has to explain that padding is now added on the top and left edges, and that users can call initial_padding(UVec2::ZERO) to get the old behaviour.
| /// The padding along the left and top edges of the `TextureAtlas` | ||
| initial_padding: UVec2, |
There was a problem hiding this comment.
I don't think there's any reason we would want the top left to have a different padding thickness. It might be clearer to just make this a bool, and reuse the value from the padding field.
…23091) # Objective - #22939 has been introducing diffs in the testbed_ui Text example, but the only thing that was changing was the order that glyphs are allocated to the font atlas. The font atlas **should be** resilient to this and produce no diffs in that case. - Another PR had similar diffs and also seemed to be due to changing the order of glyphs being allocated. ## Solution - Previously `DynamicTextureAtlasBuilder` would allocate glyph size + padding, and then copy the glyph into that rectangle - but this means that copying would start at (0,0) meaning we would "blend" with the top and left edges (which breaks things). - Now we copy to (padding, padding). - Remove (padding, padding) allocate-able space from the bottom and right to ensure we don't accidentally put textures on the right or bottom edges. ## Testing - Added unit tests to ensure that allocation works and that padding works. - The rendering change here is quite interesting! https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23091 Some cases, like the `l` in `black` in the Text screenshot look **substantially** better! Note this is orthogonal to #23056 which is about `TextureAtlasBuilder`, not `DynamicTextureAtlasBuilder`.
|
Added this to the release note included with #23132 |
…m/Wuketuke/bevy into issue-22776_texture-atlas-padding
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
# Objective - Migration guide merged in release-content ## Solution - Move it - Add a CI check that will block new merges Opened PRs that sill change that folder: - bevyengine#23467 - bevyengine#23445 - bevyengine#23373 - bevyengine#23137 - bevyengine#23132 - bevyengine#23056 - bevyengine#22917 - bevyengine#22852 - bevyengine#22782 - bevyengine#22670 - bevyengine#22557 - bevyengine#22500 - bevyengine#21929 - bevyengine#21912 - bevyengine#21897 - bevyengine#21893 - bevyengine#21890 - bevyengine#21889 - bevyengine#21839 - bevyengine#21811 - bevyengine#21772 None of them are likely to be merged in the 0.19
|
@Wuketuke do you still want to finish this, or should someone else take it over? |
fixes #22776
A
TextureAtlascan now have padding on the top and left edge.Since the user probably wants to set this together with the normal padding, i changed the
TextureAtlasBuilder::paddingfunction so that both the initial padding and the per-image padding gets set together.Should the user wish to have a different initial padding value, i added a
initial_paddingfunctionTesting
i ran the
2d/texture_atlas.rsexample, and saved each image to a file.i then manually checked if the padding is inserted correctly
for that i slightly modified the example code like this