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

RFC: TreeViewer ZeroMQ interface #593

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

rdeits
Copy link
Contributor

@rdeits rdeits commented Feb 6, 2018

Fixes #586

This is a first draft of a ZeroMQ interface for the RemoteTreeViewer in drake-visualizer. The advantages of the new interface are:

  1. Reliable communication: ZeroMQ lets clients reliably connect to the visualizer and send draw commands with some assurance that those commands are actually being received.
  2. Better performance: ZeroMQ lets us avoid LCM message size limits, and MsgPack lets us pack large arrays compactly, so we can now visualize huge pointclouds and meshes (10,000,000 points seems to work quite nicely now, and larger pointclouds work up until my machine runs out of RAM rendering them)

The new ZeroMQ interface is so much better than the LCM protocol was that I'm probably going to deprecate the LCM Tree Viewer entirely. That will mean that the Tree Viewer can be run just from the default Director build (assuming users have the ZeroMQ and MsgPack Python libraries installed). That also frees DrakeVisualizer.jl from needing to build or supply LCM, which will make my life easier.

The major downside is that I'm introducing two new dependencies: ZeroMQ and MsgPack. Both dependencies are runtime-only (they just need to be importable from python, and only if you're trying to use the new interface) and both are available from apt and brew, so I'm not terribly concerned, but I'm still always wary of creating new dependencies in a project. The ZeroMQ dependency is going to be hard to get around, but MsgPack is actually pretty lightweight so we have some options:

  1. Use the systemwide or pip-installed msgpack python library
  2. Use the systemwide or pip-installed umsgpack python library, which has the advantage of being pure-python
  3. Bundle umsgpack (which is a single 1000 line Python file and MIT-licensed) inside Director (what I'm doing now)
  4. Use ProtoBuf instead, and just distribute the generated decoder file with Director

@patmarion any opinions? I'm personally leaning towards option (3), since it means only one external dependency (zeromq). ProtoBuf has a lot of potential, since the .proto file would nicely document the interface, but I've found it surprisingly annoying to actually work with.

@gizatt if you have any thoughts about things you'd like to see in the interface, I'd be happy to make changes.

Also cc @tkoolen and @dehann since this will (at least indirectly) affect you. DrakeVisualizer.jl should continue to "just work" for the most part, but with benefits to reliability and performance, especially when rendering meshes.

Internal Changes

I also made some internal changes while I was working on this:

  • Created the ZMQTreeViewer and LCMTreeViewer classes, and made them separate components in the mainwindowapp
  • Added command-line switches to turn on and off the DrakeVisualizer LCM, TreeViewer LCM, TreeViewerZMQ, and LCMGL renderers
  • Changed drake-visualizer to not steal focus when launching (I often launch it while running other code and end up typing into the visualizer window when it steals focus)

TODOs:

  • decide on the final protocol
  • figure out how to clean up all these interfaces
    • There are now 4 different ways to draw stuff in the drake-visualizer app remotely, and they all use different protocols, dependencies, and flags. That seems like far too many. I'm planning on only using the new TreeViewer ZMQ protocol, and I've talked to @gizatt about upgrading Spartan to use that too. Drake still uses the DrakeVisualizer LCM protocol, and there's no active plan to change that. But I think we can easily get rid of TreeViewer LCM at the very least.

import zmq
HAVE_ZMQ = True
except ImportError:
HAVE_MSGPACK_ZMQ = False
Copy link
Member

Choose a reason for hiding this comment

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

should be HAVE_ZMQ?

@patmarion
Copy link
Member

very cool.

I am fine with adding zeromq as an optional dependency, no problem. I can't have it as a required dependency, but I don't think you are proposing that. I am fine with adding the pip install zmq command to the install_deps.sh so that zeromq is tested on travis-ci's mac and ubuntu builds. I agree with adding the msgpack thirdparty library in source is a good option, especially since we already have examples of this with several other thirdparty modules, as long as they are license friendly.

You can add the pip install command for ubuntu and mac in this file (or apt/brew, whatever you think makes the most sense):

https://github.com/RobotLocomotion/director/blob/master/distro/travis/install_deps.sh

@patmarion
Copy link
Member

let me know how you'd like to proceed on this PR. are you planning any more changes before code review? i think with the dependencies installed this could pass the travis-ci tests.

@rdeits
Copy link
Contributor Author

rdeits commented Feb 15, 2018

I think this is in pretty good shape, but I got myself sidetracked trying to support textures and then ended up writing a whole new viewer in Three.js so I can have fancy 3D models in my Jupyter notebooks.

I'll update the deps on Travis and let you know when it's ready.

@rdeits
Copy link
Contributor Author

rdeits commented Feb 16, 2018

Okay, CI is passing and I'm reasonably convinced that this is a positive change, regardless of what develops with the new Three.js visualizer.

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