diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-10-21 01:39:41 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-10-21 01:39:41 -0500 |
commit | 48c715c1c86301d0f25e70d3e690d04d8303c58f (patch) | |
tree | 9acf119aaf51e35a25759edd2f59218e87ba2fc7 /components | |
parent | bdc7919cef766a42694c59690a2d79e336a4edee (diff) | |
parent | d7a5f224082341f7dbb8cb66c9bdc0f5cacc64ff (diff) | |
download | servo-48c715c1c86301d0f25e70d3e690d04d8303c58f.tar.gz servo-48c715c1c86301d0f25e70d3e690d04d8303c58f.zip |
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 -->
Diffstat (limited to 'components')
-rw-r--r-- | components/style/properties/helpers/animated_properties.mako.rs | 53 |
1 files changed, 44 insertions, 9 deletions
diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 422447d582d..b7223d4b616 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -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,23 +2159,37 @@ 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 result = this.iter().zip(other).map(|(this, other)| { - this.animate(other, procedure) - }).collect::<Result<Vec<_>, _>>(); - if let Ok(list) = result { - return Ok(TransformList(if list.is_empty() { - None - } else { - Some(list) - })); + 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<_>, _>>(); + if let Ok(list) = result { + return Ok(TransformList(if list.is_empty() { + None + } else { + 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 } => { |