vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
470 stars 75 forks source link

ReadGLB infinite recursion bug #20

Closed gleblebedev closed 4 years ago

gleblebedev commented 5 years ago

The following code has infinite recursion: if (chunks.ContainsKey(glb.CHUNKBIN)) { settings.FileReader = key => string.IsNullOrEmpty(key) ? new BYTES(chunks[glb.CHUNKBIN]) : settings.FileReader.Invoke(key); }

Instead original value of settings.FileReader should be saved in a local variable and then references from the code. Something like

var originalReader = settings.FileReader; settings.FileReader = key => string.IsNullOrEmpty(key) ? new BYTES(chunks[glb.CHUNKBIN]) : originalReader.Invoke(key);

vpenades commented 5 years ago

I guess you found a GLB file that still has external binary buffers?

I was unable to find any file like that, that's why I guess that branch of the code is untested...

If you have a glb with external binary files and you can share it, I would be glad to add it to the unit tests.

gleblebedev commented 5 years ago

I accidentally made it while fixing the little Tokyo gltf. In my case it is a glb file with external textures. I can make a small one for tests.

vpenades commented 5 years ago

Ah, yes, textures might also fail.

If you can create a glb with an external texture I could use it for testing. I would like to have more fine control over how files are saved in the future, but SharpGLTF is not quite there yet.

On a side note, you might find this useful for your veldrid renderer.

gleblebedev commented 5 years ago

Thanks! By the way do you have a sample of gltf with texture UV transform?

vpenades commented 5 years ago

The glTF sample models collection has a UV transform test here.

I believe the intended usage of the texture transform extension is for texture atlas packaging... my understanding is that a sub-texture within a texture atlas, which has the WrapMode set to Repeat, would wrap over the sub-texture and not over the whole atlas, something that would need to be set at the vertex shader.