Skip to content

Commit

Permalink
Bug 1775327 - Part 2: Fix playing finished scroll animation on revers…
Browse files Browse the repository at this point in the history
…ing scrolling. r=firefox-animation-reviewers,birtles

We have to make sure the scroll animations is still responsive at
boundaries even if it's playstate is finished.

This patch includes the update of UpdateFinishedState() to match the
spec, and make sure we still tick scroll animations at finished play state.

Getting a finished state might be strange for scroll animations, and this
might be a spec issue. However, for consistency with JS-generated animations,
we'd like to align the behaviors with other browsers, and make sure we are
still match the definition of finished state in the spec.

Besides, we have to use EndTime() on the compositor so
animation-iteration-count works properly.

Tests are in the last patch.

Differential Revision: https://phabricator.services.mozilla.com/D149940
  • Loading branch information
BorisChiou committed Jul 7, 2022
1 parent ee025d7 commit db1b58d
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
27 changes: 15 additions & 12 deletions dom/animation/Animation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1610,30 +1610,32 @@ void Animation::UpdateTiming(SeekFlag aSeekFlag,
// https://drafts.csswg.org/web-animations/#update-an-animations-finished-state
void Animation::UpdateFinishedState(SeekFlag aSeekFlag,
SyncNotifyFlag aSyncNotifyFlag) {
Nullable<TimeDuration> currentTime = GetCurrentTimeAsDuration();
Nullable<TimeDuration> unconstrainedCurrentTime =
aSeekFlag == SeekFlag::NoSeek ? GetUnconstrainedCurrentTime()
: GetCurrentTimeAsDuration();
TimeDuration effectEnd = TimeDuration(EffectEnd());

if (!mStartTime.IsNull() && mPendingState == PendingState::NotPending) {
if (mPlaybackRate > 0.0 && !currentTime.IsNull() &&
currentTime.Value() >= effectEnd) {
if (!unconstrainedCurrentTime.IsNull() && !mStartTime.IsNull() &&
mPendingState == PendingState::NotPending) {
if (mPlaybackRate > 0.0 && unconstrainedCurrentTime.Value() >= effectEnd) {
if (aSeekFlag == SeekFlag::DidSeek) {
mHoldTime = currentTime;
mHoldTime = unconstrainedCurrentTime;
} else if (!mPreviousCurrentTime.IsNull()) {
mHoldTime.SetValue(std::max(mPreviousCurrentTime.Value(), effectEnd));
} else {
mHoldTime.SetValue(effectEnd);
}
} else if (mPlaybackRate < 0.0 && !currentTime.IsNull() &&
currentTime.Value() <= TimeDuration()) {
} else if (mPlaybackRate < 0.0 &&
unconstrainedCurrentTime.Value() <= TimeDuration()) {
if (aSeekFlag == SeekFlag::DidSeek) {
mHoldTime = currentTime;
mHoldTime = unconstrainedCurrentTime;
} else if (!mPreviousCurrentTime.IsNull()) {
mHoldTime.SetValue(
std::min(mPreviousCurrentTime.Value(), TimeDuration(0)));
} else {
mHoldTime.SetValue(0);
}
} else if (mPlaybackRate != 0.0 && !currentTime.IsNull() && mTimeline &&
} else if (mPlaybackRate != 0.0 && mTimeline &&
!mTimeline->GetCurrentTimeAsDuration().IsNull()) {
if (aSeekFlag == SeekFlag::DidSeek && !mHoldTime.IsNull()) {
mStartTime = StartTimeFromTimelineTime(
Expand All @@ -1644,15 +1646,16 @@ void Animation::UpdateFinishedState(SeekFlag aSeekFlag,
}
}

// We must recalculate the current time to take account of any mHoldTime
// changes the code above made.
mPreviousCurrentTime = GetCurrentTimeAsDuration();

bool currentFinishedState = PlayState() == AnimationPlayState::Finished;
if (currentFinishedState && !mFinishedIsResolved) {
DoFinishNotification(aSyncNotifyFlag);
} else if (!currentFinishedState && mFinishedIsResolved) {
ResetFinishedPromise();
}
// We must recalculate the current time to take account of any mHoldTime
// changes the code above made.
mPreviousCurrentTime = GetCurrentTimeAsDuration();
}

void Animation::UpdateEffect(PostRestyleMode aPostRestyle) {
Expand Down
8 changes: 7 additions & 1 deletion dom/animation/Animation.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,13 @@ class Animation : public DOMEventTargetHelper,
// won't be relevant and hence won't be returned by GetAnimations().
// We don't want its timeline to keep it alive (which would happen
// if we return true) since otherwise it will effectively be leaked.
PlaybackRate() != 0.0);
PlaybackRate() != 0.0) ||
// Always return true for not idle animations attached to not
// monotonically increasing timelines even if the animation is
// finished. This is required to accommodate cases where timeline
// ticks back in time.
(mTimeline && !mTimeline->IsMonotonicallyIncreasing() &&
PlayState() != AnimationPlayState::Idle);
}

/**
Expand Down
2 changes: 0 additions & 2 deletions dom/animation/TimingParams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ void TimingParams::Normalize() {
mDuration = Some(StickyTimeDuration::FromMilliseconds(
PROGRESS_TIMELINE_DURATION_MILLISEC));
mDelay = TimeDuration::FromMilliseconds(0);
mIterations = std::numeric_limits<double>::infinity();
mDirection = dom::PlaybackDirection::Alternate;

Update();
}
Expand Down
10 changes: 3 additions & 7 deletions gfx/layers/AnimationHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ namespace layers {

static dom::Nullable<TimeDuration> CalculateElapsedTimeForScrollTimeline(
const Maybe<APZSampler::ScrollOffsetAndRange> aScrollMeta,
const ScrollTimelineOptions& aOptions, const Maybe<TimeDuration>& aDuration,
const ScrollTimelineOptions& aOptions, const StickyTimeDuration& aEndTime,
const TimeDuration& aStartTime, float aPlaybackRate) {
MOZ_ASSERT(aDuration);

// We return Nothing If the associated APZ controller is not available
// (because it may be destroyed but this animation is still alive).
if (!aScrollMeta) {
Expand All @@ -59,8 +57,7 @@ static dom::Nullable<TimeDuration> CalculateElapsedTimeForScrollTimeline(
double progress = position / range;
// Just in case to avoid getting a progress more than 100%, for overscrolling.
progress = std::min(progress, 1.0);

auto timelineTime = aDuration->MultDouble(progress);
auto timelineTime = TimeDuration(aEndTime.MultDouble(progress));
return dom::Animation::CurrentTimeFromTimelineTime(timelineTime, aStartTime,
aPlaybackRate);
}
Expand All @@ -82,8 +79,7 @@ static dom::Nullable<TimeDuration> CalculateElapsedTime(
aAPZSampler->GetCurrentScrollOffsetAndRange(
aLayersId, aAnimation.mScrollTimelineOptions.value().source(),
aProofOfMapLock),
aAnimation.mScrollTimelineOptions.value(),
aAnimation.mTiming.Duration(),
aAnimation.mScrollTimelineOptions.value(), aAnimation.mTiming.EndTime(),
aAnimation.mStartTime.refOr(aAnimation.mHoldTime),
aAnimation.mPlaybackRate);
}
Expand Down

0 comments on commit db1b58d

Please sign in to comment.