From 03cf0a8cb8d2bb3f654252cc09c82e0c549cdd00 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 16 Aug 2017 11:18:26 +0800 Subject: [PATCH 1/2] Fix divided by zero problem on non-normalizable direction vector. While normalizing the direction vector, we have to make sure we don't divide the components by zero length to avoid any assertion of "NaN". --- .../helpers/animated_properties.mako.rs | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 52701c1b021..4b857c8d2ea 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -1425,11 +1425,9 @@ fn add_weighted_transform_lists(from_list: &[TransformOperation], } (&TransformOperation::Rotate(fx, fy, fz, fa), &TransformOperation::Rotate(tx, ty, tz, ta)) => { - let norm_f = ((fx * fx) + (fy * fy) + (fz * fz)).sqrt(); - let norm_t = ((tx * tx) + (ty * ty) + (tz * tz)).sqrt(); - let (fx, fy, fz) = (fx / norm_f, fy / norm_f, fz / norm_f); - let (tx, ty, tz) = (tx / norm_t, ty / norm_t, tz / norm_t); - if fx == tx && fy == ty && fz == tz { + let (fx, fy, fz, fa) = get_normalized_vector_and_angle(fx, fy, fz, fa); + let (tx, ty, tz, ta) = get_normalized_vector_and_angle(tx, ty, tz, ta); + if (fx, fy, fz) == (tx, ty, tz) { let ia = fa.add_weighted(&ta, self_portion, other_portion).unwrap(); result.push(TransformOperation::Rotate(fx, fy, fz, ia)); } else { @@ -1882,8 +1880,8 @@ impl ComputeSquaredDistance for Quaternion { impl DirectionVector { /// Create a DirectionVector. #[inline] - fn new(x: f64, y: f64, z: f64) -> Self { - DirectionVector(Point3D::new(x, y, z)) + fn new(x: f32, y: f32, z: f32) -> Self { + DirectionVector(Point3D::new(x as f64, y as f64, z as f64)) } /// Return the normalized direction vector. @@ -1907,6 +1905,19 @@ impl DirectionVector { } } +/// Return the normalized direction vector and its angle. +// A direction vector that cannot be normalized, such as [0,0,0], will cause the +// rotation to not be applied. i.e. Use an identity matrix or rotate3d(0, 0, 1, 0). +fn get_normalized_vector_and_angle(x: f32, y: f32, z: f32, angle: Angle) + -> (f32, f32, f32, Angle) { + let mut vector = DirectionVector::new(x, y, z); + if vector.normalize() { + (vector.0.x as f32, vector.0.y as f32, vector.0.z as f32, angle) + } else { + (0., 0., 1., Angle::zero()) + } +} + /// Decompose a 3D matrix. /// https://drafts.csswg.org/css-transforms/#decomposing-a-3d-matrix fn decompose_3d_matrix(mut matrix: ComputedMatrix) -> Result { @@ -2539,25 +2550,15 @@ fn compute_transform_lists_squared_distance(from_list: &[TransformOperation], } (&TransformOperation::Rotate(fx, fy, fz, fa), &TransformOperation::Rotate(tx, ty, tz, ta)) => { - // A direction vector that cannot be normalized, such as [0,0,0], will cause the - // rotation to not be applied. i.e. Use an identity matrix or rotate3d(0, 0, 1, 0). - let get_normalized_vector_and_angle = |x: f32, y: f32, z: f32, angle: Angle| - -> (DirectionVector, Angle) { - let mut vector = DirectionVector::new(x as f64, y as f64, z as f64); - if vector.normalize() { - (vector, angle) - } else { - (DirectionVector::new(0., 0., 1.), Angle::zero()) - } - }; - - let (vector1, angle1) = get_normalized_vector_and_angle(fx, fy, fz, fa); - let (vector2, angle2) = get_normalized_vector_and_angle(tx, ty, tz, ta); - if vector1 == vector2 { + let (fx, fy, fz, angle1) = get_normalized_vector_and_angle(fx, fy, fz, fa); + let (tx, ty, tz, angle2) = get_normalized_vector_and_angle(tx, ty, tz, ta); + if (fx, fy, fz) == (tx, ty, tz) { angle1.compute_squared_distance(&angle2).unwrap_or(zero_distance) } else { - let q1 = Quaternion::from_direction_and_angle(&vector1, angle1.radians64()); - let q2 = Quaternion::from_direction_and_angle(&vector2, angle2.radians64()); + let v1 = DirectionVector::new(fx, fy, fz); + let v2 = DirectionVector::new(tx, ty, tz); + let q1 = Quaternion::from_direction_and_angle(&v1, angle1.radians64()); + let q2 = Quaternion::from_direction_and_angle(&v2, angle2.radians64()); q1.compute_squared_distance(&q2).unwrap_or(zero_distance) } } From 4ed1a6be20366cc4446b11c5b320898c9559476c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 16 Aug 2017 11:20:09 +0800 Subject: [PATCH 2/2] Fix the identity transform of rotatex and rotatey. Let's see the example: "rotatex(360deg)" to "none". While we do interpolation between "rotatex" and "none", the original code path is: 1. Build an identity transform for none and always use (0, 0, 1) as the direction vector, i.e. Rotate(0, 0, 1, 0deg). 2. Do interpolation between rotatex (i.e. Rotate(1, 0, 0, 360deg)) and Rotate(0, 0, 1, 0deg). Because their direction vectors are different, so we do matrix decomposition/interpolation/recomposition on both functions. The problem is, matrix decomposition makes the 360deg disappear, so it looks like "rotatex(0deg)". 3. Obviously, we are trying to interpolate from rotatex(0deg) to none, so the interpolated matrix is always an identity matrix. I think rotatex, rotatey, and rotatez are special cases which should really rotate according to the angle, so we should build the identity transform for them according to the normalized direction vector, and so we do interpolation on the angle, instead of converting them into matrix. Replacing build_identity_transform_list() with add_weighted_transform_lists(list, list, 0, 0) might be another solution; However, I didn't do that because build_identity_transform_list() is much simpler. --- .../style/properties/helpers/animated_properties.mako.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 4b857c8d2ea..40eaa8b3eb2 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -1340,8 +1340,9 @@ fn build_identity_transform_list(list: &[TransformOperation]) -> Vec { result.push(TransformOperation::Scale(1.0, 1.0, 1.0)); } - TransformOperation::Rotate(..) => { - result.push(TransformOperation::Rotate(0.0, 0.0, 1.0, Angle::zero())); + TransformOperation::Rotate(x, y, z, a) => { + let (x, y, z, _) = get_normalized_vector_and_angle(x, y, z, a); + result.push(TransformOperation::Rotate(x, y, z, Angle::zero())); } TransformOperation::Perspective(..) | TransformOperation::AccumulateMatrix { .. } |