From 5e2100a6271808f7a84f477d9c24b52b838640a5 Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Thu, 12 Jan 2017 10:28:46 +0900 Subject: [PATCH] Bug 1325193 - Get underlying style value in the case where the last segment is missing keyframe for accumulation of iteration composite. r=birtles Both of tests in this patch fail and cause lots of assertions without this fix. MozReview-Commit-ID: CFrWSlM0Us5 --- dom/animation/KeyframeEffectReadOnly.cpp | 53 ++++++++++++------- dom/animation/KeyframeEffectReadOnly.h | 5 ++ .../file_missing-keyframe-on-compositor.html | 20 +++++++ .../test/style/file_missing-keyframe.html | 16 +++++- .../composite/AsyncCompositionManager.cpp | 8 ++- 5 files changed, 80 insertions(+), 22 deletions(-) diff --git a/dom/animation/KeyframeEffectReadOnly.cpp b/dom/animation/KeyframeEffectReadOnly.cpp index 3b2315199740..9dc4d735b9fc 100644 --- a/dom/animation/KeyframeEffectReadOnly.cpp +++ b/dom/animation/KeyframeEffectReadOnly.cpp @@ -347,6 +347,34 @@ KeyframeEffectReadOnly::CompositeValue( return StyleAnimationValue(); } +StyleAnimationValue +KeyframeEffectReadOnly::GetUnderlyingStyle( + nsCSSPropertyID aProperty, + const RefPtr& aAnimationRule) +{ + StyleAnimationValue result; + + if (aAnimationRule->HasValue(aProperty)) { + // If we have already composed style for the property, we use the style + // as the underlying style. + DebugOnly success = aAnimationRule->GetValue(aProperty, result); + MOZ_ASSERT(success, "AnimValuesStyleRule::GetValue should not fail"); + } else { + // If we are composing with composite operation that is not 'replace' + // and we have not composed style for the property yet, we have to get + // the base style for the property. + RefPtr styleContext = GetTargetStyleContext(); + result = EffectCompositor::GetBaseStyle(aProperty, + styleContext, + *mTarget->mElement, + mTarget->mPseudoType); + MOZ_ASSERT(!result.IsNull(), "The base style should be set"); + SetNeedsBaseStyle(aProperty); + } + + return result; +} + StyleAnimationValue KeyframeEffectReadOnly::CompositeValue( nsCSSPropertyID aProperty, @@ -374,23 +402,7 @@ KeyframeEffectReadOnly::CompositeValue( aCompositeOperation == CompositeOperation::Add, "InputValue should be null only if additive composite"); - if (aAnimationRule->HasValue(aProperty)) { - // If we have already composed style for the property, we use the style - // as the underlying style. - DebugOnly success = aAnimationRule->GetValue(aProperty, result); - MOZ_ASSERT(success, "AnimValuesStyleRule::GetValue should not fail"); - } else { - // If we are composing with composite operation that is not 'replace' - // and we have not composed style for the property yet, we have to get - // the base style for the property. - RefPtr styleContext = GetTargetStyleContext(); - result = EffectCompositor::GetBaseStyle(aProperty, - styleContext, - *mTarget->mElement, - mTarget->mPseudoType); - MOZ_ASSERT(!result.IsNull(), "The base style should be set"); - SetNeedsBaseStyle(aProperty); - } + result = GetUnderlyingStyle(aProperty, aAnimationRule); return CompositeValue(aProperty, aValueToComposite, @@ -533,14 +545,17 @@ KeyframeEffectReadOnly::ComposeStyle( prop.mSegments.LastElement(); // FIXME: Bug 1293492: Add a utility function to calculate both of // below StyleAnimationValues. + StyleAnimationValue lastValue = lastSegment.mToValue.IsNull() + ? GetUnderlyingStyle(prop.mProperty, aStyleRule) + : lastSegment.mToValue; fromValue = StyleAnimationValue::Accumulate(prop.mProperty, - lastSegment.mToValue, + lastValue, Move(fromValue), computedTiming.mCurrentIteration); toValue = StyleAnimationValue::Accumulate(prop.mProperty, - lastSegment.mToValue, + lastValue, Move(toValue), computedTiming.mCurrentIteration); } diff --git a/dom/animation/KeyframeEffectReadOnly.h b/dom/animation/KeyframeEffectReadOnly.h index 3515549653c5..00768bd7ae1a 100644 --- a/dom/animation/KeyframeEffectReadOnly.h +++ b/dom/animation/KeyframeEffectReadOnly.h @@ -423,6 +423,11 @@ protected: const StyleAnimationValue& aValueToComposite, CompositeOperation aCompositeOperation); + // Returns underlying style animation value for |aProperty|. + StyleAnimationValue GetUnderlyingStyle( + nsCSSPropertyID aProperty, + const RefPtr& aAnimationRule); + // Set a bit in mNeedsBaseStyleSet if |aProperty| can be run on the // compositor. void SetNeedsBaseStyle(nsCSSPropertyID aProperty); diff --git a/dom/animation/test/style/file_missing-keyframe-on-compositor.html b/dom/animation/test/style/file_missing-keyframe-on-compositor.html index b7ac33554740..ff7da8fe501d 100644 --- a/dom/animation/test/style/file_missing-keyframe-on-compositor.html +++ b/dom/animation/test/style/file_missing-keyframe-on-compositor.html @@ -490,6 +490,26 @@ promise_test(t => { }, 'Transform value for animation with no keyframe at offset 0 and with ' + 'positive delay'); +promise_test(t => { + useTestRefreshMode(t); + + var div = addDiv(t, { style: 'transform: translateX(100px)' }); + + div.animate([{ offset: 0, transform: 'translateX(200px)'}], + { duration: 100 * MS_PER_SEC, + iterationStart: 1, + iterationComposite: 'accumulate' }); + + return waitForPaintsFlushed().then(() => { + var transform = + SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform'); + assert_matrix_equals(transform, 'matrix(1, 0, 0, 1, 300, 0)', + 'Transform value for animation with no keyframe at offset 1 and its ' + + 'iterationComposite is accumulate'); + }); +}, 'Transform value for animation with no keyframe at offset 1 and its ' + + 'iterationComposite is accumulate'); + done(); diff --git a/dom/animation/test/style/file_missing-keyframe.html b/dom/animation/test/style/file_missing-keyframe.html index 144514bc0fea..ea2be90f3f0d 100644 --- a/dom/animation/test/style/file_missing-keyframe.html +++ b/dom/animation/test/style/file_missing-keyframe.html @@ -85,10 +85,24 @@ test(t => { // (200px + 200px) * 0.5 assert_equals(getComputedStyle(div).marginLeft, '200px', 'The margin-left value at 50% should be additive value of ' + - 'of the transition and animation'); + 'the transition and animation'); }, 'margin-left value at 50% for an animation with no keyframe at offset 1 ' + 'is that of transition'); +test(t => { + var div = addDiv(t, { style: 'margin-left: 100px' }); + + var animation = div.animate([{ offset: 0, marginLeft: '200px' }], + { duration: 100 * MS_PER_SEC, + iterationStart: 1, + iterationComposite: 'accumulate' }); + + assert_equals(getComputedStyle(div).marginLeft, '300px', + 'The margin-left value should be additive value of the ' + + 'accumulation of the initial value onto the base value '); +}, 'margin-left value for an animation with no keyframe at offset 1 and its ' + + 'iterationComposite is accumulate'); + done(); diff --git a/gfx/layers/composite/AsyncCompositionManager.cpp b/gfx/layers/composite/AsyncCompositionManager.cpp index ca00d4bc9229..15b56d447f7b 100644 --- a/gfx/layers/composite/AsyncCompositionManager.cpp +++ b/gfx/layers/composite/AsyncCompositionManager.cpp @@ -613,12 +613,16 @@ SampleValue(float aPortion, const Animation& aAnimation, // below StyleAnimationValues. startValue = StyleAnimationValue::Accumulate(aAnimation.property(), - aLastValue, + aLastValue.IsNull() + ? aUnderlyingValue + : aLastValue, Move(startValue), aCurrentIteration); endValue = StyleAnimationValue::Accumulate(aAnimation.property(), - aLastValue, + aLastValue.IsNull() + ? aUnderlyingValue + : aLastValue, Move(endValue), aCurrentIteration); }