OpenSceneGraph icon indicating copy to clipboard operation
OpenSceneGraph copied to clipboard

Space in node name ends the string with Collada

Open unelsson opened this issue 4 years ago • 5 comments

For example, a collada-file with a <node> with attribute id="bip01 spine" gives an OSG node with name "bip01". This may be a bug in dae-reader used by OSG, it's not necessarily a bug in the codebase of OSG's dae plugin. A workaround is to use underscores or lines instead of spaces.

unelsson avatar Dec 30 '20 18:12 unelsson

I've been researching ColladaDOM, and it looks like the default dae-method getAttribute("name") is indeed cut by whitespace, and there's no straightforward way to fix it. Digging deeper into collada data seems to be possible with getAttributeObject("name") instead, but I haven't found a way to get into the actual string data.

The following sequence placed at void daeReader::processSkeletonSkins digs up an object of type daeMetaElementArrayAttribute, which has more meta-methods, but it's starting to get complex, and requires in-depth knowledge of colladaDOM.

                    daeMetaAttribute* metaAttr = jointsAndInverseBindMatrices[j].first->getAttributeObject( currAttr );
                    daeMetaElement* metaElem = metaAttr->getContainer();
                    daeMetaElementArrayAttribute* metaCont = metaElem->getContents();

unelsson avatar Jan 23 '21 12:01 unelsson

According to this https://github.com/rdiankov/collada-dom/issues/35 , Collada 1.4.1 uses NCName for the name which doesn't support whitespaces. Collada 1.5.1 would use xs:token that supports whitespaces.

OSG plugin seems to rely on 1.4.1. Currently loading 1.5.0 collada file via OSG gives the following error: "Trying to load an invalid COLLADA version for this DOM build".

This is likely related to the version mismatch: https://github.com/openscenegraph/OpenSceneGraph/blob/711d69d2fd10a1704c0e9bbc0ddf4eb07a547eda/src/osgPlugins/dae/domSourceReader.h#L23

However, other changes may be required as well in order to support 1.5.0, but most of the new specification is about adding new features rather than changing stuff: https://www.khronos.org/files/collada_1_5_release_notes.pdf

It would be nice to have both 1.4.1 and 1.5.0 supported. Blender's Better Collada Exporter as well as the default Collada exporter use Collada 1.4.1 too, so also the exporter needs work to have Blender to OSG pipeline working.

unelsson avatar Jan 28 '21 10:01 unelsson

On Thu, 28 Jan 2021 at 10:04, unelsson [email protected] wrote:

According to this rdiankov/collada-dom#35 https://github.com/rdiankov/collada-dom/issues/35 , Collada 1.4.1 uses NCName for the name which doesn't support whitespaces. Collada 1.5.1 would use xs:token that supports whitespaces.

OSG plugin seems to rely on 1.4.1. Currently loading 1.5.0 collada file via OSG gives the following error: "Trying to load an invalid COLLADA version for this DOM build".

This is likely related to the version mismatch: https://github.com/openscenegraph/OpenSceneGraph/blob/711d69d2fd10a1704c0e9bbc0ddf4eb07a547eda/src/osgPlugins/dae/domSourceReader.h#L22

However, other changes may be required as well in order to support 1.5.0, but most of the new specification is about adding new features rather than changing stuff: https://www.khronos.org/files/collada_1_5_release_notes.pdf

It would be nice to have both 1.4.1 and 1.5.0 supported. Blender's Better Collada Exporter as well as the default Collada exporter use Collada 1.4.1 too, so also the exporter needs work to have Blender to OSG pipeline working.

I'm not the author of the OSG's Collada plugin, just had to maintain it bit by bit. A long while back I did look at trying to support 1.5, but basically had to abandon it as there really wasn't enough overlap to try and do both. Perhaps with more time it might be possible. It may be easier to just do a dedicated 1.5 loader.

Basically Collada DOM is a bit of a blow out, was, is and will always be, a bit of broken effort to unify data exchange. How much effort it's worth putting into something that will never be perfect is an open question. It may be better to spend time with other art part routes.

robertosfield avatar Jan 28 '21 13:01 robertosfield

How much effort it's worth putting into something that will never be perfect is an open question. It may be better to spend time with other art part routes.

This is very much true. Collada 1.4.1 works fine right now, especially with OSG 3.6 branch. The plugin even has a few OSG-specific extras, and it could be easy to develop those further. The effort required to support 1.5.0 just may not be worth the trouble.

For a note with collada 1.4.1 support though, I noticed a few helpful commits were missing from master, so it may be of use to cherry-pick them?

Remove "file" protocol check #969 https://github.com/openscenegraph/OpenSceneGraph/commit/b8982224c375f8f285ebd77e65a4d35fec756ada

Get names of bones and skeleton to osg nodes #968 https://github.com/openscenegraph/OpenSceneGraph/commit/8178b51956b2c3d05fe9e5628b3d2bc357398cd4

Clone pluginOptions.options #967 https://github.com/openscenegraph/OpenSceneGraph/commit/a7a7c0de8bdbfaed882c55bf31fb990ec32af713

For other formats, question is which format: fbx and glTF 2.0 seem like good candidates. I'm hoping for a plugin for the latter, but unfortunately it's not yet done. For fbx, OSG has method ReaderWriterFBX::readNode(const std::string& filenameInit, const Options* options) const, and when testing with osgViewer and osgAnimationViewer, it seems to work very well. However, method readNode for std::istream& fin is missing, so reading from memory doesn't seem to work at this moment.

unelsson avatar Jan 30 '21 15:01 unelsson

Yes use _ instead of whitespace. Matter of habit. We can sed replace all occurrences automatically before loading .dae if required.

glTF 2.0 sounds promising. But this was COLLADA once too. Never give up hope though. But focus instead of trying to get everything to work. Underscore instead of whitespace suffices. No more work required.

Cherry-picking these commits makes sense, @unelsson. Good work all involved.

faerietree avatar Mar 14 '21 20:03 faerietree