Skip to content

Commit

Permalink
Prefer UVec2 when working with texture dimensions (bevyengine#11698)
Browse files Browse the repository at this point in the history
# Objective

The physical width and height (pixels) of an image is always integers,
but for `GpuImage` bevy currently stores them as `Vec2` (`f32`).
Switching to `UVec2` makes this more consistent with the [underlying
texture data](https://docs.rs/wgpu/latest/wgpu/struct.Extent3d.html).

I'm not sure if this is worth the change in the surface level API. If
not, feel free to close this PR.

## Solution

- Replace uses of `Vec2` with `UVec2` when referring to texture
dimensions.
- Use integer types for the texture atlas dimensions and sections.


[`Sprite::rect`](https://github.com/bevyengine/bevy/blob/a81a2d1da31cc2eebcd9d431ed2b73a678ce61e3/crates/bevy_sprite/src/sprite.rs#L29)
remains unchanged, so manually specifying a sub-pixel region of an image
is still possible.

---

## Changelog

- `GpuImage` now stores its size as `UVec2` instead of `Vec2`.
- Texture atlases store their size and sections as `UVec2` and `URect`
respectively.
- `UiImageSize` stores its size as `UVec2`.

## Migration Guide

- Change floating point types (`Vec2`, `Rect`) to their respective
unsigned integer versions (`UVec2`, `URect`) when using `GpuImage`,
`TextureAtlasLayout`, `TextureAtlasBuilder`,
`DynamicAtlasTextureBuilder` or `FontAtlas`.
  • Loading branch information
CptPotato authored Feb 25, 2024
1 parent 14042b1 commit a7be8a2
Show file tree
Hide file tree
Showing 18 changed files with 89 additions and 95 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ impl FromWorld for MeshPipeline {
texture_view,
texture_format: image.texture_descriptor.format,
sampler,
size: image.size_f32(),
size: image.size(),
mip_level_count: image.texture_descriptor.mip_level_count,
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/texture/fallback_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn fallback_image_new(
texture_view,
texture_format: image.texture_descriptor.format,
sampler,
size: image.size_f32(),
size: image.size(),
mip_level_count: image.texture_descriptor.mip_level_count,
}
}
Expand Down
10 changes: 4 additions & 6 deletions crates/bevy_render/src/texture/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ pub struct GpuImage {
pub texture_view: TextureView,
pub texture_format: TextureFormat,
pub sampler: Sampler,
pub size: Vec2,
pub size: UVec2,
pub mip_level_count: u32,
}

Expand Down Expand Up @@ -851,16 +851,13 @@ impl RenderAsset for Image {
&self.data,
);

let size = self.size();
let texture_view = texture.create_view(
self.texture_view_descriptor
.or_else(|| Some(TextureViewDescriptor::default()))
.as_ref()
.unwrap(),
);
let size = Vec2::new(
self.texture_descriptor.size.width as f32,
self.texture_descriptor.size.height as f32,
);
let sampler = match self.sampler {
ImageSampler::Default => (***default_sampler).clone(),
ImageSampler::Descriptor(descriptor) => {
Expand Down Expand Up @@ -939,7 +936,6 @@ impl CompressedImageFormats {

#[cfg(test)]
mod test {

use super::*;
use crate::render_asset::RenderAssetUsages;

Expand All @@ -962,9 +958,11 @@ mod test {
image.size_f32()
);
}

#[test]
fn image_default_size() {
let image = Image::default();
assert_eq!(UVec2::ONE, image.size());
assert_eq!(Vec2::ONE, image.size_f32());
}
}
34 changes: 20 additions & 14 deletions crates/bevy_sprite/src/dynamic_texture_atlas_builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::TextureAtlasLayout;
use bevy_asset::{Assets, Handle};
use bevy_math::{IVec2, Rect, Vec2};
use bevy_math::{URect, UVec2};
use bevy_render::{
render_asset::{RenderAsset, RenderAssetUsages},
texture::{Image, TextureFormatPixelInfo},
Expand All @@ -13,7 +13,7 @@ use guillotiere::{size2, Allocation, AtlasAllocator};
/// e.g: in a font glyph [`TextureAtlasLayout`], only add the [`Image`] texture for letters to be rendered.
pub struct DynamicTextureAtlasBuilder {
atlas_allocator: AtlasAllocator,
padding: i32,
padding: u32,
}

impl DynamicTextureAtlasBuilder {
Expand All @@ -23,7 +23,7 @@ impl DynamicTextureAtlasBuilder {
///
/// * `size` - total size for the atlas
/// * `padding` - gap added between textures in the atlas, both in x axis and y axis
pub fn new(size: Vec2, padding: i32) -> Self {
pub fn new(size: UVec2, padding: u32) -> Self {
Self {
atlas_allocator: AtlasAllocator::new(to_size2(size)),
padding,
Expand All @@ -50,8 +50,8 @@ impl DynamicTextureAtlasBuilder {
atlas_texture_handle: &Handle<Image>,
) -> Option<usize> {
let allocation = self.atlas_allocator.allocate(size2(
texture.width() as i32 + self.padding,
texture.height() as i32 + self.padding,
(texture.width() + self.padding).try_into().unwrap(),
(texture.height() + self.padding).try_into().unwrap(),
));
if let Some(allocation) = allocation {
let atlas_texture = textures.get_mut(atlas_texture_handle).unwrap();
Expand All @@ -63,8 +63,8 @@ impl DynamicTextureAtlasBuilder {
);

self.place_texture(atlas_texture, allocation, texture);
let mut rect: Rect = to_rect(allocation.rectangle);
rect.max -= self.padding as f32;
let mut rect: URect = to_rect(allocation.rectangle);
rect.max = rect.max.saturating_sub(UVec2::splat(self.padding));
Some(atlas_layout.add_texture(rect))
} else {
None
Expand All @@ -78,8 +78,8 @@ impl DynamicTextureAtlasBuilder {
texture: &Image,
) {
let mut rect = allocation.rectangle;
rect.max.x -= self.padding;
rect.max.y -= self.padding;
rect.max.x -= self.padding as i32;
rect.max.y -= self.padding as i32;
let atlas_width = atlas_texture.width() as usize;
let rect_width = rect.width() as usize;
let format_size = atlas_texture.texture_descriptor.format.pixel_size();
Expand All @@ -95,13 +95,19 @@ impl DynamicTextureAtlasBuilder {
}
}

fn to_rect(rectangle: guillotiere::Rectangle) -> Rect {
Rect {
min: IVec2::new(rectangle.min.x, rectangle.min.y).as_vec2(),
max: IVec2::new(rectangle.max.x, rectangle.max.y).as_vec2(),
fn to_rect(rectangle: guillotiere::Rectangle) -> URect {
URect {
min: UVec2::new(
rectangle.min.x.try_into().unwrap(),
rectangle.min.y.try_into().unwrap(),
),
max: UVec2::new(
rectangle.max.x.try_into().unwrap(),
rectangle.max.y.try_into().unwrap(),
),
}
}

fn to_size2(vec2: Vec2) -> guillotiere::Size {
fn to_size2(vec2: UVec2) -> guillotiere::Size {
guillotiere::Size::new(vec2.x as i32, vec2.y as i32)
}
4 changes: 3 additions & 1 deletion crates/bevy_sprite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ pub fn calculate_bounds_2d(
// We default to the texture size for regular sprites
None => images.get(texture_handle).map(|image| image.size_f32()),
// We default to the drawn rect for atlas sprites
Some(atlas) => atlas.texture_rect(&atlases).map(|rect| rect.size()),
Some(atlas) => atlas
.texture_rect(&atlases)
.map(|rect| rect.size().as_vec2()),
}) {
let aabb = Aabb {
center: (-sprite.anchor.as_vec() * size).extend(0.0).into(),
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_sprite/src/mesh2d/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl FromWorld for Mesh2dPipeline {
texture_view,
texture_format: image.texture_descriptor.format,
sampler,
size: image.size_f32(),
size: image.size(),
mip_level_count: image.texture_descriptor.mip_level_count,
}
};
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl FromWorld for SpritePipeline {
texture_view,
texture_format: image.texture_descriptor.format,
sampler,
size: image.size_f32(),
size: image.size(),
mip_level_count: image.texture_descriptor.mip_level_count,
}
};
Expand Down Expand Up @@ -366,10 +366,10 @@ pub fn extract_sprites(
let rect = match (atlas_rect, sprite.rect) {
(None, None) => None,
(None, Some(sprite_rect)) => Some(sprite_rect),
(Some(atlas_rect), None) => Some(atlas_rect),
(Some(atlas_rect), None) => Some(atlas_rect.as_rect()),
(Some(atlas_rect), Some(mut sprite_rect)) => {
sprite_rect.min += atlas_rect.min;
sprite_rect.max += atlas_rect.min;
sprite_rect.min += atlas_rect.min.as_vec2();
sprite_rect.max += atlas_rect.min.as_vec2();

Some(sprite_rect)
}
Expand Down Expand Up @@ -618,7 +618,7 @@ pub fn prepare_sprites(
continue;
};

batch_image_size = Vec2::new(gpu_image.size.x, gpu_image.size.y);
batch_image_size = gpu_image.size.as_vec2();
batch_image_handle = extracted_sprite.image_handle_id;
image_bind_groups
.values
Expand Down
33 changes: 16 additions & 17 deletions crates/bevy_sprite/src/texture_atlas.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bevy_asset::{Asset, AssetId, Assets, Handle};
use bevy_ecs::component::Component;
use bevy_math::{Rect, Vec2};
use bevy_math::{URect, UVec2};
use bevy_reflect::Reflect;
use bevy_render::texture::Image;
use bevy_utils::HashMap;
Expand All @@ -19,9 +19,9 @@ use bevy_utils::HashMap;
#[reflect(Debug)]
pub struct TextureAtlasLayout {
// TODO: add support to Uniforms derive to write dimensions and sprites to the same buffer
pub size: Vec2,
pub size: UVec2,
/// The specific areas of the atlas where each texture can be found
pub textures: Vec<Rect>,
pub textures: Vec<URect>,
/// Maps from a specific image handle to the index in `textures` where they can be found.
///
/// This field is set by [`TextureAtlasBuilder`].
Expand Down Expand Up @@ -51,7 +51,7 @@ pub struct TextureAtlas {

impl TextureAtlasLayout {
/// Create a new empty layout with custom `dimensions`
pub fn new_empty(dimensions: Vec2) -> Self {
pub fn new_empty(dimensions: UVec2) -> Self {
Self {
size: dimensions,
texture_handles: None,
Expand All @@ -73,16 +73,16 @@ impl TextureAtlasLayout {
/// * `padding` - Optional padding between cells
/// * `offset` - Optional global grid offset
pub fn from_grid(
tile_size: Vec2,
columns: usize,
rows: usize,
padding: Option<Vec2>,
offset: Option<Vec2>,
tile_size: UVec2,
columns: u32,
rows: u32,
padding: Option<UVec2>,
offset: Option<UVec2>,
) -> Self {
let padding = padding.unwrap_or_default();
let offset = offset.unwrap_or_default();
let mut sprites = Vec::new();
let mut current_padding = Vec2::ZERO;
let mut current_padding = UVec2::ZERO;

for y in 0..rows {
if y > 0 {
Expand All @@ -93,18 +93,17 @@ impl TextureAtlasLayout {
current_padding.x = padding.x;
}

let cell = Vec2::new(x as f32, y as f32);

let cell = UVec2::new(x, y);
let rect_min = (tile_size + current_padding) * cell + offset;

sprites.push(Rect {
sprites.push(URect {
min: rect_min,
max: rect_min + tile_size,
});
}
}

let grid_size = Vec2::new(columns as f32, rows as f32);
let grid_size = UVec2::new(columns, rows);

Self {
size: ((tile_size + current_padding) * grid_size) - current_padding,
Expand All @@ -121,7 +120,7 @@ impl TextureAtlasLayout {
/// * `rect` - The section of the texture to be added
///
/// [`TextureAtlas`]: crate::TextureAtlas
pub fn add_texture(&mut self, rect: Rect) -> usize {
pub fn add_texture(&mut self, rect: URect) -> usize {
self.textures.push(rect);
self.textures.len() - 1
}
Expand Down Expand Up @@ -149,8 +148,8 @@ impl TextureAtlasLayout {
}

impl TextureAtlas {
/// Retrieves the current texture [`Rect`] of the sprite sheet according to the section `index`
pub fn texture_rect(&self, texture_atlases: &Assets<TextureAtlasLayout>) -> Option<Rect> {
/// Retrieves the current texture [`URect`] of the sprite sheet according to the section `index`
pub fn texture_rect(&self, texture_atlases: &Assets<TextureAtlasLayout>) -> Option<URect> {
let atlas = texture_atlases.get(&self.layout)?;
atlas.textures.get(self.index).copied()
}
Expand Down
37 changes: 16 additions & 21 deletions crates/bevy_sprite/src/texture_atlas_builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bevy_asset::AssetId;
use bevy_log::{debug, error, warn};
use bevy_math::{Rect, UVec2, Vec2};
use bevy_math::{URect, UVec2};
use bevy_render::{
render_asset::RenderAssetUsages,
render_resource::{Extent3d, TextureDimension, TextureFormat},
Expand Down Expand Up @@ -31,9 +31,9 @@ pub struct TextureAtlasBuilder<'a> {
/// Collection of texture's asset id (optional) and image data to be packed into an atlas
textures_to_place: Vec<(Option<AssetId<Image>>, &'a Image)>,
/// The initial atlas size in pixels.
initial_size: Vec2,
initial_size: UVec2,
/// The absolute maximum size of the texture atlas in pixels.
max_size: Vec2,
max_size: UVec2,
/// The texture format for the textures that will be loaded in the atlas.
format: TextureFormat,
/// Enable automatic format conversion for textures if they are not in the atlas format.
Expand All @@ -46,8 +46,8 @@ impl Default for TextureAtlasBuilder<'_> {
fn default() -> Self {
Self {
textures_to_place: Vec::new(),
initial_size: Vec2::new(256., 256.),
max_size: Vec2::new(2048., 2048.),
initial_size: UVec2::splat(256),
max_size: UVec2::splat(2048),
format: TextureFormat::Rgba8UnormSrgb,
auto_format_conversion: true,
padding: UVec2::ZERO,
Expand All @@ -59,13 +59,13 @@ pub type TextureAtlasBuilderResult<T> = Result<T, TextureAtlasBuilderError>;

impl<'a> TextureAtlasBuilder<'a> {
/// Sets the initial size of the atlas in pixels.
pub fn initial_size(mut self, size: Vec2) -> Self {
pub fn initial_size(mut self, size: UVec2) -> Self {
self.initial_size = size;
self
}

/// Sets the max size of the atlas in pixels.
pub fn max_size(mut self, size: Vec2) -> Self {
pub fn max_size(mut self, size: UVec2) -> Self {
self.max_size = size;
self
}
Expand Down Expand Up @@ -189,13 +189,11 @@ impl<'a> TextureAtlasBuilder<'a> {
/// If there is not enough space in the atlas texture, an error will
/// be returned. It is then recommended to make a larger sprite sheet.
pub fn finish(self) -> Result<(TextureAtlasLayout, Image), TextureAtlasBuilderError> {
let initial_width = self.initial_size.x as u32;
let initial_height = self.initial_size.y as u32;
let max_width = self.max_size.x as u32;
let max_height = self.max_size.y as u32;
let max_width = self.max_size.x;
let max_height = self.max_size.y;

let mut current_width = initial_width;
let mut current_height = initial_height;
let mut current_width = self.initial_size.x;
let mut current_height = self.initial_size.y;
let mut rect_placements = None;
let mut atlas_texture = Image::default();
let mut rects_to_place = GroupedRectsToPlace::<usize>::new();
Expand Down Expand Up @@ -265,16 +263,13 @@ impl<'a> TextureAtlasBuilder<'a> {
for (index, (image_id, texture)) in self.textures_to_place.iter().enumerate() {
let (_, packed_location) = rect_placements.packed_locations().get(&index).unwrap();

let min = Vec2::new(packed_location.x() as f32, packed_location.y() as f32);
let max = min
+ Vec2::new(
(packed_location.width() - self.padding.x) as f32,
(packed_location.height() - self.padding.y) as f32,
);
let min = UVec2::new(packed_location.x(), packed_location.y());
let max =
min + UVec2::new(packed_location.width(), packed_location.height()) - self.padding;
if let Some(image_id) = image_id {
texture_ids.insert(*image_id, index);
}
texture_rects.push(Rect { min, max });
texture_rects.push(URect { min, max });
if texture.texture_descriptor.format != self.format && !self.auto_format_conversion {
warn!(
"Loading a texture of format '{:?}' in an atlas with format '{:?}'",
Expand All @@ -287,7 +282,7 @@ impl<'a> TextureAtlasBuilder<'a> {

Ok((
TextureAtlasLayout {
size: atlas_texture.size_f32(),
size: atlas_texture.size(),
textures: texture_rects,
texture_handles: Some(texture_ids),
},
Expand Down
Loading

0 comments on commit a7be8a2

Please sign in to comment.