Bug 1478643 - Apply change hints for transform animation in the case where the current transform style is 'none' and no change hint produced in AddLayerChangesForAnimation. r?birtles
The applying change hints are the same as what we apply for transform style
changed from something to 'none'.
All test cases pass with this fix fail without the fix.
MozReview-Commit-ID: 7HStU26lRPq
--- a/dom/animation/test/mozilla/test_style_after_finished_on_compositor.html
+++ b/dom/animation/test/mozilla/test_style_after_finished_on_compositor.html
@@ -71,10 +71,68 @@ promise_test(async t => {
await waitForNextFrame();
const opacity = SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'opacity');
assert_equals(opacity, '', 'No opacity animation runs on the compositor');
}, 'Opacity animation is removed from compositor even when it only visits ' +
'exactly the point where the opacity: 1 value was set');
+promise_test(async t => {
+ const div = addDiv(t, { 'class': 'compositor' });
+ const anim = div.animate([ { offset: 0, transform: 'none' },
+ { offset: 1, transform: 'translateX(100px)' } ],
+ { delay: 10,
+ duration: 100 });
+
+ await anim.finished;
+
+ await waitForNextFrame();
+
+ const transform = SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
+
+ assert_equals(transform, '', 'No transform animation runs on the compositor');
+}, 'Transform animation with positive delay is removed from compositor when ' +
+ 'finished');
+
+promise_test(async t => {
+ const div = addDiv(t, { 'class': 'compositor' });
+ const anim = div.animate([ { offset: 0, transform: 'none' },
+ { offset: 0.9, transform: 'none' },
+ { offset: 1, transform: 'translateX(100px)' } ],
+ { duration: 100 });
+
+ await anim.finished;
+
+ await waitForNextFrame();
+
+ const transform = SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
+
+ assert_equals(transform, '', 'No transform animation runs on the compositor');
+}, 'Transform animation initially transform: none is removed from compositor ' +
+ 'when finished');
+
+
+promise_test(async t => {
+ const div = addDiv(t, { 'class': 'compositor' });
+ const anim = div.animate([ { offset: 0, transform: 'translateX(100px)' },
+ { offset: 0.5, transform: 'none' },
+ { offset: 0.9, transform: 'none' },
+ { offset: 1, transform: 'translateX(100px)' } ],
+ { delay: 10, duration: 100 });
+
+ await waitForAnimationFrames(2);
+
+ // Setting the current time at the offset generating transform: none.
+ anim.currentTime = 60;
+
+ await anim.finished;
+
+ await waitForNextFrame();
+
+ const transform = SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
+
+ assert_equals(transform, '', 'No transform animation runs on the compositor');
+}, 'Transform animation is removed from compositor even when it only visits ' +
+ 'exactly the point where the transform: none value was set');
+
</script>
</body>
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1743,41 +1743,61 @@ RestyleManager::IncrementAnimationGenera
if (!mInStyleRefresh) {
++mAnimationGeneration;
}
}
/* static */ void
RestyleManager::AddLayerChangesForAnimation(nsIFrame* aFrame,
nsIContent* aContent,
+ nsChangeHint aHintForThisFrame,
nsStyleChangeList&
aChangeListToProcess)
{
if (!aFrame || !aContent) {
return;
}
uint64_t frameGeneration =
RestyleManager::GetAnimationGenerationForFrame(aFrame);
nsChangeHint hint = nsChangeHint(0);
for (const LayerAnimationInfo::Record& layerInfo :
LayerAnimationInfo::sRecords) {
layers::Layer* layer =
FrameLayerBuilder::GetDedicatedLayer(aFrame, layerInfo.mLayerType);
if (layer && frameGeneration != layer->GetAnimationGeneration()) {
- // If we have a transform layer but don't have any transform style, we
+ // If we have a transform layer bug don't have any transform style, we
// probably just removed the transform but haven't destroyed the layer
- // yet. In this case we will add the appropriate change hint
- // (nsChangeHint_UpdateContainingBlock) when we compare styles so we can
- // skip adding any change hint here. (If we *were* to add
- // nsChangeHint_UpdateTransformLayer, ApplyRenderingChangeToTree would
- // complain that we're updating a transform layer without a transform).
+ // yet. In this case we will typically add the appropriate change hint
+ // (nsChangeHint_UpdateContainingBlock) when we compare styles so in
+ // theory we could skip adding any change hint here.
+ //
+ // However, sometimes when we compare styles we'll get no change. For
+ // example, if the transform style was 'none' when we sent the transform
+ // animation to the compositor and the current transform style is now
+ // 'none' we'll think nothing changed but actually we still need to
+ // trigger an update to clear whatever style the transform animation set
+ // on the compositor. To handle this case we simply set all the change
+ // hints relevant to removing transform style (since we don't know exactly
+ // what changes happened while the animation was running on the
+ // compositor).
+ //
+ // Note that we *don't* add nsChangeHint_UpdateTransformLayer since if we
+ // did, ApplyRenderingChangeToTree would complain that we're updating a
+ // transform layer without a transform.
if (layerInfo.mLayerType == DisplayItemType::TYPE_TRANSFORM &&
!aFrame->StyleDisplay()->HasTransformStyle()) {
+ // Add all the hints for a removing a transform if they are not already
+ // set for this frame.
+ if (!(NS_IsHintSubset(
+ nsChangeHint_ComprehensiveAddOrRemoveTransform,
+ aHintForThisFrame))) {
+ hint |= nsChangeHint_ComprehensiveAddOrRemoveTransform;
+ }
continue;
}
hint |= layerInfo.mChangeHint;
}
// We consider it's the first paint for the frame if we have an animation
// for the property but have no layer.
// Note that in case of animations which has properties preventing running
@@ -2744,17 +2764,17 @@ RestyleManager::ProcessPostTraversal(
// the Web Animations API won't affect an element's style but still
// requires to update the animation on the layer.
//
// We can sometimes reach this when the animated style is being removed.
// Since AddLayerChangesForAnimation checks if |styleFrame| has a transform
// style or not, we need to call it *after* setting |newStyle| to
// |styleFrame| to ensure the animated transform has been removed first.
AddLayerChangesForAnimation(
- styleFrame, aElement, aRestyleState.ChangeList());
+ styleFrame, aElement, changeHint, aRestyleState.ChangeList());
childrenFlags |= SendA11yNotifications(mPresContext,
aElement,
oldOrDisplayContentsStyle,
upToDateContext,
aFlags);
}
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -392,18 +392,30 @@ public:
// Update the animation generation count to mark that animation state
// has changed.
//
// This is normally performed automatically by ProcessPendingRestyles
// but it is also called when we have out-of-band changes to animations
// such as changes made through the Web Animations API.
void IncrementAnimationGeneration();
+ // Apply change hints for animations on the compositor.
+ //
+ // There are some cases where we forcibly apply change hints for animations
+ // even if there is no change hint produced in order to synchronize with
+ // animations running on the compositor.
+ //
+ // For example:
+ //
+ // a) Pausing animations via the Web Animations API
+ // b) When the style before sending the animation to the compositor exactly
+ // the same as the current style
static void AddLayerChangesForAnimation(nsIFrame* aFrame,
nsIContent* aContent,
+ nsChangeHint aHintForThisFrame,
nsStyleChangeList&
aChangeListToProcess);
/**
* Whether to clear all the style data (including the element itself), or just
* the descendants' data.
*/
enum class IncludeRoot {