Geometry bug fixes & tests (#1171)

modules/core/src/geometries/ico-sphere-geometry.js

@@ -20,7 +20,7 @@ export default class IcoSphereGeometry extends Geometry {
   }
 }
 
-function tesselateIcosaHedron(props = {}) {
+function tesselateIcosaHedron(props) {
   const {iterations = 0} = props;
 
   const PI = Math.PI;
@@ -80,8 +80,8 @@ function tesselateIcosaHedron(props = {}) {
   }
 
   // Calculate texCoords and normals
-  const normals = new Array(indices.length * 3);
-  const texCoords = new Array(indices.length * 2);
+  const normals = new Array(positions.length);
+  const texCoords = new Array((positions.length / 3) * 2);
 
   const l = indices.length;
   for (let i = l - 3; i >= 0; i -= 3) {

modules/core/src/geometries/plane-geometry.js

@@ -26,8 +26,8 @@ function tesselatePlane(props) {
 
   const coords = type.split(',');
   // width, height
-  let c1len = props[`${coords[0]}len`];
-  const c2len = props[`${coords[1]}len`];
+  let c1len = props[`${coords[0]}len`] || 1;
+  const c2len = props[`${coords[1]}len`] || 1;
   // subdivisionsWidth, subdivisionsDepth
   const subdivisions1 = props[`n${coords[0]}`] || 1;
   const subdivisions2 = props[`n${coords[1]}`] || 1;
@@ -82,7 +82,7 @@ function tesselatePlane(props) {
           break;
 
         default:
-          break;
+          throw new Error('PlaneGeometry: unknown type');
       }
 
       i2 += 2;

modules/core/src/geometry/geometry-utils.js

@@ -1,33 +1,29 @@
-// TODO - generalize to arbitrary attributes
 export function unpackIndexedGeometry(geometry) {
   const {indices, attributes} = geometry;
   if (!indices) {
     return geometry;
   }
 
-  const {POSITION, NORMAL, TEXCOORD_0} = attributes;
+  const vertexCount = indices.value.length;
+  const unpackedAttributes = {};
 
-  const unpackedPositions = new Float32Array(indices.length * 3);
-  const unpackedNormals = new Float32Array(indices.length * 3);
-  const unpackedTexCoords = new Float32Array(indices.length * 2);
-
-  for (let x = 0; x < indices.length; ++x) {
-    const index = indices[x];
-    unpackedPositions[x * 3 + 0] = POSITION[index * 3 + 0];
-    unpackedPositions[x * 3 + 1] = POSITION[index * 3 + 1];
-    unpackedPositions[x * 3 + 2] = POSITION[index * 3 + 2];
-    unpackedNormals[x * 3 + 0] = NORMAL[index * 3 + 0];
-    unpackedNormals[x * 3 + 1] = NORMAL[index * 3 + 1];
-    unpackedNormals[x * 3 + 2] = NORMAL[index * 3 + 2];
-    unpackedTexCoords[x * 2 + 0] = TEXCOORD_0[index * 2 + 0];
-    unpackedTexCoords[x * 2 + 1] = TEXCOORD_0[index * 2 + 1];
+  for (const attributeName in attributes) {
+    const attribute = attributes[attributeName];
+    const {constant, value, size} = attribute;
+    if (constant || !size) {
+      continue; // eslint-disable-line
+    }
+    const unpackedValue = new value.constructor(vertexCount * size);
+    for (let x = 0; x < vertexCount; ++x) {
+      const index = indices.value[x];
+      for (let i = 0; i < size; i++) {
+        unpackedValue[x * size + i] = value[index * size + i];
+      }
+    }
+    unpackedAttributes[attributeName] = {size, value: unpackedValue};
   }
 
   return {
-    attributes: {
-      POSITION: unpackedPositions,
-      NORMAL: unpackedNormals,
-      TEXCOORD_0: unpackedTexCoords
-    }
+    attributes: Object.assign({}, attributes, unpackedAttributes)
   };
 }

modules/core/src/geometry/geometry.js

@@ -22,14 +22,13 @@ export default class Geometry {
     const {
       id = uid('geometry'),
       drawMode = DRAW_MODE.TRIANGLES,
-      mode,
       attributes = {},
       indices = null,
       vertexCount = null
     } = props;
 
     this.id = id;
-    this.drawMode = drawMode | 0 || mode | 0;
+    this.drawMode = drawMode | 0;
     this.attributes = {};
     this.userData = {};
 
@@ -88,18 +87,24 @@ export default class Geometry {
         `${this._print(attributeName)}: must be typed array or object with value as typed array`
       );
 
+      if ((attributeName === 'POSITION' || attributeName === 'positions') && !attribute.size) {
+        attribute.size = 3;
+      }
+
       // Move indices to separate field
       if (attributeName === 'indices') {
         assert(!this.indices);
         this.indices = attribute;
-        if (this.indices.isIndexed !== undefined) {
-          this.indices = Object.assign({}, this.indices);
-          delete this.indices.isIndexed;
-        }
       } else {
         this.attributes[attributeName] = attribute;
       }
     }
+
+    if (this.indices && this.indices.isIndexed !== undefined) {
+      this.indices = Object.assign({}, this.indices);
+      delete this.indices.isIndexed;
+    }
+
     return this;
   }
 
@@ -116,14 +121,6 @@ export default class Geometry {
       }
     }
 
-    // TODO - Magic, should this be removed?
-    if (!Number.isFinite(vertexCount)) {
-      const attribute = attributes.POSITION || attributes.positions;
-      if (attribute) {
-        vertexCount = attribute.value && attribute.value.length / (attribute.size || 3);
-      }
-    }
-
     assert(Number.isFinite(vertexCount));
     return vertexCount;
   }

modules/core/test/geometry/geometries.spec.js

@@ -0,0 +1,83 @@
+import test from 'tape-catch';
+import {
+  ConeGeometry,
+  CubeGeometry,
+  CylinderGeometry,
+  IcoSphereGeometry,
+  PlaneGeometry,
+  SphereGeometry,
+  TruncatedConeGeometry
+} from '@luma.gl/core';
+
+const GEOMETRY_TESTS = [
+  {name: 'ConeGeometry', Geometry: ConeGeometry},
+  {name: 'CubeGeometry', Geometry: CubeGeometry},
+  {
+    name: 'CylinderGeometry',
+    Geometry: CylinderGeometry,
+    props: [{verticalAxis: 'z'}, {topCap: true, bottomCap: true}]
+  },
+  {
+    name: 'IcoSphereGeometry',
+    Geometry: IcoSphereGeometry,
+    props: [{iterations: 2}]
+  },
+  {
+    name: 'PlaneGeometry',
+    Geometry: PlaneGeometry,
+    props: [
+      {flipCull: true, unpack: true},
+      {type: 'x,z'},
+      {type: 'x,z', flipCull: true},
+      {type: 'y,z'},
+      {type: 'y,z', flipCull: true},
+      {type: null, _shouldThrow: true}
+    ]
+  },
+  {name: 'SphereGeometry', Geometry: SphereGeometry},
+  {name: 'TruncatedConeGeometry', Geometry: TruncatedConeGeometry}
+];
+
+function checkAttribute(attribute, type = [Float32Array]) {
+  return (
+    attribute &&
+    attribute.value &&
+    type.some(t => attribute.value instanceof t) &&
+    attribute.value.length > 0 &&
+    attribute.value.every(Number.isFinite)
+  );
+}
+
+test('Object#Geometries', t => {
+  for (const geometryTest of GEOMETRY_TESTS) {
+    const {name, Geometry} = geometryTest;
+    // `undefined` tests the default props
+    const testProps = [undefined].concat(geometryTest.props || []);
+
+    for (const props of testProps) {
+      if (props && props._shouldThrow) {
+        t.throws(() => new Geometry(props), `${name}: should throw`);
+        continue; // eslint-disable-line
+      }
+
+      const geometry = new Geometry(props);
+
+      t.is(typeof geometry.drawMode, 'number', `${name}: .drawMode is a number`);
+      t.is(typeof geometry.mode, 'number', `${name}: .mode is a number`);
+      t.is(typeof geometry.vertexCount, 'number', `${name}: .vertexCount is a number`);
+
+      const attributes = geometry.getAttributes();
+
+      t.ok(checkAttribute(attributes.POSITION), `${name}: POSITION is Float32Array`);
+      t.ok(checkAttribute(attributes.NORMAL), `${name}: NORMAL is Float32Array`);
+      t.ok(checkAttribute(attributes.TEXCOORD_0), `${name}: TEXCOORD_0 is Float32Array`);
+      if (attributes.indices) {
+        t.ok(
+          checkAttribute(attributes.indices, [Uint16Array, Uint32Array]),
+          `${name}: indices is Uint{16/32}Array`
+        );
+      }
+    }
+  }
+  t.end();
+});

modules/core/test/geometry/geometry-utils.spec.js

@@ -0,0 +1,57 @@
+import test from 'tape-catch';
+import {unpackIndexedGeometry} from '@luma.gl/core/geometry/geometry-utils';
+
+const TEST_ATTRIBUTES = {
+  POSITION: {value: new Float32Array([0, 0, 0, 1, 0, 0, 1, 1, 0, 1, 0, 0]), size: 3},
+  NORMAL: {value: new Float32Array([0, 1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0]), size: 3},
+  TEXCOORD_0: {value: new Float32Array([0, 0, 0, 1, 1, 1, 1, 0]), size: 2},
+  COLOR: {constant: true, value: new Float32Array([255, 255, 255]), size: 3}
+};
+
+const TEST_CASES = [
+  {
+    title: 'no indices',
+    input: {
+      attributes: TEST_ATTRIBUTES
+    },
+    output: {
+      attributes: TEST_ATTRIBUTES
+    }
+  },
+  {
+    title: 'with indices',
+    input: {
+      indices: {value: new Uint16Array([0, 1, 2, 3, 1, 2])},
+      attributes: TEST_ATTRIBUTES
+    },
+    output: {
+      attributes: {
+        POSITION: {
+          value: new Float32Array([0, 0, 0, 1, 0, 0, 1, 1, 0, 1, 0, 0, 1, 0, 0, 1, 1, 0]),
+          size: 3
+        },
+        NORMAL: {
+          value: new Float32Array([0, 1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0, 0, 1, 0]),
+          size: 3
+        },
+        TEXCOORD_0: {value: new Float32Array([0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1]), size: 2},
+        COLOR: TEST_ATTRIBUTES.COLOR
+      }
+    }
+  }
+];
+
+test('unpackIndexedGeometry', t => {
+  for (const testCase of TEST_CASES) {
+    const {attributes} = unpackIndexedGeometry(testCase.input);
+    for (const name in testCase.output.attributes) {
+      t.deepEqual(
+        attributes[name],
+        testCase.output.attributes[name],
+        `${testCase.title}: ${name} matches`
+      );
+    }
+  }
+
+  t.end();
+});