-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
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: Screen.Recording.2024-11-04.at.08.51.02.mp4 |
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 |
There was a problem hiding this comment.
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. 👍
@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
AnimationContext
s to theCoreVisualizer
. That stack is empty by default, butatframe()
pushes a new(animation, frame)
context onto the stack before calling the user's provided function. This causes anysetprop!
orsettransform!
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 justAnimation()
. 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 theAnimation
didn't know whatCoreVisualizer
it pertained to, so we allowed merging and mixing animations from differentCoreVisualizer
s, which doesn't make any sense.Would you mind trying this out and seeing if it resolves the MechCatMechanisms issues you were seeing?