Auto merge of #20189 - servo:squared-distance-for-real, r=emilio

Never store a squared root in SquaredDistance

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20189)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-03-03 20:11:41 -05:00 committed by GitHub
commit 86a7010082
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 27 additions and 31 deletions

View file

@ -811,7 +811,7 @@ impl Animate for Visibility {
impl ComputeSquaredDistance for Visibility { impl ComputeSquaredDistance for Visibility {
#[inline] #[inline]
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
Ok(SquaredDistance::Value(if *self == *other { 0. } else { 1. })) Ok(SquaredDistance::from_sqrt(if *self == *other { 0. } else { 1. }))
} }
} }
@ -1921,7 +1921,7 @@ impl ComputeSquaredDistance for Quaternion {
// so we can get their angle difference by: // so we can get their angle difference by:
// cos(theta/2) = (q1 dot q2) / (|q1| * |q2|) = q1 dot q2. // cos(theta/2) = (q1 dot q2) / (|q1| * |q2|) = q1 dot q2.
let distance = self.dot(other).max(-1.0).min(1.0).acos() * 2.0; let distance = self.dot(other).max(-1.0).min(1.0).acos() * 2.0;
Ok(SquaredDistance::Value(distance * distance)) Ok(SquaredDistance::from_sqrt(distance))
} }
} }

View file

@ -166,19 +166,19 @@ impl ComputeSquaredDistance for Color {
// All comments from the Animate impl also applies here. // All comments from the Animate impl also applies here.
if self.foreground_ratio == other.foreground_ratio { if self.foreground_ratio == other.foreground_ratio {
if self.is_currentcolor() { if self.is_currentcolor() {
Ok(SquaredDistance::Value(0.)) Ok(SquaredDistance::from_sqrt(0.))
} else { } else {
self.color.compute_squared_distance(&other.color) self.color.compute_squared_distance(&other.color)
} }
} else if self.is_currentcolor() && other.is_numeric() { } else if self.is_currentcolor() && other.is_numeric() {
Ok( Ok(
RGBA::transparent().compute_squared_distance(&other.color)? + RGBA::transparent().compute_squared_distance(&other.color)? +
SquaredDistance::Value(1.), SquaredDistance::from_sqrt(1.),
) )
} else if self.is_numeric() && other.is_currentcolor() { } else if self.is_numeric() && other.is_currentcolor() {
Ok( Ok(
self.color.compute_squared_distance(&RGBA::transparent())? + self.color.compute_squared_distance(&RGBA::transparent())? +
SquaredDistance::Value(1.), SquaredDistance::from_sqrt(1.),
) )
} else { } else {
let self_color = self.effective_intermediate_rgba(); let self_color = self.effective_intermediate_rgba();

View file

@ -28,38 +28,43 @@ pub trait ComputeSquaredDistance {
/// A distance between two animatable values. /// A distance between two animatable values.
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug)]
pub enum SquaredDistance { pub struct SquaredDistance {
/// Represented as the square root of the squared distance. value: f64,
Sqrt(f64), }
/// Represented as the squared distance itself.
Value(f64), impl SquaredDistance {
/// Returns a squared distance from its square root.
#[inline]
pub fn from_sqrt(sqrt: f64) -> Self {
Self { value: sqrt * sqrt }
}
} }
impl ComputeSquaredDistance for u16 { impl ComputeSquaredDistance for u16 {
#[inline] #[inline]
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
Ok(SquaredDistance::Sqrt(((*self as f64) - (*other as f64)).abs())) Ok(SquaredDistance::from_sqrt(((*self as f64) - (*other as f64)).abs()))
} }
} }
impl ComputeSquaredDistance for i32 { impl ComputeSquaredDistance for i32 {
#[inline] #[inline]
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
Ok(SquaredDistance::Sqrt((*self - *other).abs() as f64)) Ok(SquaredDistance::from_sqrt((*self - *other).abs() as f64))
} }
} }
impl ComputeSquaredDistance for f32 { impl ComputeSquaredDistance for f32 {
#[inline] #[inline]
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
Ok(SquaredDistance::Sqrt((*self - *other).abs() as f64)) Ok(SquaredDistance::from_sqrt((*self - *other).abs() as f64))
} }
} }
impl ComputeSquaredDistance for f64 { impl ComputeSquaredDistance for f64 {
#[inline] #[inline]
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
Ok(SquaredDistance::Sqrt((*self - *other).abs())) Ok(SquaredDistance::from_sqrt((*self - *other).abs()))
} }
} }
@ -77,7 +82,7 @@ impl<T> ComputeSquaredDistance for Option<T>
fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> { fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
match (self.as_ref(), other.as_ref()) { match (self.as_ref(), other.as_ref()) {
(Some(this), Some(other)) => this.compute_squared_distance(other), (Some(this), Some(other)) => this.compute_squared_distance(other),
(None, None) => Ok(SquaredDistance::Value(0.)), (None, None) => Ok(SquaredDistance::from_sqrt(0.)),
_ => Err(()), _ => Err(()),
} }
} }
@ -94,21 +99,16 @@ impl<T> ComputeSquaredDistance for Size2D<T>
impl SquaredDistance { impl SquaredDistance {
/// Returns the square root of this squared distance. /// Returns the square root of this squared distance.
#[inline]
pub fn sqrt(self) -> f64 { pub fn sqrt(self) -> f64 {
match self { self.value.sqrt()
SquaredDistance::Sqrt(this) => this,
SquaredDistance::Value(this) => this.sqrt(),
}
} }
} }
impl From<SquaredDistance> for f64 { impl From<SquaredDistance> for f64 {
#[inline] #[inline]
fn from(distance: SquaredDistance) -> Self { fn from(distance: SquaredDistance) -> Self {
match distance { distance.value
SquaredDistance::Sqrt(this) => this * this,
SquaredDistance::Value(this) => this,
}
} }
} }
@ -117,19 +117,15 @@ impl Add for SquaredDistance {
#[inline] #[inline]
fn add(self, rhs: Self) -> Self { fn add(self, rhs: Self) -> Self {
SquaredDistance::Value(f64::from(self) + f64::from(rhs)) SquaredDistance { value: self.value + rhs.value }
} }
} }
impl Sum for SquaredDistance { impl Sum for SquaredDistance {
fn sum<I>(mut iter: I) -> Self fn sum<I>(iter: I) -> Self
where where
I: Iterator<Item = Self>, I: Iterator<Item = Self>,
{ {
let first = match iter.next() { iter.fold(SquaredDistance::from_sqrt(0.), Add::add)
Some(first) => first,
None => return SquaredDistance::Value(0.),
};
iter.fold(first, Add::add)
} }
} }

View file

@ -28,7 +28,7 @@ pub fn derive(input: DeriveInput) -> Tokens {
let (this_pattern, this_info) = cg::ref_pattern(&variant, "this"); let (this_pattern, this_info) = cg::ref_pattern(&variant, "this");
let (other_pattern, other_info) = cg::ref_pattern(&variant, "other"); let (other_pattern, other_info) = cg::ref_pattern(&variant, "other");
let sum = if this_info.is_empty() { let sum = if this_info.is_empty() {
quote! { ::values::distance::SquaredDistance::Value(0.) } quote! { ::values::distance::SquaredDistance::from_sqrt(0.) }
} else { } else {
let mut sum = quote!(); let mut sum = quote!();
sum.append_separated(this_info.iter().zip(&other_info).map(|(this, other)| { sum.append_separated(this_info.iter().zip(&other_info).map(|(this, other)| {