Skip to content

Commit c495cd4

Browse files
authored
Fix DynamicTextureAtlasBuilder incorrectly padding added textures. (#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`.
1 parent 7b65256 commit c495cd4

File tree

1 file changed

+220
-9
lines changed

1 file changed

+220
-9
lines changed

crates/bevy_image/src/dynamic_texture_atlas_builder.rs

Lines changed: 220 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,15 @@ impl DynamicTextureAtlasBuilder {
3737
/// # Arguments
3838
///
3939
/// * `size` - total size for the atlas
40-
/// * `padding` - gap added between textures in the atlas, both in x axis and y axis
40+
/// * `padding` - gap added between textures in the atlas (and the atlas edge), both in x axis
41+
/// and y axis
4142
pub fn new(size: UVec2, padding: u32) -> Self {
43+
// This doesn't need to be >= since `AtlasAllocator` requires non-zero size.
44+
debug_assert!(size.x > padding && size.y > padding);
4245
Self {
43-
atlas_allocator: AtlasAllocator::new(to_size2(size)),
46+
// Leave out padding at the right and bottom, so we don't put textures on the edge of
47+
// atlas.
48+
atlas_allocator: AtlasAllocator::new(to_size2(size - padding)),
4449
padding,
4550
}
4651
}
@@ -62,20 +67,25 @@ impl DynamicTextureAtlasBuilder {
6267
texture: &Image,
6368
atlas_texture: &mut Image,
6469
) -> Result<usize, DynamicTextureAtlasBuilderError> {
70+
// Allocate enough space for the texture and the padding to the top and left (bottom and
71+
// right padding are taken care off since the allocator size omits it on creation).
6572
let allocation = self.atlas_allocator.allocate(size2(
6673
(texture.width() + self.padding).try_into().unwrap(),
6774
(texture.height() + self.padding).try_into().unwrap(),
6875
));
69-
if let Some(allocation) = allocation {
76+
if let Some(mut allocation) = allocation {
7077
assert!(
7178
atlas_texture.asset_usage.contains(RenderAssetUsages::MAIN_WORLD),
7279
"The atlas_texture image must have the RenderAssetUsages::MAIN_WORLD usage flag set"
7380
);
81+
let rect = &mut allocation.rectangle;
82+
// Remove the padding from the top and left (bottom and right padding is taken care of
83+
// by the "next" allocation and the border restriction).
84+
rect.min.x += self.padding as i32;
85+
rect.min.y += self.padding as i32;
7486

7587
self.place_texture(atlas_texture, allocation, texture)?;
76-
let mut rect: URect = to_rect(allocation.rectangle);
77-
rect.max = rect.max.saturating_sub(UVec2::splat(self.padding));
78-
Ok(atlas_layout.add_texture(rect))
88+
Ok(atlas_layout.add_texture(to_rect(allocation.rectangle)))
7989
} else {
8090
Err(DynamicTextureAtlasBuilderError::FailedToAllocateSpace)
8191
}
@@ -87,9 +97,7 @@ impl DynamicTextureAtlasBuilder {
8797
allocation: Allocation,
8898
texture: &Image,
8999
) -> Result<(), DynamicTextureAtlasBuilderError> {
90-
let mut rect = allocation.rectangle;
91-
rect.max.x -= self.padding as i32;
92-
rect.max.y -= self.padding as i32;
100+
let rect = &allocation.rectangle;
93101
let atlas_width = atlas_texture.width() as usize;
94102
let rect_width = rect.width() as usize;
95103
let format_size = atlas_texture.texture_descriptor.format.pixel_size()?;
@@ -127,3 +135,206 @@ fn to_rect(rectangle: guillotiere::Rectangle) -> URect {
127135
fn to_size2(vec2: UVec2) -> guillotiere::Size {
128136
guillotiere::Size::new(vec2.x as i32, vec2.y as i32)
129137
}
138+
139+
#[cfg(test)]
140+
mod tests {
141+
use bevy_asset::RenderAssetUsages;
142+
use bevy_math::{URect, UVec2};
143+
144+
use crate::{DynamicTextureAtlasBuilder, Image, TextureAtlasLayout};
145+
146+
fn make_filled_image(size: UVec2, pixel_rgba_bytes: [u8; 4]) -> Image {
147+
Image::new_fill(
148+
wgpu_types::Extent3d {
149+
width: size.x,
150+
height: size.y,
151+
depth_or_array_layers: 1,
152+
},
153+
wgpu_types::TextureDimension::D2,
154+
&pixel_rgba_bytes,
155+
wgpu_types::TextureFormat::Rgba8Unorm,
156+
RenderAssetUsages::all(),
157+
)
158+
}
159+
160+
fn rect_contains_value(image: &Image, rect: URect, pixel_rgba_bytes: [u8; 4]) -> bool {
161+
let image_data = image.data.as_ref().unwrap();
162+
for y in rect.min.y..rect.max.y {
163+
for x in rect.min.x..rect.max.x {
164+
let byte_start = ((x + y * image.width()) * 4) as usize;
165+
if image_data[byte_start..(byte_start + 4)] != pixel_rgba_bytes {
166+
return false;
167+
}
168+
}
169+
}
170+
true
171+
}
172+
173+
#[test]
174+
fn allocate_textures() {
175+
let size = UVec2::new(30, 30);
176+
177+
let mut atlas_texture = make_filled_image(size, [0, 0, 0, 0]);
178+
let mut layout = TextureAtlasLayout::new_empty(size);
179+
let mut builder = DynamicTextureAtlasBuilder::new(size, 0);
180+
181+
let square = UVec2::new(10, 10);
182+
let colors = [
183+
[255, 0, 0, 255],
184+
[0, 255, 0, 255],
185+
[0, 0, 255, 255],
186+
[255, 0, 255, 255],
187+
[0, 255, 255, 255],
188+
[0, 255, 255, 255],
189+
];
190+
let texture_0 = builder
191+
.add_texture(
192+
&mut layout,
193+
&make_filled_image(square, colors[0]),
194+
&mut atlas_texture,
195+
)
196+
.unwrap();
197+
let texture_1 = builder
198+
.add_texture(
199+
&mut layout,
200+
&make_filled_image(square, colors[1]),
201+
&mut atlas_texture,
202+
)
203+
.unwrap();
204+
let texture_2 = builder
205+
.add_texture(
206+
&mut layout,
207+
&make_filled_image(square, colors[2]),
208+
&mut atlas_texture,
209+
)
210+
.unwrap();
211+
let texture_3 = builder
212+
.add_texture(
213+
&mut layout,
214+
&make_filled_image(square, colors[3]),
215+
&mut atlas_texture,
216+
)
217+
.unwrap();
218+
let texture_4 = builder
219+
.add_texture(
220+
&mut layout,
221+
&make_filled_image(square, colors[4]),
222+
&mut atlas_texture,
223+
)
224+
.unwrap();
225+
let texture_5 = builder
226+
.add_texture(
227+
&mut layout,
228+
&make_filled_image(square, colors[5]),
229+
&mut atlas_texture,
230+
)
231+
.unwrap();
232+
233+
let expected_rects = [
234+
URect::from_corners(UVec2::new(0, 0), UVec2::new(10, 10)),
235+
URect::from_corners(UVec2::new(10, 0), UVec2::new(20, 10)),
236+
URect::from_corners(UVec2::new(20, 0), UVec2::new(30, 10)),
237+
URect::from_corners(UVec2::new(0, 10), UVec2::new(10, 20)),
238+
URect::from_corners(UVec2::new(0, 20), UVec2::new(10, 30)),
239+
URect::from_corners(UVec2::new(10, 10), UVec2::new(20, 20)),
240+
];
241+
assert_eq!(layout.textures[texture_0], expected_rects[0]);
242+
assert_eq!(layout.textures[texture_1], expected_rects[1]);
243+
assert_eq!(layout.textures[texture_2], expected_rects[2]);
244+
assert_eq!(layout.textures[texture_3], expected_rects[3]);
245+
assert_eq!(layout.textures[texture_4], expected_rects[4]);
246+
assert_eq!(layout.textures[texture_5], expected_rects[5]);
247+
248+
assert!(rect_contains_value(
249+
&atlas_texture,
250+
expected_rects[0],
251+
colors[0]
252+
));
253+
assert!(rect_contains_value(
254+
&atlas_texture,
255+
expected_rects[1],
256+
colors[1]
257+
));
258+
assert!(rect_contains_value(
259+
&atlas_texture,
260+
expected_rects[2],
261+
colors[2]
262+
));
263+
assert!(rect_contains_value(
264+
&atlas_texture,
265+
expected_rects[3],
266+
colors[3]
267+
));
268+
assert!(rect_contains_value(
269+
&atlas_texture,
270+
expected_rects[4],
271+
colors[4]
272+
));
273+
assert!(rect_contains_value(
274+
&atlas_texture,
275+
expected_rects[5],
276+
colors[5]
277+
));
278+
}
279+
280+
#[test]
281+
fn allocate_textures_with_padding() {
282+
let size = UVec2::new(12, 12);
283+
284+
let mut atlas_texture = make_filled_image(size, [0, 0, 0, 0]);
285+
let mut layout = TextureAtlasLayout::new_empty(size);
286+
let mut builder = DynamicTextureAtlasBuilder::new(size, 1);
287+
288+
let square = UVec2::new(3, 3);
289+
let colors = [[255, 0, 0, 255], [0, 255, 0, 255], [0, 0, 255, 255]];
290+
let texture_0 = builder
291+
.add_texture(
292+
&mut layout,
293+
&make_filled_image(square, colors[0]),
294+
&mut atlas_texture,
295+
)
296+
.unwrap();
297+
let texture_1 = builder
298+
.add_texture(
299+
&mut layout,
300+
&make_filled_image(square, colors[1]),
301+
&mut atlas_texture,
302+
)
303+
.unwrap();
304+
let texture_2 = builder
305+
.add_texture(
306+
&mut layout,
307+
&make_filled_image(square, colors[2]),
308+
&mut atlas_texture,
309+
)
310+
.unwrap();
311+
312+
let expected_rects = [
313+
URect::from_corners(UVec2::new(1, 1), UVec2::new(4, 4)),
314+
URect::from_corners(UVec2::new(5, 1), UVec2::new(8, 4)),
315+
// If we didn't pad the right of the texture atlas, there would be just enough space to
316+
// fit this in the first row, but since we do pad the right, this gets pushed to the
317+
// next row.
318+
URect::from_corners(UVec2::new(1, 5), UVec2::new(4, 8)),
319+
];
320+
assert_eq!(layout.textures[texture_0], expected_rects[0]);
321+
assert_eq!(layout.textures[texture_1], expected_rects[1]);
322+
assert_eq!(layout.textures[texture_2], expected_rects[2]);
323+
324+
assert!(rect_contains_value(
325+
&atlas_texture,
326+
expected_rects[0],
327+
colors[0]
328+
));
329+
assert!(rect_contains_value(
330+
&atlas_texture,
331+
expected_rects[1],
332+
colors[1]
333+
));
334+
assert!(rect_contains_value(
335+
&atlas_texture,
336+
expected_rects[2],
337+
colors[2]
338+
));
339+
}
340+
}

0 commit comments

Comments
 (0)