Skip to content

Commit

Permalink
Bug 1744850 - Drop ScrollTimeline:sTiming and introduce a normalized …
Browse files Browse the repository at this point in the history
…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 w3c/csswg-drafts#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
  • Loading branch information
BorisChiou committed Jun 22, 2022
1 parent 8976ddf commit 6c723e6
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 38 deletions.
13 changes: 9 additions & 4 deletions dom/animation/Animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ void Animation::SetTimelineNoUpdate(AnimationTimeline* aTimeline) {
}

mTimeline = aTimeline;

if (mEffect) {
mEffect->UpdateNormalizedTiming();
}

if (!mStartTime.IsNull()) {
mHoldTime.SetNull();
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -1792,7 +1797,7 @@ StickyTimeDuration Animation::EffectEnd() const {
return StickyTimeDuration(0);
}

return mEffect->SpecifiedTiming().EndTime();
return mEffect->NormalizedTiming().EndTime();
}

Document* Animation::GetRenderedDocument() const {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
16 changes: 15 additions & 1 deletion dom/animation/AnimationEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ void AnimationEffect::SetSpecifiedTiming(TimingParams&& aTiming) {

mTiming = aTiming;

UpdateNormalizedTiming();

if (mAnimation) {
Maybe<nsAutoAnimationMutationBatch> mb;
if (AsKeyframeEffect() && AsKeyframeEffect()->GetAnimationTarget()) {
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<TimeDuration> AnimationEffect::GetLocalTime() const {
// Since the *animation* start time is currently always zero, the local
// time is equal to the parent time.
Expand Down
27 changes: 17 additions & 10 deletions dom/animation/AnimationEffect.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
//
Expand Down Expand Up @@ -98,6 +104,7 @@ class AnimationEffect : public nsISupports, public nsWrapperCache {
RefPtr<Document> mDocument;
RefPtr<Animation> mAnimation;
TimingParams mTiming;
Maybe<TimingParams> mNormalizedTiming;
};

} // namespace dom
Expand Down
2 changes: 1 addition & 1 deletion dom/animation/CSSAnimation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 2 additions & 0 deletions dom/animation/KeyframeEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
15 changes: 2 additions & 13 deletions dom/animation/ScrollTimeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#include "nsIScrollableFrame.h"
#include "nsLayoutUtils.h"

#define SCROLL_TIMELINE_DURATION_MILLISEC 100000

namespace mozilla::dom {

// ---------------------------------
Expand All @@ -43,22 +41,13 @@ 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()),
mDocument(aDocument),
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<float>::infinity(),
PlaybackDirection::Alternate, FillMode::Both);
}

/* static */
Expand Down Expand Up @@ -222,7 +211,7 @@ Nullable<TimeDuration> 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();
Expand All @@ -240,7 +229,7 @@ Nullable<TimeDuration> 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 {
Expand Down
8 changes: 0 additions & 8 deletions dom/animation/ScrollTimeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -177,8 +176,6 @@ class ScrollTimeline final : public AnimationTimeline {

bool ScrollingDirectionIsAvailable() const;

static constexpr const TimingParams& GetTiming() { return sTiming; }

protected:
virtual ~ScrollTimeline() { Teardown(); }

Expand Down Expand Up @@ -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;
};

/**
Expand Down
12 changes: 12 additions & 0 deletions dom/animation/TimingParams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<double>::infinity();
mDirection = dom::PlaybackDirection::Alternate;
mFill = dom::FillMode::Both;

Update();
}

} // namespace mozilla
4 changes: 4 additions & 0 deletions dom/animation/TimingParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "mozilla/dom/AnimationEffectBinding.h" // for FillMode
// and PlaybackDirection

#define PROGRESS_TIMELINE_DURATION_MILLISEC 100000

namespace mozilla {

namespace dom {
Expand Down Expand Up @@ -202,6 +204,8 @@ struct TimingParams {
return mFunction;
}

void Normalize();

private:
void Update() {
mActiveDuration = CalcActiveDuration(mDuration, mIterations);
Expand Down
2 changes: 1 addition & 1 deletion gfx/layers/AnimationInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[at-scroll-timeline-default-descriptors-iframe-print.html]
expected: FAIL
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1774505
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[at-scroll-timeline-specified-scroller-print.html]
expected: FAIL
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1774505

0 comments on commit 6c723e6

Please sign in to comment.