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

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

Closed
wants to merge 5 commits into from

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