nem0 / OpenFBX

Lightweight open source FBX importer
MIT License
1.15k stars 135 forks source link

Token parsing failure with "ascii" format with embedded texture data due to a ',' #85

Closed TomTheFurry closed 1 year ago

TomTheFurry commented 1 year ago

A quirk in how FBX stores embedded texture in "ascii" format like below:

Video: 2669981292352, "Video::Map #7", "Clip" {
    Type: "Clip"
    Properties70:  {...}
    UseMipMap: 0
    Filename: "C:\Users\HTC Vive 1\Desktop\uv_test.png"
    RelativeFilename: "..\..\..\Desktop\uv_test.png"
    Content: ,
"iVBORw0KGgoAAAANSUhEUgAAAgAAAAIACAIAAAB7GkOtAAAAAXNSR0IArs4c6QAAAA..."
 }

The Content: , "iVBO...." line causes a token parsing failure as the property parsing in readTextProperty(...) did not expect a 0 sized property followed by a comma.

(Note that this issue isn't reporting that OpenFBX does not read the texture data, but that the parser failed when encountering any "ascii" FBX files with embedded texture.)

A related (closed) issue in 'three.js' also contains more discussion regarding this quirk: https://github.com/mrdoob/three.js/issues/11624#issuecomment-312403033

Our C# port fix is to check in the readTextElement() that if a comma appears where a property should start, and if so, emit a void / default / 0 sized Property.

Background info:

This group of issues are discovered when our team is porting OpenFBX library to run natively on C#. Our company is planning to use OpenFBX to expend model loading support for our game (and game engine), as we are currently using block bench parsing and loading only which provides limited features. While the modified C# port is currently closed source and for internal use only, it is undecided whether we will release the port in any format once it is to a stage we are happy with.

nem0 commented 1 year ago

Thanks, fixed