Bug 1279403 - Part 2: Set NS_FRAME_MAY_BE_TRANSFORMED bit if the target nsIFrame has transform when setting target or keyframes. r?birtles
MozReview-Commit-ID: InQyXpENsSY
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -488,16 +488,17 @@ KeyframeEffectReadOnly::SetKeyframes(nsT
KeyframeUtils::ApplyDistributeSpacing(mKeyframes);
if (mAnimation && mAnimation->IsRelevant()) {
nsNodeUtils::AnimationChanged(mAnimation);
}
if (aStyleContext) {
UpdateProperties(aStyleContext);
+ MaybeUpdateFrameForCompositor();
}
}
const AnimationProperty*
KeyframeEffectReadOnly::GetAnimationOfProperty(nsCSSProperty aProperty) const
{
for (size_t propIdx = 0, propEnd = mProperties.Length();
propIdx != propEnd; ++propIdx) {
@@ -1499,16 +1500,38 @@ KeyframeEffectReadOnly::CanIgnoreIfNotVi
}
// FIXME: For further sophisticated optimization we need to check
// change hint on the segment corresponding to computedTiming.progress.
return NS_IsHintSubset(
mCumulativeChangeHint, nsChangeHint_Hints_CanIgnoreIfNotVisible);
}
+void
+KeyframeEffectReadOnly::MaybeUpdateFrameForCompositor()
+{
+ nsIFrame* frame = GetAnimationFrame();
+ if (!frame) {
+ return;
+ }
+
+ // We don't check mWinsInCascade flag here because, at this point,
+ // UpdateCascadeResults has not yet run.
+ // FIXME: Bug 1272495: If this effect does not win in the cascade, the
+ // NS_FRAME_MAY_BE_TRANSFORMED flag should be removed when the animation
+ // will be removed from effect set or the transform keyframes are removed
+ // by setKeyframes. The latter case will be hard to solve though.
+ for (const AnimationProperty& property : mProperties) {
+ if (property.mProperty == eCSSProperty_transform) {
+ frame->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
+ return;
+ }
+ }
+}
+
//---------------------------------------------------------------------
//
// KeyframeEffect
//
//---------------------------------------------------------------------
KeyframeEffect::KeyframeEffect(nsIDocument* aDocument,
const Maybe<OwningAnimationTarget>& aTarget,
@@ -1598,16 +1621,18 @@ KeyframeEffect::SetTarget(const Nullable
UpdateTargetRegistration();
RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
if (styleContext) {
UpdateProperties(styleContext);
} else if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
KeyframeUtils::ApplyDistributeSpacing(mKeyframes);
}
+ MaybeUpdateFrameForCompositor();
+
RequestRestyle(EffectCompositor::RestyleType::Layer);
nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
if (mAnimation) {
nsNodeUtils::AnimationAdded(mAnimation);
}
} else if (mEffectOptions.mSpacingMode == SpacingMode::paced) {
// New target is null, so fall back to distribute spacing.
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -384,16 +384,22 @@ protected:
// owning Animation's timing.
void UpdateTargetRegistration();
// Remove the current effect target from its EffectSet.
void UnregisterTarget();
void RequestRestyle(EffectCompositor::RestyleType aRestyleType);
+ // Update the associated frame state bits so that, if necessary, a stacking
+ // context will be created and the effect sent to the compositor. We
+ // typically need to do this when the properties referenced by the keyframe
+ // have changed, or when the target frame might have changed.
+ void MaybeUpdateFrameForCompositor();
+
// Looks up the style context associated with the target element, if any.
// We need to be careful to *not* call this when we are updating the style
// context. That's because calling GetStyleContextForElement when we are in
// the process of building a style context may trigger various forms of
// infinite recursion.
// If aDoc is nullptr, we will use the owner doc of the target element.
already_AddRefed<nsStyleContext>
GetTargetStyleContext(nsIDocument* aDoc = nullptr);
--- a/layout/reftests/web-animations/reftest.list
+++ b/layout/reftests/web-animations/reftest.list
@@ -1,5 +1,8 @@
test-pref(dom.animations-api.core.enabled,true) == 1246046-1.html green-box.html
test-pref(dom.animations-api.core.enabled,true) == 1267937-1.html 1267937-ref.html
test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-none-animation-before-appending-element.html stacking-context-animation-ref.html
test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-keyframe.html stacking-context-animation-ref.html
test-pref(dom.animations-api.core.enabled,true) == stacking-context-opacity-changing-target.html stacking-context-animation-changing-target-ref.html
+test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-keyframe.html stacking-context-animation-ref.html
+test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-target.html stacking-context-animation-changing-target-ref.html
+test-pref(dom.animations-api.core.enabled,true) == stacking-context-transform-changing-display-property.html stacking-context-animation-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-display-property.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html class="reftest-wait reftest-no-flush">
+<title>
+Transform animation creates a stacking context when changing its display style
+</title>
+<style>
+span {
+ height: 100px;
+ width: 100px;
+ position: fixed;
+ background: green;
+ top: 50px;
+}
+div {
+ width: 100px; height: 100px;
+ background: blue;
+ display: none;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<script>
+ var anim = document.getElementById("test")
+ .animate({ transform: ['none', 'none'] }, { duration: 100000 });
+ anim.ready.then(function() {
+ anim.effect.target.style.display = 'block';
+ requestAnimationFrame(function() {
+ document.documentElement.classList.remove("reftest-wait");
+ });
+ });
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-keyframe.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html class="reftest-wait reftest-no-flush">
+<title>Changing keyframes to transform frames creates a stacking context</title>
+<style>
+span {
+ height: 100px;
+ width: 100px;
+ position: fixed;
+ background: green;
+ top: 50px;
+}
+#test {
+ width: 100px; height: 100px;
+ background: blue;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<script>
+ var anim = document.getElementById("test")
+ .animate({ }, { duration: 100000 });
+ anim.ready.then(function() {
+ anim.effect.setKeyframes({ transform: ['none', 'none'] });
+ requestAnimationFrame(function() {
+ document.documentElement.classList.remove("reftest-wait");
+ });
+ });
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/web-animations/stacking-context-transform-changing-target.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html class="reftest-wait reftest-no-flush">
+<title>
+Transform animation creates a stacking context when changing its target
+</title>
+<style>
+span {
+ height: 100px;
+ width: 100px;
+ position: fixed;
+ background: green;
+ top: 50px;
+}
+div {
+ width: 100px; height: 100px;
+ background: blue;
+}
+</style>
+<span></span>
+<div id="test"></div>
+<div id="another"></div>
+<script>
+ var anim = document.getElementById("test")
+ .animate({ transform: ['none', 'none'] }, { duration: 100000 });
+ anim.ready.then(function() {
+ anim.effect.target = document.getElementById("another");
+ requestAnimationFrame(function() {
+ document.documentElement.classList.remove("reftest-wait");
+ });
+ });
+</script>