Skip to content

Add camera pose editors plugins#1397

Open
fracapuano wants to merge 6 commits intohaosulab:mainfrom
fracapuano:main
Open

Add camera pose editors plugins#1397
fracapuano wants to merge 6 commits intohaosulab:mainfrom
fracapuano:main

Conversation

@fracapuano
Copy link
Copy Markdown

Adds an interactive camera-pose editor directly in the renderer, so that users can modify (and read!) the pose of different cameras more easily. Demo 👇

0317

Run the same command with:

 python3 -m mani_skill.examples.demo_random_action --render-mode human --pause --edit-camera-poses

As an aside: wow, Codex (@Vaibhavs10, 🥹)---simply wow.

Copy link
Copy Markdown
Member

@StoneT2000 StoneT2000 left a comment

Choose a reason for hiding this comment

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

Incredible work! This has been a requested feature in the past by some people, glad you made a great implementation for it! See my comments



_VIEWER_GL_POSE = sapien.Pose([0, 0, 0], [-0.5, -0.5, 0.5, 0.5])
_CAMERA_LINESET_VERTICES = [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can the _CAMERA_LINESET_VERTICES variable be made a single line (and add a comment to ignore lint checks for that line)

@@ -0,0 +1,411 @@
from __future__ import annotations

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice use of the sapien plugin class.

I think this won't be the only non sapien provided plugin we may wish to provide given there could be some other useful features (e.g. reward tracking à la mujoco/mjx tooling).

Moreover we kind of shoehorned some other viewer related code into this folder https://github.com/haosulab/ManiSkill/tree/main/mani_skill/render which I never really liked.

Thus, I think we should make a new folder mani_skill/viewer and then another folder mani_skill/viewer/plugins and add this plugin here.

pause: Annotated[bool, tyro.conf.arg(aliases=["-p"])] = False
"""If using human render mode, auto pauses the simulation upon loading"""

edit_camera_poses: bool = False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The demo random action file should not be demoing specific plugins like camera editing. I recommend making a new function to demonstrate how to add plugins to a viewer by a user in code examples/demo_plugins.py. Let user specify in the CLI which plugins to load, and print a message to the user to tell them to read the demo's code for how to use non-default plugins

We can include some widely used plugins in the future as part of the default set of plugins added to sapien in maniskill (and this could be one of them, at the moment I'd hold off from making it a default).

If True, the environment will reset the episode RNG upon each reset regardless of whether a seed is provided.
Generally enhanced_determinisim is not needed and users are recommended to pass seeds into the env reset function instead.

enable_camera_pose_editing (bool): If True, the human viewer exposes an in-window camera editor that lets you click camera frustums
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

given I think we should make new plugins a bit more standardized, we should not add specific viewer plugin related code here like one just for camera editing.

Instead, we have two (not mutually exclusive) options

  • Add a function attach_viewer_plugin that accepts a created plugin object or maybe the plugin class and adds it to the viewer once viewer is created (we will need to maintain a list of plugins).
  • In the BaseEnv init args, add a argument plugins and you can pass in a list of plugins that we add upon viewer creation

@fracapuano
Copy link
Copy Markdown
Author

(sorry for the gymnasium confusion, pushed to the wrong branch!)

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