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

Space in node name ends the string with Collada #1016

Open
unelsson opened this issue Dec 30, 2020 · 5 comments
Open

Space in node name ends the string with Collada #1016

unelsson opened this issue Dec 30, 2020 · 5 comments

Comments

@unelsson
Copy link
Contributor

unelsson commented Dec 30, 2020

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

unelsson commented Jan 23, 2021

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

unelsson commented Jan 28, 2021

According to this rdiankov/collada-dom#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:

using namespace ColladaDOM141;

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.

@robertosfield
Copy link
Collaborator

robertosfield commented Jan 28, 2021 via email

@unelsson
Copy link
Contributor Author

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
b898222

Get names of bones and skeleton to osg nodes #968
8178b51

Clone pluginOptions.options #967
a7a7c0d

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.

@faerietree
Copy link

faerietree commented Mar 14, 2021

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.

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

No branches or pull requests

3 participants