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

Default values & serialization #196

Open
Selmar opened this issue Aug 20, 2019 · 0 comments
Open

Default values & serialization #196

Selmar opened this issue Aug 20, 2019 · 0 comments

Comments

@Selmar
Copy link
Contributor

Selmar commented Aug 20, 2019

Continued discussion from #193 .

I noticed that there is currently some inconsistency between which default values are always set to defaults and/or serialized and which aren't. For example, node translation/rotation/scale is just parsed/serialized as-is, ignoring defaults. What is your preferred approach?

This may be a difficult question.

For a short-term perspective, I'd prefer ignore default values when serializing(this will save .gltf file size) and set default values when there is no corresponding entries in glTF at parsing.

For a mid-term perspective, it would be better to introduce an option whether ignore/not-set or export/set default values in serializer/parser, since default value may be changed in the future glTF spec(by minor updating) and some people may want force output full description of glTF data in the serializer.

For a long-term perspective, we are better to transit to automatically generate parser/serializer based on JSON schema to avoid human errors > #10

I see. I don't think I'll get around to change this any time soon, but let's discuss it anyway (for others).

The mid-term solution is logical. It is not a lot of extra work to begin with this; simply a boolean or define to set? I remember you weren't fond of adding more defines in the past, do you have a preferred approach to that?

The long-term solution is nice, but that seems quite far away indeed. Let's get RapidJSON to work first. 👍

@syoyo
Copy link
Owner

syoyo commented Aug 21, 2019

The mid-term solution is logical. It is not a lot of extra work to begin with this; simply a boolean or define to set?

I have just added API to set boolean value to control serialize default value or not.

a472a3f

The long-term solution is nice, but that seems quite far away indeed. Let's get RapidJSON to work first.

rapidjson branch and master now has some divergence. It requires some effort to transit to use RapidJson only and deprecate json.hpp.

Before the RapidJson transition, I'm considering to let tinygltf co-exist rapidjson and json.hpp in master branch and switch JSON backend by using a define(e.g. TINYGLTF_USE_JSON_BACKEND_RAPIDJSON).

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