From 79060fd3285295cd1e280e60b2d2b8a7d01b1d7e Mon Sep 17 00:00:00 2001 From: Peter Gagliardi Date: Fri, 22 Oct 2021 10:31:07 -0400 Subject: [PATCH 1/2] Handle stringOffsetType and arrayOffssetType from EXT_mesh_features --- Source/Scene/MetadataTableProperty.js | 34 ++++-- .../BoxInstanced/glTF/box-instanced.gltf | 4 +- .../glTF/box-instanced-interleaved.gltf | 4 +- Specs/MetadataTester.js | 21 +++- Specs/Scene/MetadataTablePropertySpec.js | 104 ++++++++++++++++++ 5 files changed, 153 insertions(+), 14 deletions(-) diff --git a/Source/Scene/MetadataTableProperty.js b/Source/Scene/MetadataTableProperty.js index dca4e78928d6..77dc3a2742d8 100644 --- a/Source/Scene/MetadataTableProperty.js +++ b/Source/Scene/MetadataTableProperty.js @@ -12,7 +12,9 @@ import MetadataType from "./MetadataType.js"; /** * A binary property in a {@MetadataTable} *

- * For 3D Tiles Next details, see the {@link https://github.com/CesiumGS/3d-tiles/tree/3d-tiles-next/extensions/3DTILES_metadata|3DTILES_metadata Extension} for 3D Tiles, as well as the {@link https://github.com/CesiumGS/glTF/tree/3d-tiles-next/extensions/2.0/Vendor/EXT_feature_metadata|EXT_feature_metadata Extension} for glTF. + * For 3D Tiles Next details, see the {@link https://github.com/CesiumGS/3d-tiles/tree/3d-tiles-next/extensions/3DTILES_metadata|3DTILES_metadata Extension} + * for 3D Tiles, as well as the {@link https://github.com/CesiumGS/glTF/tree/3d-tiles-next/extensions/2.0/Vendor/EXT_mesh_features|EXT_mesh_features Extension} + * for glTF. For the legacy glTF extension, see {@link https://github.com/CesiumGS/glTF/tree/3d-tiles-next/extensions/2.0/Vendor/EXT_feature_metadata|EXT_feature_metadata Extension} *

* * @param {Object} options Object with the following properties: @@ -53,16 +55,20 @@ function MetadataTableProperty(options) { var hasStrings = valueType === MetadataComponentType.STRING; var hasBooleans = valueType === MetadataComponentType.BOOLEAN; - var offsetType = defaultValue( - MetadataComponentType[property.offsetType], - MetadataComponentType.UINT32 - ); - var arrayOffsets; if (isVariableSizeArray) { + // EXT_mesh_features uses arrayOffsetType, EXT_feature_metadata uses offsetType for both arrays and strings + var arrayOffsetType = defaultValue( + property.arrayOffsetType, + property.offsetType + ); + arrayOffsetType = defaultValue( + MetadataComponentType[arrayOffsetType], + MetadataComponentType.UINT32 + ); arrayOffsets = new BufferView( bufferViews[property.arrayOffsetBufferView], - offsetType, + arrayOffsetType, count + 1 ); } @@ -78,9 +84,18 @@ function MetadataTableProperty(options) { var stringOffsets; if (hasStrings) { + // EXT_mesh_features uses arrayOffsetType, EXT_feature_metadata uses offsetType for both arrays and strings + var stringOffsetType = defaultValue( + property.stringOffsetType, + property.offsetType + ); + stringOffsetType = defaultValue( + MetadataComponentType[stringOffsetType], + MetadataComponentType.UINT32 + ); stringOffsets = new BufferView( bufferViews[property.stringOffsetBufferView], - offsetType, + stringOffsetType, componentCount + 1 ); } @@ -670,6 +685,9 @@ function BufferView(bufferView, componentType, length) { this.dataView = new DataView(typedArray.buffer, typedArray.byteOffset); this.get = getFunction; this.set = setFunction; + + // for unit testing + this.componentType = componentType; } export default MetadataTableProperty; diff --git a/Specs/Data/Models/GltfLoader/BoxInstanced/glTF/box-instanced.gltf b/Specs/Data/Models/GltfLoader/BoxInstanced/glTF/box-instanced.gltf index 659407c8f34d..4b134f5cb0cc 100644 --- a/Specs/Data/Models/GltfLoader/BoxInstanced/glTF/box-instanced.gltf +++ b/Specs/Data/Models/GltfLoader/BoxInstanced/glTF/box-instanced.gltf @@ -42,7 +42,7 @@ "count": 4, "properties": { "name": { - "offsetType": "UINT16", + "stringOffsetType": "UINT16", "bufferView": 7, "stringOffsetBufferView": 8 }, @@ -57,7 +57,7 @@ "count": 2, "properties": { "name": { - "offsetType": "UINT16", + "stringOffsetType": "UINT16", "bufferView": 10, "stringOffsetBufferView": 11 }, diff --git a/Specs/Data/Models/GltfLoader/BoxInstancedInterleaved/glTF/box-instanced-interleaved.gltf b/Specs/Data/Models/GltfLoader/BoxInstancedInterleaved/glTF/box-instanced-interleaved.gltf index 9d5e5a7263bc..0773212df4bd 100644 --- a/Specs/Data/Models/GltfLoader/BoxInstancedInterleaved/glTF/box-instanced-interleaved.gltf +++ b/Specs/Data/Models/GltfLoader/BoxInstancedInterleaved/glTF/box-instanced-interleaved.gltf @@ -41,7 +41,7 @@ "count": 4, "properties": { "name": { - "offsetType": "UINT16", + "stringOffsetType": "UINT16", "bufferView": 4, "stringOffsetBufferView": 5 }, @@ -55,7 +55,7 @@ "count": 2, "properties": { "name": { - "offsetType": "UINT16", + "stringOffsetType": "UINT16", "bufferView": 7, "stringOffsetBufferView": 8 }, diff --git a/Specs/MetadataTester.js b/Specs/MetadataTester.js index 813bbba4cbaf..237ce6128d32 100644 --- a/Specs/MetadataTester.js +++ b/Specs/MetadataTester.js @@ -32,7 +32,11 @@ MetadataTester.createProperty = function (options) { var table = MetadataTester.createMetadataTable({ properties: properties, propertyValues: propertyValues, + // offsetType is for legacy EXT_feature_metadata, arrayOffsetType and + // stringOffsetType are for EXT_mesh_features offsetType: options.offsetType, + arrayOffsetType: options.arrayOffsetType, + stringOffsetType: options.stringOffsetType, enums: options.enums, disableBigIntSupport: options.disableBigIntSupport, disableBigInt64ArraySupport: options.disableBigInt64ArraySupport, @@ -47,6 +51,8 @@ function createProperties(options) { var classId = options.classId; var propertyValues = options.propertyValues; var offsetType = options.offsetType; + var stringOffsetType = options.stringOffsetType; + var arrayOffsetType = options.arrayOffsetType; var bufferViews = defined(options.bufferViews) ? options.bufferViews : {}; var enums = defined(schema.enums) ? schema.enums : {}; @@ -86,16 +92,26 @@ function createProperties(options) { properties[propertyId] = property; + // for legacy EXT_feature_metadata if (defined(offsetType)) { property.offsetType = offsetType; } + if (defined(stringOffsetType)) { + property.stringOffsetType = offsetType; + } + + if (defined(arrayOffsetType)) { + property.arrayOffsetType = arrayOffsetType; + } + if ( classProperty.type === MetadataType.ARRAY && !defined(classProperty.componentCount) ) { + var arrayOffsetBufferType = defaultValue(arrayOffsetType, offsetType); var arrayOffsetBuffer = addPadding( - createArrayOffsetBuffer(values, offsetType) + createArrayOffsetBuffer(values, arrayOffsetBufferType) ); var arrayOffsetBufferView = bufferViewIndex++; bufferViews[arrayOffsetBufferView] = arrayOffsetBuffer; @@ -103,8 +119,9 @@ function createProperties(options) { } if (classProperty.componentType === MetadataComponentType.STRING) { + var stringOffsetBufferType = defaultValue(stringOffsetType, offsetType); var stringOffsetBuffer = addPadding( - createStringOffsetBuffer(values, offsetType) + createStringOffsetBuffer(values, stringOffsetBufferType) ); var stringOffsetBufferView = bufferViewIndex++; bufferViews[stringOffsetBufferView] = stringOffsetBuffer; diff --git a/Specs/Scene/MetadataTablePropertySpec.js b/Specs/Scene/MetadataTablePropertySpec.js index fe8e599c1d98..9afec73972aa 100644 --- a/Specs/Scene/MetadataTablePropertySpec.js +++ b/Specs/Scene/MetadataTablePropertySpec.js @@ -2,6 +2,7 @@ import { defaultValue, Cartesian3, MetadataClassProperty, + MetadataComponentType, MetadataTableProperty, } from "../../Source/Cesium.js"; import MetadataTester from "../MetadataTester.js"; @@ -61,6 +62,109 @@ describe("Scene/MetadataTableProperty", function () { expect(property.extensions).toBe(extensions); }); + it("constructs properties with stringOffset and arrayOffset", function () { + var extras = { + other: 0, + }; + + var extensions = { + EXT_other_extension: {}, + }; + + var a = 97; + var b = 98; + var c = 99; + var d = 100; + var e = 101; + + var property = new MetadataTableProperty({ + count: 2, + property: { + bufferView: 0, + extras: extras, + extensions: extensions, + stringOffsetType: "UINT16", + stringOffsetBufferView: 1, + arrayOffsetType: "UINT8", + arrayOffsetBufferView: 2, + }, + classProperty: new MetadataClassProperty({ + id: "property", + property: { + type: "ARRAY", + componentType: "STRING", + }, + }), + bufferViews: { + 0: new Uint8Array([a, b, b, c, c, c, d, d, d, d, e, e, e, e, e]), + 1: new Uint8Array([0, 0, 1, 0, 3, 0, 6, 0, 10, 0, 15, 0]), + 2: new Uint8Array([0, 3, 5]), + }, + }); + + expect(property.extras).toBe(extras); + expect(property.extensions).toBe(extensions); + expect(property._stringOffsets.componentType).toBe( + MetadataComponentType.UINT16 + ); + expect(property._arrayOffsets.componentType).toBe( + MetadataComponentType.UINT8 + ); + expect(property.get(0)).toEqual(["a", "bb", "ccc"]); + expect(property.get(1)).toEqual(["dddd", "eeeee"]); + }); + + it("constructs property with EXT_feature_metadata offsetType", function () { + var extras = { + other: 0, + }; + + var extensions = { + EXT_other_extension: {}, + }; + + var a = 97; + var b = 98; + var c = 99; + var d = 100; + var e = 101; + + var property = new MetadataTableProperty({ + count: 2, + property: { + bufferView: 0, + extras: extras, + extensions: extensions, + offsetType: "UINT16", + stringOffsetBufferView: 1, + arrayOffsetBufferView: 2, + }, + classProperty: new MetadataClassProperty({ + id: "property", + property: { + type: "ARRAY", + componentType: "STRING", + }, + }), + bufferViews: { + 0: new Uint8Array([a, b, b, c, c, c, d, d, d, d, e, e, e, e, e]), + 1: new Uint8Array([0, 0, 1, 0, 3, 0, 6, 0, 10, 0, 15, 0]), + 2: new Uint8Array([0, 0, 3, 0, 5, 0]), + }, + }); + + expect(property.extras).toBe(extras); + expect(property.extensions).toBe(extensions); + expect(property._stringOffsets.componentType).toBe( + MetadataComponentType.UINT16 + ); + expect(property._arrayOffsets.componentType).toBe( + MetadataComponentType.UINT16 + ); + expect(property.get(0)).toEqual(["a", "bb", "ccc"]); + expect(property.get(1)).toEqual(["dddd", "eeeee"]); + }); + it("constructor throws without count", function () { expect(function () { return new MetadataTableProperty({ From 9abd454ce45f0e57d91b45faaa37e7797c3c261b Mon Sep 17 00:00:00 2001 From: Peter Gagliardi Date: Fri, 22 Oct 2021 16:12:19 -0400 Subject: [PATCH 2/2] PR feedback --- Source/Scene/MetadataTableProperty.js | 4 ++-- Specs/Scene/MetadataTablePropertySpec.js | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/Scene/MetadataTableProperty.js b/Source/Scene/MetadataTableProperty.js index 77dc3a2742d8..739d5ba07579 100644 --- a/Source/Scene/MetadataTableProperty.js +++ b/Source/Scene/MetadataTableProperty.js @@ -84,7 +84,7 @@ function MetadataTableProperty(options) { var stringOffsets; if (hasStrings) { - // EXT_mesh_features uses arrayOffsetType, EXT_feature_metadata uses offsetType for both arrays and strings + // EXT_mesh_features uses stringOffsetType, EXT_feature_metadata uses offsetType for both arrays and strings var stringOffsetType = defaultValue( property.stringOffsetType, property.offsetType @@ -687,7 +687,7 @@ function BufferView(bufferView, componentType, length) { this.set = setFunction; // for unit testing - this.componentType = componentType; + this._componentType = componentType; } export default MetadataTableProperty; diff --git a/Specs/Scene/MetadataTablePropertySpec.js b/Specs/Scene/MetadataTablePropertySpec.js index 9afec73972aa..3bfded8cfcbd 100644 --- a/Specs/Scene/MetadataTablePropertySpec.js +++ b/Specs/Scene/MetadataTablePropertySpec.js @@ -104,10 +104,10 @@ describe("Scene/MetadataTableProperty", function () { expect(property.extras).toBe(extras); expect(property.extensions).toBe(extensions); - expect(property._stringOffsets.componentType).toBe( + expect(property._stringOffsets._componentType).toBe( MetadataComponentType.UINT16 ); - expect(property._arrayOffsets.componentType).toBe( + expect(property._arrayOffsets._componentType).toBe( MetadataComponentType.UINT8 ); expect(property.get(0)).toEqual(["a", "bb", "ccc"]); @@ -155,10 +155,10 @@ describe("Scene/MetadataTableProperty", function () { expect(property.extras).toBe(extras); expect(property.extensions).toBe(extensions); - expect(property._stringOffsets.componentType).toBe( + expect(property._stringOffsets._componentType).toBe( MetadataComponentType.UINT16 ); - expect(property._arrayOffsets.componentType).toBe( + expect(property._arrayOffsets._componentType).toBe( MetadataComponentType.UINT16 ); expect(property.get(0)).toEqual(["a", "bb", "ccc"]);