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

Switch to using MeshCat.MeshFileGeometry and MechanismGeometries.MeshFile. #32

Merged
merged 5 commits into from
Jul 12, 2019

Conversation

tkoolen
Copy link
Contributor

@tkoolen tkoolen commented May 15, 2019

On top of #31.

Part of the plan from rdeits/MeshCat.jl#92 (comment). Ref JuliaRobotics/MechanismGeometries.jl#17.

Requires version tags for MeshCat, ValkyrieRobot, and MechanismGeometries.

Seems to be working though!

image

It does seem to be a lower-poly model than before though... Is there a way we can control that?

Also turned some really long short-form functions into long-form functions to improve readability; sorry, should have split that out.

  • update lower bounds in Project.toml once tags are in.

@tkoolen
Copy link
Contributor Author

tkoolen commented May 15, 2019

I tried this with https://github.com/tkoolen/universal-robot-dae-jl.

For tag="collision" (STL files), everything works perfectly. However for the tag="visual" DAE files, only the base shows for some reason. If I change all the links in the URDF to point to the base DAE file, then I do get multiple base links at different locations. I thought it might be a file size issue, as the base file is smaller than most, but the wrist3 file is even smaller so maybe that's not it. Any ideas?

@rdeits
Copy link
Collaborator

rdeits commented May 15, 2019

Hm, interesting. Does anything show up in the browser's console when you try to load the bad meshes? I can also look into this after work tonight.

@tkoolen
Copy link
Contributor Author

tkoolen commented May 15, 2019

Looks like there are indeed some issues:

Screen Shot 2019-05-15 at 9 30 32 AM

@tkoolen
Copy link
Contributor Author

tkoolen commented May 17, 2019

Chrome on Linux seems to give a more helpful error message:

Uncaught TypeError: Cannot set property 'uuid' of undefined
    at handle_special_geometry (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at ExtensibleObjectLoader.delegate (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at ExtensibleObjectLoader.parseGeometries (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at ExtensibleObjectLoader.parse (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:3)
    at Viewer.set_object_from_json (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at Viewer.handle_command (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at Viewer.handle_command_message (73edcf236a5dc82f052d45ffa75c1c3d7774fe94-main.min.js:37)
    at Object.eval (eval at <anonymous> (bundle.js:3059), <anonymous>:2:353)
    at Object.dispatch (bundle.js:177)
    at WebSocket.ws.onmessage (mux_setup.js:25)

image

@tkoolen
Copy link
Contributor Author

tkoolen commented May 17, 2019

@tkoolen
Copy link
Contributor Author

tkoolen commented May 17, 2019

I think I'm starting to understand what's going on. The meshcat code assumes that obj.scene.children[0] is the object we care about, but for these meshes it's just some random camera:

image

Indeed, for the .dae that does render, there's only one child.

@rdeits
Copy link
Collaborator

rdeits commented May 19, 2019

Oh, wow, good catch. Hmm, I'm not sure what the right way to hand this is--general mesh loading is weirdly complicated precisely because each file tries to be an entire scene rather than just a single geometry. I think I just need to adjust the loader in meshcat to try to add all of the geometries in the file.

@tkoolen
Copy link
Contributor Author

tkoolen commented May 19, 2019

Yeah, that sounds good. Most likely all the lamps and cameras left in the .dae file for a robot part aren't really meant to be in there anyway.

@rdeits
Copy link
Collaborator

rdeits commented May 19, 2019

Oh man, this is compilcated. I think I need to re-think the way mesh files are handled--it's currently assumed internally that each mesh file produces some kind of three.js Geometry, but a .DAE file is really more like its own entire scene, which we'd need to represent as a three.js Group. A Group is an Object (a class containing its own geometries and materials), not a Geometry in three.js-land, so this will take some doing.

A related question is: what exactly do we do with the materials inside a mesh file? It seems reasonable to provide an STL mesh geometry and a separate material specification, but a DAE file includes all of its material specifications internally. Should we let a user provide a material in that case? And what object would we apply that material to?

I think the most sensible answer is probably to change MeshFileGeometry <: MeshCat.AbstractGeometry into MeshFileObject <: MeshCat.AbstractObject and then load it on the javascript side as a Group (with potentially many geometries and materials). That means we'd be taking whatever material is specified in the file, without letting the user override that material in Julia.

Then, this gets even more complicated because OBJ files store their material as a reference to an .mtl file, so having the full file contents isn't sufficient to render the mesh and its materials.

@rdeits
Copy link
Collaborator

rdeits commented May 19, 2019

Ok, with meshcat-dev/meshcat#51 and this PR and all its friends, the UR10 now loads properly:

meshcat

It looks a little goofy, though, because the DAE files include their own light sources for no apparent reason. We can either handle that by stripping them out in meshcat (on the javascript side), or just declare that we're doing the right thing and the mesh itself is silly.

@rdeits rdeits closed this Jul 12, 2019
@rdeits rdeits reopened this Jul 12, 2019
@rdeits
Copy link
Collaborator

rdeits commented Jul 12, 2019

whoops, still need a tag for MechanismGeometries 0.4.0: JuliaRegistries/General#1966

@codecov-io
Copy link

codecov-io commented Jul 12, 2019

Codecov Report

Merging #32 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
+ Coverage   96.69%   96.72%   +0.02%     
==========================================
  Files           5        5              
  Lines         121      122       +1     
==========================================
+ Hits          117      118       +1     
  Misses          4        4
Impacted Files Coverage Δ
src/MeshCatMechanisms.jl 100% <ø> (ø) ⬆️
src/visualizer.jl 94.11% <100%> (+0.11%) ⬆️

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 dfbbc2d...fa1ef92. Read the comment docs.

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 12, 2019

OK, build is passing. I'm going to quickly merge and tag a new version, since the latest release is now incompatible with the new MechanismGeometries version.

@tkoolen tkoolen merged commit c06238b into master Jul 12, 2019
@tkoolen tkoolen deleted the tk/meshfile branch July 12, 2019 12:35
@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 12, 2019

Hmm, so with the new versions of everything I tried the UR10 again (https://github.com/tkoolen/universal-robot-dae-jl), and ran into #32 (comment) again.

We're currently creating MeshFileGeometrys in this package. So we end up over here in meshcat:

https://github.com/rdeits/meshcat/blob/3122cecd5da022ad96bb0c8dc0f811a1bc492350/src/index.js#L63-L66

But the changes to those lines in meshcat-dev/meshcat#51 are not present in master.

Should we be creating MeshFileObjects here instead? Or does the handling of meshfile_geometry need to be fixed in meshcat?

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 12, 2019

With https://github.com/JuliaRobotics/MeshCatMechanisms.jl/compare/tk/meshfileobject-hack?expand=1, I the .daes at least load without error, but the poses are all wrong:

image

And I was under the impression that the idea was to use MeshFileGeometry.

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 12, 2019

Another thing I just noticed is some strange behavior with .obj files and transparency:

image

Maybe a face orientation issue?

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 13, 2019

We can either handle that by stripping them out in meshcat (on the javascript side), or just declare that we're doing the right thing and the mesh itself is silly.

I think we should just do the latter (as we are currently).

I do wonder why you got the right link-relative poses in #32 (comment) but I didn't in #32 (comment).

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 13, 2019

@tkoolen
Copy link
Contributor Author

tkoolen commented Jul 15, 2019

#32 (comment) is fixed by meshcat-dev/meshcat#58.

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.

3 participants