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

Clarification on min/max float precision #628

Merged
merged 2 commits into from
Jun 23, 2016
Merged

Clarification on min/max float precision #628

merged 2 commits into from
Jun 23, 2016

Conversation

lexaknyazev
Copy link
Member

@lexaknyazev lexaknyazev commented Jun 17, 2016

I've put it in accessor's definitions, because it's the only case of direct comparison between buffer-stored and json-stored floats.

@lexaknyazev
Copy link
Member Author

Maybe we should also add backwards requirement: a json-stored float string must be equal to the decimal gotten from buffer data:

2.3 to Float32Array: [51, 51, 19, 64]
[51, 51, 19, 64] from Float32Array: 2.299999952316284

@pjcozzi
Copy link
Member

pjcozzi commented Jun 17, 2016

Thanks @lexaknyazev! Can you also update the accessor's schema file to keep the schema and spec in-sync? The original spec was actually generated from the schema using https://github.com/AnalyticalGraphicsInc/wetzel.

Maybe we should also add backwards requirement: a json-stored float string must be equal to the decimal gotten from buffer data...

Maybe, can you provide the context and use case for this?

@lexaknyazev
Copy link
Member Author

Let's say we have accessor.min: 2.3. Its single-precision bytes are [51, 51, 19, 64] and let's say this is the actual minimum value stored in binary buffer. So, 2.3 conforms PR's requirement.

JavaScript treats all numbers as doubles. When js-based processing tool (such as gltfPipeline) extracts such value from Float32Array it will get 2.299999952316284 which is less than 2.3. So, strictly speaking, 2.3 isn't an actual minimum.

@pjcozzi
Copy link
Member

pjcozzi commented Jun 17, 2016

So basically this enforces that exporters need to export the double-precision value for min/max?

@lexaknyazev
Copy link
Member Author

lexaknyazev commented Jun 17, 2016

I think that there are two ways to resolve this:

  1. Ensure that a numeric value that is converted to a string will be parsed back into the same numeric value (exporter can use C# approach or always print doubles with enough precision).
  2. glTF tools can convert JSON doubles to singles in runtime before comparing them with Float32Array values.

@pjcozzi
Copy link
Member

pjcozzi commented Jun 19, 2016

Sounds good. Before merging this, can you:

@lexaknyazev
Copy link
Member Author

The question remains: should spec enforce two-way match of values or not? The burden will be on exporters or on clients.

@pjcozzi
Copy link
Member

pjcozzi commented Jun 19, 2016

The question remains: should spec enforce two-way match of values or not?

Yes, I think this is fine. Do you agree?

However, we need to put the burden on exporters; otherwise, clients will always require the workaround.

@lexaknyazev
Copy link
Member Author

The question remains: should spec enforce two-way match of values or not?

Yes, I think this is fine. Do you agree?

Yes. I'll update this PR asap.

@lexaknyazev
Copy link
Member Author

Is it OK now?

@pjcozzi
Copy link
Member

pjcozzi commented Jun 23, 2016

Looks good, thanks @lexaknyazev!

@pjcozzi pjcozzi merged commit 6c27c06 into KhronosGroup:1.0.1 Jun 23, 2016
@lexaknyazev lexaknyazev deleted the 1.0.1-floats branch June 23, 2016 12:36
lexaknyazev pushed a commit that referenced this pull request Feb 2, 2017
Clarification on min/max float precision
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants