diff --git a/src/validation/gltfExtensions/ExtStructuralMetadataValidator.ts b/src/validation/gltfExtensions/ExtStructuralMetadataValidator.ts index 304f782e..7d61e0f4 100644 --- a/src/validation/gltfExtensions/ExtStructuralMetadataValidator.ts +++ b/src/validation/gltfExtensions/ExtStructuralMetadataValidator.ts @@ -397,7 +397,6 @@ export class ExtStructuralMetadataValidator { } } - // TODO - WIP return result; } } diff --git a/src/validation/gltfExtensions/GltfDataReader.ts b/src/validation/gltfExtensions/GltfDataReader.ts index 0348f95a..4ffc1112 100644 --- a/src/validation/gltfExtensions/GltfDataReader.ts +++ b/src/validation/gltfExtensions/GltfDataReader.ts @@ -155,18 +155,7 @@ export class GltfDataReader { ): Promise { 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 diff --git a/src/validation/gltfExtensions/ImageData.ts b/src/validation/gltfExtensions/ImageData.ts index 3d461620..8c93a3cc 100644 --- a/src/validation/gltfExtensions/ImageData.ts +++ b/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[]; diff --git a/src/validation/gltfExtensions/PropertyAttributePropertyModel.ts b/src/validation/gltfExtensions/PropertyAttributePropertyModel.ts index 3525b512..302cc764 100644 --- a/src/validation/gltfExtensions/PropertyAttributePropertyModel.ts +++ b/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. */ diff --git a/src/validation/gltfExtensions/PropertyAttributePropertyValidator.ts b/src/validation/gltfExtensions/PropertyAttributePropertyValidator.ts index ff07390b..1167ebd1 100644 --- a/src/validation/gltfExtensions/PropertyAttributePropertyValidator.ts +++ b/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, diff --git a/src/validation/gltfExtensions/PropertyAttributeValuesValidator.ts b/src/validation/gltfExtensions/PropertyAttributeValuesValidator.ts index 43d9b665..2cf20d3f 100644 --- a/src/validation/gltfExtensions/PropertyAttributeValuesValidator.ts +++ b/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; - } } } diff --git a/src/validation/gltfExtensions/PropertyTextureBooleanMetadataPropertyModel.ts b/src/validation/gltfExtensions/PropertyTextureBooleanMetadataPropertyModel.ts index a408086f..6ea76c21 100644 --- a/src/validation/gltfExtensions/PropertyTextureBooleanMetadataPropertyModel.ts +++ b/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, diff --git a/src/validation/gltfExtensions/PropertyTexturePropertyValidator.ts b/src/validation/gltfExtensions/PropertyTexturePropertyValidator.ts index 4a0f5866..97e2b033 100644 --- a/src/validation/gltfExtensions/PropertyTexturePropertyValidator.ts +++ b/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; } diff --git a/src/validation/gltfExtensions/PropertyTextureValidator.ts b/src/validation/gltfExtensions/PropertyTextureValidator.ts index 9c74ba84..2b60e0cb 100644 --- a/src/validation/gltfExtensions/PropertyTextureValidator.ts +++ b/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 */ diff --git a/src/validation/gltfExtensions/PropertyTextureValuesValidator.ts b/src/validation/gltfExtensions/PropertyTextureValuesValidator.ts index 8841de04..34b27e98 100644 --- a/src/validation/gltfExtensions/PropertyTextureValuesValidator.ts +++ b/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 diff --git a/src/validation/metadata/BinaryPropertyTableValidator.ts b/src/validation/metadata/BinaryPropertyTableValidator.ts index c7d81699..dc255251 100644 --- a/src/validation/metadata/BinaryPropertyTableValidator.ts +++ b/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 diff --git a/src/validation/metadata/MetadataPropertyValuesValidator.ts b/src/validation/metadata/MetadataPropertyValuesValidator.ts index 4a534074..5df16b82 100644 --- a/src/validation/metadata/MetadataPropertyValuesValidator.ts +++ b/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