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

Remove WebIO and build directly on Mux.jl and HTTP.jl #141

Merged
merged 8 commits into from
Feb 12, 2020
Merged

Conversation

rdeits
Copy link
Owner

@rdeits rdeits commented Feb 10, 2020

I've been considering this for a whlie, and I think it's time to do it. MeshCat doesn't actually gain much from being built on top of WebIO, and there have been a fair number of drawbacks, mostly just the churn of having a pretty complicated dependency in the stack. This PR removes the WebIO layer and re-implements the meshcat protocol directly with Mux.jl, HTTP.jl, and WebSockets.jl. Those are the same packages that power WebIO under the hood, so nothing is changing except the way we interact with those tools. We even get to maintain compatibility with WebIO (by implementing WebIO.render with Requires.jl), so integration with other WebIO tools should keep working.

What works:

  • Creating and opening visualizers in the browser
  • IJulia cell integration
  • Blink window integration
  • Juno plot pane integration
  • VSCode plot pane integration
  • ElectronDisplay.jl integration
  • animations
  • WebIO.render() integration (now handled via a plain iframe and lazily loaded via Requires.jl)

What doesn't work:

  • Serving visualizers from remote jupyter servers (e.g. NextJournal). This was the primary motivation for the switch to WebIO, but I've never actually used it successfully. I don't think this hypothetical feature is worth all the complexity it requires. And in fact, I have an idea for how we can make this work anyway: rather than creating a live connection from the remote server to the client, we can create a new function which sends a fully static rendering of the entire scene, including any animations. That won't deliver the full MeshCat experience, but it should be pretty stable and cover most cases.
  • The whole AbstractControls infrastructure. This never worked anyway, so I'm removing it.

Pros:

Cons:

  • Slightly more boilerplate for IJulia and Blink? Edit: not really
  • Probably won't work in Juno without some additional integration. Edit: this works now
  • Might require some adjustment to work with the RigidBodySim GUI stuff

@codecov-io
Copy link

codecov-io commented Feb 10, 2020

Codecov Report

Merging #141 into master will increase coverage by 2.12%.
The diff coverage is 55.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage   79.27%   81.39%   +2.12%     
==========================================
  Files          15       18       +3     
  Lines         468      473       +5     
==========================================
+ Hits          371      385      +14     
+ Misses         97       88       -9
Impacted Files Coverage Δ
src/lowering.jl 98.48% <ø> (+8.2%) ⬆️
src/commands.jl 83.33% <ø> (+11.9%) ⬆️
src/assets.jl 0% <0%> (ø)
src/servers.jl 4% <0%> (-0.55%) ⬇️
src/integrations.jl 100% <100%> (ø)
src/MeshCat.jl 80% <100%> (+46.66%) ⬆️
src/render.jl 33.33% <33.33%> (ø)
src/visualizer.jl 91.42% <90.69%> (+15.37%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b4d0be1...d5d75bf. Read the comment docs.

@rdeits
Copy link
Owner Author

rdeits commented Feb 11, 2020

@travigd I wanted to let you know about this. Basically, I reimplemented the core MeshCat interface directly in Mux/HTTP/WebSockets, rather than using WebIO to handle communications and asset serving. This helped with comms performance (by skipping the JSON encode) and overall reduced the complexity of the system a bit. Integration with other WebIO stuff should keep working, since MeshCat now defines WebIO.render via Requires.jl. I suspect this will be a better way for these tools to coexist, and it means that MeshCat should be much less sensitive to internal changes in WebIO. Good news all around 🙂

@rdeits
Copy link
Owner Author

rdeits commented Feb 11, 2020

@tkoolen this might slightly break the RigidBodySim GUI controls, but I think it will be easy to fix. It also might just work as-is, since I'm still defining WebIO.render. Anyway, I'll make sure this is a new minor version, so we can update RigidBodySim later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants