[tor-commits] [tor-browser] 30/311: Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro, a=dsmith
gitolite role
git at cupani.torproject.org
Tue Apr 26 15:27:10 UTC 2022
This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1
in repository tor-browser.
commit 136cd85c3504630133d8f9cad673647f2e42b33b
Author: Emilio Cobos Álvarez <emilio at crisal.io>
AuthorDate: Wed Jan 5 19:10:28 2022 +0000
Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro, a=dsmith
The issue here is that we end up with a transition between mismatched
transform lists that ends up generating an InterpolateMatrix {}
operation. So far so good, but we end up interpolating that a lot of
times and generating an unboundedly-deep operation list.
This implementas an optimization that flattens them to a single matrix
when possible (when there's no dependencies on the containing box).
This is similar to:
https://chromium.googlesource.com/chromium/src.git/+/2b89cc4df436e672ef9cf940d1c0dc73fef82a4a
We fix the to_pixel_length() behavior for LenghtPercentage to be
correct (and update callers to preserve behavior).
Differential Revision: https://phabricator.services.mozilla.com/D134784
---
.../components/style/values/animated/transform.rs | 108 ++++++++++++---------
.../components/style/values/generics/transform.rs | 40 ++++----
2 files changed, 83 insertions(+), 65 deletions(-)
diff --git a/servo/components/style/values/animated/transform.rs b/servo/components/style/values/animated/transform.rs
index 542b4c3102313..8600281ce5b00 100644
--- a/servo/components/style/values/animated/transform.rs
+++ b/servo/components/style/values/animated/transform.rs
@@ -891,25 +891,8 @@ impl Animate for ComputedTransform {
match (this_remainder, other_remainder) {
// If there is a remainder from *both* lists we must have had mismatched functions.
// => Add the remainders to a suitable ___Matrix function.
- (Some(this_remainder), Some(other_remainder)) => match procedure {
- Procedure::Add => {
- debug_assert!(false, "Should have already dealt with add by the point");
- return Err(());
- },
- Procedure::Interpolate { progress } => {
- result.push(TransformOperation::InterpolateMatrix {
- from_list: Transform(this_remainder.to_vec().into()),
- to_list: Transform(other_remainder.to_vec().into()),
- progress: Percentage(progress as f32),
- });
- },
- Procedure::Accumulate { count } => {
- result.push(TransformOperation::AccumulateMatrix {
- from_list: Transform(this_remainder.to_vec().into()),
- to_list: Transform(other_remainder.to_vec().into()),
- count: cmp::min(count, i32::max_value() as u64) as i32,
- });
- },
+ (Some(this_remainder), Some(other_remainder)) => {
+ result.push(TransformOperation::animate_mismatched_transforms(this_remainder, other_remainder, procedure)?);
},
// If there is a remainder from just one list, then one list must be shorter but
// completely match the type of the corresponding functions in the longer list.
@@ -923,36 +906,19 @@ impl Animate for ComputedTransform {
let identity = transform.to_animated_zero().unwrap();
match transform {
- // We can't interpolate/accumulate ___Matrix types directly with a
- // matrix. Instead we need to wrap it in another ___Matrix type.
TransformOperation::AccumulateMatrix { .. } |
TransformOperation::InterpolateMatrix { .. } => {
- let transform_list = Transform(vec![transform.clone()].into());
- let identity_list = Transform(vec![identity].into());
- let (from_list, to_list) = if fill_right {
- (transform_list, identity_list)
+ let (from, to) = if fill_right {
+ (transform, &identity)
} else {
- (identity_list, transform_list)
+ (&identity, transform)
};
- match procedure {
- Procedure::Add => Err(()),
- Procedure::Interpolate { progress } => {
- Ok(TransformOperation::InterpolateMatrix {
- from_list,
- to_list,
- progress: Percentage(progress as f32),
- })
- },
- Procedure::Accumulate { count } => {
- Ok(TransformOperation::AccumulateMatrix {
- from_list,
- to_list,
- count: cmp::min(count, i32::max_value() as u64)
- as i32,
- })
- },
- }
+ TransformOperation::animate_mismatched_transforms(
+ &[from.clone()],
+ &[to.clone()],
+ procedure,
+ )
},
_ => {
let (lhs, rhs) = if fill_right {
@@ -981,9 +947,13 @@ impl ComputeSquaredDistance for ComputedTransform {
// Roll back to matrix interpolation if there is any Err(()) in the
// transform lists, such as mismatched transform functions.
+ //
+ // FIXME: Using a zero size here seems a bit sketchy but matches the
+ // previous behavior.
if squared_dist.is_err() {
- let matrix1: Matrix3D = self.to_transform_3d_matrix(None)?.0.into();
- let matrix2: Matrix3D = other.to_transform_3d_matrix(None)?.0.into();
+ let rect = euclid::Rect::zero();
+ let matrix1: Matrix3D = self.to_transform_3d_matrix(Some(&rect))?.0.into();
+ let matrix2: Matrix3D = other.to_transform_3d_matrix(Some(&rect))?.0.into();
return matrix1.compute_squared_distance(&matrix2);
}
@@ -1141,6 +1111,52 @@ impl Animate for ComputedTransformOperation {
}
}
+impl ComputedTransformOperation {
+ /// If there are no size dependencies, we try to animate in-place, to avoid
+ /// creating deeply nested Interpolate* operations.
+ fn try_animate_mismatched_transforms_in_place(
+ left: &[Self],
+ right: &[Self],
+ procedure: Procedure,
+ ) -> Result<Self, ()> {
+ let (left, _left_3d) = Transform::components_to_transform_3d_matrix(left, None)?;
+ let (right, _right_3d) = Transform::components_to_transform_3d_matrix(right, None)?;
+ ComputedTransformOperation::Matrix3D(left.into()).animate(&ComputedTransformOperation::Matrix3D(right.into()), procedure)
+ }
+
+ fn animate_mismatched_transforms(
+ left: &[Self],
+ right: &[Self],
+ procedure: Procedure,
+ ) -> Result<Self, ()> {
+ if let Ok(op) = Self::try_animate_mismatched_transforms_in_place(left, right, procedure) {
+ return Ok(op);
+ }
+ let from_list = Transform(left.to_vec().into());
+ let to_list = Transform(right.to_vec().into());
+ Ok(match procedure {
+ Procedure::Add => {
+ debug_assert!(false, "Addition should've been handled earlier");
+ return Err(())
+ },
+ Procedure::Interpolate { progress } => {
+ Self::InterpolateMatrix {
+ from_list,
+ to_list,
+ progress: Percentage(progress as f32),
+ }
+ }
+ Procedure::Accumulate { count } => {
+ Self::AccumulateMatrix {
+ from_list,
+ to_list,
+ count: cmp::min(count, i32::max_value() as u64) as i32,
+ }
+ }
+ })
+ }
+}
+
// This might not be the most useful definition of distance. It might be better, for example,
// to trace the distance travelled by a point as its transform is interpolated between the two
// lists. That, however, proves to be quite complicated so we take a simple approach for now.
diff --git a/servo/components/style/values/generics/transform.rs b/servo/components/style/values/generics/transform.rs
index 816dde92e4f94..1179001ccf884 100644
--- a/servo/components/style/values/generics/transform.rs
+++ b/servo/components/style/values/generics/transform.rs
@@ -404,15 +404,7 @@ impl ToAbsoluteLength for ComputedLength {
impl ToAbsoluteLength for ComputedLengthPercentage {
#[inline]
fn to_pixel_length(&self, containing_len: Option<ComputedLength>) -> Result<CSSFloat, ()> {
- match containing_len {
- Some(relative_len) => Ok(self.resolve(relative_len).px()),
- // If we don't have reference box, we cannot resolve the used value,
- // so only retrieve the length part. This will be used for computing
- // distance without any layout info.
- //
- // FIXME(emilio): This looks wrong.
- None => Ok(self.resolve(Zero::zero()).px()),
- }
+ Ok(self.maybe_percentage_relative_to(containing_len).ok_or(())?.px())
}
}
@@ -572,12 +564,21 @@ impl<T> Transform<T> {
impl<T: ToMatrix> Transform<T> {
/// Return the equivalent 3d matrix of this transform list.
+ ///
/// We return a pair: the first one is the transform matrix, and the second one
/// indicates if there is any 3d transform function in this transform list.
#[cfg_attr(rustfmt, rustfmt_skip)]
pub fn to_transform_3d_matrix(
&self,
reference_box: Option<&Rect<ComputedLength>>
+ ) -> Result<(Transform3D<CSSFloat>, bool), ()> {
+ Self::components_to_transform_3d_matrix(&self.0, reference_box)
+ }
+
+ /// Converts a series of components to a 3d matrix.
+ pub fn components_to_transform_3d_matrix(
+ ops: &[T],
+ reference_box: Option<&Rect<ComputedLength>>
) -> Result<(Transform3D<CSSFloat>, bool), ()> {
let cast_3d_transform = |m: Transform3D<f64>| -> Transform3D<CSSFloat> {
use std::{f32, f64};
@@ -590,26 +591,27 @@ impl<T: ToMatrix> Transform<T> {
)
};
- let (m, is_3d) = self.to_transform_3d_matrix_f64(reference_box)?;
+ let (m, is_3d) = Self::components_to_transform_3d_matrix_f64(ops, reference_box)?;
Ok((cast_3d_transform(m), is_3d))
}
/// Same as Transform::to_transform_3d_matrix but a f64 version.
- pub fn to_transform_3d_matrix_f64(
- &self,
+ fn components_to_transform_3d_matrix_f64(
+ ops: &[T],
reference_box: Option<&Rect<ComputedLength>>,
) -> Result<(Transform3D<f64>, bool), ()> {
- // We intentionally use Transform3D<f64> during computation to avoid error propagation
- // because using f32 to compute triangle functions (e.g. in rotation()) is not
- // accurate enough. In Gecko, we also use "double" to compute the triangle functions.
- // Therefore, let's use Transform3D<f64> during matrix computation and cast it into f32
- // in the end.
+ // We intentionally use Transform3D<f64> during computation to avoid
+ // error propagation because using f32 to compute triangle functions
+ // (e.g. in rotation()) is not accurate enough. In Gecko, we also use
+ // "double" to compute the triangle functions. Therefore, let's use
+ // Transform3D<f64> during matrix computation and cast it into f32 in
+ // the end.
let mut transform = Transform3D::<f64>::identity();
let mut contain_3d = false;
- for operation in &*self.0 {
+ for operation in ops {
let matrix = operation.to_3d_matrix(reference_box)?;
- contain_3d |= operation.is_3d();
+ contain_3d = contain_3d || operation.is_3d();
transform = matrix.then(&transform);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tor-commits
mailing list