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.
stylo: Bug 1362896 - Implement ComputeSquaredDistance for TransformList
We implement ComputeSquaredDistance for TransformList.
---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1362896](https://bugzilla.mozilla.org/show_bug.cgi?id=1362896).
- [X] These changes do not require tests because Gecko has related tests.
<!-- 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/18086)
<!-- Reviewable:end -->
We need to use enum instead of Either since we can't interpolate stroke-* between
unitless length and unit length(e.g. '5' -> '10px').
This coomit make following:
* Introduce SVGLengthOrPercentageOrNumber on computed and specified values.
* Make SVGLengthOrPercentageOrNumber animatable.
* Make stroke-dasharray not-accumulate.
In SMIL we don't expect the 'none' value of the 'fill' property to be
additive and hence the following animation should have no effect:
<rect width="100" height="100" y="100" fill="blue">
<animate attributeName="fill" dur="3s" from="red" by="none"/>
</rect>
Although SMIL doesn't make this entirely clear, [1] says that "by
animation" and "from-by animation" may only be used "with attributes
that support addition (e.g. most numeric attributes)" and [2] says that
<paint>s are "only additive if each value can be converted to an RGB
color". As a result, the animation above should have no effect.
By extrapolation, animating from 'none' by 'none' should also have no
effect:
<rect width="100" height="100" y="100" fill="blue">
<animate attributeName="fill" dur="3s" from="none" by="none"/>
</rect>
However, in Servo's interpolation of <paint>s we special case the
interpolation and addition of 'none' such that if both values are 'none'
it is allowed.
We should disallow this in order to produce the expected behavior and in
order to match Gecko's behavior.
[1] https://www.w3.org/TR/smil-animation/#AnimFuncValues
[2] https://www.w3.org/TR/SVG11/animate.html#AnimationAttributesAndProperties
For CSS Transitions we want this case to return Err() so we know that the two
values are not interpolable.
For CSS Animations/Web Animations we implement discrete animation as the
fallback behavior when Err() is returned.
We convert a perspective function into a ComputedMatrix, but we apply the
perspective length to a wrong matrix item. We should put it into m34,
instead of m43. m43 is for translate parameter Z.
set_list_style_type() for gecko needs an additional argument 'Device' [1], and
making list-style-type animtable makes AnimatedProperty::update() call the
set_list_style_type(). To avoid introducing the Device argument to the setter in
servo, this patch makes simply PropertyAnimation conditional build.
(Whereas in stylo, we don't use the setter for animation, we use clone_xx()
instead.)
[1] https://hg.mozilla.org/mozilla-central/file/1d042bcb2632/servo/components/style/properties/properties.mako.rs#l2667
Remove style/testing feature
We added this because a year ago we had no reliable Gecko CI. This meant that Gecko-only properties needed to be tested *somehow*, and we solved that by making it so that for unit tests we compile all properties, not just the servo ones.
This was useful back then, but I don't think we need this anymore. We have reliable Gecko CI, and all the gecko-only stuff we tested is adequately handled by the properties-database parsing mochitests. It's a bit of annoying cruft that just complicates things; we probably should remove it.
r? @emilio or @SimonSapin
<!-- 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/17984)
<!-- Reviewable:end -->
stylo: Bug 1374233 - Clamp interpolated values for properties which need to be restricted
Some properties only accept non-negative values, or values greater than or equal to one. It is possible to produce an negative interpolated values while using negative timing functions, so we have to apply a restriction to these values to avoid getting invalid values.
For example, line-height must be non-negative, but the output progress of some timing functions (e,g. cubic-bezier(0.25, -2, 0.75, 1)) may be a negative value, so the interpolated result of line-height is also negative.
---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix Bug 1374233.
- [X] These changes do not require tests because we have tests in Gecko side 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/17783)
<!-- Reviewable:end -->
We already have NonNegativeLength and NonNegativeAu, so we can re-use it
to define the specified value and the computed value of border-spacing.
And then implement ToAnimatedValue for it.
MozReview-Commit-ID: CLckpKMYVXU
Add NonNegativeLength, which could be computed to NonNegativeAu. So we
can declare Either<NonNegativeLength, X>, X=Auto, Normal, or Number.
NonNegativeLengthOrAuto is for column-width.
NonNegativeLengthOrNormal is for column-gap.
NonNegativeLengthOrNumber is for -moz-tab-size.
MozReview-Commit-ID: AfU8XpA1um0
Add values::computed::NonNegativeAu, so BorderSideWith could be computed
to this non-negative Au, for the following properties:
1. outline-width
2. border-{*}-width
3. column-rule-width
4. -webkit-text-stroke-width
MozReview-Commit-ID: ASHaB2F7VtM
NonNegativeNumber: for -moz-box-flex, flex-grow, and flex-shrink.
GreaterThanOrEqualToOneNumber: for stroke-miterlimit.
MozReview-Commit-ID: Kgbt99BPdVA
When computing the distance between two filter functions, e.g.
HueRotate(0deg) and HueRotate(-360deg), we got an Err(()) and unwrap it
immediately in compute_filter_square_distance(), which causes a crash.
The root-cause is that we don't implement computed_distance of Angle, and we
assume we always get a valid result from compute_squared_distance() for
each filter function.
In the case where we accumulate transform:none onto decomposed matrix for
iteration composite whose iteration count is over 2, we pass zero
other_portion and self_portion which is over 1.0. We should care about the
case.
