glTF extension validation

[javagl]

It should be possible to validate the EXT_mesh_features and EXT_structural_metadata extensions.

These are glTF extensions, and thus, somewhat unrelated to 3D Tiles. One could make a case for implementing the validation in the glTF-Validator. But after internal considerations, the first implementation will happen in the 3d-tiles-validator.

(Reasons for that are, roughly: The extensions are relatively complex. They are not ratified. The glTF-Validator is using a different language. It is not clear whether the validation can be implemented in the glTF-Validator without considerable refactorings. The 3d-tiles-validator already contains much of the infrastructure for validating 3D Metadata. This particularly refers to the binary representation of metadata - and the 3D Metadata is supposed to be structurally the same (or at least very similar) for 3D Tiles and glTF after all...)

---

This PR is currently a DRAFT. There are some details to be sorted out, but also some higher-level considerations. The following it therefore only a short summary of things that have to be addressed:

• There has to be a mechanism for registering the glTF extension validators.
  In the current state (a first draft), there is just a GltfExtensionValidators class that is called at the end of the GltfValidator, and calls the actual extension validators (which is only a first draft of an ExtMeshFeaturesValidator right now).
  The 3D Tiles validator already offers a mechanism for "extension validators". But this only refers to 3D Tiles extensions (for example, for validating the 3DTILES_bounding_volume_S2 extension). It operates on the RootProperty type. But this only defines the extras/extensions. This type, however, is structurally equal to the gltfProperty. So ... some of this infrastructure could actually be used for generalizing the glTF extension validation...
• Talking about types: The 3d-tiles-validator generally operates on the types that are defined in the 3d-tiles-tools with the structure classes.

  An aside: There is no "built-in" type checking! Even with a const tileset : Tileset = JSON.parse(data) as Tileset, the resulting object is essentially of type any (so each validation function still has to do the lowest-level, nitty-gritty JSON-level type checks, e.g. whether tileset.asset.version is in fact a string or a number...)

There are no types for the glTF structures (i.e. there is no Accessor or Mesh type). So all the validation functions have to operate on any for now. Some minor tweaks will be necessary in the 3d-tiles-tools. These are tracked in CesiumGS/3d-tiles-tools#70

---

The current state (which is a first DRAFT) contains first test cases for EXT_mesh_features validation. Running the GltfExtensionValidators directly on some of the first spec files generates reports like this;

Validation result for ./specs/data/gltfExtensions/meshFeatures/FeatureIdAttributeFeatureCountInvalidValue.gltf:
{
  "date": "2023-09-16T13:27:58.792Z",
  "numErrors": 1,
  "numWarnings": 0,
  "numInfos": 0,
  "issues": [
    {
      "type": "VALUE_NOT_IN_RANGE",
      "path": "./specs/data/gltfExtensions/meshFeatures/FeatureIdAttributeFeatureCountInvalidValue.gltf/featureIds/0/featureCount",
      "message": "The 'featureCount' property must be greater than or equal to 1, but is -12345",
      "severity": "ERROR"
    }
  ]
}
Valid? false

[javagl]

The current state should largely cover the validation of EXT_mesh_features.

It validates the constraints that are established on the JSON level and the specification text. It also performs some validation of "binary" data, insofar that for feature ID attributes and textures, it obtains the set of feature ID values, and checks whether they match the declared featureCount. It does not yet validate the propertyTable, because that is a connection to EXT_structural_metadata which is not implemented yet.

Note that the current state does not yet have any specs. I locally have a list of glTF assets, where the names already give a hint about the features that are checked:

FeatureIdAttributeAccessorNormalized.gltf
FeatureIdAttributeAccessorNotScalar.gltf
FeatureIdAttributeAttributeInvalidType.gltf
FeatureIdAttributeAttributeInvalidValue.gltf
FeatureIdAttributeFeatureCountInvalidType.gltf
FeatureIdAttributeFeatureCountInvalidValue.gltf
FeatureIdAttributeFeatureCountMismatch.gltf
FeatureIdAttributeFeatureCountMissing.gltf
FeatureIdAttributeLabelInvalidType.gltf
FeatureIdAttributeLabelInvalidValue.gltf
FeatureIdAttributeNullFeatureIdInvalidType.gltf
FeatureIdAttributeNullFeatureIdInvalidValue.gltf
FeatureIdTextureFeatureCountMismatch.gltf
FeatureIdTextureSamplerInvalidFilterMode.gltf
FeatureIdTextureTextureChannelsInvalidElementType.gltf
FeatureIdTextureTextureChannelsInvalidType.gltf
FeatureIdTextureTextureChannelsTooManyChannels.gltf
FeatureIdTextureTextureChannelsTooManyElements.gltf
FeatureIdTextureTextureImageDataInvalid.gltf
FeatureIdTextureTextureIndexInvalidType.gltf
FeatureIdTextureTextureIndexInvalidValue.gltf
FeatureIdTextureTextureInvalidType.gltf
FeatureIdTextureTextureTexCoordInvalidAccessorComponentType.gltf
FeatureIdTextureTextureTexCoordInvalidAccessorNormalization.gltf
FeatureIdTextureTextureTexCoordInvalidAccessorType.gltf
FeatureIdTextureTextureTexCoordInvalidType.gltf
FeatureIdTextureTextureTexCoordInvalidValue.gltf
ValidFeatureIdAttribute.gltf
ValidFeatureIdAttributeWithByteStride.glb
ValidFeatureIdTexture.glb
ValidFeatureIdTexture.gltf
ValidFeatureIdTextureUsingDefaultChannels.gltf

(further ones may still be added).

The specs for the tileset-level validation currently have the pattern

  it("detects issues in assetVersionInvalidType", async function () {
    const result = await Validators.validateTilesetFile(
      "specs/data/tilesets/assetVersionInvalidType.json"
    );
    expect(result.length).toEqual(1);
    expect(result.get(0).type).toEqual("TYPE_MISMATCH");
  });

I'll try to achieve a same pattern for the glTF tests, by essentially wrapping the glTF assets (files) into a tileset object at runtime. (The point is that I don't want to create "dummy" tileset.json files for each glTF asset...).

Some rather internal notes/TODOs:

• Validate featureId.propertyTable
• Move "extractBinaryFromGlb" into 3d-tiles-tools (+corresponding cleanups!)
• Can feature IDs be in sparse accessors?

The last one may have to be discussed separately.

[javagl]

The last commits should largely cover the validation of the EXT_mesh_features.

I have opened a PR at CesiumGS/3d-tiles-tools#70 that contains minor changes to the 3d-tiles-tools that are required for this PR, and that will have to be part of an updated release of the 3d-tiles-tools before merging this one.

There are some TODOs related to that, mainly revolving around one issue that has to be sorted out:

Right now, the validation can work on .glb files and on embedded .gltf files (and the latter are used for the specs). It does not yet work for 'default' .gltf files (which may refer to external buffers or textures). This is currently marked as a TODO. From a practical perspective: Nearly all tile contents will be .GLB files (and not .gltf ones with external resources), and there are different options for dealing with that (ignoring it, reporting it as a WARNING, waiting for an update in the downstream dependencies, implementing some workaround...), but it is not clear which way to go here.

[javagl]

The last commits covered most of the EXT_structural_metadata validation, but still in a "draft" state. They include the validation of property textures and property attributes, both in terms of structure and values. The structure validation refers to the basic JSON structure validity. The value validation refers to the validation of the actual metadata values, in view of the min/max definitions and valid ENUM values.

The latter is... a bit of a mess right now, and I intend to clean that up. For the property textures and property attributes (and for the binary property tables, FWIW), the validation follows the same pattern:

    // Perform the checks that only apply to ENUM types,
    if (classProperty.type === "ENUM") {
      if (!...) {
        result = false;
      }
    }

    // Perform the checks that only apply to numeric types
    if (ClassProperties.hasNumericType(classProperty)) {
      // When the ClassProperty defines a minimum, then the metadata
      // values MUST not be smaller than this minimum
      if (defined(classProperty.min)) {
        if (!...) {
          result = false;
        }
      }

      // When the PropertyAttributeProperty defines a minimum, then the metadata
      // values MUST not be smaller than this minimum
      if (defined(propertyAttributeProperty.min)) {
        if (!...) {
          result = false;
        } else {
          // When none of the values is smaller than the minimum from
          // the PropertyAttributeProperty, make sure that this minimum
          // matches the computed minimum of all metadata values
          if (!...) {
            result = false;
          }
        }
      }
      
      // (The same for "max"...)
    }

This is currently implemented as hundreds of structurally similar, complex, repetitive lines in the ...ValuesValidator classes.

The difficulties for simplifying that are revolving around...

• The different types that may or may not be valid. Property Tables support everything. Property Textures support only few types. Property Attributes are somewhere in-between
• The different ways of accessing the data. One could try to find a common abstraction for Property Tables and Property Attributes, as in propertyValue = magicSource.getValue(index);. But ...
  • For details of the validation, it is necessary to access the "raw" values and the "processed" values, meaning that they should either be numeric values or string values (for ENUM), or values that do or do not include the offset/scale defintions (for numeric values)
  • Property Textures are not accessed with a single index, but with (x,y) pixel coordinates
• I'd like to create helpful messages, and the contexts are different....

The latter refers to the fact that, for example, a mismatch of the given min and the computed minimum value could just be reported as

"Something doesn't add up there..."

But I'm currently trying to create messages that clearly and specifically say what exactly is wrong. A current example is

For property 'intensity', the property attribute property defines a minimum of 0.3, but the computed minimum value for attribute _INTENSITY of primitive 1 of mesh 0 is 0.5

The attribute name and mesh/primitive indices have to be carried along, but this "context" is different for each type of metadata source.

There are some obvious and simple cleanups for the current state, but I'll look for further possibilities for implementing this "nicely".

---

Another open point is that of the specs. The EXT_mesh_features should largely be covered. For EXT_structural_metadata, I already have ~30 specs for the structural validation. But for this (and particulary for the values validation in this case), there are many degrees of freedom. For example, I think that there's nothing that forbids storing a 23-element fixed length BOOLEAN array in channels [3,0,1] of a property texture 🥴 I'll start with some of the more common configurations, though...

[javagl]

The last commits addressed the issue of the repetitive code for the validation of metadata values against the min/max that may have been defined in the ClassProperty or in the respective metadata property definition.

The summary is:

• There now is a MetadataPropertyModel interface that has two methods:

  • getRawPropertyValue returns the "raw" value, without normalization/offset/scale
  • getPropertyValue returns the final value, including normalization/offset/scale

  In most "real" applications, only the latter would be required. But the validator needs both.

• These methods receive an index : number in most cases. This is the index (row) of the property table, or the index into the accessor of property attributes. But for property textures, it receives pixelCoordinates: [number, number].

• From whatever the source of the metadata values is, a MetadataPropertyModel can be created.

  • Some of the implementations are quite a mouthful, like PropertyTextureEnumMetadataPropertyModel. Maybe a shorter name can be found, but it is a model for a metadata property that was created for an enum-typed property texture property, so...

• The calling code also creates the "keys" for accessing this model. This is just an iterable over either the row indices (number), or the pixel coordinates ([number, number]).

• All this is passed to the MetadataPropertyValuesValidator, which contains the code that was summarized in the previous comment - but this class contains this code only once.

---

To keep track of this: Some issue have been discovered in the validator during this PR.

• There had been some confusion about the numeric-vs-string representations of ENUM values. The root of this was located in the tools, and is tracked and addressed in Binary representations of enum values are not translated 3d-tiles-tools#71 . The corresponding fix for the validation was done in 2f4d63c
• The validator erroneously validated the min/max/offset/scale properties against the range of the underlying data type. For example, it reported an error for a property with type UINT8/normalized and a max of 1000.0, claiming that the 1000.0 was out of range of the UINT8. As of 7daf2ba , only the noData and default values are validated against these ranges
• The validator did not check the ranges of property values that had been given directly in the JSON representation of metadata entities against the min/max that had been declared in the Class Property. This was fixed via db22c1f , with the specs being added in 32257a7
• There was an await missing when tracking the "found extension". This might have caused a race condition, but has been fixed in f719aaa in any case

[javagl]

The last commit added the validations that are specific to the combination of both extensions, EXT_mesh_features and EXT_structural_metadata. Specifically, when a feature ID (attribute or texture) refers to a propertyTable, then ...

• there must be an EXT_structural_metadata extension object that defines this property table and its count
• the values of the feature ID attribute/texture must be in the range [0, propertyTable.count].

---

^{EDIT: The following originally talked about "matching" values. This is not what the validation should do. Edited to be more precise:}

There is one (minor) issue to be sorted out: Currently, the validator assumes that the featureCount of a feature ID defines the number of different features. For example, when an accessor has 10 different values, then the featureCount is required to be at least 10. But... it seems like the presence of a nullFeatureId might change this. It will probably have to be changed so that...

• when there is no nullFeatureId, then the number of values may not be larger than the featureIdCount
• when there is a nullFeatureId, then the nullFeatureId should be removed from the values that are found, and afterwards the number of values may not be larger than the featureIdCount

(to be confirmed)

---

Apart from that, this PR could largely be ready for review, but before it can be merged, it will be necessary to merge the PR for CesiumGS/3d-tiles-tools#70 and publish the new tools version (right now, the package.json here refers to a local file...).

[javagl]

I did a "proofreading"/cleanup pass

• removed obsolete TODOs
• addressed minor open TODOs (e.g. stemming from the intermediate state where the 3d-tiles-tools had been a local file dependency)
• minor TSDoc fixes
• minor fixes for typos in validation messages

There are some cleanups that I'd still like to do. For example: The current state of the validator code (as a whole) often uses the defined/defaultValue functions that originate from CesiumJS. In the context of TypeScript, probably all uses of defaultValue could be replaced by the Null Coalescing Operator (??). But this will be part of another PR.