Commit 0ce761a4 authored by Kevin Ellis's avatar Kevin Ellis Committed by Chromium LUCI CQ

Fix tests for numerical precision problems

Animation times were change from Optional<double> to
Optional<AnimationTimeDelta> in the following CL:

https://chromium-review.googlesource.com/c/chromium/src/+/2591488

This change eroded the numerical precision of calculations involving
time; however, we only require a time precision down to 1 microsecond
(https://drafts.csswg.org/web-animations/#precision-of-time-values).

Thus, it is safe to relax fuzzy equality constraints at the upper bounds
of an animation.

This patch, also addresses a previous inaccuracy where it was possible
to recognize that an animation was limited (i.e. finished) yet not set
the hold time when updating the finished state.


Bug: 1169674, 1169111
Change-Id: I0d7869cac8809eb118b09b740a35a0fee4655303
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2644740
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: default avatarXida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846370}
parent a36f19ef
......@@ -160,6 +160,16 @@ AtomicString GetCSSTransitionCSSPropertyName(const Animation* animation) {
->TransitionCSSPropertyName()
.ToAtomicString();
}
bool GreaterThanOrEqualWithinTimeTolerance(const AnimationTimeDelta& a,
const AnimationTimeDelta& b) {
double a_ms = a.InMillisecondsF();
double b_ms = b.InMillisecondsF();
if (std::abs(a_ms - b_ms) < Animation::kTimeToleranceMs)
return true;
return a_ms > b_ms;
}
} // namespace
Animation* Animation::Create(AnimationEffect* effect,
......@@ -287,9 +297,8 @@ bool Animation::Limited(base::Optional<AnimationTimeDelta> current_time) const {
return (EffectivePlaybackRate() < 0 &&
current_time <= AnimationTimeDelta()) ||
(EffectivePlaybackRate() > 0 &&
GreaterThanOrEqualToWithinEpsilon(
current_time.value().InMillisecondsF(),
EffectEnd().InMillisecondsF()));
GreaterThanOrEqualWithinTimeTolerance(current_time.value(),
EffectEnd()));
}
Document* Animation::GetDocument() const {
......@@ -1497,12 +1506,14 @@ void Animation::UpdateFinishedState(UpdateType update_type,
double playback_rate = EffectivePlaybackRate();
base::Optional<AnimationTimeDelta> hold_time;
TimelinePhase hold_phase;
if (playback_rate > 0 &&
unconstrained_current_time.value() >= EffectEnd()) {
GreaterThanOrEqualWithinTimeTolerance(
unconstrained_current_time.value(), EffectEnd())) {
if (did_seek) {
hold_time = unconstrained_current_time;
} else {
if (previous_current_time_ >= EffectEnd()) {
if (previous_current_time_ > EffectEnd()) {
hold_time = previous_current_time_;
} else {
hold_time = EffectEnd();
......
......@@ -99,6 +99,10 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
// relative position.
enum CompareAnimationsOrdering { kTreeOrder, kPointerOrder };
// Only expect timing accuracy to within 1 microsecond.
// drafts.csswg.org/web-animations/#precision-of-time-values.
static constexpr double kTimeToleranceMs = 0.001;
static Animation* Create(AnimationEffect*,
AnimationTimeline*,
ExceptionState& = ASSERT_NO_EXCEPTION);
......
......@@ -53,6 +53,10 @@ namespace {
base::TimeTicks TimeTicksFromMillisecondsD(double seconds) {
return base::TimeTicks() + base::TimeDelta::FromMillisecondsD(seconds);
}
#define EXPECT_TIME_NEAR(expected, value) \
EXPECT_NEAR((expected).InMillisecondsF(), (value).InMillisecondsF(), \
Animation::kTimeToleranceMs)
} // namespace
namespace blink {
......@@ -463,9 +467,9 @@ TEST_F(AnimationDocumentTimelineRealTimeTest,
DocumentTimeline* timeline =
MakeGarbageCollected<DocumentTimeline>(document.Get(), origin_time);
timeline->SetPlaybackRate(0.5);
EXPECT_DOUBLE_EQ((AnimationTimeDelta(origin_time) * 2).InMillisecondsF(),
(timeline->ZeroTime() - document->Timeline().ZeroTime())
.InMillisecondsF());
EXPECT_TIME_NEAR(AnimationTimeDelta(origin_time) * 2,
timeline->ZeroTime() - document->Timeline().ZeroTime());
}
} // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment