From 29c632b5242980cd69895140c0060acfb93918cf Mon Sep 17 00:00:00 2001 From: Han Damin Date: Tue, 10 Sep 2024 01:04:41 +0900 Subject: [PATCH] Add common aspect ratio constants and improve documentation (#15091) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hello, I'd like to contribute to this project by adding some useful constants and improving the documentation for the AspectRatio struct. Here's a summary of the changes I've made: 1. Added new constants for common aspect ratios: - SIXTEEN_NINE (16:9) - FOUR_THREE (4:3) - ULTRAWIDE (21:9) 2. Enhanced the overall documentation: - Improved module-level documentation with an overview and use cases - Expanded explanation of the AspectRatio struct with examples - Added detailed descriptions and examples for all methods (both existing and new) - Included explanations for the newly introduced constant values - Added clarifications for From trait implementations These changes aim to make the AspectRatio API more user-friendly and easier to understand. The new constants provide convenient access to commonly used aspect ratios, which I believe will be helpful in many scenarios. --------- Co-authored-by: Gonçalo Rica Pais da Silva Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com> --- .../bevy_core_pipeline/src/bloom/settings.rs | 4 +- crates/bevy_math/src/aspect_ratio.rs | 84 +++++++++++++++++-- crates/bevy_pbr/src/cluster/mod.rs | 5 +- crates/bevy_render/src/camera/projection.rs | 4 +- crates/bevy_render/src/texture/image.rs | 4 +- 5 files changed, 87 insertions(+), 14 deletions(-) diff --git a/crates/bevy_core_pipeline/src/bloom/settings.rs b/crates/bevy_core_pipeline/src/bloom/settings.rs index 8fe6628214..4e8e0ff1f7 100644 --- a/crates/bevy_core_pipeline/src/bloom/settings.rs +++ b/crates/bevy_core_pipeline/src/bloom/settings.rs @@ -229,7 +229,9 @@ impl ExtractComponent for BloomSettings { viewport: UVec4::new(origin.x, origin.y, size.x, size.y).as_vec4() / UVec4::new(target_size.x, target_size.y, target_size.x, target_size.y) .as_vec4(), - aspect: AspectRatio::from_pixels(size.x, size.y).into(), + aspect: AspectRatio::try_from_pixels(size.x, size.y) + .expect("Valid screen size values for Bloom settings") + .ratio(), uv_offset: settings.uv_offset, }; diff --git a/crates/bevy_math/src/aspect_ratio.rs b/crates/bevy_math/src/aspect_ratio.rs index 97960015a5..5b0fc47946 100644 --- a/crates/bevy_math/src/aspect_ratio.rs +++ b/crates/bevy_math/src/aspect_ratio.rs @@ -1,6 +1,7 @@ //! Provides a simple aspect ratio struct to help with calculations. use crate::Vec2; +use thiserror::Error; #[cfg(feature = "bevy_reflect")] use bevy_reflect::Reflect; @@ -11,23 +12,74 @@ use bevy_reflect::Reflect; pub struct AspectRatio(f32); impl AspectRatio { - /// Create a new `AspectRatio` from a given `width` and `height`. + /// Standard 16:9 aspect ratio + pub const SIXTEEN_NINE: Self = Self(16.0 / 9.0); + /// Standard 4:3 aspect ratio + pub const FOUR_THREE: Self = Self(4.0 / 3.0); + /// Standard 21:9 ultrawide aspect ratio + pub const ULTRAWIDE: Self = Self(21.0 / 9.0); + + /// Attempts to create a new [`AspectRatio`] from a given width and height. + /// + /// # Errors + /// + /// Returns an `Err` with `AspectRatioError` if: + /// - Either width or height is zero (`AspectRatioError::Zero`) + /// - Either width or height is infinite (`AspectRatioError::Infinite`) + /// - Either width or height is NaN (`AspectRatioError::NaN`) #[inline] - pub fn new(width: f32, height: f32) -> Self { - Self(width / height) + pub fn try_new(width: f32, height: f32) -> Result { + match (width, height) { + (w, h) if w == 0.0 || h == 0.0 => Err(AspectRatioError::Zero), + (w, h) if w.is_infinite() || h.is_infinite() => Err(AspectRatioError::Infinite), + (w, h) if w.is_nan() || h.is_nan() => Err(AspectRatioError::NaN), + _ => Ok(Self(width / height)), + } } - /// Create a new `AspectRatio` from a given amount of `x` pixels and `y` pixels. + /// Attempts to create a new [`AspectRatio`] from a given amount of x pixels and y pixels. #[inline] - pub fn from_pixels(x: u32, y: u32) -> Self { - Self::new(x as f32, y as f32) + pub fn try_from_pixels(x: u32, y: u32) -> Result { + Self::try_new(x as f32, y as f32) + } + + /// Returns the aspect ratio as a f32 value. + #[inline] + pub fn ratio(&self) -> f32 { + self.0 + } + + /// Returns the inverse of this aspect ratio (height/width). + #[inline] + pub fn inverse(&self) -> Self { + Self(1.0 / self.0) + } + + /// Returns true if the aspect ratio represents a landscape orientation. + #[inline] + pub fn is_landscape(&self) -> bool { + self.0 > 1.0 + } + + /// Returns true if the aspect ratio represents a portrait orientation. + #[inline] + pub fn is_portrait(&self) -> bool { + self.0 < 1.0 + } + + /// Returns true if the aspect ratio is exactly square. + #[inline] + pub fn is_square(&self) -> bool { + self.0 == 1.0 } } -impl From for AspectRatio { +impl TryFrom for AspectRatio { + type Error = AspectRatioError; + #[inline] - fn from(value: Vec2) -> Self { - Self::new(value.x, value.y) + fn try_from(value: Vec2) -> Result { + Self::try_new(value.x, value.y) } } @@ -37,3 +89,17 @@ impl From for f32 { aspect_ratio.0 } } + +/// An Error type for when [`super::AspectRatio`] is provided invalid width or height values +#[derive(Error, Debug, PartialEq, Eq, Clone, Copy)] +pub enum AspectRatioError { + /// Error due to width or height having zero as a value. + #[error("AspectRatio error: width or height is zero")] + Zero, + /// Error due towidth or height being infinite. + #[error("AspectRatio error: width or height is infinite")] + Infinite, + /// Error due to width or height being Not a Number (NaN). + #[error("AspectRatio error: width or height is NaN")] + NaN, +} diff --git a/crates/bevy_pbr/src/cluster/mod.rs b/crates/bevy_pbr/src/cluster/mod.rs index 7da6c5da02..5ebb19dc3d 100644 --- a/crates/bevy_pbr/src/cluster/mod.rs +++ b/crates/bevy_pbr/src/cluster/mod.rs @@ -257,8 +257,9 @@ impl ClusterConfig { ClusterConfig::FixedZ { total, z_slices, .. } => { - let aspect_ratio: f32 = - AspectRatio::from_pixels(screen_size.x, screen_size.y).into(); + let aspect_ratio: f32 = AspectRatio::try_from_pixels(screen_size.x, screen_size.y) + .expect("Failed to calculate aspect ratio for Cluster: screen dimensions must be positive, non-zero values") + .ratio(); let mut z_slices = *z_slices; if *total < z_slices { warn!("ClusterConfig has more z-slices than total clusters!"); diff --git a/crates/bevy_render/src/camera/projection.rs b/crates/bevy_render/src/camera/projection.rs index 2aa5f8acc0..bb6b047690 100644 --- a/crates/bevy_render/src/camera/projection.rs +++ b/crates/bevy_render/src/camera/projection.rs @@ -190,7 +190,9 @@ impl CameraProjection for PerspectiveProjection { } fn update(&mut self, width: f32, height: f32) { - self.aspect_ratio = AspectRatio::new(width, height).into(); + self.aspect_ratio = AspectRatio::try_new(width, height) + .expect("Failed to update PerspectiveProjection: width and height must be positive, non-zero values") + .ratio(); } fn far(&self) -> f32 { diff --git a/crates/bevy_render/src/texture/image.rs b/crates/bevy_render/src/texture/image.rs index 8d92395b63..55b1560a90 100644 --- a/crates/bevy_render/src/texture/image.rs +++ b/crates/bevy_render/src/texture/image.rs @@ -666,7 +666,9 @@ impl Image { /// Returns the aspect ratio (width / height) of a 2D image. #[inline] pub fn aspect_ratio(&self) -> AspectRatio { - AspectRatio::from_pixels(self.width(), self.height()) + AspectRatio::try_from_pixels(self.width(), self.height()).expect( + "Failed to calculate aspect ratio: Image dimensions must be positive, non-zero values", + ) } /// Returns the size of a 2D image as f32.