Skip to content

Commit

Permalink
Bug 1850841 - Tweak the formula of computing the ratio of quaternionA…
Browse files Browse the repository at this point in the history
… in Slerp algorithm. r=emilio

When the progress is 100%, the result from the original formula of calculating
`left_weight` may not be 0 exactly (i.e. approximate zero, e.g. -2.22e-16), and
so this imprecision makes the recomposed Matrix3D have some approximate zeros.

Those approximate zeros (e.g. `_m13` is 2e-16, and `_m23` is 8e-17 in WPT)
make us failed to treat this Matrix3D as a 2D matrix when we serializing it
(because we use `!= 0.0f` to check if the matrix components are not equal to
zero in `Matrix4x4::Is2D()`, and other places).

Differential Revision: https://phabricator.services.mozilla.com/D187814
  • Loading branch information
BorisChiou committed Sep 11, 2023
1 parent 3cf3fd0 commit 024f384
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 33 deletions.
52 changes: 33 additions & 19 deletions servo/components/style/values/animated/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::values::generics::transform::{Rotate, Scale, Translate};
use crate::values::CSSFloat;
use crate::Zero;
use std::cmp;
use std::ops::Add;

// ------------------------------------
// Animations for Matrix/Matrix3D.
Expand Down Expand Up @@ -388,6 +389,14 @@ impl Quaternion {
}
}

impl Add for Quaternion {
type Output = Self;

fn add(self, other: Self) -> Self {
Self(self.0 + other.0, self.1 + other.1, self.2 + other.2, self.3 + other.3)
}
}

impl Animate for Quaternion {
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
let (this_weight, other_weight) = procedure.weights();
Expand Down Expand Up @@ -434,32 +443,37 @@ impl Animate for Quaternion {
));
}

// Straight from gfxQuaternion::Slerp.
// https://drafts.csswg.org/css-transforms-2/#interpolation-of-decomposed-3d-matrix-values
//
// Dot product, clamped between -1 and 1.
let dot = (self.0 * other.0 + self.1 * other.1 + self.2 * other.2 + self.3 * other.3)
.min(1.0)
.max(-1.0);
let cos_half_theta =
(self.0 * other.0 + self.1 * other.1 + self.2 * other.2 + self.3 * other.3)
.min(1.0)
.max(-1.0);

if dot.abs() == 1.0 {
if cos_half_theta.abs() == 1.0 {
return Ok(*self);
}

let theta = dot.acos();
let rsintheta = 1.0 / (1.0 - dot * dot).sqrt();

let right_weight = (other_weight * theta).sin() * rsintheta;
let left_weight = (other_weight * theta).cos() - dot * right_weight;

let left = self.scale(left_weight);
let right = other.scale(right_weight);
let half_theta = cos_half_theta.acos();
let sin_half_theta = (1.0 - cos_half_theta * cos_half_theta).sqrt();

let right_weight = (other_weight * half_theta).sin() / sin_half_theta;
// The spec would like to use
// "(other_weight * half_theta).cos() - cos_half_theta * right_weight". However, this
// formula may produce some precision issues of floating-point number calculation, e.g.
// when the progress is 100% (i.e. |other_weight| is 1), the |left_weight| may not be
// perfectly equal to 0. It could be something like -2.22e-16, which is approximately equal
// to zero, in the test. And after we recompose the Matrix3D, these approximated zeros
// make us failed to treat this Matrix3D as a Matrix2D, when serializating it.
//
// Therefore, we use another formula to calculate |left_weight| here. Blink and WebKit also
// use this formula, which is defined in:
// https://www.euclideanspace.com/maths/algebra/realNormedAlgebra/quaternions/slerp/index.htm
// https://github.com/w3c/csswg-drafts/issues/9338
let left_weight = (this_weight * half_theta).sin() / sin_half_theta;

Ok(Quaternion(
left.0 + right.0,
left.1 + right.1,
left.2 + right.2,
left.3 + right.3,
))
Ok(self.scale(left_weight) + other.scale(right_weight))
}
}

Expand Down

This file was deleted.

0 comments on commit 024f384

Please sign in to comment.