Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proportional scaling for the sprite's texture. #17258

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,17 @@ description = "Animates a sprite in response to an event"
category = "2D Rendering"
wasm = true

[[example]]
name = "sprite_scale"
path = "examples/2d/sprite_scale.rs"
doc-scrape-examples = true

[package.metadata.example.sprite_scale]
name = "Sprite Scale"
description = "Shows how a sprite could be scaled into a rectangle while keeping the aspect ratio"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description = "Shows how a sprite could be scaled into a rectangle while keeping the aspect ratio"
description = "Shows how a sprite can be scaled into a rectangle while keeping the aspect ratio"

category = "2D Rendering"
wasm = true

[[example]]
name = "sprite_flipping"
path = "examples/2d/sprite_flipping.rs"
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_sprite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub mod prelude {
pub use crate::{
sprite::{Sprite, SpriteImageMode},
texture_slice::{BorderRect, SliceScaleMode, TextureSlice, TextureSlicer},
ColorMaterial, MeshMaterial2d,
ColorMaterial, MeshMaterial2d, ScalingMode,
};
}

Expand Down
137 changes: 123 additions & 14 deletions crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use core::ops::Range;

use crate::{ComputedTextureSlices, Sprite, SPRITE_SHADER_HANDLE};
use crate::{ComputedTextureSlices, ScalingMode, Sprite, SPRITE_SHADER_HANDLE};
use bevy_asset::{AssetEvent, AssetId, Assets};
use bevy_color::{ColorToComponents, LinearRgba};
use bevy_core_pipeline::{
Expand Down Expand Up @@ -339,6 +339,7 @@ pub struct ExtractedSprite {
/// For cases where additional [`ExtractedSprites`] are created during extraction, this stores the
/// entity that caused that creation for use in determining visibility.
pub original_entity: Option<Entity>,
pub scaling_mode: Option<ScalingMode>,
}

#[derive(Resource, Default)]
Expand Down Expand Up @@ -430,6 +431,7 @@ pub fn extract_sprites(
image_handle_id: sprite.image.id(),
anchor: sprite.anchor.as_vec(),
original_entity: Some(original_entity),
scaling_mode: sprite.image_mode.scale(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer it if the sprite geometry was calculated during extraction rather than in prepare.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the geometry logic happens in prepare for now. I am going with less resistance and fewer changes in the codebase to achieve the necessary functionality.

},
);
}
Expand Down Expand Up @@ -700,21 +702,37 @@ pub fn prepare_sprite_image_bind_groups(
// By default, the size of the quad is the size of the texture
let mut quad_size = batch_image_size;

// Calculate vertex data for this item
let mut uv_offset_scale: Vec4;

// If a rect is specified, adjust UVs and the size of the quad
if let Some(rect) = extracted_sprite.rect {
let mut uv_offset_scale = if let Some(rect) = extracted_sprite.rect {
let rect_size = rect.size();
uv_offset_scale = Vec4::new(
quad_size = rect_size;
Vec4::new(
rect.min.x / batch_image_size.x,
rect.max.y / batch_image_size.y,
rect_size.x / batch_image_size.x,
-rect_size.y / batch_image_size.y,
);
quad_size = rect_size;
)
} else {
uv_offset_scale = Vec4::new(0.0, 1.0, 1.0, -1.0);
Vec4::new(0.0, 1.0, 1.0, -1.0)
};

// Override the size if a custom one is specified
if let Some(custom_size) = extracted_sprite.custom_size {
quad_size = custom_size;
}

// Used for translation of the quad if `TextureScale::Fit...` is specified.
let mut quad_translation = Vec2::ZERO;

// Scales the texture based on the `texture_scale` field.
if let Some(scaling_mode) = extracted_sprite.scaling_mode {
apply_scaling(
scaling_mode,
batch_image_size,
&mut quad_size,
&mut quad_translation,
&mut uv_offset_scale,
);
}

if extracted_sprite.flip_x {
Expand All @@ -726,15 +744,13 @@ pub fn prepare_sprite_image_bind_groups(
uv_offset_scale.w *= -1.0;
}

// Override the size if a custom one is specified
if let Some(custom_size) = extracted_sprite.custom_size {
quad_size = custom_size;
}
let transform = extracted_sprite.transform.affine()
* Affine3A::from_scale_rotation_translation(
quad_size.extend(1.0),
Quat::IDENTITY,
(quad_size * (-extracted_sprite.anchor - Vec2::splat(0.5))).extend(0.0),
((quad_size + quad_translation)
* (-extracted_sprite.anchor - Vec2::splat(0.5)))
.extend(0.0),
);

// Store the vertex data and add the item to the render phase
Expand Down Expand Up @@ -875,3 +891,96 @@ impl<P: PhaseItem> RenderCommand<P> for DrawSpriteBatch {
RenderCommandResult::Success
}
}

/// Scales a texture to fit within a given quad size with keeping the aspect ratio.
fn apply_scaling(
scaling_mode: ScalingMode,
texture_size: Vec2,
quad_size: &mut Vec2,
quad_translation: &mut Vec2,
uv_offset_scale: &mut Vec4,
) {
let quad_ratio = quad_size.x / quad_size.y;
let texture_ratio = texture_size.x / texture_size.y;

match scaling_mode {
ScalingMode::FillCenter => {
let scale = if quad_ratio > texture_ratio {
// Quad is wider than the image
Vec2::new(1., -texture_ratio / quad_ratio)
} else {
// Quad is taller than the image
Vec2::new(quad_ratio / texture_ratio, -1.)
};
let offset = (1.0 - scale) * 0.5;

// override all previous scaling and offset
*uv_offset_scale = Vec4::new(offset.x, offset.y, scale.x, scale.y);
}
ScalingMode::FillStart => {
if quad_ratio > texture_ratio {
let scale = Vec2::new(1., -texture_ratio / quad_ratio);
let offset = (1.0 - scale) * 0.5;
*uv_offset_scale = Vec4::new(offset.x, scale.y.abs(), scale.x, scale.y);
} else {
let scale = Vec2::new(quad_ratio / texture_ratio, -1.);
let offset = (1.0 - scale) * 0.5;
*uv_offset_scale = Vec4::new(0.0, offset.y, scale.x, scale.y);
}
}
ScalingMode::FillEnd => {
if quad_ratio > texture_ratio {
let scale = Vec2::new(1., -texture_ratio / quad_ratio);
let offset = (1.0 - scale) * 0.5;
*uv_offset_scale = Vec4::new(offset.x, 1.0, scale.x, scale.y);
} else {
let scale = Vec2::new(quad_ratio / texture_ratio, -1.);
let offset = (1.0 - scale) * 0.5;
*uv_offset_scale = Vec4::new(1.0 - scale.x, offset.y, scale.x, scale.y);
}
}
ScalingMode::FitCenter => {
let scale = if texture_ratio > quad_ratio {
// Scale based on width
Vec2::new(1.0, quad_ratio / texture_ratio)
} else {
// Scale based on height
Vec2::new(texture_ratio / quad_ratio, 1.0)
};

*quad_size *= scale;
}
ScalingMode::FitStart => {
if texture_ratio > quad_ratio {
// The quad is scaled to match the image ratio, and the quad translation is adjusted
// to start of the quad within the original quad size.
let scale = Vec2::new(1.0, quad_ratio / texture_ratio);
let new_quad = *quad_size * scale;
let offset = *quad_size - new_quad;
*quad_translation = Vec2::new(0.0, -offset.y);
*quad_size = new_quad;
} else {
let scale = Vec2::new(texture_ratio / quad_ratio, 1.0);
let new_quad = *quad_size * scale;
let offset = *quad_size - new_quad;
*quad_translation = Vec2::new(offset.x, 0.0);
*quad_size = new_quad;
}
}
ScalingMode::FitEnd => {
if texture_ratio > quad_ratio {
let scale = Vec2::new(1.0, quad_ratio / texture_ratio);
let new_quad = *quad_size * scale;
let offset = *quad_size - new_quad;
*quad_translation = Vec2::new(0.0, offset.y);
*quad_size = new_quad;
} else {
let scale = Vec2::new(texture_ratio / quad_ratio, 1.0);
let new_quad = *quad_size * scale;
let offset = *quad_size - new_quad;
*quad_translation = Vec2::new(-offset.x, 0.0);
*quad_size = new_quad;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are identical except for 1 minus sign, and there are similar duplication in the other Start/End cases. On one hand it would be nice if the repetition could be factored out so e.g. FitEnd is just FitStart and set the minus. On the other hand this code is pretty simple and easy to read.

It might also become simpler if you match on (scaling_mode, quad_ratio > texture_ratio) since every case has that if inside.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your concern about code duplication, and I agree that maintaining clarity is important. However, considering the minimal amount of code duplication we currently have, I believe it's reasonable to keep things as they are. The existing structure seems to be functioning well without introducing unnecessary complexity.

}
}
48 changes: 48 additions & 0 deletions crates/bevy_sprite/src/sprite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ pub enum SpriteImageMode {
/// The sprite will take on the size of the image by default, and will be stretched or shrunk if [`Sprite::custom_size`] is set.
#[default]
Auto,
/// The texture will be scaled to fit the rect bounds defined in [`Sprite::custom_size`].
/// Otherwise no scaling will be applied.
Scale(ScalingMode),
/// The texture will be cut in 9 slices, keeping the texture in proportions on resize
Sliced(TextureSlicer),
/// The texture will be repeated if stretched beyond `stretched_value`
Expand All @@ -185,6 +188,51 @@ impl SpriteImageMode {
SpriteImageMode::Sliced(..) | SpriteImageMode::Tiled { .. }
)
}

/// Returns [`ScalingMode`] if scale is presented or [`Option::None`] otherwise.
#[inline]
#[must_use]
pub const fn scale(&self) -> Option<ScalingMode> {
if let SpriteImageMode::Scale(scale) = self {
Some(*scale)
} else {
None
}
}
}

/// Represents various modes for proportional scaling of a texture.
///
/// Can be used in [`SpriteImageMode::Scale`].
#[derive(Debug, Clone, Copy, PartialEq, Default, Reflect)]
#[reflect(Debug)]
pub enum ScalingMode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe this type might be better split up into different two enums, one Fill/Fit and the other Start/Center/End.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe this type might be better split up into different two enums, one Fill/Fit and the other Start/Center/End.

I don't think that will be convenient for the end user. There are only six variants and no more are planned to add I guess. That led to the single field in Sprite that corresponds to scaling, adding one more enum adds more complexity.

Then SpriteImageMode::Scale needs to be a struct enum variant with two fields, one for Fill/Fit and the second one for direction Start/Center/End. In my humble opinion, it will not bring convenience for the current API.

/// Scale the texture uniformly (maintain the texture's aspect ratio)
/// so that both dimensions (width and height) of the texture will be equal
/// to or larger than the corresponding dimension of the rect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you mean the rect field of the sprite? Needs to be clearer I think.

/// Fill rect with a centered texture.
#[default]
FillCenter,
/// Scale the texture to fill the rect with a start of the texture,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Scale the texture to fill the rect with a start of the texture,
/// Scale the texture to fill the rect with the start of the texture,

This isn't really very clear about what the "start" of the texture is.

/// maintaining the aspect ratio.
FillStart,
/// Scale the texture to fill the rect with a end of the texture,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Scale the texture to fill the rect with a end of the texture,
/// Scale the texture to fill the rect with the end of the texture,

/// maintaining the aspect ratio.
FillEnd,
/// Scaling the texture will maintain the original aspect ratio
/// and ensure that the original texture fits entirely inside the rect.
/// At least one axis (X or Y) will fit exactly. The result is centered inside the rect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// At least one axis (X or Y) will fit exactly. The result is centered inside the rect.
/// At least one axis (x or y) will fit exactly. The result is centered inside the rect.

FitCenter,
/// Scaling the texture will maintain the original aspect ratio
/// and ensure that the original texture fits entirely inside rect.
/// At least one axis (X or Y) will fit exactly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// At least one axis (X or Y) will fit exactly.
/// At least one axis (x or y) will fit exactly.

/// Aligns the result to the left and top edges of rect.
FitStart,
/// Scaling the texture will maintain the original aspect ratio
/// and ensure that the original texture fits entirely inside rect.
/// At least one axis (X or Y) will fit exactly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// At least one axis (X or Y) will fit exactly.
/// At least one axis (x or y) will fit exactly.

/// Aligns the result to the right and bottom edges of rect.
FitEnd,
}

/// How a sprite is positioned relative to its [`Transform`].
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_sprite/src/texture_slice/computed_slices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ impl ComputedTextureSlices {
flip_y,
image_handle_id: sprite.image.id(),
anchor: Self::redepend_anchor_from_sprite_to_slice(sprite, slice),
scaling_mode: sprite.image_mode.scale(),
}
})
}
Expand Down Expand Up @@ -123,6 +124,9 @@ fn compute_sprite_slices(
SpriteImageMode::Auto => {
unreachable!("Slices should not be computed for SpriteImageMode::Stretch")
}
SpriteImageMode::Scale(_) => {
unreachable!("Slices should not be computed for SpriteImageMode::Scale")
}
};
Some(ComputedTextureSlices(slices))
}
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_text/src/text2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ pub fn extract_text2d_sprite(
flip_y: false,
anchor: Anchor::Center.as_vec(),
original_entity: Some(original_entity),
scaling_mode: None,
},
);
}
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_ui/src/render/ui_texture_slice_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,10 @@ fn compute_texture_slices(
[[0., 0., 1., 1.], [0., 0., 1., 1.], [1., 1., rx, ry]]
}
SpriteImageMode::Auto => {
unreachable!("Slices should not be computed for ImageScaleMode::Stretch")
unreachable!("Slices should not be computed for SpriteImageMode::Stretch")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unreachable!("Slices should not be computed for SpriteImageMode::Stretch")
unreachable!("Slices can not be computed for SpriteImageMode::Auto")

}
SpriteImageMode::Scale(_) => {
unreachable!("Slices should not be computed for SpriteImageMode::Scale")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unreachable!("Slices should not be computed for SpriteImageMode::Scale")
unreachable!("Slices can not be computed for SpriteImageMode::Scale")

}
}
}
Expand Down
Loading
Loading