KhronosGroup / OpenCOLLADA

652 stars 252 forks source link

Maya exporter concatenate joint names #423

Open pixelnerve opened 8 years ago

pixelnerve commented 8 years ago

When exporting a skin from maya, joint names get concatenated as it goes down the hierarchy.

Colllada snipped here

RemiArnaud commented 8 years ago

@jerome-jacob @mycheckgithub @mbarnes-sb please chime in, but my take is that 'name' should not concatenate, only 'id' should.

RemiArnaud commented 8 years ago

I replied too fast, I think you are talking about the SIDs (which are stored in NAME_ARRAY, as in XML NAME type, not the name attribute)

And yes, I see no reason for adding the bone hierarchy, only thing needed is that bone sid is unique in hierarchy from the node referenced by the #.... in the instance_controller.

but maybe this was done to create unique sids when they are not unique to begin with ?

@jerome-jacob @mycheckgithub - could you give some details?

mbarnes-sb commented 8 years ago

SIDREF stored in Name_Array do not have to be baked to be resolved. SID only need to be unique within the scope of their parent element. SIDREF_array was introduced in COLLADA 1.5 to clarify usage and semantics. See Chapter 3 in the specs.

jerome-jacob commented 8 years ago

SIDs were generated like IDs. However this should not be an issue. Or maybe a readability of the produced document issue. Anyway it is fixed in https://github.com/KhronosGroup/OpenCOLLADA/issues/426.

RemiArnaud commented 8 years ago

@pixelnerve could you please test the exporter created by the PR ?

the bot in the PR says:

binaries: look for folder: PULL_REQUEST#426/COLLADAMaya/mac/2016-04-2203-57-11#229 http://opencollada.fl4re.com

pixelnerve commented 8 years ago

That worked. It didn't solve the real problem which is resolving the id. Still get the concatenated string but i'd guess that would not be something that collada would change, right?

RemiArnaud commented 8 years ago

I am not sure what you mean by it did not solve the real problem. What exactly did you get out of the exporter, and what did you expect?

pixelnerve commented 8 years ago

Our gltf importer was using the object keys (collada ids) to structure data but we have a limit on name lengths. Collada files exported from maya have joint names concatenated as Ids, therefore so does gltf. There is cases of string lengths of about 500 chars. That is not acceptable for us.

I've changed the code to use real names and all seems to work, but having those gigantic strings still makes me wonder if it could be avoided. Probably not on the collada exporter level but maybe when converting to gltf.

Either way, i was able to work around it and the change you made helped cutting down the jointNames block size.

RemiArnaud commented 8 years ago

PR was merged - concatenation of hierarchy to make joint Sid unique is not only unnecessary, but also inconvenient as the joint name no longer matches the name in the source data, and result in larger exports. Just thinking about what would happen after a series of import/export makes this a bug and not only an inconvenience.

It is possible to reduce this more. For instance, Id - as opposed to name - do not have to be readable by a human, they just have to be unique. One can use a simple counter to be used for ids and sid, it would work the same and reduce the size of the data, especially if using a base>10 for the counter.

The issue is that ids are unique to a dae, (sid unique to a specific node hierarchy) and can be referenced outside by other daes or any software. For instance an annimation can be retargetted to different skeletons, using the sids, or different skins can be used with same skeleton. All this happening with no modification possible to the referenced dataset.

So I would not recommend making changes to id and sid in the collada->gltf by default, but certainly possible as an option. Giving the user the choice to reduce content size at the expense of not being able to rely on id and sid. However it is possible to create a dictionary to provide the original id and sid, which could be queried optionally.

Where does the 500 characters string size limit comes from? Is that a limit of json? Or a limit of your particular implementation? Why is this not acceptable?

pixelnerve commented 8 years ago

It's a limit we have internally. Strings have a limit of 128 chars.

Why does this id concatenation only happens in Maya? Tested with 3dsmax and Blender and each software does things differently. Blender seems to be the official choice as it's mentioned in the glTF repo. I don't know what collide implementation they have but i don't really trust it. Try saving a collada and load it again. Here it messes everything up when loading. 3dsmax saves joints by naming it "joint" and a index suffix so it seems. Works but then fails to find animation data as the animation data refers to some other name.

Only software that works nicely for me is still Maya, except for generation those long string ids which leak into the gltf. I guess this is a discussion that could be taken on to glTF team

RemiArnaud commented 8 years ago

It's a limit we have internally. Strings have a limit of 128 chars. Then you could provide an option to the converter to 'compress' the output by renaming ids and sids to short versions. You should probably output a dictionary as well so its possible for an application to find original ids/sids, and more important use for next conversion to keep generated ids consistent, or allow multiple separate external dae to correctly reference objects once converted to multiple? glTF. FYI - please see the Maya exporter, there is an option to export each animation clip in a separate dae document, which is very useful for production.

Why does this id concatenation only happens in Maya?

Each software is free to do whatever they want when generating id. The spec says that id should be preserved when importing and then exporting again (which probably no tool has correctly implemented). But more important use case is when exporting again the same content, the ids should be the same. Using dag name concatenation is the way Maya exporter chose to consistently provide ids on successive exports. This allow parts of the maya scene to be modified, and ids for the untouched sections to stay unchanged.

The other tools should probably implement a similar scheme, unless they internally correctly manage ids. Do you have a better idea?

Blender seems to be the official choice as it's mentioned in the glTF repo

Really? There should not be an official choice for a modeling tool in glTF !

You are right that Maya is probably the most stable exporter at the moment. We use openCOLLADA Maya on a daily basis for production and contribute all of it to this repo.

jerome-jacob commented 8 years ago

Maybe we could add an option to generate a hash from id?

pixelnerve commented 8 years ago

It's worth a try.

mbarnes-sb commented 8 years ago

The reason both id and name attributes exist is because of the functional requirements on id.

The name attribute is non-functional metadata and no COLLADA tool should use name values to perform any meaningful calculation. Tools outside of the COLLADA domain can do whatever they want with name attributes.