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

Collada bug #31

Merged
merged 3 commits into from Sep 10, 2021
Merged

Collada bug #31

merged 3 commits into from Sep 10, 2021

Conversation

afrankel7861
Copy link
Collaborator

As a newbie to Kotlin, kotlin-graphics/assimp, and Assimp in general, any guidance on what is needed, how things are done, is appreciated.

commit 301d1ae
Repair readMaterialLibrary's handling of duplicate names.
The native Assimp's ColladaParser::ReadMaterialLibrary has, to my mind, an excessive use of high power machinery obscuring both the intent and the actual action. The initial translation to Kotlin missed a bit of syntactic cleverness, resulting in an array bounds exception. If three Collada elements have name="Mary", the action is to name the material library entries "Mary, "Mary 1", and "Mary 2".
Test by manually editing a Collada .dae file and just duplicate one of the
elements.

commit 15bae11
Repair fallback texture CHANNEL guess.
This failure was exposed by a Collada file that actually fell to this fallback calculation of the texture channel number. But the coding resulted in the ASCII code for
'0'.

commit 2ff8c85
Propagate Collada import failure info through to user as importer.errorString.
The exception thrown because of a duplicate resulted in readFile returning scene=null, but then getErrorString() returned "", and calling from Java invoked under NetBeans, the traceback did not appear anywhere obvious. Even though that exception is now quieted, tracking it down revealed that any unexpected exception in the Collada loader would be caught in BaseImporter where the exception's message was passed to the logger but not copied through to Importer.errorString where the user could get it.
My solution is to catch unexpected exceptions in the Collada loader and pass on a message that at least indicates where in the input the problem occurred, and then to put the resulting message where it gets copied to where the user expects a problem report. Checking other exceptions, from Collada loader or elsewhere, seems futile, not not on board with the directive to to the right thing with correct input and spend no significant effort on verifying compliance of the input data.

@elect86 elect86 merged commit 536857c into master Sep 10, 2021
@elect86
Copy link
Collaborator

elect86 commented Sep 10, 2021

Thanks afrankel7861!

Copy link
Collaborator

@elect86 elect86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine

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

Successfully merging this pull request may close these issues.

None yet

2 participants