Cleanups...

- removed obsolete TODOs
- addressed minor open TODOs
- minor TSDoc fixes
- minor fixes for typos in validation messages

src/validation/gltfExtensions/ExtStructuralMetadataValidator.ts

@@ -397,7 +397,6 @@ export class ExtStructuralMetadataValidator {
       }
     }
 
-    // TODO - WIP
     return result;
   }
 }

src/validation/gltfExtensions/GltfDataReader.ts

@@ -155,18 +155,7 @@ export class GltfDataReader {
   ): Promise<Document | undefined> {
     try {
       const io = await GltfTransform.getIO();
-      const doc = await io.readBinary(input);
-
-      // TODO This obscure line avoids the error
-      // > Type 'import("...3d-tiles-tools.......").Document' is not assignable to
-      // > type 'import("...3d-tiles-validator...").Document'.
-      // >   Types have separate declarations of a private property '_graph'.ts(2322)
-      // that is probably caused by using the local, file-based,
-      // non-npm version of the 3d-tiles-tools. Verify that this
-      // works without this line when using the proper npm
-      // dependency to the tools!
-      const gltfDocument = doc as any as Document;
-
+      const gltfDocument = await io.readBinary(input);
       return gltfDocument;
     } catch (error) {
       // This may happen when the glTF is invalid. The exact reason should
@@ -249,18 +238,7 @@ export class GltfDataReader {
       const io = await GltfTransform.getIO();
       const json = JSON.parse(input.toString());
       const jsonDoc = { json, resources } as JSONDocument;
-      const doc = await io.readJSON(jsonDoc);
-
-      // TODO This obscure line avoids the error
-      // > Type 'import("...3d-tiles-tools.......").Document' is not assignable to
-      // > type 'import("...3d-tiles-validator...").Document'.
-      // >   Types have separate declarations of a private property '_graph'.ts(2322)
-      // that is probably caused by using the local, file-based,
-      // non-npm version of the 3d-tiles-tools. Verify that this
-      // works without this line when using the proper npm
-      // dependency to the tools!
-
-      const gltfDocument = doc as any as Document;
+      const gltfDocument = await io.readJSON(jsonDoc);
       return gltfDocument;
     } catch (error) {
       // This may happen when the glTF is invalid. The exact reason should

src/validation/gltfExtensions/ImageData.ts

@@ -23,7 +23,7 @@ export interface ImageData {
   /**
    * The pixels.
    *
-   * The channel `c` of the pixel at `x`, `y` is ndexed
+   * The channel `c` of the pixel at `x`, `y` is indexed
    * by `index = ((y * sizeX) + x) * channels) + c`
    */
   pixels: number[];

src/validation/gltfExtensions/PropertyAttributePropertyModel.ts

@@ -3,7 +3,7 @@ import { MetadataValues } from "3d-tiles-tools";
 import { MetadataPropertyModel } from "../metadata/MetadataPropertyModel";
 
 /**
- * Implementation of a metadata property model that for a
+ * Implementation of a metadata property model for a
  * property attribute property, backed by glTF accessor
  * data.
  */

src/validation/gltfExtensions/PropertyAttributePropertyValidator.ts

@@ -49,7 +49,7 @@ export class PropertyAttributePropertyValidator {
     // The attribute MUST be defined
     // The attribute MUST be a string
     const attribute = propertyAttributeProperty.attribute;
-    const attributePath = path + "/atribute";
+    const attributePath = path + "/attribute";
     if (
       !BasicValidator.validateString(
         attributePath,

src/validation/gltfExtensions/PropertyAttributeValuesValidator.ts

@@ -26,9 +26,6 @@ import { RangeIterables } from "../metadata/RangeIterables";
  * by a `PropertyAttributeValidator`.
  *
  * @internal
- *
- * TODO There is a lot of "structural" overlap between this and
- * other classes - see PropertyTextureValuesValidator
  */
 export class PropertyAttributeValuesValidator {
   /**
@@ -128,36 +125,28 @@ export class PropertyAttributeValuesValidator {
           path + "/properties/" + propertyName;
         const metadataClassName = propertyAttribute.class;
 
+        // Assuming structural validity for the classProperty
         const classProperty = MetadataValidationUtilities.computeClassProperty(
           schema,
           metadataClassName,
           propertyName
-        );
-        if (!classProperty) {
-          const message =
-            `Could not obtain class property for property ` +
-            `${propertyName} of class ${classProperty}`;
-          const issue = ValidationIssues.INTERNAL_ERROR(path, message);
-          context.addIssue(issue);
+        )!;
+        const propertyValuesValid =
+          await PropertyAttributeValuesValidator.validatePropertyAttributePropertyValues(
+            propertyAttributePropertyPath,
+            propertyName,
+            propertyAttributeProperty,
+            meshPrimitive,
+            meshIndex,
+            primitiveIndex,
+            schema,
+            metadataClassName,
+            classProperty,
+            gltfData,
+            context
+          );
+        if (!propertyValuesValid) {
           result = false;
-        } else {
-          const propertyValuesValid =
-            await PropertyAttributeValuesValidator.validatePropertyAttributePropertyValues(
-              propertyAttributePropertyPath,
-              propertyName,
-              propertyAttributeProperty,
-              meshPrimitive,
-              meshIndex,
-              primitiveIndex,
-              schema,
-              metadataClassName,
-              classProperty,
-              gltfData,
-              context
-            );
-          if (!propertyValuesValid) {
-            result = false;
-          }
         }
       }
     }
@@ -240,30 +229,24 @@ export class PropertyAttributeValuesValidator {
 
     // Perform the checks that only apply to ENUM types,
     if (classProperty.type === "ENUM") {
+      // Assuming structural validity for the validEnumValueValues
       const validEnumValueValues =
         MetadataValidationUtilities.computeValidEnumValueValues(
           schema,
           metadataClassName,
           propertyName
-        );
-      if (!validEnumValueValues) {
-        const message = `Could not read valid enum values for property`;
-        const issue = ValidationIssues.INTERNAL_ERROR(path, message);
-        context.addIssue(issue);
+        )!;
+      if (
+        !MetadataPropertyValuesValidator.validateEnumValues(
+          path,
+          propertyName,
+          keys,
+          metadataPropertyModel,
+          validEnumValueValues,
+          context
+        )
+      ) {
         result = false;
-      } else {
-        if (
-          !MetadataPropertyValuesValidator.validateEnumValues(
-            path,
-            propertyName,
-            keys,
-            metadataPropertyModel,
-            validEnumValueValues,
-            context
-          )
-        ) {
-          result = false;
-        }
       }
     }
 

src/validation/gltfExtensions/PropertyTextureBooleanMetadataPropertyModel.ts

@@ -36,13 +36,6 @@ export class PropertyTextureBooleanMetadataPropertyModel
    * @param imageData - The image data
    * @param propertyTextureProperty - The property texture property
    * @param classProperty - The class property
-   * @param enumValueType - The `valueType` of the enum type of
-   * the given class property (or undefined if the class property
-   * is not an ENUM)
-   * @param valueValueNames - The mapping from enum value values
-   * to enum value names for the enum type of the given class
-   * property (or an empty dictionary when the class property is
-   * not an ENUM)
    */
   constructor(
     imageData: ImageData,

src/validation/gltfExtensions/PropertyTexturePropertyValidator.ts

@@ -188,7 +188,7 @@ export class PropertyTexturePropertyValidator {
    * Validates that the given number of channels matches the
    * number of bytes of the type of the given `classProperty`.
    *
-   * The given number is either the length of the `channels` array of
+   * The given number is the length of the `channels` array of
    * a property texture property (if it is defined). If the
    * `channels` array is not defined, then this value should be
    * `1`, because the default value for the `channels` is `[0]`.
@@ -232,7 +232,7 @@ export class PropertyTexturePropertyValidator {
         const totalByteSize = classProperty.count! * byteSize;
         if (totalByteSize !== numberOfChannels) {
           const message =
-            `The property '${propertyName}' is has the enum type ` +
+            `The property '${propertyName}' has the enum type ` +
             `${enumType} with a value type of ${enumValueType} which ` +
             `consists of ${byteSize} bytes, and the property is an ` +
             `array with ${classProperty.count} elements, resulting in ` +
@@ -250,7 +250,7 @@ export class PropertyTexturePropertyValidator {
         // Handle properties that are single enums
         if (byteSize !== numberOfChannels) {
           const message =
-            `The property '${propertyName}' is has the enum type ` +
+            `The property '${propertyName}' has the enum type ` +
             `${enumType} with a value type of ${enumValueType}, which ` +
             `consists of ${byteSize} bytes, but the number of channels ` +
             `in the property texture property was ${numberOfChannels}`;
@@ -267,12 +267,37 @@ export class PropertyTexturePropertyValidator {
       return true;
     }
 
-    // TODO Special handling for `BOOLEAN` required here!
+    // When the type is ENUM, compute the size based on the
+    // number of required bits
+    const type = classProperty.type;
+    if (type === "BOOLEAN") {
+      if (classProperty.array === true) {
+        // Handle BOOLEAN properties that are arrays
+        const count = classProperty.count!;
+        const totalByteSize = Math.ceil(count / 8);
+        if (totalByteSize !== numberOfChannels) {
+          const message =
+            `The property '${propertyName}' is has the type 'BOOLEAN' and it ` +
+            `is an array with ${count} elements, resulting in a total number of ` +
+            `ceil(${count}/8) = ${totalByteSize} bytes, but the number of channels ` +
+            `in the property texture property was ${numberOfChannels}`;
+          const issue =
+            GltfExtensionValidationIssues.TEXTURE_CHANNELS_OUT_OF_RANGE(
+              path,
+              message
+            );
+          context.addIssue(issue);
+          return false;
+        }
+      }
+      // For BOOLEAN properties that are not arrays, even a single
+      // channel is sufficient
+      return true;
+    }
 
-    // For non-enum types, compute the required size based
-    // on the number of bytes per component, and the number
+    // For non-ENUM and non-BOOLEAN types, compute the required size
+    // based on the number of bytes per component, and the number
     // of components per element
-    const type = classProperty.type;
     const componentType = classProperty.componentType;
     const componentCount = MetadataTypes.componentCountForType(type);
     const componentByteSize = MetadataComponentTypes.byteSizeForComponentType(
@@ -285,7 +310,7 @@ export class PropertyTexturePropertyValidator {
       const totalByteSize = classProperty.count! * elementByteSize;
       if (totalByteSize !== numberOfChannels) {
         const message =
-          `The property '${propertyName}' is has the component type ` +
+          `The property '${propertyName}' has the component type ` +
           `${componentType}, with a size of ${componentByteSize} bytes, ` +
           `and the type ${type} with ${componentCount} components, ` +
           `resulting in ${elementByteSize} bytes per element, and it ` +
@@ -304,7 +329,7 @@ export class PropertyTexturePropertyValidator {
       // Handle properties that are not arrays
       if (elementByteSize !== numberOfChannels) {
         const message =
-          `The property '${propertyName}' is has the component type ` +
+          `The property '${propertyName}' has the component type ` +
           `${componentType}, with a size of ${componentByteSize} bytes, ` +
           `and the type ${type} with ${componentCount} components, ` +
           `resulting in ${elementByteSize} bytes per element, but ` +
@@ -356,7 +381,6 @@ export class PropertyTexturePropertyValidator {
           message
         );
       context.addIssue(issue);
-
       return false;
     }
 

src/validation/gltfExtensions/PropertyTextureValidator.ts

@@ -22,7 +22,8 @@ import { PropertyTexturePropertyValidator } from "./PropertyTexturePropertyValid
  * a property texture has to start at the mesh primitive
  * that refers to the property texture, because it requires
  * knowledge about the attributes (texture coordinates)
- * that are defined in the referring mesh primitive.
+ * that are defined in the referring mesh primitive, and
+ * the glTF texture that the definition refers to.
  *
  * @internal
  */

src/validation/gltfExtensions/PropertyTextureValuesValidator.ts

@@ -25,17 +25,6 @@ import { MetadataValidationUtilities } from "../metadata/MetadataValidationUtili
  * by a `PropertyTextureValidator`.
  *
  * @internal
- *
- * TODO There is a lot of "structural" overlap between this and
- * the BinaryPropertyTableValuesValidator: They both check the
- * enum values, min/max, and the main difference is that the
- * values are once fetched from a "BinaryPropertyTable", and
- * once from a "PropertyTexturePropertyModel". Whether or not
- * it is worthwhile to try and extract the common parts
- * (considering that they are once accessed with indices, and
- * once with pixel coordinates, and these different ways of
- * accessign the data will affect the validation issue messages)
- * has to be decided.
  */
 export class PropertyTextureValuesValidator {
   /**
@@ -49,7 +38,8 @@ export class PropertyTextureValuesValidator {
    *
    * It assumes that they are structurally valid, and ONLY checks the
    * validity of the values in the context of the mesh primitive
-   * that refers to the property texture.
+   * that refers to the property texture, and the glTF texture
+   * that the property texture refers to.
    *
    * @param path - The path for the `ValidationIssue` instances
    * @param propertyTextureIndex - The index that was given in the

src/validation/metadata/BinaryPropertyTableValidator.ts

@@ -241,11 +241,13 @@ export class BinaryPropertyTableValidator {
       propertyId,
       binaryPropertyTable
     );
-    const componentTypeByteSize =
-      MetadataComponentTypes.byteSizeForComponentType(componentType);
-    let expectedByteLength = numValues * componentTypeByteSize;
+    let expectedByteLength;
     if (type === "BOOLEAN") {
-      expectedByteLength = Math.ceil(numValues / 8) * componentTypeByteSize;
+      expectedByteLength = Math.ceil(numValues / 8);
+    } else {
+      const componentTypeByteSize =
+        MetadataComponentTypes.byteSizeForComponentType(componentType);
+      expectedByteLength = numValues * componentTypeByteSize;
     }
 
     // Validate that the length of the 'values' buffer

src/validation/metadata/MetadataPropertyValuesValidator.ts

@@ -158,8 +158,8 @@ export class MetadataPropertyValuesValidator {
         result = false;
       } else {
         // When none of the values is smaller than the minimum from
-        // the PropertyTextureProperty, make sure that this minimum
-        // matches the computed minimum of all metadata values
+        // the property, make sure that this minimum matches the
+        // computed minimum of all metadata values
         const computedMin = MetadataPropertyValuesValidator.computeMin(
           keys,
           metadataPropertyModel
@@ -220,8 +220,8 @@ export class MetadataPropertyValuesValidator {
         result = false;
       } else {
         // When none of the values is greater than the maximum from
-        // the PropertyTextureProperty, make sure that this maximum
-        // matches the computed maximum of all metadata values
+        // the property, make sure that this maximum matches the
+        // computed maximum of all metadata values
         const computedMax = MetadataPropertyValuesValidator.computeMax(
           keys,
           metadataPropertyModel