vsg-dev / vsgXchange

Utility library for converting data+materials to/from VulkanSceneGraph
MIT License
65 stars 39 forks source link

Fixed 'Optional Texture Embedding' feature #196

Closed appcodegen closed 1 month ago

appcodegen commented 1 month ago

Description

It seems that commit https://github.com/vsg-dev/vsgXchange/commit/68edd8f22af38cdd883e2cf81c1652b6031c8e94 broke the 'Optional texture embedding' (see PR https://github.com/vsg-dev/vsgXchange/pull/172) feature, so that vsgb/vsgt files created using vsgconv will (at best) not work and (at worst) lead to the loading code to crash.

For example it happened to me that I had a top-level CullNode.. the vsg::External object referenced an external texture, but its path was not set/saved (that is the actual bug introduced in the merge), so the StartID_EndID_Filename values were incorrect (an empty object with the ID=3 was created on loading), which means the the vsg::MatrixTransform node was not loaded (since the empty object with ID=3 already cached), so I ended up with a CullNode without child node, which leads to an access violation on traversal (CullNode on purpose does not check for this due to performance considerations)

vsga 1.1.7

Root id=1 vsg::CullNode { userObjects 1 key "external" object id=2 vsg::External { userObjects 0 options id=0 NumEntries 1 StartID_EndID_Filename 3 3 "" } bound 2.28881835938e-05 2.22340196371 45.2545385361 63.7056769332 child id=3 vsg::MatrixTransform {

For reference, after the fix, the file looks like this

vsga 1.1.7

Root id=1 vsg::CullNode { userObjects 1 key "external" object id=2 vsg::External { userObjects 0 options id=0 NumEntries 1 StartID_EndID_Filename 3 4 "myTexture.bmp" } bound 2.28881835938e-05 2.22340196371 45.2545385361 63.7056769332 child id=4 vsg::MatrixTransform {

Fixes #195

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

I converted some test models using vsgconvd.exe, using the command line

then loaded the resulting files into the vsgViewer example

Test Configuration: Windows 10, VS2022

Checklist: