Major error handling bugs

[ivalylo]

There is a function called handleFWLError() which returns true if the parsing should stop (either because of the IErrorHandler or a critical error). On most places, it's used like this though:

success = handleFWLError ( )

I.e. it does exactly the opposite.
There are also palaces like this one where the result is not handled at all:

if ( positionInput == 0 )
handleFWLError ( SaxFWLError::ERROR_DATA_NOT_VALID, "No positions, can't import!", IError::SEVERITY_CRITICAL );
mPositionsOffset = positionInput->getOffset ();

The code directly crashes of course. Tested also with the Maya importer.

[RemiArnaud]

Thanks for this report.
Any chance you could create a PR ?

[ivalylo]

I work with forked version of OpenCOLLADA and can't contribute a patch since I can't test it with the official version. I can upload the fork itself on github if it will be of any interest when I've tested it. Here is what I'm changing from the official version:

• new hybrid parsing - IWriter methods should be able to return new code that means - don't delete the item, add it to the loader (similar to the items added for the post-processor). Then the loader itself provides a DOM fashion interface with the requested items. The idea is to simplify the loading and SAX parse just the meshes or any other items you can process out of order and it's worth the trouble.
• consistent use of FW_NEW/FW_DELETE. Or even converting them to more flexible custom allocator functions (like in the FBX SDK)
  • custom file IO interface.
• fix the Texture and Light object report in the IExtraDataCallbackHandler::parseElement. The getObject() methods didn't implement this
• the color/uv sets have some weird initial index which offsets the indices reported for additional sets. There is also a comment in the COLLADA to glTF converter about this weirdness. I removed it,
  • I would want to fix the index data duplication in the mesh loader, so it can be reused, but I don't know if I will have time since it's more involved. For example normals/tangents/binormals would likely use the same indices, but currently (if I understand the code properly), the same indices will be loaded 3 times.

[RemiArnaud]

if you forked from this repo, you should be able to create a branch matching the common ancestry and do the patch. you can merge the updated openCOLLADA changes too.