Skip to content

Fix CSS step timing functions jump-both, jump-start, start at timestamp 0; Early return when possible#354

Open
yezhizhen wants to merge 3 commits intomainfrom
fix-quantization
Open

Fix CSS step timing functions jump-both, jump-start, start at timestamp 0; Early return when possible#354
yezhizhen wants to merge 3 commits intomainfrom
fix-quantization

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Apr 20, 2026

Make early return happen earlier:

  1. total_progress ≥ 1.0 and total_progress < 0 now handled immediately. Also make it direction aware.
  2. At timestamp 0, for normal direction, non‑jump step functions (jump‑end, jump‑none, end) now return the start‑keyframe value early, instead of proceeding to interpolation.
  3. Zero-interval: we exit early to avoid division‑by‑zero in percentage_between_keyframes

Try
Servo PR: servo/servo#44365

@yezhizhen
Copy link
Copy Markdown
Member Author

@Loirooriol Just friendly reminder.

Comment thread style/servo/animation.rs Outdated

let prev_keyframe = &self.computed_steps[prev_keyframe_index];
let Some(next_keyframe_index) = next_keyframe_index else {
debug_assert!(false, "next_keyframe_index should always be Some");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using debug_unreachable may be clearer.
Also the message isn't providing much, it should rather explain why.

Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2701332

The explanation is a bit lengthy tho. We are relying on

debug_assert!(intermediate_steps.first().unwrap().start_percentage == 0.);
debug_assert!(intermediate_steps.last().unwrap().start_percentage == 1.);

Comment thread style/servo/animation.rs Outdated
// Progress clamped to the current iteration (0.0 to 1.0).
let total_progress = progress.min(self.current_iteration_end_progress()).max(0.0);

if total_progress >= 1.0 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can this handle the equality, but above you check progress < 0.0?

Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 2701332 I updated to ==1.0 as it is always in [0,1].

but above you check progress < 0.0?

At progress == 0.0, we don't want to early return because we need to reach the step‑function handling block at L878 we added.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But don't we have the same jumping problem at the end?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

servo/servo#43086

End has always been fine. We only have problem at the beginning.

@yezhizhen yezhizhen marked this pull request as draft May 1, 2026 06:01
@yezhizhen yezhizhen force-pushed the fix-quantization branch from 398c241 to 1d1f9ce Compare May 1, 2026 06:32
@yezhizhen yezhizhen marked this pull request as ready for review May 1, 2026 06:55
yezhizhen added 3 commits May 1, 2026 16:36
…rst keyframe to be selected at progress 0; changed to < for normal direction (kept <= for reverse to avoid #342 (comment)) and added direction‑aware step‑function handling.

- Guard against division‑by‑zero for zero‑length keyframe intervals.

1. total_progress ≥ 1.0 and total_progress < 0 now handled immediately. Also make it direction aware.
2. At timestamp 0, for normal direction, non‑jump step functions (jump‑end, jump‑none, end) now return the start‑keyframe value early, instead of proceeding to interpolation.
3. Zero-interval: we exit early to avoid division‑by‑zero in `percentage_between_keyframes`

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen yezhizhen force-pushed the fix-quantization branch from dbc9ddd to 0cf455c Compare May 1, 2026 08:36
@yezhizhen yezhizhen requested a review from Loirooriol May 1, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants