Add mjviser web viewer backend (--viewer=viser).#1266
Add mjviser web viewer backend (--viewer=viser).#1266kevinzakka merged 2 commits intogoogle-deepmind:mainfrom
Conversation
erikfrey
left a comment
There was a problem hiding this comment.
Very cool! Would love to get this in, mostly nits below.
mujoco_warp/viewer.py
Outdated
| try: | ||
| from mjviser import Viewer as MjViserViewer | ||
| except ImportError: | ||
| raise SystemExit("mjviser required for --viewer=viser: pip install mjviser") |
There was a problem hiding this comment.
Is mjvser dep heavy? Should we add it to dev dependencies?
Lines 50 to 59 in 368bdca
There was a problem hiding this comment.
It's not, added it to dev.
mujoco_warp/viewer.py
Outdated
|
|
||
|
|
||
| def _make_warp_step_fn(m, d, graph, ctrls=None): | ||
| ctrlid = [0] |
There was a problem hiding this comment.
haha, it took me a moment to understand why you were creating a list here!
I think this is what nonlocal is for.
so:
ctrlid = 0
def step_fn(mjm, mjd):
nonlocal ctrlid
...
mujoco_warp/viewer.py
Outdated
|
|
||
| def _make_c_step_fn(ctrls=None): | ||
| if ctrls is None: | ||
| return None |
There was a problem hiding this comment.
OOC why does this return none?
mujoco_warp/viewer.py
Outdated
| if _ENGINE.value == EngineOptions.WARP: | ||
| step_fn = _make_warp_step_fn(m, d, graph, ctrls) | ||
| else: | ||
| step_fn = _make_c_step_fn(ctrls) |
There was a problem hiding this comment.
it looks like you've gone through some effort to abstract out the step function logic which is nice
can't we reuse it with the passive viewer below?
ideally we could have something like:
step_fn = foo
viewer_fn = bar
viewer_fn(step_fn)|
Thanks @erikfrey, addressed all your review feedback. |
Adds support for mjviser as an alternative viewer backend via --viewer=viser. mjviser is a web-based MuJoCo visualizer built on viser, useful when running on headless machines or when you want browser-based visualization.
Did not add mjviser as an explicit dependency so it should be run as follows: