rdeits / MeshCat.jl

WebGL-based 3D visualizer in Julia
MIT License
234 stars 43 forks source link

Try replacing Mux and WebSockets with pure HTTP.jl #241

Closed rdeits closed 1 year ago

rdeits commented 1 year ago

Fixes #235 (by removing WebSockets.jl entirely).

Fixes #236, I hope (by allowing us to upgrade Blink.jl to v0.12.6). Edit: Blink.jl is gone, so the problem is gone too.

This PR rewrites the core web server to only use HTTP.jl, eliminating the dependencies on Mux.jl and WebSockets.jl. I'm hoping this will avoid the issues we've seen in recent versions of those packages (https://github.com/rdeits/MeshCat.jl/issues/234, https://github.com/rdeits/MeshCat.jl/issues/233), and it should allow us to benefit from the fact that HTTP.jl seems to be much more reliably updated.

We were previously using Mux.jl to handle routing the basic URLs that the meshcat javascript needs, but the HTTP.jl Router class works just as well. Dispatching streams to the HTTP or websocket handler is also pretty easy without using Mux.jl at all. And the actual WebSocket implementation is now provided by HTTP.jl, so we don't need WebSockets.jl at all.

The downside is that we have to upgrade the Julia requirement to 1.8 due to Blink requiring WebIO which requires WebSockets.jl (which we're not actually using) which requires Julia 1.8.2. Edit: Removed Blink.jl in favor of Electron.jl to fix this (fixes #242 ).

rdeits commented 1 year ago

@ferrolho would you mind helping me test this out? I've made sure the very basic stuff like opening (and re-opening) a visualizer works, but I won't have time to do much more in the next day or two.

rdeits commented 1 year ago

Turns out it was easy to eliminate the dependency on Blink.jl entirely, so I did that. We should be able to support Julia 1.6 again.

ferrolho commented 1 year ago

Using the Julia REPL for the commands + Chrome to open the visualiser:

Using the Julia REPL for the commands + Electron to open the visualiser:

I've also tested running stuff from a notebook on Jupyter lab, and it worked well too.

All the tests passed locally when running ] test from the project root. There are only a few deprecation warnings (for which we could open an issue):

┌ Warning: `rotation_between(from::AbstractVector, to::AbstractVector)` is deprecated, use `rotation_between(SVector{3}(from), SVector{3}(to))` instead.
│   caller = intrinsic_transform(g::Cylinder3{Float64}) at geometry.jl:76
└ @ MeshCat ~/git/MeshCat.jl/src/geometry.jl:76
┌ Warning: `rotation_between(from::AbstractVector, to::AbstractVector)` is deprecated, use `rotation_between(SVector{3}(from), SVector{3}(to))` instead.
│   caller = intrinsic_transform(g::Cone{3, Float64}) at geometry.jl:82
└ @ MeshCat ~/git/MeshCat.jl/src/geometry.jl:82
┌ Warning: `rotation_between(from::AbstractVector, to::AbstractVector)` is deprecated, use `rotation_between(SVector{3}(from), SVector{3}(to))` instead.
│   caller = settransform!(vis::ArrowVisualizer{Visualizer}, base::Point3{Int64}, vec::Vec3{Float64}; shaft_radius::Float64, max_head_radius::Float64, max_head_length::Float64) at arrow_visualizer.jl:48
└ @ MeshCat ~/git/MeshCat.jl/src/arrow_visualizer.jl:48
┌ Warning: `rotation_between(from::AbstractVector, to::AbstractVector)` is deprecated, use `rotation_between(SVector{3}(from), SVector{3}(to))` instead.
│   caller = settransform!(vis::ArrowVisualizer{Visualizer}, base::Point3{Int64}, vec::Vec3{Int64}; shaft_radius::Float64, max_head_radius::Float64, max_head_length::Float64) at arrow_visualizer.jl:48
└ @ MeshCat ~/git/MeshCat.jl/src/arrow_visualizer.jl:48

Overall, things seem to work pretty well! 🎉 Here's a screenshot of a relatively busy scene using this MeshCat branch (and a develop version of MeshCatMechanisms), running on macOS with Julia 1.9 and on an Electron window:

image
rdeits commented 1 year ago

Thank you--that's extremely helpful!

I tested this on Windows as well and verified that https://github.com/rdeits/MeshCat.jl/issues/234 is still fixed on this branch. I'll see if I can fix that IOError after closing the electron window and then merge this.