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

save a file of .glb with .bin buffer embed after json? #181

Open
qiukaijia opened this issue Jul 25, 2019 · 11 comments
Open

save a file of .glb with .bin buffer embed after json? #181

qiukaijia opened this issue Jul 25, 2019 · 11 comments

Comments

@qiukaijia
Copy link

qiukaijia commented Jul 25, 2019

Does this project support?I could not find yet.
If not, I consider implement it.

@syoyo
Copy link
Owner

syoyo commented Jul 25, 2019

Not yet. Currently tinygltf save .bin to a separated file or serialize it to BASE64 string when saving a model as .glb.

If not, I consider implement it.

Awesome! PR is always welcome.

@epajarre
Copy link
Contributor

Have you worked on this? If not I might take a look. My first target use for tinygltf is model writer for some existing model handling code of mine.

Couple of questions / opinion-requests?

API-wise it looks like that we could either add an extra parameter to WriteGltfSceneToStream/File which would create the embedded buffer to .glb save or we could just change the behavior so that the BIN buffer is always added to binary saves.

I guess data for the .bin buffer should come from what ever is the data in the first buffer of the Scene
(especially if there is no file URL related to the buffer?)

After some consideration, for maximum backwards compatibility, I would add an extra parameter flag, which would create the BIN block for binary writes from the first buffer, if it is not bound to an external URI.

comments?

Eero

@syoyo
Copy link
Owner

syoyo commented Nov 17, 2019

Have you worked on this? If not I might take a look.

@epajarre Awesome! No one is not working on this yet, so your help is appreciated.

API-wise it looks like that we could either add an extra parameter to WriteGltfSceneToStream/File

WriteGltfSceneToStream/File now has many default arguments. It would be better to introduce SetSerializeOption() or similar method to TinyGLTF class to control serialization flags.

I guess data for the .bin buffer should come from what ever is the data in the first buffer of the Scene

https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/README.md#glb-file-format-specification

Currently, bin data is saved as separate .bin, or embed to .gltf as BASE64 string(SerializeGltfBufferData)
We'll need to implement .glb + embede bin version of SerializeGltfBuffer(concatenate all buffer binary to single blob, calculate offsets to blob for buffer, ...) and use it here: https://github.com/syoyo/tinygltf/blob/master/tiny_gltf.h#L7305

@epajarre
Copy link
Contributor

I forked the repo for testing purposes, and I will see how it works out.

I think that at least first, I will not be concatenating the buffers, while doing the save, just save the first buffer as the binary blob. The concatenation also involves adjusting the related bufferviews so maybe it could be a separate operation? Also existing .glb files are already adjusted to use only one "internal buffer". So rewriting then out will need no fixup.

@epajarre
Copy link
Contributor

I have a work in progress version in https://github.com/epajarre/tinygltf/tree/glb-bin-blob
which "seems to work in my personal test case", which is a program building an gltf model from scratch.

The branch contains two non related commits, which I did while working on my test case, I am not too fluent with the github flow, in any case it is possible to separate the directly bin-blob related code,

@syoyo
Copy link
Owner

syoyo commented Nov 18, 2019

I have a work in progress version in https://github.com/epajarre/tinygltf/tree/glb-bin-blob
which "seems to work in my personal test case

Awesome!

Also existing .glb files are already adjusted to use only one "internal buffer". So rewriting then out will need no fixup.

I see. so no fixup required(no buffer info modification required in JSON part) when embedding .bin to .glb.

@syoyo
Copy link
Owner

syoyo commented Nov 18, 2019

@epajarre I have glimpsed the code and it looks perfect, so could you please send it as Pull Req?

@epajarre
Copy link
Contributor

I can send a pull request, but if I wait till tomorrow, I might be able to add/do some more testing

OK?
.

@syoyo
Copy link
Owner

syoyo commented Nov 18, 2019

OK?

@epajarre Absolutely yes. adding/doing more tests is appreciated.

@epajarre
Copy link
Contributor

Unfortunately it looks like that testing is more difficult than I had hoped. Going a round trip from from .glb file into memory and then back does not work as I expected. At least one problem is that tinygltf will unpack the texture images into memory, even in case they were in bufferViews originally. As my current .glb writer does not recreate the bin buffer on output, the textures will end into different implementation, either as external files or as data URI

The gltf-validator does not really like data URI with the GLB container, it also notices that the original images in the BIN data block become unused. Neither of these are actual errors, I think but certainly problems.

I will open the pull-request in any case, use it if you want. I will probably try some other cases with my original workflow which creates the GLtf model inside a program directly.

@syoyo
Copy link
Owner

syoyo commented Nov 18, 2019

At least one problem is that tinygltf will unpack the texture images into memory

For Image, you can provide your own custom ImageLoader and set as_is flag true to indicate thatImage.image has original byte data from .glb/.gltf file.

https://github.com/syoyo/tinygltf/blob/master/tiny_gltf.h#L648

I will open the pull-request in any case,

Thanks. it has been merged > #226

https://github.com/syoyo/tinygltf/blob/master/tiny_gltf.h#L648

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

3 participants