Merge of 'collada_metadata' branch breaks loading of collada files

[robindegen]

After the merge of branch collada_metadata ( rev 7b6cb57 ) the loading of certain collada files has broken.

I see that ColladaParser::SkipElement is trying to find the end tag of "author" for my file, but it never actually finds this. This results in the while loop reading until the reader has nothing left to read. The result is that the entire file is not loaded.

After reverting back to the commit right before this merge ( 0b9da72 ) fixes the issue and the file is loadable again.

I'm testing this with a file exported with the collada exporter for 3ds max (generated back in 2016).

The file can be found here:
https://github.com/aeon-engine/aeon-engine/blob/master/bin/resources/meshes/elementalist-warrior-female-character-f/x-elemetal.dae

[RichardTea]

It seems I misunderstood the limitations of IrrXML.

IrrXML doesn't recognise the construction: <author></author> as being an empty element, and so ColladaParser::TestTextContent advances the element stream into the </author> element.

Should be fixed by pull request #2516

[kimkulling]

I guess we should replace irrXml with somethink like pugiXml. Any opinions or feature requests?

Thanks!

[Cgettys]

Hi Kim, I know I haven't contributed anything in a while, but I have been using pugixml in the hobby project I've been working on for a while. It has some really nice functionality, but it also has some drawbacks. The biggest being that the root document object isn't copyable (if I remember correctly anyway), making it a bit annoying at times. You end up just passing around references to the root node, but it's worth noting. It does also have some nice features like a walker interface to make tree traversal easy, and good load/save support. It definitely has a learning curve though - also beware of the fact that it's easy to not actually create nodes & attributes and not even realize it because of the way the sort-of fluent interface works, and it won't warn you. Just thought you might find this info useful.

[RichardTea]

I don't really think it's worth changing XML parser.
IrrXML has very limited features, but it's a pretty fast streaming parser and the learning curve isn't too bad.

I don't think a DOM style parser is sensible for this application.
Models in the XML 3D formats are often huge, and so constructing a DOM would be very expensive in time and memory.
Even at best, it'd mean traversing the tree one more time than at present. Most of the XML formats only ever refer to items defined earlier in the file so there's very few occasions where a DOM is at all useful.

[turol]

IrrXML is causing some truly weird warnings with high warning levels on clang and GCC. Something about overflows in IrrString. I'd like get rid of it just to get rid of those.

[RichardTea]

Perhaps Expat? https://libexpat.github.io/
They use the MIT/X Consortium license, which is commercially-friendly.
It's still actively developed and has a CMake config.

That said, all this is a long way from the original issue. @kimkulling can you create a new issue for updating the XML parser and move these comments there?

[RichardTea]

@robindegen @kimkulling I believe this issue now corrected, can you confirm and close it?

[robindegen]

@RichardTea Yes it was fixed. Thanks