tinygltf icon indicating copy to clipboard operation
tinygltf copied to clipboard

Make writing GLTF scene to a stream more flexible

Open TheMostDiligent opened this issue 2 years ago • 7 comments

Describe the issue

Currently, WriteGltfSceneToStream always embeds images and buffers into the output stream, which is not very flexible. In our usage scenario we would like to write images to separate files as they are shared between multiple scenes, but at the same time to be able to output the scene to a custom stream. At the moment we have to write the scene to a file on disk and then immediately read it back.

To Reproduce

  • OS: any
  • Compiler, compiler version, compile options: any

Expected behaviour

WriteGltfSceneToStream takes additional arguments embedImages and embedBuffers that control how images and buffers are embedded. The following function overload could be added which will let the existing function work as before:

  bool WriteGltfSceneToStream(Model *model, std::ostream &stream,
                              const std::string &base_dir,
                              bool embedImages, bool embedBuffers,
                              bool prettyPrint, bool writeBinary);

Screenshots

n/a

Additional context

n/a

TheMostDiligent avatar May 22 '22 01:05 TheMostDiligent

It may be better to introduce callback handlers to WriteGltfSceneToStream for customizing Image output and Buffer output.

Your contribution is much appreciated!

syoyo avatar May 22 '22 06:05 syoyo

There are already file system callbacks that can be set with SetFsCallbacks. And as a matter of fact this is how I tried to implement it initially - by proving a custom WriteWholeFile function. I was expecting that the callback will be called for the GLTF file. It however turned out that WriteWholeFile is only used to write images when TINYGLTF_NO_STB_IMAGE_WRITE is not defined and is never used otherwise. WriteGltfFile always uses std::ofstream for writing the file.

TheMostDiligent avatar May 23 '22 05:05 TheMostDiligent

It however turned out that WriteWholeFile is only used to write images when TINYGLTF_NO_STB_IMAGE_WRITE is not defined and is never used otherwise.

Oh, I see.. Writing API is not mature and needs an improvement.

Anyway, You can propose PR to fix/improve writing API!

syoyo avatar May 23 '22 07:05 syoyo

How would you suggest to implement this - to make the callbacks work or use the new version of WriteGltfSceneToStream function?

TheMostDiligent avatar May 25 '22 15:05 TheMostDiligent

@TheMostDiligent Propose new API considering your usecase senario recommended. Current API design of WriteGltfSceneToStream is not so good... 😩

syoyo avatar May 25 '22 16:05 syoyo

I am trying to understand the architecture and there are few questions that I have. In particular, why WriteGltfSceneToFile is not implemented through WriteGltfSceneToStream? And similar with WriteBinaryGltfFile/WriteBinaryGltfStream and WriteGltfFile/WriteGltfStream. It looked to me that the "File" version should be a simple wrapper over the "Stream" versions, where they create the std::ofstream and then just call the stream version.

TheMostDiligent avatar May 25 '22 19:05 TheMostDiligent

Serialization feature is primarily contributed by @AurL and there are some inconsistency between loader(primarily implemented by me) and serializer API.

There is no strong reason not implementing WriteGltfSceneToFile(just we forget to implement as far as I know), so you can implement WriteGltfSceneToFile as a simple wrapper over "Stream" version.

syoyo avatar May 26 '22 12:05 syoyo