Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make writing GLTF scene to a stream more flexible #359

Open
TheMostDiligent opened this issue May 22, 2022 · 7 comments
Open

Make writing GLTF scene to a stream more flexible #359

TheMostDiligent opened this issue May 22, 2022 · 7 comments

Comments

@TheMostDiligent
Copy link

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

@syoyo
Copy link
Owner

syoyo commented May 22, 2022

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

Your contribution is much appreciated!

@TheMostDiligent
Copy link
Author

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.

@syoyo
Copy link
Owner

syoyo commented May 23, 2022

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!

@TheMostDiligent
Copy link
Author

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

@syoyo
Copy link
Owner

syoyo commented May 25, 2022

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

@TheMostDiligent
Copy link
Author

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.

@syoyo
Copy link
Owner

syoyo commented May 26, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants