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 <alice.i.cecile@gmail.com>
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
This commit is contained in:
ZoOL 2025-06-05 11:59:20 +08:00 committed by GitHub
parent d0f1b3e9f1
commit a35eed0ea4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 189 additions and 99 deletions

View File

@ -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<Self> 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<Self> 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<Self> 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<Self> 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.

View File

@ -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));
}
}

View File

@ -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::<FadeIn>();
@ -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();
}

View File

@ -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);
}
```