vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
454 stars 72 forks source link

Make `MemoryImage(Byte[] image, string filePath)` constructor public #162

Closed xPaw closed 1 year ago

xPaw commented 1 year ago

See https://github.com/SteamDatabase/ValveResourceFormat/pull/484/commits/5c0c940649fdd47bda51d2a2a9ec0ef81a4154b3#diff-a740f76f43bd153e9126ae3abe86209608d28825f381ed37b2d74aa3bcbc9c93R1026 for context

We have a use case where we want to write textures straight to disk, without sharpgltf keeping them in memory (it adds up!), but unfortunately the current soluton we arrived at is writing a small png to disk so that sharpgltf loads and verifies it, and after that we write the real texture to disk.

At least with the (byte[], string) constructor we would avoid writing the temp file to disk and sharpgltf reading it back.

Ideally we could do MemoryImage(null, filePath), but that would fail when writing gltf because of image validation.

// Write a 44 byte PNG file that can be replaced with the full image after linking to modelRoot.
// SharpGLTF loads images into memory, which isn't necessary with satellite images.

using (var fs = File.Open(exportedTexturePath, FileMode.Create))
{
    fs.Write(tempPng);
}

image = model.CreateImage(fileName);
image.Content = new SharpGLTF.Memory.MemoryImage(exportedTexturePath);

// Replace with full image
using (var fs = File.Open(exportedTexturePath, FileMode.Create))
{
    fs.Write(TextureExtract.ToPngImageChannels(bitmap, channel));
}
vpenades commented 1 year ago

The reason the API discourages using texture file names at all costs is because any logic depending on texture file names would break when using GLBs, because GLBs are missing that information. This is a point of discussion in the glTF boards because it prevents a perfect GLB to glTF roundtrip conversion.

So, that "file name" is just a hint used to knowing which files the glTF model originally used, but it is not involved in neither loading or saving the model. so if you set the fileName to something, it would still save the texture to the predetermined location and name. So exposing that constructor would not have the effect you want.

Anyway, I am trying to understand the problem; it is just a matter of not willing to load all the images in RAM because they use a lot of memory? or there's another reason?

I am available at discord from time to time if you're willing to discuss.

vpenades commented 1 year ago

I've uploaded some changes to allow creating a memory image using a lambda as source. Not sure if that will allow you to close the circle.

Anyway, with this and the previously added feature I think it's all I can do for now