-
Notifications
You must be signed in to change notification settings - Fork 13
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
Improve custom cameras support in MujocoVisualizer
and MujocoVideoRecorder
#206
Improve custom cameras support in MujocoVisualizer
and MujocoVideoRecorder
#206
Conversation
d5744ad
to
07094fd
Compare
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.
Thanks! I left some comments
6b86a15
to
f0e6f0c
Compare
Co-authored-by: Filippo Luca Ferretti <filippo.ferretti@iit.it>
f0e6f0c
to
b25c338
Compare
I spotted a bug in the computation of camera parameters from the desired view. Solved in https://github.com/ami-iit/jaxsim/compare/f0e6f0c1b075e664cfcbc1236729ba6035594d09..b25c33837794a796eda85e1121f6327b379e47c8. |
@@ -5,7 +5,7 @@ dependencies: | |||
# =========================== | |||
# Dependencies from setup.cfg | |||
# =========================== | |||
- python=3.11 | |||
- python >= 3.12.0 |
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.
Why is this necessary?
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.
This was just a maintenance dependecy bump. Since readthedocs built successfully, I thought why not updating to the latest python version. Did you have any reason to stay one version behind?
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.
It's okay to update the default Python version used in the build environment, I was just wondering if it was a constraint for scipy
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.
Nope no reason. I was just passing through the yaml file and I saw this pinning that could have been updated.
MujocoVisualizer
can now be opened with a configuration1 that initializes the camera towards a specific target. Then, the user is free to roam around interactively.jaxsim.mujoco.RodModelToMjcf.convert
function now accepts alsoMujocoCamera
objects of a sequence of such objects, removing the need to serialize them to dictionary. However, for the most generic camera support, the method still accepts a plain dictionary or list of dictionaries.track
camera of theMujocoVideoRecorder
is now more explicit as it is defined as default argument.I found interesting the approach used to specify the initial viewpoint of the interactive viewer. It seemed to me much easier to setup than the parameters necessary to create a custom camera. We might think to understand what is the mapping logic between them, and provide a newMujocoCamera.build_from_target_view
helper.I eventually found the time to also implement this approach. Now having the recorder taking exactly the same image that is shown by default in the interactive viewer should be straightforward. Furthermore, we can now use the interactive viewer to find the desired view, and then use the resulting parameters in the recorder.
📚 Documentation preview 📚: https://jaxsim--206.org.readthedocs.build//206/
Footnotes
https://mujoco.readthedocs.io/en/stable/XMLreference.html#visual-global ↩
https://mujoco.readthedocs.io/en/stable/XMLreference.html#body-camera ↩