vpenades / SharpGLTF

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

Fix SatelliteFile to use existing files on disk #52

Closed tomspilman closed 4 years ago

tomspilman commented 4 years ago

The SatelliteFile option on saving a model writes new textures when the existing material links to a valid texture file.

It would be nice to fix it so that it checks and uses the correct path and doesn't create extra texture files when not needed.

tomspilman commented 4 years ago

@vpenades i started looking at this but got tied up in the various material conversions.

If you have some idea on how best to fix this let me know and i'll submit a PR.

vpenades commented 4 years ago

I understand you want to share the textures already being used by other models, right?

There's two possible solutions:

In either case, both implementations could be handled in a hook in the saving context, so you could inject your own texture saving strategy.

I'll look into it.

tomspilman commented 4 years ago

property to keep the path of the "source texture" that is used only for reference

From my perspective. I created a material giving it the absolute path to the texture. I by default expected that texture to be referenced by the final gltf/glb and not embedded or duplicated.

So to me this would be ideal.

tell the library to scan a directory looking for already existing textures

That could work as well i guess. Just seems redundant, error prone,and slow if you have a folder with lots of textures.

both implementations could be handled in a hook in the saving context

Oh... can you point me at that? I can give it a shot.

vpenades commented 4 years ago

Oh... can you point me at that? I can give it a shot.

It's not commited yet, I'm writing a unit test right now, I'll commit the changes in a few hours.

vpenades commented 4 years ago

I've just pushed the changes that support the image write hook,

here's an example/unit test demonstrating how it works:

https://github.com/vpenades/SharpGLTF/blob/16c7137f48a82e9a100e6c83dd306e686d620168/tests/SharpGLTF.Toolkit.Tests/Materials/ImageSharingTests.cs#L58

Let me know if this is what you're looking for.

tomspilman commented 4 years ago

That looks perfect... thanks @vpenades