Bug 1390384 - Apply SMIL's calcMode=discrete handling to discretely animatable properties in Stylo; r?hiro
In SMIL if a discrete calcMode is used, the keyTimes attribute may be used to
specify the times at which the various animation values are used, overriding the
regular 50% flip behavior (see nsSMILAnimationFunction::ScaleSimpleProgress).
However, this behavior is only applied when the calcMode is known to be
discrete. If the author specifies calcMode="linear" but
attributeType="stroke-linecap" then SMIL should fall back to discrete calcMode
including applying the special keyTimes behavior.
For the Servo backend, Servo_AnimationValues_Interpolate does not return an
error for discretely animated properties so SMIL does not recognize when we fall
back to discrete calcMode. This patch adds a check before performing
interpolation that tests if the property is a discretely interpolable one and,
if it is, returns an error so that we run SMIL's special discrete calcMode
handling.
MozReview-Commit-ID: FOp8XcNbvdu
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -521,28 +521,88 @@ InterpolateForGecko(const ValueWrapper*
*endCSSValue,
aUnitDistance, resultValue)) {
aResult.mU.mPtr = new ValueWrapper(aEndWrapper.mPropID, resultValue);
return NS_OK;
}
return NS_ERROR_FAILURE;
}
+static bool
+IsPropertyDiscretelyAnimatable(nsCSSPropertyID aProperty)
+{
+ // For shorthands, Servo_Property_IsDiscreteAnimatable will always return
+ // false. That makes sense for shorthands like 'font' which have smoothly
+ // interpolable longhand components. However, for shorthands where all the
+ // components are discretely animatable, like 'font-variant', we should
+ // treat the shorthand as discretely animatable so that we apply SMIL's
+ // special discrete handling to it and all its longhand components.
+ if (nsCSSProps::IsShorthand(aProperty)) {
+ // We could iterate over all the longhand components and call
+ // Servo_Property_IsDiscreteAnimatable on each of them, but that's a lot of
+ // FFI calls do be doing each time we interpolate. Instead, since there are
+ // only about six shorthands in the set of properties that can be animated
+ // by SMIL, we just hard code the discrete ones below then add a debug-mode
+ // check that this list matches what the result would be if we performed
+ // all the FFI calls.
+ bool result;
+ switch (aProperty) {
+ case eCSSProperty_font_variant:
+ case eCSSProperty_marker:
+ case eCSSProperty_overflow:
+ result = true;
+ break;
+ default:
+ result = false;
+ break;
+ }
+
+#ifdef DEBUG
+ bool resultAccordingToServo = true;
+ CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aProperty,
+ CSSEnabledState::eForAllContent) {
+ // If the shorthand has one or more non-discrete components, it should not
+ // be treated as discrete.
+ if (!Servo_Property_IsDiscreteAnimatable(*p)) {
+ resultAccordingToServo = false;
+ }
+ }
+ MOZ_ASSERT(result == resultAccordingToServo,
+ "Gecko and Servo should agree on which shorthands should be"
+ " treated as discretely animatable");
+#endif
+
+ return result;
+ }
+
+ return Servo_Property_IsDiscreteAnimatable(aProperty);
+}
+
static nsresult
InterpolateForServo(const ValueWrapper* aStartWrapper,
const ValueWrapper& aEndWrapper,
double aUnitDistance,
nsSMILValue& aResult)
{
+ // For discretely-animated properties Servo_AnimationValues_Interpolate will
+ // perform the discrete animation (i.e. 50% flip) and return a success result.
+ // However, SMIL has its own special discrete animation behavior that it uses
+ // when keyTimes are specified, but we won't run that unless that this method
+ // returns a failure to indicate that the property cannot be smoothly
+ // interpolated, i.e. that we need to use a discrete calcMode.
+ if (IsPropertyDiscretelyAnimatable(aEndWrapper.mPropID)) {
+ return NS_ERROR_FAILURE;
+ }
+
ServoAnimationValues results;
size_t len = aEndWrapper.mServoValues.Length();
results.SetCapacity(len);
MOZ_ASSERT(!aStartWrapper || aStartWrapper->mServoValues.Length() == len,
"Start and end values length should be the same if "
- "The start value exists");
+ "the start value exists");
for (size_t i = 0; i < len; i++) {
const RefPtr<RawServoAnimationValue>*
startValue = aStartWrapper
? &aStartWrapper->mServoValues[i]
: nullptr;
const RefPtr<RawServoAnimationValue>* endValue = &aEndWrapper.mServoValues[i];
RefPtr<RawServoAnimationValue> zeroValueStorage;
if (!FinalizeServoAnimationValues(startValue, endValue, zeroValueStorage)) {
--- a/dom/smil/test/mochitest.ini
+++ b/dom/smil/test/mochitest.ini
@@ -37,17 +37,16 @@ skip-if = stylo
skip-if = toolkit == 'android'
[test_smilFillMode.xhtml]
[test_smilGetSimpleDuration.xhtml]
[test_smilGetStartTime.xhtml]
[test_smilHyperlinking.xhtml]
[test_smilInvalidValues.html]
[test_smilKeySplines.xhtml]
[test_smilKeyTimes.xhtml]
-skip-if = stylo
[test_smilKeyTimesPacedMode.xhtml]
[test_smilMappedAttrFromBy.xhtml]
skip-if = stylo
[test_smilMappedAttrFromTo.xhtml]
skip-if = stylo
[test_smilMappedAttrPaced.xhtml]
skip-if = stylo
[test_smilMinTiming.html]