From a35eed0ea4e5739a668e5661cddc87f69750484d Mon Sep 17 00:00:00 2001 From: ZoOL Date: Thu, 5 Jun 2025 11:59:20 +0800 Subject: [PATCH] fix: Ensure linear volume subtraction does not go below zero (#19423) fix: [Ensure linear volume subtraction does not go below zero ](https://github.com/bevyengine/bevy/issues/19417) ## Solution - Clamp the result of linear volume subtraction to a minimum of 0.0 - Add a new test case to verify behavior when subtracting beyond zero --------- Co-authored-by: Alice Cecile Co-authored-by: Jan Hohenheim --- crates/bevy_audio/src/volume.rs | 249 +++++++++++------- examples/audio/audio_control.rs | 6 +- examples/audio/soundtrack.rs | 8 +- .../remove_the_add_sub_impls_on_volume.md | 25 ++ 4 files changed, 189 insertions(+), 99 deletions(-) create mode 100644 release-content/migration-guides/remove_the_add_sub_impls_on_volume.md diff --git a/crates/bevy_audio/src/volume.rs b/crates/bevy_audio/src/volume.rs index 3c19d189ef..1f1f417594 100644 --- a/crates/bevy_audio/src/volume.rs +++ b/crates/bevy_audio/src/volume.rs @@ -34,7 +34,7 @@ impl GlobalVolume { #[derive(Clone, Copy, Debug, Reflect)] #[reflect(Clone, Debug, PartialEq)] pub enum Volume { - /// Create a new [`Volume`] from the given volume in linear scale. + /// Create a new [`Volume`] from the given volume in the linear scale. /// /// In a linear scale, the value `1.0` represents the "normal" volume, /// meaning the audio is played at its original level. Values greater than @@ -144,7 +144,7 @@ impl Volume { /// Returns the volume in decibels as a float. /// - /// If the volume is silent / off / muted, i.e. its underlying linear scale + /// If the volume is silent / off / muted, i.e., its underlying linear scale /// is `0.0`, this method returns negative infinity. pub fn to_decibels(&self) -> f32 { match self { @@ -155,57 +155,95 @@ impl Volume { /// The silent volume. Also known as "off" or "muted". pub const SILENT: Self = Volume::Linear(0.0); -} -impl core::ops::Add for Volume { - type Output = Self; - - fn add(self, rhs: Self) -> Self { - use Volume::{Decibels, Linear}; - - match (self, rhs) { - (Linear(a), Linear(b)) => Linear(a + b), - (Decibels(a), Decibels(b)) => Decibels(linear_to_decibels( - decibels_to_linear(a) + decibels_to_linear(b), - )), - // {Linear, Decibels} favors the left hand side of the operation by - // first converting the right hand side to the same type as the left - // hand side and then performing the operation. - (Linear(..), Decibels(db)) => self + Linear(decibels_to_linear(db)), - (Decibels(..), Linear(l)) => self + Decibels(linear_to_decibels(l)), - } + /// Increases the volume by the specified percentage. + /// + /// This method works in the linear domain, where a 100% increase + /// means doubling the volume (equivalent to +6.02dB). + /// + /// # Arguments + /// * `percentage` - The percentage to increase (50.0 means 50% increase) + /// + /// # Examples + /// ``` + /// use bevy_audio::Volume; + /// + /// let volume = Volume::Linear(1.0); + /// let increased = volume.increase_by_percentage(100.0); + /// assert_eq!(increased.to_linear(), 2.0); + /// ``` + pub fn increase_by_percentage(&self, percentage: f32) -> Self { + let factor = 1.0 + (percentage / 100.0); + Volume::Linear(self.to_linear() * factor) } -} -impl core::ops::AddAssign for Volume { - fn add_assign(&mut self, rhs: Self) { - *self = *self + rhs; + /// Decreases the volume by the specified percentage. + /// + /// This method works in the linear domain, where a 50% decrease + /// means halving the volume (equivalent to -6.02dB). + /// + /// # Arguments + /// * `percentage` - The percentage to decrease (50.0 means 50% decrease) + /// + /// # Examples + /// ``` + /// use bevy_audio::Volume; + /// + /// let volume = Volume::Linear(1.0); + /// let decreased = volume.decrease_by_percentage(50.0); + /// assert_eq!(decreased.to_linear(), 0.5); + /// ``` + pub fn decrease_by_percentage(&self, percentage: f32) -> Self { + let factor = 1.0 - (percentage / 100.0).clamp(0.0, 1.0); + Volume::Linear(self.to_linear() * factor) } -} -impl core::ops::Sub for Volume { - type Output = Self; - - fn sub(self, rhs: Self) -> Self { - use Volume::{Decibels, Linear}; - - match (self, rhs) { - (Linear(a), Linear(b)) => Linear(a - b), - (Decibels(a), Decibels(b)) => Decibels(linear_to_decibels( - decibels_to_linear(a) - decibels_to_linear(b), - )), - // {Linear, Decibels} favors the left hand side of the operation by - // first converting the right hand side to the same type as the left - // hand side and then performing the operation. - (Linear(..), Decibels(db)) => self - Linear(decibels_to_linear(db)), - (Decibels(..), Linear(l)) => self - Decibels(linear_to_decibels(l)), - } + /// Scales the volume to a specific linear factor relative to the current volume. + /// + /// This is different from `adjust_by_linear` as it sets the volume to be + /// exactly the factor times the original volume, rather than applying + /// the factor to the current volume. + /// + /// # Arguments + /// * `factor` - The scaling factor (2.0 = twice as loud, 0.5 = half as loud) + /// + /// # Examples + /// ``` + /// use bevy_audio::Volume; + /// + /// let volume = Volume::Linear(0.8); + /// let scaled = volume.scale_to_factor(1.25); + /// assert_eq!(scaled.to_linear(), 1.0); + /// ``` + pub fn scale_to_factor(&self, factor: f32) -> Self { + Volume::Linear(self.to_linear() * factor) } -} -impl core::ops::SubAssign for Volume { - fn sub_assign(&mut self, rhs: Self) { - *self = *self - rhs; + /// Creates a fade effect by interpolating between current volume and target volume. + /// + /// This method performs linear interpolation in the linear domain, which + /// provides a more natural-sounding fade effect. + /// + /// # Arguments + /// * `target` - The target volume to fade towards + /// * `factor` - The interpolation factor (0.0 = current volume, 1.0 = target volume) + /// + /// # Examples + /// ``` + /// use bevy_audio::Volume; + /// + /// let current = Volume::Linear(1.0); + /// let target = Volume::Linear(0.0); + /// let faded = current.fade_towards(target, 0.5); + /// assert_eq!(faded.to_linear(), 0.5); + /// ``` + pub fn fade_towards(&self, target: Volume, factor: f32) -> Self { + let current_linear = self.to_linear(); + let target_linear = target.to_linear(); + let factor_clamped = factor.clamp(0.0, 1.0); + + let interpolated = current_linear + (target_linear - current_linear) * factor_clamped; + Volume::Linear(interpolated) } } @@ -337,8 +375,9 @@ mod tests { Linear(f32::NEG_INFINITY).to_decibels().is_infinite(), "Negative infinite linear scale is equivalent to infinite decibels" ); - assert!( - Decibels(f32::NEG_INFINITY).to_linear().abs() == 0.0, + assert_eq!( + Decibels(f32::NEG_INFINITY).to_linear().abs(), + 0.0, "Negative infinity decibels is equivalent to zero linear scale" ); @@ -361,6 +400,74 @@ mod tests { ); } + #[test] + fn test_increase_by_percentage() { + let volume = Linear(1.0); + + // 100% increase should double the volume + let increased = volume.increase_by_percentage(100.0); + assert_eq!(increased.to_linear(), 2.0); + + // 50% increase + let increased = volume.increase_by_percentage(50.0); + assert_eq!(increased.to_linear(), 1.5); + } + + #[test] + fn test_decrease_by_percentage() { + let volume = Linear(1.0); + + // 50% decrease should halve the volume + let decreased = volume.decrease_by_percentage(50.0); + assert_eq!(decreased.to_linear(), 0.5); + + // 25% decrease + let decreased = volume.decrease_by_percentage(25.0); + assert_eq!(decreased.to_linear(), 0.75); + + // 100% decrease should result in silence + let decreased = volume.decrease_by_percentage(100.0); + assert_eq!(decreased.to_linear(), 0.0); + } + + #[test] + fn test_scale_to_factor() { + let volume = Linear(0.8); + let scaled = volume.scale_to_factor(1.25); + assert_eq!(scaled.to_linear(), 1.0); + } + + #[test] + fn test_fade_towards() { + let current = Linear(1.0); + let target = Linear(0.0); + + // 50% fade should result in 0.5 linear volume + let faded = current.fade_towards(target, 0.5); + assert_eq!(faded.to_linear(), 0.5); + + // 0% fade should keep current volume + let faded = current.fade_towards(target, 0.0); + assert_eq!(faded.to_linear(), 1.0); + + // 100% fade should reach target volume + let faded = current.fade_towards(target, 1.0); + assert_eq!(faded.to_linear(), 0.0); + } + + #[test] + fn test_decibel_math_properties() { + let volume = Linear(1.0); + + // Adding 20dB should multiply linear volume by 10 + let adjusted = volume * Decibels(20.0); + assert_approx_eq(adjusted, Linear(10.0)); + + // Subtracting 20dB should divide linear volume by 10 + let adjusted = volume / Decibels(20.0); + assert_approx_eq(adjusted, Linear(0.1)); + } + fn assert_approx_eq(a: Volume, b: Volume) { const EPSILON: f32 = 0.0001; @@ -380,52 +487,6 @@ mod tests { } } - #[test] - fn volume_ops_add() { - // Linear to Linear. - assert_approx_eq(Linear(0.5) + Linear(0.5), Linear(1.0)); - assert_approx_eq(Linear(0.5) + Linear(0.1), Linear(0.6)); - assert_approx_eq(Linear(0.5) + Linear(-0.5), Linear(0.0)); - - // Decibels to Decibels. - assert_approx_eq(Decibels(0.0) + Decibels(0.0), Decibels(6.0206003)); - assert_approx_eq(Decibels(6.0) + Decibels(6.0), Decibels(12.020599)); - assert_approx_eq(Decibels(-6.0) + Decibels(-6.0), Decibels(0.020599423)); - - // {Linear, Decibels} favors the left hand side of the operation. - assert_approx_eq(Linear(0.5) + Decibels(0.0), Linear(1.5)); - assert_approx_eq(Decibels(0.0) + Linear(0.5), Decibels(3.521825)); - } - - #[test] - fn volume_ops_add_assign() { - // Linear to Linear. - let mut volume = Linear(0.5); - volume += Linear(0.5); - assert_approx_eq(volume, Linear(1.0)); - } - - #[test] - fn volume_ops_sub() { - // Linear to Linear. - assert_approx_eq(Linear(0.5) - Linear(0.5), Linear(0.0)); - assert_approx_eq(Linear(0.5) - Linear(0.1), Linear(0.4)); - assert_approx_eq(Linear(0.5) - Linear(-0.5), Linear(1.0)); - - // Decibels to Decibels. - assert_eq!(Decibels(0.0) - Decibels(0.0), Decibels(f32::NEG_INFINITY)); - assert_approx_eq(Decibels(6.0) - Decibels(4.0), Decibels(-7.736506)); - assert_eq!(Decibels(-6.0) - Decibels(-6.0), Decibels(f32::NEG_INFINITY)); - } - - #[test] - fn volume_ops_sub_assign() { - // Linear to Linear. - let mut volume = Linear(0.5); - volume -= Linear(0.5); - assert_approx_eq(volume, Linear(0.0)); - } - #[test] fn volume_ops_mul() { // Linear to Linear. diff --git a/examples/audio/audio_control.rs b/examples/audio/audio_control.rs index 19bb8c807a..3a6e4a609a 100644 --- a/examples/audio/audio_control.rs +++ b/examples/audio/audio_control.rs @@ -1,6 +1,6 @@ //! This example illustrates how to load and play an audio file, and control how it's played. -use bevy::{audio::Volume, math::ops, prelude::*}; +use bevy::{math::ops, prelude::*}; fn main() { App::new() @@ -105,9 +105,9 @@ fn volume( if keyboard_input.just_pressed(KeyCode::Equal) { let current_volume = sink.volume(); - sink.set_volume(current_volume + Volume::Linear(0.1)); + sink.set_volume(current_volume.increase_by_percentage(10.0)); } else if keyboard_input.just_pressed(KeyCode::Minus) { let current_volume = sink.volume(); - sink.set_volume(current_volume - Volume::Linear(0.1)); + sink.set_volume(current_volume.increase_by_percentage(-10.0)); } } diff --git a/examples/audio/soundtrack.rs b/examples/audio/soundtrack.rs index 8a6a0dfb9a..27163b90d0 100644 --- a/examples/audio/soundtrack.rs +++ b/examples/audio/soundtrack.rs @@ -115,7 +115,9 @@ fn fade_in( ) { for (mut audio, entity) in audio_sink.iter_mut() { let current_volume = audio.volume(); - audio.set_volume(current_volume + Volume::Linear(time.delta_secs() / FADE_TIME)); + audio.set_volume( + current_volume.fade_towards(Volume::Linear(1.0), time.delta_secs() / FADE_TIME), + ); if audio.volume().to_linear() >= 1.0 { audio.set_volume(Volume::Linear(1.0)); commands.entity(entity).remove::(); @@ -132,7 +134,9 @@ fn fade_out( ) { for (mut audio, entity) in audio_sink.iter_mut() { let current_volume = audio.volume(); - audio.set_volume(current_volume - Volume::Linear(time.delta_secs() / FADE_TIME)); + audio.set_volume( + current_volume.fade_towards(Volume::Linear(0.0), time.delta_secs() / FADE_TIME), + ); if audio.volume().to_linear() <= 0.0 { commands.entity(entity).despawn(); } diff --git a/release-content/migration-guides/remove_the_add_sub_impls_on_volume.md b/release-content/migration-guides/remove_the_add_sub_impls_on_volume.md new file mode 100644 index 0000000000..859fe2384f --- /dev/null +++ b/release-content/migration-guides/remove_the_add_sub_impls_on_volume.md @@ -0,0 +1,25 @@ +--- +title: remove the Add/Sub impls on Volume +pull_requests: [ 19423 ] +--- + +Linear volumes are like percentages, and it does not make sense to add or subtract percentages. +As such, use the new `increase_by_percentage` function instead of addition or subtraction. + +```rust +// 0.16 +fn audio_system() { + let linear_a = Volume::Linear(0.5); + let linear_b = Volume::Linear(0.1); + let linear_c = linear_a + linear_b; + let linear_d = linear_a - linear_b; +} + +// 0.17 +fn audio_system() { + let linear_a = Volume::Linear(0.5); + let linear_b = Volume::Linear(0.1); + let linear_c = linear_a.increase_by_percentage(10.0); + let linear_d = linear_a.increase_by_percentage(-10.0); +} +```