meshcat-dev / meshcat

Remotely-controllable 3D viewer, built on top of three.js
MIT License
250 stars 44 forks source link

Workaround for mergeBufferGeometries failure when loading Collada files with inconsistent attributes #139

Closed wxmerkt closed 6 months ago

wxmerkt commented 1 year ago

Currently mergeBufferGeometries fails when a Collada file is loaded that has an inconsistent set of attributes in the contained geometries. I noticed this when loading a mesh (anymal_base.dae) that contains uv attributes for some of the nodes but not for all of them. In those cases, mergeBufferGeometries would write an error to the console and then return a null object resulting in a load failure.

This PR is a workaround which simply removes all textures from the loaded and merged geometry if this failure would otherwise be triggered. It's not a pretty solution but is better than not showing any geometry at all. Open to hear about better suggestions on how to fix it :-)

jwnimmer-tri commented 8 months ago

In your case, are you loading the DAE using _meshfile_object or _meshfile_geometry? (See #166 for documentation of each -- on master once that lands.) The anymal_base.dae looks to me like it has defined material(s), texture image(s), etc. so to me it should be loaded with _meshfile_object and not _meshfile_geometry. (Currently meshcat-python lacks the ability to send _meshfile_object messages, but support for that really should be added, and should be simple to do.)

At the moment, I think the problem would still occur with _meshfile_object but it does open up another path to a solution: when loading with _meshfile_object we don't really need to call ExtensibleObjectLoader and merge_geometries. Instead, we can do what the *.gltf loader already does -- load the *.dae file as a scene and just plop the whole scene tree into three.js, without any geometry merging. In that case, threejs would just show the loaded scene however it got loaded; meshcat doesn't need to get in the way.

jwnimmer-tri commented 6 months ago

Please see #170 for active work on having _meshfile_object support DAE more carefully. We would welcome any help getting that one over the finish line. In the meantime, I'll close this PR to help focus our efforts on a single branch.