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.

Sign up for GitHub

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

Describe numbers encoding in JSON #1574

Closed
wants to merge 2 commits into from
Closed

Describe numbers encoding in JSON #1574

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7717876
Describe numbers encoding in JSON
lexaknyazev Mar 6, 2019
2173d8f
Wording tweaks in JSON section.
Mar 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions specification/2.0/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ To simplify client-side implementation, glTF has additional restrictions on JSON

> **Implementation Note:** This allows generic glTF client implementations to not have full Unicode support. Application-specific strings (e.g., values of `"name"` properties or content of `extras` fields) may use any symbols.
3. Names (keys) within JSON objects must be unique, i.e., duplicate keys aren't allowed.
4. Numbers defined as integers in the schema must be written without a fractional part (e.g., `.0`). Exporters should not produce integer values greater than 2<sup>53</sup> because some client implementations will not be able to read them correctly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the "without a fractional part" requirement, could you include the motivation for that? A value of 1.0 would pass JSON validation for { "type": "number", "multipleOf": 1.0 }, so this requirement is really because some (many?) JSON libraries for typed languages depend on it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@donmccurdy Related: KhronosGroup/glTF-Validator#109

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I think the rationale should be included explicitly in the specification – otherwise it sounds like we're inventing our own subset of JSON, which would be a bad look. 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the issues that are related to the original integer type have already been linked to in other comments. Some of the related hiccups can probably be explained by the fact that earlier versions of the ("official") JSON-Schema included the integer type, but it has been removed in the meantime (cf. json-schema-org/json-schema-spec#146 ).

One specific issue is caused by the combination of a typed language and automatic code generation from the schema. In doubt, the code generation has to be tweaked and configured so that a { "type": "number", "multipleOf": 1.0 } is translated into a long, and that the resulting parser does a parseLong instead of a parseDouble ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"type": "integer" was added back at some point. Latest definitions are:

  • concept of "integer" is from a vocabulary, not the data model

  • For consistency, integer JSON numbers SHOULD NOT be encoded with a fractional part.

  • String values MUST be one of the six primitive types ("null", "boolean", "object", "array", "number", or "string"), or "integer" which matches any number with a zero fractional part.

Sources:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there's an important distinction between "SHOULD NOT" (the JSON spec wording) and "must be written without a fractional part" (this PR). The JSON wording doesn't outright prevent encodings like 1.1e1 for integers, although they are strongly discouraged.

It may still make sense for glTF to have stronger enforcement, for the sake of strongly-typed glTF readers that don't wish to take extra steps of parsing floats and rounding. But I agree with Don's concern that this needs to avoid becoming a subset of JSON.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, they update the JSON schema draft regularly, and I wonder about their roadmap for a ratification - without that, #929 remains a moving target... :-/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should copy the JSON wording here, changing "must be written without a fractional part" to "should not be written without a fractional part."

5. Floating-point numbers must be written in a way that preserves original values when these numbers are read back.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an implementer I don't know what to do with this as a normative requirement... no one should be writing a JSON serializer from scratch, and I would not know how to evaluate this requirement on a JSON library, other than (as you suggest below) looking for something common and well-tested. Some developers (e.g. on the web) will not have a choice in their JSON implementation.

Perhaps this should just be an implementation note – that implementers should be aware of this when choosing a serialization method, and that common JSON libraries handle it automatically.

Is this something we can detect in validation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All "reasonable" approaches (such as JSON.stringify on the Web, platform-provided JSON implementations in languages like Python, commonly-used C++ libraries) are already aligned with this requirement.

Validating it would require comparing glTF JSON with some "source" data.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevertheless, I think this is too vague to be a normative requirement, and should probably be an implementation note.

It is probably also worth saying at the top of this section that all of these "additional restrictions on JSON" are already implemented by common JSON libraries, even if they are not required by the JSON spec.


> **Implementation Note:** This is typically achieved with algorithms like Grisu2 used by common JSON libraries. This restriction enables efficient and predictable data round-trip with binary JSON representations such as UBJSON.

## URIs

Expand Down