From 6c723e6e0a3217418c01766b0bcac15703fe8e25 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 22 Jun 2022 21:19:30 +0000 Subject: [PATCH] Bug 1744850 - Drop ScrollTimeline:sTiming and introduce a normalized timing. r=birtles `sTiming` is a hack and I believe animation-delay, animation-iteration-count, animation-direction, and animation-fill-mode should be meaningful for scroll-linked animations. (I will add the tentative wpt in Bug 1775327.) So we need to introduce a normalized timing when resolving the specified timing. Also, this patch makes the bug of printing scroll animations detectable. No behavior is changed and I'd like to remove the magic values and do normalization in Bug 1775327. Note: Based on https://github.com/w3c/csswg-drafts/issues/4862 and web-animations-2, we will introudce CSSNumberish for duration, current time, and delay. That is, we will accept percentage for animation-duration, animation-delay. However, Gecko doesn't support CSSNumberish for those values, so we'd like to normalize these time values in Bug 1775327. This patch is the 1st step: split the normalized timing from the specified timing, and use it when resolving the timing, for progress-based timeline. Differential Revision: https://phabricator.services.mozilla.com/D149683 --- dom/animation/Animation.cpp | 13 ++++++--- dom/animation/AnimationEffect.cpp | 16 ++++++++++- dom/animation/AnimationEffect.h | 27 ++++++++++++------- dom/animation/CSSAnimation.h | 2 +- dom/animation/KeyframeEffect.cpp | 2 ++ dom/animation/ScrollTimeline.cpp | 15 ++--------- dom/animation/ScrollTimeline.h | 8 ------ dom/animation/TimingParams.cpp | 12 +++++++++ dom/animation/TimingParams.h | 4 +++ gfx/layers/AnimationInfo.cpp | 2 +- ...-default-descriptors-iframe-print.html.ini | 3 +++ ...fault-descriptors-print.tentative.html.ini | 2 ++ ...timeline-specified-scroller-print.html.ini | 1 + 13 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-iframe-print.html.ini diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index 2c85b5c7cf561..0bb6fe0acd512 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -255,6 +255,11 @@ void Animation::SetTimelineNoUpdate(AnimationTimeline* aTimeline) { } mTimeline = aTimeline; + + if (mEffect) { + mEffect->UpdateNormalizedTiming(); + } + if (!mStartTime.IsNull()) { mHoldTime.SetNull(); } @@ -992,7 +997,7 @@ TimeStamp Animation::AnimationTimeToTimeStamp( TimeStamp Animation::ElapsedTimeToTimeStamp( const StickyTimeDuration& aElapsedTime) const { TimeDuration delay = - mEffect ? mEffect->SpecifiedTiming().Delay() : TimeDuration(); + mEffect ? mEffect->NormalizedTiming().Delay() : TimeDuration(); return AnimationTimeToTimeStamp(aElapsedTime + delay); } @@ -1792,7 +1797,7 @@ StickyTimeDuration Animation::EffectEnd() const { return StickyTimeDuration(0); } - return mEffect->SpecifiedTiming().EndTime(); + return mEffect->NormalizedTiming().EndTime(); } Document* Animation::GetRenderedDocument() const { @@ -1916,7 +1921,7 @@ StickyTimeDuration Animation::IntervalStartTime( "Should be called for CSS animations or transitions"); static constexpr StickyTimeDuration zeroDuration = StickyTimeDuration(); return std::max( - std::min(StickyTimeDuration(-mEffect->SpecifiedTiming().Delay()), + std::min(StickyTimeDuration(-mEffect->NormalizedTiming().Delay()), aActiveDuration), zeroDuration); } @@ -1932,7 +1937,7 @@ StickyTimeDuration Animation::IntervalEndTime( "Should be called for CSS animations or transitions"); static constexpr StickyTimeDuration zeroDuration = StickyTimeDuration(); - return std::max(std::min((EffectEnd() - mEffect->SpecifiedTiming().Delay()), + return std::max(std::min((EffectEnd() - mEffect->NormalizedTiming().Delay()), aActiveDuration), zeroDuration); } diff --git a/dom/animation/AnimationEffect.cpp b/dom/animation/AnimationEffect.cpp index 3ed190121845d..17c3741898ef7 100644 --- a/dom/animation/AnimationEffect.cpp +++ b/dom/animation/AnimationEffect.cpp @@ -75,6 +75,8 @@ void AnimationEffect::SetSpecifiedTiming(TimingParams&& aTiming) { mTiming = aTiming; + UpdateNormalizedTiming(); + if (mAnimation) { Maybe mb; if (AsKeyframeEffect() && AsKeyframeEffect()->GetAnimationTarget()) { @@ -91,6 +93,7 @@ void AnimationEffect::SetSpecifiedTiming(TimingParams&& aTiming) { AsKeyframeEffect()->RequestRestyle(EffectCompositor::RestyleType::Layer); } } + // For keyframe effects, NotifyEffectTimingUpdated above will eventually // cause KeyframeEffect::NotifyAnimationTimingUpdated to be called so it can // update its registration with the target element as necessary. @@ -266,7 +269,7 @@ ComputedTiming AnimationEffect::GetComputedTiming( const TimingParams* aTiming) const { double playbackRate = mAnimation ? mAnimation->PlaybackRate() : 1; return GetComputedTimingAt( - GetLocalTime(), aTiming ? *aTiming : SpecifiedTiming(), playbackRate); + GetLocalTime(), aTiming ? *aTiming : NormalizedTiming(), playbackRate); } // Helper function for generating an (Computed)EffectTiming dictionary @@ -333,6 +336,17 @@ void AnimationEffect::UpdateTiming(const OptionalEffectTiming& aTiming, SetSpecifiedTiming(std::move(timing)); } +void AnimationEffect::UpdateNormalizedTiming() { + mNormalizedTiming.reset(); + + if (!mAnimation || !mAnimation->UsingScrollTimeline()) { + return; + } + + mNormalizedTiming.emplace(mTiming); + mNormalizedTiming->Normalize(); +} + Nullable AnimationEffect::GetLocalTime() const { // Since the *animation* start time is currently always zero, the local // time is equal to the parent time. diff --git a/dom/animation/AnimationEffect.h b/dom/animation/AnimationEffect.h index 0986650f11834..7c71c3cb96a49 100644 --- a/dom/animation/AnimationEffect.h +++ b/dom/animation/AnimationEffect.h @@ -41,7 +41,7 @@ class AnimationEffect : public nsISupports, public nsWrapperCache { bool IsCurrent() const; bool IsInEffect() const; bool HasFiniteActiveDuration() const { - return SpecifiedTiming().ActiveDuration() != TimeDuration::Forever(); + return NormalizedTiming().ActiveDuration() != TimeDuration::Forever(); } // AnimationEffect interface @@ -50,17 +50,23 @@ class AnimationEffect : public nsISupports, public nsWrapperCache { virtual void UpdateTiming(const OptionalEffectTiming& aTiming, ErrorResult& aRv); - const TimingParams& SpecifiedTiming() const { - return mAnimation && mAnimation->UsingScrollTimeline() - // This returns the TimingParams which is used for getting - // ComputedTiming. For now, we assume all the callers of - // SpecifiedTiming() want to use this for scroll-timelines - // (and let the animation events be an undefined part). - ? ScrollTimeline::GetTiming() - : mTiming; - } + const TimingParams& SpecifiedTiming() const { return mTiming; } void SetSpecifiedTiming(TimingParams&& aTiming); + const TimingParams& NormalizedTiming() const { + MOZ_ASSERT((mAnimation && mAnimation->UsingScrollTimeline() && + mNormalizedTiming) || + !mNormalizedTiming, + "We do normalization only for progress-based timeline"); + return mNormalizedTiming ? *mNormalizedTiming : mTiming; + } + + // There are 3 conditions where we have to update the normalized timing: + // 1. mAnimation is changed, or + // 2. the timeline of mAnimation is changed, or + // 3. mTiming is changed. + void UpdateNormalizedTiming(); + // This function takes as input the timing parameters of an animation and // returns the computed timing at the specified local time. // @@ -98,6 +104,7 @@ class AnimationEffect : public nsISupports, public nsWrapperCache { RefPtr mDocument; RefPtr mAnimation; TimingParams mTiming; + Maybe mNormalizedTiming; }; } // namespace dom diff --git a/dom/animation/CSSAnimation.h b/dom/animation/CSSAnimation.h index 9210dfd04bd32..ce6c392146eed 100644 --- a/dom/animation/CSSAnimation.h +++ b/dom/animation/CSSAnimation.h @@ -158,7 +158,7 @@ class CSSAnimation final : public Animation { // This is used for setting the elapsedTime member of CSS AnimationEvents. TimeDuration InitialAdvance() const { return mEffect ? std::max(TimeDuration(), - mEffect->SpecifiedTiming().Delay() * -1) + mEffect->NormalizedTiming().Delay() * -1) : TimeDuration(); } diff --git a/dom/animation/KeyframeEffect.cpp b/dom/animation/KeyframeEffect.cpp index 245a587388498..7769a935d6b78 100644 --- a/dom/animation/KeyframeEffect.cpp +++ b/dom/animation/KeyframeEffect.cpp @@ -1862,6 +1862,8 @@ void KeyframeEffect::SetAnimation(Animation* aAnimation) { mAnimation = aAnimation; + UpdateNormalizedTiming(); + // The order of these function calls is important: // NotifyAnimationTimingUpdated() need the updated mIsRelevant flag to check // if it should create the effectSet or not, and MarkCascadeNeedsUpdate() diff --git a/dom/animation/ScrollTimeline.cpp b/dom/animation/ScrollTimeline.cpp index 3da0c828471fa..6c380fc482103 100644 --- a/dom/animation/ScrollTimeline.cpp +++ b/dom/animation/ScrollTimeline.cpp @@ -15,8 +15,6 @@ #include "nsIScrollableFrame.h" #include "nsLayoutUtils.h" -#define SCROLL_TIMELINE_DURATION_MILLISEC 100000 - namespace mozilla::dom { // --------------------------------- @@ -43,8 +41,6 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(ScrollTimeline, AnimationTimeline) -TimingParams ScrollTimeline::sTiming; - ScrollTimeline::ScrollTimeline(Document* aDocument, const Scroller& aScroller, StyleScrollAxis aAxis) : AnimationTimeline(aDocument->GetParentObject()), @@ -52,13 +48,6 @@ ScrollTimeline::ScrollTimeline(Document* aDocument, const Scroller& aScroller, mSource(aScroller), mAxis(aAxis) { MOZ_ASSERT(aDocument); - - // Use default values except for |mDuration| and |mFill|. - // Use a fixed duration defined in SCROLL_TIMELINE_DURATIONMILLISEC, and use - // FillMode::Both to make sure the animation is in effect at 100%. - sTiming = TimingParams(SCROLL_TIMELINE_DURATION_MILLISEC, 0.0, - std::numeric_limits::infinity(), - PlaybackDirection::Alternate, FillMode::Both); } /* static */ @@ -222,7 +211,7 @@ Nullable ScrollTimeline::GetCurrentTimeAsDuration() const { // If this orientation is not ready for scrolling (i.e. the scroll range is // not larger than or equal to one device pixel), we make it 100%. if (!scrollFrame->GetAvailableScrollingDirections().contains(orientation)) { - return TimeDuration::FromMilliseconds(SCROLL_TIMELINE_DURATION_MILLISEC); + return TimeDuration::FromMilliseconds(PROGRESS_TIMELINE_DURATION_MILLISEC); } const nsPoint& scrollOffset = scrollFrame->GetScrollPosition(); @@ -240,7 +229,7 @@ Nullable ScrollTimeline::GetCurrentTimeAsDuration() const { // https://drafts.csswg.org/scroll-animations-1/#progress-calculation-algorithm double progress = position / range; return TimeDuration::FromMilliseconds(progress * - SCROLL_TIMELINE_DURATION_MILLISEC); + PROGRESS_TIMELINE_DURATION_MILLISEC); } layers::ScrollDirection ScrollTimeline::Axis() const { diff --git a/dom/animation/ScrollTimeline.h b/dom/animation/ScrollTimeline.h index b6226b77d6b08..c118462e7cdbe 100644 --- a/dom/animation/ScrollTimeline.h +++ b/dom/animation/ScrollTimeline.h @@ -12,7 +12,6 @@ #include "mozilla/HashTable.h" #include "mozilla/PairHash.h" #include "mozilla/ServoStyleConsts.h" -#include "mozilla/TimingParams.h" #include "mozilla/WritingModes.h" class nsIScrollableFrame; @@ -177,8 +176,6 @@ class ScrollTimeline final : public AnimationTimeline { bool ScrollingDirectionIsAvailable() const; - static constexpr const TimingParams& GetTiming() { return sTiming; } - protected: virtual ~ScrollTimeline() { Teardown(); } @@ -208,11 +205,6 @@ class ScrollTimeline final : public AnimationTimeline { // nearest scroller. Scroller mSource; StyleScrollAxis mAxis; - - // Note: it's unfortunate TimingParams cannot be a const variable because - // we have to use StickyTimingDuration::FromMilliseconds() in its - // constructor. - static TimingParams sTiming; }; /** diff --git a/dom/animation/TimingParams.cpp b/dom/animation/TimingParams.cpp index 039d466438e56..be269545274b9 100644 --- a/dom/animation/TimingParams.cpp +++ b/dom/animation/TimingParams.cpp @@ -214,4 +214,16 @@ bool TimingParams::operator==(const TimingParams& aOther) const { mFunction == aOther.mFunction; } +void TimingParams::Normalize() { + // FIXME: Bug 1775327, do normalization, instead of using these magic numbers. + mDuration = Some(StickyTimeDuration::FromMilliseconds( + PROGRESS_TIMELINE_DURATION_MILLISEC)); + mDelay = TimeDuration::FromMilliseconds(0); + mIterations = std::numeric_limits::infinity(); + mDirection = dom::PlaybackDirection::Alternate; + mFill = dom::FillMode::Both; + + Update(); +} + } // namespace mozilla diff --git a/dom/animation/TimingParams.h b/dom/animation/TimingParams.h index 2bffc17c6790c..9bf193c784859 100644 --- a/dom/animation/TimingParams.h +++ b/dom/animation/TimingParams.h @@ -21,6 +21,8 @@ #include "mozilla/dom/AnimationEffectBinding.h" // for FillMode // and PlaybackDirection +#define PROGRESS_TIMELINE_DURATION_MILLISEC 100000 + namespace mozilla { namespace dom { @@ -202,6 +204,8 @@ struct TimingParams { return mFunction; } + void Normalize(); + private: void Update() { mActiveDuration = CalcActiveDuration(mDuration, mIterations); diff --git a/gfx/layers/AnimationInfo.cpp b/gfx/layers/AnimationInfo.cpp index e6e142fec0b1e..f69a54e71621e 100644 --- a/gfx/layers/AnimationInfo.cpp +++ b/gfx/layers/AnimationInfo.cpp @@ -427,7 +427,7 @@ void AnimationInfo::AddAnimationForProperty( ? AddAnimationForNextTransaction() : AddAnimation(); - const TimingParams& timing = aAnimation->GetEffect()->SpecifiedTiming(); + const TimingParams& timing = aAnimation->GetEffect()->NormalizedTiming(); // If we are starting a new transition that replaces an existing transition // running on the compositor, it is possible that the animation on the diff --git a/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-iframe-print.html.ini b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-iframe-print.html.ini new file mode 100644 index 0000000000000..652adcfd787ce --- /dev/null +++ b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-iframe-print.html.ini @@ -0,0 +1,3 @@ +[at-scroll-timeline-default-descriptors-iframe-print.html] + expected: FAIL + bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1774505 diff --git a/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-print.tentative.html.ini b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-print.tentative.html.ini index 84bc531888ef3..a6c937f9e2e80 100644 --- a/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-print.tentative.html.ini +++ b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-default-descriptors-print.tentative.html.ini @@ -1,2 +1,4 @@ [at-scroll-timeline-default-descriptors-print.tentative.html] max-asserts: 1 # Bug 1746116, an assertion to check "nextFrame || kidStatus.NextInFlowNeedsReflow()" in nsCanvasFrame::Reflow(). + expected: FAIL + bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1774505 diff --git a/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-specified-scroller-print.html.ini b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-specified-scroller-print.html.ini index 0b807bd5ab32b..6fadc30600ae8 100644 --- a/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-specified-scroller-print.html.ini +++ b/testing/web-platform/meta/scroll-animations/css/at-scroll-timeline-specified-scroller-print.html.ini @@ -1,2 +1,3 @@ [at-scroll-timeline-specified-scroller-print.html] expected: FAIL + bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1774505