Bug 1311620 - Part 4: Implement keyframe composite(add). r?birtles
This patch also fixes expected computed offset values in frame at 0.5 offset for
add composite.
MozReview-Commit-ID: 8PNp237NoV4
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -311,23 +311,27 @@ KeyframeEffectReadOnly::CompositeValue(
nsCSSPropertyID aProperty,
const StyleAnimationValue& aValueToComposite,
const StyleAnimationValue& aUnderlyingValue,
CompositeOperation aCompositeOperation)
{
switch (aCompositeOperation) {
case dom::CompositeOperation::Replace:
return aValueToComposite;
- case dom::CompositeOperation::Add:
- // So far nothing to do since we come to here only in case of missing
- // keyframe, that means we have only to use the base value or the
- // underlying value as the composited value.
- // FIXME: Bug 1311620: Once we implement additive operation, we need to
- // calculate it here.
- return aUnderlyingValue;
+ case dom::CompositeOperation::Add: {
+ // Just return the underlying value if |aValueToComposite| is null (i.e.
+ // missing keyframe).
+ if (aValueToComposite.IsNull()) {
+ return aUnderlyingValue;
+ }
+ StyleAnimationValue result(aValueToComposite);
+ return StyleAnimationValue::Add(aProperty,
+ aUnderlyingValue,
+ Move(result));
+ }
case dom::CompositeOperation::Accumulate: {
StyleAnimationValue result(aValueToComposite);
return StyleAnimationValue::Accumulate(aProperty,
aUnderlyingValue,
Move(result));
}
default:
MOZ_ASSERT_UNREACHABLE("Unknown compisite operation type");
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -809,20 +809,17 @@ ConvertKeyframeSequence(JSContext* aCx,
if (!keyframe) {
return false;
}
if (!keyframeDict.mOffset.IsNull()) {
keyframe->mOffset.emplace(keyframeDict.mOffset.Value());
}
if (keyframeDict.mComposite.WasPassed()) {
- // FIXME: Bug 1311620: We don't support additive animation yet.
- if (keyframeDict.mComposite.Value() != dom::CompositeOperation::Add) {
- keyframe->mComposite.emplace(keyframeDict.mComposite.Value());
- }
+ keyframe->mComposite.emplace(keyframeDict.mComposite.Value());
}
ErrorResult rv;
keyframe->mTimingFunction =
TimingParams::ParseEasing(keyframeDict.mEasing, aDocument, rv);
if (rv.MaybeSetPendingException(aCx)) {
return false;
}
@@ -1414,20 +1411,17 @@ GetKeyframeListFromPropertyIndexedKeyfra
if (!keyframeDict.Init(aCx, aValue, "BasePropertyIndexedKeyframe argument",
false)) {
aRv.Throw(NS_ERROR_FAILURE);
return;
}
Maybe<dom::CompositeOperation> composite;
if (keyframeDict.mComposite.WasPassed()) {
- // FIXME: Bug 1311620: We don't support additive animation yet.
- if (keyframeDict.mComposite.Value() != dom::CompositeOperation::Add) {
- composite.emplace(keyframeDict.mComposite.Value());
- }
+ composite.emplace(keyframeDict.mComposite.Value());
}
Maybe<ComputedTimingFunction> easing =
TimingParams::ParseEasing(keyframeDict.mEasing, aDocument, aRv);
if (aRv.Failed()) {
return;
}
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -733,16 +733,31 @@ ExtractComplexColor(const StyleAnimation
case StyleAnimationValue::eUnit_ComplexColor:
return ComplexColorData(aValue.GetComplexColorData());
default:
MOZ_ASSERT_UNREACHABLE("Unknown unit");
return ComplexColorData({0, 0, 0, 0}, 0.0f);
}
}
+StyleAnimationValue
+StyleAnimationValue::Add(nsCSSPropertyID aProperty,
+ const StyleAnimationValue& aA,
+ StyleAnimationValue&& aB)
+{
+ StyleAnimationValue result(Move(aB));
+
+ Unused << AddWeighted(aProperty,
+ 1.0, result,
+ 1, aA,
+ result);
+
+ return result;
+}
+
double
StyleAnimationValue::ComputeColorDistance(const RGBAColorData& aStartColor,
const RGBAColorData& aEndColor)
{
// http://www.w3.org/TR/smil-animation/#animateColorElement says
// that we should use Euclidean RGB cube distance. However, we
// have to extend that to RGBA. For now, we'll just use the
// Euclidean distance in the (part of the) 4-cube of premultiplied
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -57,16 +57,25 @@ public:
*/
static MOZ_MUST_USE bool
Add(nsCSSPropertyID aProperty, StyleAnimationValue& aDest,
const StyleAnimationValue& aValueToAdd, uint32_t aCount) {
return AddWeighted(aProperty, 1.0, aDest, aCount, aValueToAdd, aDest);
}
/**
+ * Alternative version of Add that reflects the naming used in Web Animations
+ * and which returns the result using the return value.
+ */
+ static StyleAnimationValue
+ Add(nsCSSPropertyID aProperty,
+ const StyleAnimationValue& aA,
+ StyleAnimationValue&& aB);
+
+ /**
* Calculates a measure of 'distance' between two colors.
*
* @param aStartColor The start of the interval for which the distance
* should be calculated.
* @param aEndColor The end of the interval for which the distance
* should be calculated.
* @return the result of the calculation.
*/
--- a/testing/web-platform/meta/web-animations/animation-model/combining-effects/effect-composition.html.ini
+++ b/testing/web-platform/meta/web-animations/animation-model/combining-effects/effect-composition.html.ini
@@ -1,14 +1,5 @@
[effect-composition.html]
type: testharness
- [add onto the base value]
- expected: FAIL
-
- [add onto an underlying animation value]
- expected: FAIL
-
- [Composite when mixing add and replace]
- expected: FAIL
-
[add specified on a keyframe overrides the composite mode of the effect]
expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-[animate.html]
- type: testharness
- [Element.animate() accepts a keyframe sequence with different composite values, but the same composite value for a given offset]
- expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1291468
-
--- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini
+++ b/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini
@@ -1,18 +1,6 @@
[constructor.html]
type: testharness
- [composite values are parsed correctly when passed to the KeyframeEffectReadOnly constructor in property-indexed keyframes]
- expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1291468
-
- [composite values are parsed correctly when passed to the KeyframeEffectReadOnly constructor in regular keyframes]
- expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1291468
-
[composite values are parsed correctly when passed to the KeyframeEffectReadOnly constructor in KeyframeTimingOptions]
expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1311620 # This needs additive animation
+ bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1311620
- [a KeyframeEffectReadOnly can be constructed with a keyframe sequence with different composite values, but the same composite value for a given offset]
- expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1291468
-
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini
+++ /dev/null
@@ -1,6 +0,0 @@
-[setKeyframes.html]
- type: testharness
- [Keyframes can be replaced with a keyframe sequence with different composite values, but the same composite value for a given offset]
- expected: FAIL
- bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1291468
-
--- a/testing/web-platform/tests/web-animations/resources/keyframe-utils.js
+++ b/testing/web-platform/tests/web-animations/resources/keyframe-utils.js
@@ -336,19 +336,19 @@ var gKeyframeSequenceTests = [
{ offset: 0.5, composite: "add", left: "30px" },
{ offset: 0.5, composite: "add", top: "40px" },
{ offset: 1.0, composite: "replace", left: "50px" },
{ offset: 1.0, composite: "replace", top: "60px" }],
output: [{ offset: 0.0, computedOffset: 0.0, easing: "linear",
composite: "replace", left: "10px" },
{ offset: 0.0, computedOffset: 0.0, easing: "linear",
composite: "replace", top: "20px" },
- { offset: 0.5, computedOffset: 0.0, easing: "linear",
+ { offset: 0.5, computedOffset: 0.5, easing: "linear",
composite: "add", left: "30px" },
- { offset: 0.5, computedOffset: 0.0, easing: "linear",
+ { offset: 0.5, computedOffset: 0.5, easing: "linear",
composite: "add", top: "40px" },
{ offset: 1.0, computedOffset: 1.0, easing: "linear",
composite: "replace", left: "50px" },
{ offset: 1.0, computedOffset: 1.0, easing: "linear",
composite: "replace", top: "60px" }] },
{ desc: "a one property two keyframe sequence that needs to stringify"
+ " its values",
input: [{ offset: 0, opacity: 0 },