Auto merge of #18966 - chenpighead:Bug1399049-transform-animate-refactoring, r=hiikezoe

stylo: Avoid using InterpolateMatrix as a fallback for matched transform function pair

In the current implementation, if there is any interpolation error in a matched
transform function pair, we fall-back to use InterpolateMatrix unconditionally.
However, the error could be caused by:

1. mismatched transform function pair
2. matched transform function pair within at least one undecomposable matrix.

Using InterpolateMatrix for case 1 makes sense, however, using InterpolateMatrix
for case 2 does not. According to the spec, we should just report error for
case 2, and let the caller do the fallback procedure. Using InterpolateMatrix
for case 2 will go through more unnecessary code path, and produce more memory
usage and calculation cost, which should be avoidable.

In this patch, we add an extra pass to check if a transform function pair have
matched operations in advance. With this information, we can easily tell whether
the interpolation error in a equal-length transform function pair is caused by
case 1 or case 2. So, we can avoid the unnecessary cost.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1399049](https://bugzilla.mozilla.org/show_bug.cgi?id=1399049)
- [X] These changes do not require tests because the change is for some performance gain, and we have tests to ensure that we won't regress the existing behavior already.

<!-- 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/18966)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-10-21 01:39:41 -05:00 committed by GitHub
commit 48c715c1c8

View file

@ -1157,6 +1157,27 @@ impl Animate for TransformOperation {
}
}
fn is_matched_operation(first: &TransformOperation, second: &TransformOperation) -> bool {
match (first, second) {
(&TransformOperation::Matrix(..),
&TransformOperation::Matrix(..)) |
(&TransformOperation::MatrixWithPercents(..),
&TransformOperation::MatrixWithPercents(..)) |
(&TransformOperation::Skew(..),
&TransformOperation::Skew(..)) |
(&TransformOperation::Translate(..),
&TransformOperation::Translate(..)) |
(&TransformOperation::Scale(..),
&TransformOperation::Scale(..)) |
(&TransformOperation::Rotate(..),
&TransformOperation::Rotate(..)) |
(&TransformOperation::Perspective(..),
&TransformOperation::Perspective(..)) => true,
// InterpolateMatrix and AccumulateMatrix are for mismatched transform.
_ => false
}
}
/// <https://www.w3.org/TR/css-transforms-1/#Rotate3dDefined>
fn rotate_to_matrix(x: f32, y: f32, z: f32, a: Angle) -> ComputedMatrix {
let half_rad = a.radians() / 2.0;
@ -2138,10 +2159,16 @@ impl Animate for TransformList {
Cow::Owned(self.to_animated_zero()?)
};
// For matched transform lists.
{
let this = (*this).0.as_ref().map_or(&[][..], |l| l);
let other = (*other).0.as_ref().map_or(&[][..], |l| l);
if this.len() == other.len() {
let is_matched_transforms = this.iter().zip(other).all(|(this, other)| {
is_matched_operation(this, other)
});
if is_matched_transforms {
let result = this.iter().zip(other).map(|(this, other)| {
this.animate(other, procedure)
}).collect::<Result<Vec<_>, _>>();
@ -2152,9 +2179,17 @@ impl Animate for TransformList {
Some(list)
}));
}
// Can't animate for a pair of matched transform lists?
// This means we have at least one undecomposable matrix,
// so we should report Err here, and let the caller do
// the fallback procedure.
return Err(());
}
}
}
// For mismatched transform lists.
match procedure {
Procedure::Add => Err(()),
Procedure::Interpolate { progress } => {