Skip to content

Conversation

@wxmerkt
Copy link
Contributor

@wxmerkt wxmerkt commented Dec 14, 2022

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 :-)

The ColladaLoader fails for some DAE files that contain textures for
some geometries but not for others. This workaround (currently
deactivated) would remove all textures if this were to be the case. This
prevents "non-loading" of geometries and falls back to loading them
without textures
…thon

Unclear why we need to specialise this type since the switch from
msgpack-lite to msgpack - but without it the geometries do not get
decoded.
@jwnimmer-tri
Copy link
Contributor

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
Copy link
Contributor

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.

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