From eb1ff2ce4bdc8f35a4b2247b022d551e3901b752 Mon Sep 17 00:00:00 2001
From: Don McCurdy <don.r.mccurdy@gmail.com>
Date: Sat, 3 Feb 2018 22:35:56 +0000
Subject: [PATCH] BufferGeometryUtils: Fix groups on indexed geometry.

---
 docs/api/core/BufferGeometry.html              |  5 +++--
 examples/js/BufferGeometryUtils.js             | 13 +++++++++++--
 test/unit/example/BufferGeometryUtils.tests.js |  5 +++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/docs/api/core/BufferGeometry.html b/docs/api/core/BufferGeometry.html
index 1ab5420828..1e1ce18cc8 100644
--- a/docs/api/core/BufferGeometry.html
+++ b/docs/api/core/BufferGeometry.html
@@ -134,8 +134,9 @@
 
 			Each group is an object of the form:
 			<code>{ start: Integer, count: Integer, materialIndex: Integer }</code>
-			where start specifies the index of the first vertex in this draw call, count specifies
-			how many vertices are included, and materialIndex specifies the material array index to use.<br /><br />
+			where start specifies the first element in this draw call – the first vertex for non-indexed geometry,
+			otherwise the first triangle index. Count specifies how many vertices (or indices) are included, and
+			materialIndex specifies the material array index to use.<br /><br />
 
 			Use [page:.addGroup] to add groups, rather than modifying this array directly.
 		</div>
diff --git a/examples/js/BufferGeometryUtils.js b/examples/js/BufferGeometryUtils.js
index b6b479b609..c2c35fa2d5 100644
--- a/examples/js/BufferGeometryUtils.js
+++ b/examples/js/BufferGeometryUtils.js
@@ -245,8 +245,17 @@ THREE.BufferGeometryUtils = {
 
 			// create new group for this geometry
 
-			mergedGeometry.addGroup( offset, geometry.attributes.position.count, i );
-			offset += geometry.attributes.position.count;
+			if ( isIndexed ) {
+
+				mergedGeometry.addGroup( offset, geometry.index.count, i );
+				offset += geometry.index.count;
+
+			} else {
+
+				mergedGeometry.addGroup( offset, geometry.attributes.position.count, i );
+				offset += geometry.attributes.position.count;
+
+			}
 
 		}
 
diff --git a/test/unit/example/BufferGeometryUtils.tests.js b/test/unit/example/BufferGeometryUtils.tests.js
index 666d559531..6d306059f4 100644
--- a/test/unit/example/BufferGeometryUtils.tests.js
+++ b/test/unit/example/BufferGeometryUtils.tests.js
@@ -64,7 +64,7 @@ export default QUnit.module( 'BufferGeometryUtils', () => {
 
     var geometry1 = new THREE.BufferGeometry();
     geometry1.addAttribute( 'position', new THREE.BufferAttribute( new Float32Array( [ 1, 2, 3 ] ), 1, false ) );
-    geometry1.setIndex( new THREE.BufferAttribute( new Uint16Array( [ 0, 1, 2 ] ), 1, false ) );
+    geometry1.setIndex( new THREE.BufferAttribute( new Uint16Array( [ 0, 1, 2, 2, 1, 0 ] ), 1, false ) );
 
     var geometry2 = new THREE.BufferGeometry();
     geometry2.addAttribute( 'position', new THREE.BufferAttribute( new Float32Array( [ 4, 5, 6 ] ), 1, false ) );
@@ -74,7 +74,8 @@ export default QUnit.module( 'BufferGeometryUtils', () => {
 
     assert.ok( mergedGeometry, 'merge succeeds' );
     assert.smartEqual( Array.from( mergedGeometry.attributes.position.array ), [ 1, 2, 3, 4, 5, 6 ], 'merges elements' );
-    assert.smartEqual( Array.from( mergedGeometry.index.array ), [ 0, 1, 2, 3, 4, 5 ], 'merges indices' );
+    assert.smartEqual( Array.from( mergedGeometry.index.array ), [ 0, 1, 2, 2, 1, 0, 3, 4, 5 ], 'merges indices' );
+    assert.smartEqual( [ { start: 0, count: 6, materialIndex: 0 }, { start: 6, count: 3, materialIndex: 1 } ], mergedGeometry.groups, 'creates groups' );
     assert.equal( mergedGeometry.attributes.position.itemSize, 1, 'retains .itemSize' );
 
   } );
-- 
GitLab