Skip to content

fix(scene): give the first frame of an animation a real dt - #4912

Closed
msdh wants to merge 1 commit into
ManimCommunity:mainfrom
msdh:fix/first-frame-dt
Closed

fix(scene): give the first frame of an animation a real dt#4912
msdh wants to merge 1 commit into
ManimCommunity:mainfrom
msdh:fix/first-frame-dt

Conversation

@msdh

@msdh msdh commented Aug 4, 2026

Copy link
Copy Markdown

Overview: What does this pull request change?

Closes #3005. Closes #4611.

Scene.compile_animation_data() resets self.last_t = 0 before every animation, and get_time_progression() starts the frame times at 0 as well, so update_to_time() works out dt = 0 for the first frame of every play() and wait(). This sets last_t to minus one frame period instead. The very first frame of a scene is left alone, since nothing precedes it. That is the fix @behackl described in #3005.

Motivation and Explanation: Why and how do your changes improve the library?

Two symptoms, one cause.

A dt-based updater holds still for one frame at the start of every animation. #4611 is a clean demo of it: an updater that shifts by 2 * dt * RIGHT gives a duplicated frame at the start of each of the ten animations in its loop, and at 5 fps that is a fifth of a second of nothing, ten times over.

And the dt an updater accumulates over an animation comes out at run_time - 1/frame_rate, so anything integrating dt runs behind — and the error grows with the number of play() calls instead of staying put. Measured on b3560c5 at 15 fps:

scene clock dt the updater received
after 1 play of 0.5 s 0.5000 0.4667
after 2 1.0000 0.9333
after 4 2.0000 1.8667
after 8 4.0000 3.7333

That shape is what makes it awkward to catch: it is one frame at the start of a render and a visible offset by the end. I ran into it with an accumulator driven off dt that kept drifting behind self.renderer.time. Reading the renderer's clock directly is the workaround, but an updater's own dt ought to add up.

With the patch, at 15 fps and again at 60:

anim 0 (the scene's first): first dt 0.00000   sum 0.933333
anim 1:                     first dt 0.06667   sum 1.000000
anim 2:                     first dt 0.06667   sum 1.000000
wait(2.0):                                     sum 2.000000

Links to added or changed documentation pages

None.

Further Information and Comments

The scene's opening frame stays at dt = 0 on purpose. Nothing comes before it, and leaving it alone means no existing scene's first frame moves.

control_data/speed/SpeedModifier.npz needed regenerating. That test rotates a dot with ChangeSpeed.add_updater(c, lambda x, dt: x.rotate_about_origin(PI / 3.7 * dt)) across three play() calls, so the dot now sits two frames further round than the stored frames have it. I diffed the old data against the new rather than just trusting --set_test: 21 of the 31 frames change, by 52 to 57 pixels each out of 102,480 (about 0.05%), and in every one of them the changed pixels sit inside a ~14x13 px box that follows the dot along its arc. Frames 0-9 are the scene's first animation and are untouched, which is the exception above doing its job.

Nothing else in the suite moves. I ran tests/ (minus the OpenGL directories) on b3560c5 and on this branch on the same machine: 779 passed / 38 failed both times, and the same 38 names both times — typst, the CLI subcommands, and vp9, none of which work on this box.

Two tests added in tests/test_scene_rendering/test_play_logic.py. One is parametrised over 15/30/60 fps and asserts that every rendered frame of an animation carries exactly one frame period and that the dts sum to the run_time; it fails on main at all three rates, one mismatched element per animation. The other pins the opening frame at zero so the exception does not get tidied away later.

compile_animation_data() reset Scene.last_t to 0 before every animation
and the frame times start at 0 as well, so update_to_time() worked out
dt = 0 for the first frame of every play() and wait(). Every dt-based
updater held still for that frame, and the dt an updater accumulated
over an animation came out at run_time - 1/frame_rate, an error that
grew with the number of calls rather than staying constant.

Set last_t to minus one frame period instead. The very first frame of a
scene keeps dt = 0, since no frame precedes it.

control_data/speed/SpeedModifier.npz is regenerated: its dot is rotated
by a dt updater across three play() calls, so it now lands two frames
further round.

Closes ManimCommunity#3005
Closes ManimCommunity#4611
@msdh

msdh commented Aug 6, 2026

Copy link
Copy Markdown
Author

Worth recording a limit of this patch that I measured but didn't put in the description.

A skipped animation gets a single update_to_time(run_time) call rather than a frame sequence, so with last_t = -1/frame_rate it now advances dt-updaters by run_time + 1/frame_rate. Rendering with -n 2 at 15 fps:

animation before after
0 — skipped, scene's first 1.000000 1.000000
1 — skipped 1.000000 1.066667
2, 3 — rendered 0.933333 1.000000

The gap between a skipped and a rendered animation is 1/frame_rate before this patch and 1/frame_rate after it, so a cached re-render diverges from a fresh one by exactly as much as it always did. The rendered path is exact now; the skipped one still isn't.

I left it alone deliberately rather than also guarding on renderer.skip_animations. That guard would catch -n, but a cache hit sets skip_animations after compile_animation_data has run, so cached animations would keep the offset while -n lost it — two skip paths disagreeing, which seems worse than one consistent offset. Happy to take it further if you'd rather it were handled here.

@behackl

behackl commented Aug 7, 2026

Copy link
Copy Markdown
Member

Hi! Indeed, there is a good reason why I never implemented the solution suggested in #3005 -- as of right now, the way how global scene time is handled is still quite fragile. There is a first step (#4916) towards changing the ownership model of the rendering process a bit to introduce, down the line, a more stable scene time. I'd honestly rather wait until a bit more progress has happened on that front.

As a bandaid fix, the proposed solution is okay for individual scenes (and where you don't necessarily care about working caching / consistent scene times), but I don't really want to include this patch with the weird negative time solution as is.

Thanks, regardless, for your (attempted) contribution!

@behackl behackl closed this Aug 7, 2026
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.

Extra frame of animation with dt-updater 0 dt value passed into updater between animations/waits

2 participants