remove per-primitive bounding spheres, clean up

Author: donmccurdy

packages/engine/Source/Scene/BufferPoint.js

@@ -42,6 +42,9 @@ class BufferPoint extends BufferPrimitive {
     __BYTE_LENGTH: BufferPrimitive.Layout.__BYTE_LENGTH + 4,
   };
 
+  /////////////////////////////////////////////////////////////////////////////
+  // LIFECYCLE
+
   /**
    * @param {BufferPoint} point
    * @param {BufferPoint} result

packages/engine/Source/Scene/BufferPolygon.js

@@ -2,7 +2,6 @@
 
 import assert from "../Core/assert.js";
 import BufferPrimitive from "./BufferPrimitive.js";
-import BoundingSphere from "../Core/BoundingSphere.js";
 import BufferPrimitiveCollection from "./BufferPrimitiveCollection.js";
 import defined from "../Core/defined.js";
 
@@ -31,68 +30,48 @@ class BufferPolygon extends BufferPrimitive {
    */
   _collection = null;
 
-  /**
-   * @type {BoundingSphere}
-   * @protected
-   * @ignore
-   */
-  _boundingSphere = new BoundingSphere();
-
   /** @ignore */
   static Layout = {
     ...BufferPrimitive.Layout,
 
-    /**
-     * Bounding sphere for polygon.
-     * @type {number}
-     */
-    BOUNDING_SPHERE: BufferPrimitive.Layout.__BYTE_LENGTH,
-
     /**
      * Offset in position array to first vertex in polygon, number of VEC3 elements.
      * @type {number}
      */
-    POSITION_OFFSET_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength,
+    POSITION_OFFSET_U32: BufferPrimitive.Layout.__BYTE_LENGTH,
 
     /**
      * Count of positions (vertices) in this polygon, number of VEC3 elements.
      * @type {number}
      */
-    POSITION_COUNT_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 4,
+    POSITION_COUNT_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 4,
 
     /**
      * Offset in holes array to first hole in polygon, number of integer elements.
      * @type {number}
      */
-    HOLE_OFFSET_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 8,
+    HOLE_OFFSET_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 8,
 
     /**
      * Count of holes (indices) in this polygon, number of integer elements.
      * @type {number}
      */
-    HOLE_COUNT_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 12,
+    HOLE_COUNT_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 12,
 
     /**
      * Offset in triangles array to first triangle in polygon, number of VEC3 elements.
      * @type {number}
      */
-    TRIANGLE_OFFSET_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 16,
+    TRIANGLE_OFFSET_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 16,
 
     /**
      * Count of triangles (3x uint32) in this polygon, number of VEC3 elements.
      * @type {number}
      */
-    TRIANGLE_COUNT_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 20,
+    TRIANGLE_COUNT_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 20,
 
     /** @type {number} */
-    __BYTE_LENGTH:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 24,
+    __BYTE_LENGTH: BufferPrimitive.Layout.__BYTE_LENGTH + 24,
   };
 
   /////////////////////////////////////////////////////////////////////////////

packages/engine/Source/Scene/BufferPolyline.js

@@ -1,6 +1,5 @@
 // @ts-check
 
-import BoundingSphere from "../Core/BoundingSphere.js";
 import BufferPrimitive from "./BufferPrimitive.js";
 import assert from "../Core/assert.js";
 import BufferPrimitiveCollection from "./BufferPrimitiveCollection.js";
@@ -32,35 +31,25 @@ class BufferPolyline extends BufferPrimitive {
     ...BufferPrimitive.Layout,
 
     /**
-     * Bounding sphere for polygon.
-     * @type {number}
-     */
-    BOUNDING_SPHERE: BufferPrimitive.Layout.__BYTE_LENGTH,
-
-    /**
-     * Width of polyline, 0–255.
+     * Offset in position array to first vertex in polyline, number of VEC3 elements.
      * @type {number}
      */
-    WIDTH_U8:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength,
+    POSITION_OFFSET_U32: BufferPrimitive.Layout.__BYTE_LENGTH,
 
     /**
-     * Offset in position array to first vertex in polyline, number of VEC3 elements.
+     * Count of positions (vertices) in this polyline, number of VEC3 elements.
      * @type {number}
      */
-    POSITION_OFFSET_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 4,
+    POSITION_COUNT_U32: BufferPrimitive.Layout.__BYTE_LENGTH + 4,
 
     /**
-     * Count of positions (vertices) in this polyline, number of VEC3 elements.
+     * Width of polyline, 0–255.
      * @type {number}
      */
-    POSITION_COUNT_U32:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 8,
+    WIDTH_U8: BufferPrimitive.Layout.__BYTE_LENGTH + 8,
 
     /** @type {number} */
-    __BYTE_LENGTH:
-      BufferPrimitive.Layout.__BYTE_LENGTH + BoundingSphere.packedLength + 12,
+    __BYTE_LENGTH: BufferPrimitive.Layout.__BYTE_LENGTH + 12,
   };
 
   /////////////////////////////////////////////////////////////////////////////

packages/engine/Source/Scene/BufferPrimitive.js

@@ -10,7 +10,9 @@ const scratchColor = new Color();
  * View bound to the underlying buffer data of a {@link BufferPrimitiveCollection}. Abstract.
  *
  * <p>BufferPrimitive instances are intended to be reused when iterating over large collections,
- * and temporarily bound to a primitive index while performing read/write operations on that primitive.</p>
+ * and temporarily bound to a primitive index while performing read/write operations on that primitive,
+ * before being rebound to the next primitive, using the
+ * {@link https://en.wikipedia.org/wiki/Flyweight_pattern|flyweight pattern}.</p>
  *
  * @example
  * const primitive = new BufferPrimitive();
@@ -34,24 +36,43 @@ const scratchColor = new Color();
  */
 class BufferPrimitive {
   /**
+   * Collecton containing the primitive(s) for which this instance currently
+   * provides a view.
+   *
    * @type {BufferPrimitiveCollection<BufferPrimitive>}
    * @ignore
    */
   _collection = null;
 
   /**
+   * Index of the primitive for which this instance currently provides a view.
+   *
    * @type {number}
    * @ignore
    */
   _index = -1;
 
   /**
+   * Byte offset, into the collection's primitive buffer, of the primitive for
+   * which this instance currently provides a view,
+   *
    * @type {number}
    * @ignore
    */
   _byteOffset = -1;
 
-  /** @ignore */
+  /**
+   * Binary layout for this primitive type in collection's primitive buffer.
+   * Each `Layout.MY_KEY_U32` entry is an offset in bytes, relative to the
+   * start offset of the primitive, with the suffix indicating the data type
+   * stored at that offset.
+   *
+   * The final entry, `__BYTE_LENGTH`, is not a pointer into a buffer — its
+   * literal value is the total byte length of one primitive in the primitive
+   * buffer, exclusive of other buffers.
+   *
+   * @ignore
+   */
   static Layout = {
     /**
      * Feature ID associated with the primitive; not required to be unique.
@@ -77,7 +98,11 @@ class BufferPrimitive {
      */
     COLOR_U32: 8,
 
-    /** @type {number} */
+    /**
+     * Byte length of one primitive in the primitive buffer, exclusive of
+     * other buffers. Literal value, not a pointer.
+     * @type {number}
+     */
     __BYTE_LENGTH: 12,
   };
 
@@ -101,6 +126,10 @@ class BufferPrimitive {
   }
 
   /**
+   * Returns true if this primitive's memory footprint is resizable. Only the
+   * newest (most recently created) primitive in a collection can be resized,
+   * to guarantee fast and stable performance.
+   *
    * @returns {boolean}
    * @protected
    * @ignore
@@ -112,7 +141,10 @@ class BufferPrimitive {
   /////////////////////////////////////////////////////////////////////////////
   // ACCESSORS
 
-  /** @type {number} */
+  /**
+   * Feature ID associated with the primitive; not required to be unique.
+   * @type {number}
+   */
   get featureId() {
     return this._getUint32(BufferPrimitive.Layout.FEATURE_ID_U32);
   }
@@ -121,7 +153,10 @@ class BufferPrimitive {
     this._setUint32(BufferPrimitive.Layout.FEATURE_ID_U32, featureId);
   }
 
-  /** @type {boolean} */
+  /**
+   * Whether primitive is shown.
+   * @type {boolean}
+   */
   get show() {
     return this._getUint8(BufferPrimitive.Layout.SHOW_U8) === 1;
   }
@@ -131,8 +166,16 @@ class BufferPrimitive {
   }
 
   /**
+   * Whether the primitive requires an update on next render. Renderers should
+   * _not_ iterate over all primitives each frame, but must instead inspect
+   * only the dirty range of the parent collection. This flag is managed
+   * automatically, by primitive setters and collection renderers.
+   *
    * @type {boolean}
    * @ignore
+   *
+   * @see BufferPrimitiveCollection#_dirtyOffset
+   * @see BufferPrimitiveCollection#_dirtyCount
    */
   get _dirty() {
     return this._getUint8(BufferPrimitive.Layout.DIRTY_U8) === 1;
@@ -154,6 +197,7 @@ class BufferPrimitive {
   }
 
   /**
+   * Returns the color of primitive.
    * @param {Color} result
    * @returns {Color}
    */
@@ -165,14 +209,15 @@ class BufferPrimitive {
   }
 
   /**
+   * Updates the color of primitive.
    * @param {Color} color
    */
   setColor(color) {
     this._setUint32(BufferPrimitive.Layout.COLOR_U32, color.toRgba());
   }
 
   /////////////////////////////////////////////////////////////////////////////
-  // DATAVIEW ACCESSORS
+  // BUFFER ACCESSORS
 
   /**
    * @param {number} itemByteOffset

packages/engine/Source/Scene/BufferPrimitiveCollection.js

@@ -22,8 +22,8 @@ import assert from "../Core/assert.js";
  *
  * To get the full performance benefit of using a BufferPrimitiveCollection containing "N" primitives,
  * be careful to avoid allocating "N" instances of any related JavaScript object. {@link BufferPrimitive},
- * {@link Color}, {@link Cartesian3}, and other objects can all be reused when working with large collections. See
- * {@linkcode BufferPrimitiveCollection#add} and {@linkcode BufferPrimitiveCollection#get}.
+ * {@link Color}, {@link Cartesian3}, and other objects can all be reused when working with large collections,
+ * using the {@link https://en.wikipedia.org/wiki/Flyweight_pattern|flyweight pattern}.
  *
  * @abstract
  * @template T extends BufferPrimitive

packages/engine/Specs/Scene/BufferPolygonCollectionSpec.js

@@ -164,7 +164,7 @@ describe("BufferPolygonCollection", () => {
       triangleCountMax: 1,
     });
 
-    expect(collection.sizeInBytes).toBe(40 + 72 + 12);
+    expect(collection.sizeInBytes).toBe(36 + 72 + 12);
 
     collection = new BufferPolygonCollection({
       primitiveCountMax: 128,
@@ -173,7 +173,7 @@ describe("BufferPolygonCollection", () => {
       triangleCountMax: 1024,
     });
 
-    expect(collection.sizeInBytes).toBe(5120 + 24576 + 512 + 12288);
+    expect(collection.sizeInBytes).toBe(4608 + 24576 + 512 + 12288);
   });
 
   it("clone", () => {

packages/engine/Specs/Scene/BufferPolylineCollectionSpec.js

@@ -91,14 +91,14 @@ describe("BufferPolylineCollection", () => {
       vertexCountMax: 1,
     });
 
-    expect(collection.sizeInBytes).toBe(28 + 24);
+    expect(collection.sizeInBytes).toBe(24 + 24);
 
     collection = new BufferPolylineCollection({
       primitiveCountMax: 128,
       vertexCountMax: 128,
     });
 
-    expect(collection.sizeInBytes).toBe((28 + 24) * 128);
+    expect(collection.sizeInBytes).toBe((24 + 24) * 128);
   });
 
   it("clone", () => {