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

Validator and friends versioning policy #1

Open
lexaknyazev opened this issue Jul 21, 2016 · 6 comments
Open

Validator and friends versioning policy #1

lexaknyazev opened this issue Jul 21, 2016 · 6 comments

Comments

@lexaknyazev
Copy link
Member

At the moment, we have glTF spec version 1.0.1, with WebGL 1.0.3 profile (this also implies GLSL ES 1.0).

First release version of Validator will obviously have version 1.0. However numbering future versions could become a mess, because it's unclear should Validator's version be aligned with spec or not.

Some possible strategies:

  • fork validation code each time;
  • incorporate a version check into every validation function;
  • drop support for old spec versions, or make something like a sliding window (like GPU drivers handle old hardware).

Ideas?

@javagl
Copy link

javagl commented Aug 5, 2016

It would be awesome if the validator could validate against "all" versions (which would mean to implement the second of your listed strategies). But I can imagine that it will be difficult to organize this in view of possible future changes.

When the RFQ for the validator was opened, I thought about this, and thought that there are several forms or "levels" of validation - most importantly:

  • Schema validation: Whether the input matches the JSON schema
  • Conceptual validation: Whether the IDs refer to valid entries etc.

The crucial thing is that they might be conceptually independent:

The schema validation could be done in a form that is, in fact, independent of glTF: It could be implemented as a generic validator that takes an arbitrary JSON and an arbitrary (v03) JSON schema, and blindly does all the checks for invalid properties, missing required properties and basic range checks of the values. The conceptual validation, e.g. checking whether the IDs refer to valid entries, would then be specific for glTF, but independent of the checks that are covered with the schema validation.

The nice thing would be:

  • The schema validation would be "version agnostic" in the sense that it only validates a JSON against an arbitrary schema (overly suggestive pseudocode: validate(gltf, schemaRepository[gltf.asset.version]))
  • The conceptual validation would be "version agnostic" in the sense that (most of) the validity checks will not depend on the version: An ID has to be valid, regardless of whether it's used in version 1.0.0 or 3.4.5.

(Of course, the latter is a bit idealistic. E.g. the ID binary_glTF already is "invalid" in this sense. But special cases like this will always have to be handled somehow...)

I haven't yet digged into the source code, and am not familiar with dart or the general approach you took. Do you validate against a schema, or did you "bake" the current schema into your validation code?

@lexaknyazev
Copy link
Member Author

lexaknyazev commented Aug 5, 2016

Thanks for the feedback, @javagl!

One of the goals for validation is to ensure that glTF asset won't trigger any runtime errors. This implies checking not only schema and IDs: actually we need to check correctness of all values which will be used as parameters in GL calls.

The main issue with "blind" schema validation is that it's not possible to express lots of "sibling" properties limitations with it.

E.g. (accessor object):

  • Stage 1: schema + "intra" checks
    • componentType can be UNSIGNED_INT IFF
      type: "SCALAR" AND glExtensionsUsed contains OES_element_index_uint;
    • min and max must be:
      • the size of type,
      • each array value must be type of componentType;
    • byteOffset must be a multiple of componentType;
    • byteStride must be a multiple of componentType;
    • byteStride mustn't be less than one attribute byteLength.
  • Stage 2: cross object checks:
    • referenced bufferView exists;
    • accessor's byteOffset and its total length fit into referenced bufferView;
    • total byteOfset (accessor.byteOffset + bufferView.byteOffset) is a multiple of componentType;
    • there are limitations on valid values of componentType and type, if accessor references bufferView with target: ELEMENT_ARRAY_BUFFER;
    • accessor with componentType: UNSIGNED_INT can reference only bufferView with target: ELEMENT_ARRAY_BUFFER.

Moreover, glTF extensions can add valid values for existing enums (see CESIUM_RTC).

Currently, everything is baked into code with possibility to load extension validation code externally.
I'd wait for glTF 1.1 (or what the next version would be) before making any fundamental changes.

@javagl
Copy link

javagl commented Aug 6, 2016

Sure, these comments have just been initial thoughts. (I'm not even sure whether this is the right place to discuss the details. The basic question about the versioning policy could be answered independently of the general validation strategy. I just mentioned these points because I think that making the validation as version-agnostic as possible could avoid some hassle in the future. But if you think that this should be discussed elsewhere, just point it out).

Regarding the given example: One could argue that one main problem here is that the schema simply cannot capture semantics. But that's why I thought that a clean separation could help to avoid mixing unrelated concepts.

I'd consider all the checks that you described as "conceptual" checks (particularly, I don't see where Stage 1 refers to the schema). For example: In 1.0.0, the componentType cannot be UNSIGNED_INT. In 1.0.1, it can be UNSIGNED_INT. This check solely depends on the schema. But checking the additional conditions that you described does not depend on the schema. Similarly, one could check an accessor against the 1.0.1 schema, and see that it indeed has min and max and both are arrays of numbers with 1...16 elements. Fine. But checking the additional constraints (basically: What is described verbally in gltf_detailedDescription) is a separate step.

However, I see that this separation may not always be trivial, and maybe not even sufficient, maybe not even beneficial in all cases, and regardless of that, it probably makes sense to further split the conceptual checks into "intra-object" and "cross-object" checks.


Some nitpicking side notes (because I think that a validator is a place where nitpicking is appropriate ;-))

total byteOfset (accessor.byteOffset + bufferView.byteOffset) is a multiple of componentType;

I don't think this is the case. The accessor.byteOffset has to be a multiple, but the bufferView.byteOffset can be arbitrary (unless this constraint is somehow related to GL- or ArrayBuffer internals!?).

accessor with componentType: UNSIGNED_INT can reference only bufferView with target: ELEMENT_ARRAY_BUFFER.

I think the constraint is in the other direction: A bufferView with target ELEMENT_ARRAY_BUFFER can only be referenced by an accessor with an integral componentType - and I think, for GL, it has to be an unsigned type, but can be any of UNSIGNED_BYTE, UNSIGNED_SHORT or UNSIGNED_INT.

@lexaknyazev
Copy link
Member Author

One could argue that one main problem here is that the schema simply cannot capture semantics.

If an an asset conforms only to schema, we can't say anything on its validity. So I don't think there is any reason to have schema-only validation.

Currently, schema validation is always the first step of Stage 1. If future spec updates make support of several versions troublesome, such separation could be done.


I don't think this is the case. The accessor.byteOffset has to be a multiple, but the bufferView.byteOffset can be arbitrary (unless this constraint is somehow related to GL- or ArrayBuffer internals!?).

BufferView and Accessor Byte Alignment

I think the constraint is in the other direction: A bufferView with target ELEMENT_ARRAY_BUFFER can only be referenced by an accessor with an integral componentType - and I think, for GL, it has to be an unsigned type, but can be any of UNSIGNED_BYTE, UNSIGNED_SHORT or UNSIGNED_INT.

There're both constraints, because UNSIGNED_INT is allowed only for indices.

@javagl
Copy link

javagl commented Aug 7, 2016

The argument for the schema-only-validation mainly aimed at some sort of "separation of concerns": It could be done mechanically, and would handle changes like a missing accessor.max which is fine for 1.0.0, but should cause an error in 1.0.1. This could be captured without having to manually write explicit code like if (version > 1.0.0) checkForExisting(accessor.max). But of course, there still will have to be the additional checks, as in if (version > 1.0.0) checkForSameSize(accessor.min,accessor.max). The advantage could then just be that, at this point, one already knew that min and max are present and contained numbers (because that was checked against the schema).

(EDIT: Although, this would raise the question of whether the validator would bail out early, or try to eagerly detect all errors)

However, I see that these points are rather high-level design issues, and maybe they can be tackled later (independent of the original question about the versioning).


Regarding the alignment: So this is indeed justified with the use of an ArrayBuffer. Only mentioning this in the text seems to be a bit vague. One could consider adding something like

"gltf_detailedDescription" : "This must be a multiple of the maximum size of any data type of any accessor that refers to this bufferView",

in the bufferView.byteOffset. But this is certainly off-topic for this issue here.

@pjcozzi
Copy link
Member

pjcozzi commented Aug 10, 2016

Regarding the alignment: So this is indeed justified with the use of an ArrayBuffer. Only mentioning this in the text seems to be a bit vague. One could consider adding something like...

Pull requests into the glTF 1.0.1 branch that clarify things are welcome!

If an an asset conforms only to schema, we can't say anything on its validity...

From the end user perspective, I totally agree here. I need to know that my engine or tool is getting validate glTF, not just valid with respect to what the JSON schema can represent. From a validator implementation perspective, totally up to @lexaknyazev.

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

No branches or pull requests

3 participants