Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop Cassette.jl by rewriting animation handlers #257

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Conversation

rdeits
Copy link
Owner

@rdeits rdeits commented Nov 4, 2024

@ferrolho I was inspired by your question from #255 (comment) to try to get rid of Cassette. It's a super cool tool, but probably not worth the complexity for our very minor usage.

This PR is the best I could come up with today as a way to (mostly) preserve the animation system without using Cassette. It adds a stack of AnimationContexts to the CoreVisualizer. That stack is empty by default, but atframe() pushes a new (animation, frame) context onto the stack before calling the user's provided function. This causes any setprop! or settransform! calls to be redirected to that animation, which is exactly what we had before with Cassette.

Unfortunately, I do need to make a breaking change to the API: You now have to do Animation(vis) instead of just Animation(). That''ll necessitate a major version increment, but I don't really have a problem with doing that. It's also probably a good idea anyway -- previously the Animation didn't know what CoreVisualizer it pertained to, so we allowed merging and mixing animations from different CoreVisualizers, which doesn't make any sense.

Would you mind trying this out and seeing if it resolves the MechCatMechanisms issues you were seeing?

Project.toml Show resolved Hide resolved
@ferrolho
Copy link
Collaborator

ferrolho commented Nov 4, 2024

I have just tried it out and it is working great with Julia 1.11! It does solve the issue that I found in #255 (comment).

I actually found that somehow as a byproduct of dropping cassette, the time it takes to render long animations with lots of entities has decreased massively. For example, the method that prepares the animation below used to take what probably felt like a good 10 seconds before the animation would start playing; but after these changes, the animation is prepared and starts playing almost instantaneously! 🚀

Edit: 29.94 seconds before vs 0.57 seconds after. 😆

Screen.Recording.2024-11-04.at.08.51.02.mp4

Comment on lines 77 to 85
function atframe(f, animation::Animation, frame::Integer)
Cassette.overdub(AnimationCtx(metadata=(animation, frame)), f)
push!(animation.visualizer.animation_contexts, AnimationContext(animation, frame))
try
f()
finally
pop!(animation.visualizer.animation_contexts)
end
return animation
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I finally understood how animations are implemented in MeshCat.jl thanks to these changes.

There is a stack of objects called animation contexts, which are a kind of "helper" object to keep track of which transformations and properties are applied on different objects at different frames. This works well because the methods that apply the transformations and properties are first checking for the existence of a context before applying said transformation or property to the object. I.e., if there is no animation context, we apply the transforms directly to the objects in the visualisation; otherwise, if there is an animation context, it must be because the settransform, etc. methods are being called within an atframe call, and so we apply the transforms to the last animation context in the stack instead of to the visualiser directly.

Initially, I was confused with the pop! instruction of the context. I now understand that we push! it temporarily, then apply the "state of the world" as specified in an atframe call, and finally we pop! it back so that future settransform calls are applied directly to the visualiser again rather than to some animation context.

I suppose that there are other ways of achieving this without the need for pushing and popping contexts to a stack, but I guess that doesn't really matter as this is working really well and is a nice way of implementing the logic for conditionally applying settransform! and setprop! to either an animation context or directly to the visualiser depending on whether there is a context in the stack. 👍

@rdeits rdeits marked this pull request as ready for review November 8, 2024 01:27
@rdeits rdeits merged commit ee42997 into master Nov 8, 2024
10 checks passed
@ferrolho ferrolho deleted the rd/drop-cassette branch November 8, 2024 08:14
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