Merge pull request #15084 from donmccurdy/bug-draco-primitive-comparison

GLTFLoader + DRACOLoader: Fix bugs in glTF+Draco decompression.

examples/js/loaders/DRACOLoader.js

@@ -84,18 +84,19 @@ THREE.DRACOLoader.prototype = {
     },
 
     /**
-     * |attributeUniqueIdMap| specifies attribute unique id for an attribute in
-     * the geometry to be decoded. The name of the attribute must be one of the
-     * supported attribute type in Three.JS, including:
-     *     'position',
-     *     'color',
-     *     'normal',
-     *     'uv',
-     *     'uv2',
-     *     'skinIndex',
-     *     'skinWeight'.
-     * The format is:
-     *     attributeUniqueIdMap[attributeName] = attributeId
+     * Decompresses a Draco buffer. Names of attributes (for ID and type maps)
+     * must be one of the supported three.js types, including: position, color,
+     * normal, uv, uv2, skinIndex, skinWeight.
+     *
+     * @param {ArrayBuffer} rawBuffer
+     * @param {Function} callback
+     * @param {Object|undefined} attributeUniqueIdMap Provides a pre-defined ID
+     *     for each attribute in the geometry to be decoded. If given,
+     *     `attributeTypeMap` is required and `nativeAttributeMap` will be
+     *     ignored.
+     * @param {Object|undefined} attributeTypeMap Provides a predefined data
+     *     type (as a typed array constructor) for each attribute in the
+     *     geometry to be decoded.
      */
     decodeDracoFile: function(rawBuffer, callback, attributeUniqueIdMap,
                               attributeTypeMap) {
@@ -103,7 +104,7 @@ THREE.DRACOLoader.prototype = {
       THREE.DRACOLoader.getDecoderModule()
           .then( function ( module ) {
             scope.decodeDracoFileInternal( rawBuffer, module.decoder, callback,
-              attributeUniqueIdMap || {}, attributeTypeMap || {});
+              attributeUniqueIdMap, attributeTypeMap);
           });
     },
 
@@ -231,6 +232,7 @@ THREE.DRACOLoader.prototype = {
     convertDracoGeometryTo3JS: function(dracoDecoder, decoder, geometryType,
                                         buffer, attributeUniqueIdMap,
                                         attributeTypeMap) {
+        // TODO: Should not assume native Draco attribute IDs apply.
         if (this.getAttributeOptions('position').skipDequantization === true) {
           decoder.SkipAttributeTransform(dracoDecoder.POSITION);
         }
@@ -278,6 +280,7 @@ THREE.DRACOLoader.prototype = {
         }
 
         // Verify if there is position attribute.
+        // TODO: Should not assume native Draco attribute IDs apply.
         var posAttId = decoder.GetAttributeId(dracoGeometry,
                                               dracoDecoder.POSITION);
         if (posAttId == -1) {
@@ -294,11 +297,23 @@ THREE.DRACOLoader.prototype = {
         // Import data to Three JS geometry.
         var geometry = new THREE.BufferGeometry();
 
-        // Add native Draco attribute type to geometry.
-        for (var attributeName in this.nativeAttributeMap) {
-          // The native attribute type is only used when no unique Id is
-          // provided. For example, loading .drc files.
-          if (attributeUniqueIdMap[attributeName] === undefined) {
+        // Do not use both the native attribute map and a provided (e.g. glTF) map.
+        if ( attributeUniqueIdMap ) {
+
+          // Add attributes of user specified unique id. E.g. GLTF models.
+          for (var attributeName in attributeUniqueIdMap) {
+            var attributeType = attributeTypeMap[attributeName];
+            var attributeId = attributeUniqueIdMap[attributeName];
+            var attribute = decoder.GetAttributeByUniqueId(dracoGeometry,
+                                                           attributeId);
+            this.addAttributeToGeometry(dracoDecoder, decoder, dracoGeometry,
+                attributeName, attributeType, attribute, geometry, geometryBuffer);
+          }
+
+        } else {
+
+          // Add native Draco attribute type to geometry.
+          for (var attributeName in this.nativeAttributeMap) {
             var attId = decoder.GetAttributeId(dracoGeometry,
                 dracoDecoder[this.nativeAttributeMap[attributeName]]);
             if (attId !== -1) {
@@ -310,16 +325,7 @@ THREE.DRACOLoader.prototype = {
                   attributeName, Float32Array, attribute, geometry, geometryBuffer);
             }
           }
-        }
 
-        // Add attributes of user specified unique id. E.g. GLTF models.
-        for (var attributeName in attributeUniqueIdMap) {
-          var attributeType = attributeTypeMap[attributeName] || Float32Array;
-          var attributeId = attributeUniqueIdMap[attributeName];
-          var attribute = decoder.GetAttributeByUniqueId(dracoGeometry,
-                                                         attributeId);
-          this.addAttributeToGeometry(dracoDecoder, decoder, dracoGeometry,
-              attributeName, attributeType, attribute, geometry, geometryBuffer);
         }
 
         // For mesh, we need to generate the faces.
@@ -354,6 +360,9 @@ THREE.DRACOLoader.prototype = {
                 THREE.Uint32BufferAttribute : THREE.Uint16BufferAttribute)
               (geometryBuffer.indices, 1));
         }
+
+        // TODO: Should not assume native Draco attribute IDs apply.
+        // TODO: Can other attribute types be quantized?
         var posTransform = new dracoDecoder.AttributeQuantizationTransform();
         if (posTransform.InitFromAttribute(posAttribute)) {
           // Quantized attribute. Store the quantization parameters into the

examples/js/loaders/GLTFLoader.js

@@ -1187,16 +1187,10 @@ THREE.GLTFLoader = ( function () {
 		POSITION: 'position',
 		NORMAL: 'normal',
 		TEXCOORD_0: 'uv',
-		TEXCOORD0: 'uv', // deprecated
-		TEXCOORD: 'uv', // deprecated
 		TEXCOORD_1: 'uv2',
 		COLOR_0: 'color',
-		COLOR0: 'color', // deprecated
-		COLOR: 'color', // deprecated
 		WEIGHTS_0: 'skinWeight',
-		WEIGHT: 'skinWeight', // deprecated
 		JOINTS_0: 'skinIndex',
-		JOINT: 'skinIndex' // deprecated
 	};
 
 	var PATH_PROPERTIES = {
@@ -1347,6 +1341,7 @@ THREE.GLTFLoader = ( function () {
 
 			if ( hasMorphPosition ) {
 
+				// TODO: Error-prone use of a callback inside a loop.
 				var accessor = target.POSITION !== undefined
 					? parser.getDependency( 'accessor', target.POSITION )
 						.then( function ( accessor ) {
@@ -1361,6 +1356,7 @@ THREE.GLTFLoader = ( function () {
 
 			if ( hasMorphNormal ) {
 
+				// TODO: Error-prone use of a callback inside a loop.
 				var accessor = target.NORMAL !== undefined
 					? parser.getDependency( 'accessor', target.NORMAL )
 						.then( function ( accessor ) {
@@ -1505,6 +1501,17 @@ THREE.GLTFLoader = ( function () {
 
 	function isPrimitiveEqual( a, b ) {
 
+		var dracoExtA = a.extensions ? a.extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ] : undefined;
+		var dracoExtB = b.extensions ? b.extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ] : undefined;
+
+		if ( dracoExtA && dracoExtB ) {
+
+			if ( dracoExtA.bufferView !== dracoExtB.bufferView ) return false;
+
+			return isObjectEqual( dracoExtA.attributes, dracoExtB.attributes );
+
+		}
+
 		if ( a.indices !== b.indices ) {
 
 			return false;
@@ -1633,6 +1640,7 @@ THREE.GLTFLoader = ( function () {
 
 			if ( primitive0.mode !== primitive.mode ) return false;
 			if ( primitive.indices === undefined ) return false;
+			if ( primitive.extensions && primitive.extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ] ) return false;
 			if ( ! isObjectEqual( primitive0.attributes, primitive.attributes ) ) return false;
 
 			var targets = primitive.targets || [];
@@ -1887,6 +1895,7 @@ THREE.GLTFLoader = ( function () {
 			var type = types[ i ];
 			var value = this.getDependencies( type );
 
+			// TODO: Error-prone use of a callback inside a loop.
 			value = value.then( function ( key, value ) {
 
 				results[ key ] = value;
@@ -1978,7 +1987,7 @@ THREE.GLTFLoader = ( function () {
 			// Ignore empty accessors, which may be used to declare runtime
 			// information about attributes coming from another source (e.g. Draco
 			// compression extension).
-			return null;
+			return Promise.resolve( null );
 
 		}
 
@@ -2487,6 +2496,18 @@ THREE.GLTFLoader = ( function () {
 
 		}
 
+		function createDracoPrimitive( primitive ) {
+
+			return extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ]
+				.decodePrimitive( primitive, parser )
+				.then( function ( geometry ) {
+
+					return addPrimitiveAttributes( geometry, primitive, parser );
+
+				} );
+
+		}
+
 		var pending = [];
 
 		for ( var i = 0, il = primitives.length; i < il; i ++ ) {
@@ -2508,13 +2529,7 @@ THREE.GLTFLoader = ( function () {
 				if ( primitive.extensions && primitive.extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ] ) {
 
 					// Use DRACO geometry if available
-					geometryPromise = extensions[ EXTENSIONS.KHR_DRACO_MESH_COMPRESSION ]
-						.decodePrimitive( primitive, parser )
-						.then( function ( geometry ) {
-
-							return addPrimitiveAttributes( geometry, primitive, parser );
-
-						} );
+					geometryPromise = createDracoPrimitive( primitive );
 
 				} else {