From 47630c07c3c493302058eb0c03395953b924b977 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Fri, 15 Nov 2019 16:34:15 -0800 Subject: [PATCH 1/4] Optimize Box3.expandByObject Instead of recomputing the bounding box every time, we now use the cached bounding box from the object, and transform it by the world transform. Note that this results in slightly less accurate bounding box compared to the previous implementation. This seems like a fair tradeoff - it's possible to get the precise box by using Box3.setFromBufferAttribute if desired. In addition to making the box more precise, this honors morph target data and supports arbitrary future improvements to object bounding box computation automatically. --- src/math/Box3.js | 33 +++++++-------------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/src/math/Box3.js b/src/math/Box3.js index eb1aea375c5d50..516a667c86f414 100644 --- a/src/math/Box3.js +++ b/src/math/Box3.js @@ -251,36 +251,17 @@ Object.assign( Box3.prototype, { if ( geometry !== undefined ) { - if ( geometry.isGeometry ) { + if ( geometry.boundingBox === null ) { - var vertices = geometry.vertices; + geometry.computeBoundingBox(); - for ( i = 0, l = vertices.length; i < l; i ++ ) { - - _vector.copy( vertices[ i ] ); - _vector.applyMatrix4( object.matrixWorld ); - - this.expandByPoint( _vector ); - - } - - } else if ( geometry.isBufferGeometry ) { - - var attribute = geometry.attributes.position; - - if ( attribute !== undefined ) { - - for ( i = 0, l = attribute.count; i < l; i ++ ) { - - _vector.fromBufferAttribute( attribute, i ).applyMatrix4( object.matrixWorld ); - - this.expandByPoint( _vector ); - - } + } - } + _box.copy( geometry.boundingBox ); + _box.applyMatrix4( object.matrixWorld ); - } + this.expandByPoint( _box.min ); + this.expandByPoint( _box.max ); } From 11b8bf3d83d9edc065fc73caffc4cf31d50f04ff Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 16 Nov 2019 05:46:54 +0100 Subject: [PATCH 2/4] Fix non-bundled build - _box is defined in a different .js file --- src/math/Box3.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/math/Box3.js b/src/math/Box3.js index 516a667c86f414..2ea564e840a4fe 100644 --- a/src/math/Box3.js +++ b/src/math/Box3.js @@ -257,11 +257,13 @@ Object.assign( Box3.prototype, { } - _box.copy( geometry.boundingBox ); - _box.applyMatrix4( object.matrixWorld ); + var box = new Box3(); - this.expandByPoint( _box.min ); - this.expandByPoint( _box.max ); + box.copy( geometry.boundingBox ); + box.applyMatrix4( object.matrixWorld ); + + this.expandByPoint( box.min ); + this.expandByPoint( box.max ); } From 43cd006f03b87fe479110e1f0bbc1edb24623538 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 16 Nov 2019 06:03:02 +0100 Subject: [PATCH 3/4] Eliminate redundant object creation Instead of creating a new GC'ed object for each object we traverse, pre-create the _box similarly to other globals. This is slightly awkward since this needs to be done after we define the ctor. --- src/math/Box3.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/math/Box3.js b/src/math/Box3.js index 2ea564e840a4fe..a5504d29467013 100644 --- a/src/math/Box3.js +++ b/src/math/Box3.js @@ -41,6 +41,8 @@ function Box3( min, max ) { } +var _box = new Box3(); + Object.assign( Box3.prototype, { isBox3: true, @@ -257,13 +259,11 @@ Object.assign( Box3.prototype, { } - var box = new Box3(); - - box.copy( geometry.boundingBox ); - box.applyMatrix4( object.matrixWorld ); + _box.copy( geometry.boundingBox ); + _box.applyMatrix4( object.matrixWorld ); - this.expandByPoint( box.min ); - this.expandByPoint( box.max ); + this.expandByPoint( _box.min ); + this.expandByPoint( _box.max ); } From 57dab2c4567a093a8bc13d02e37e1070c8ce73fb Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 18 Nov 2019 09:16:52 +0100 Subject: [PATCH 4/4] docs: Add a caveat about expandByObject/setFromObject The comment is worded to not interfere too much with possible future changes to the function's implementation. --- docs/api/en/math/Box3.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/api/en/math/Box3.html b/docs/api/en/math/Box3.html index 40f1c1069e1037..464b30c9baad83 100644 --- a/docs/api/en/math/Box3.html +++ b/docs/api/en/math/Box3.html @@ -138,6 +138,7 @@

[method:Box3 expandByObject]( [param:Object3D object] )

Expands the boundaries of this box to include [page:Object3D object] and its children, accounting for the object's, and children's, world transforms. + The function may result in a larger box than strictly necessary.

@@ -279,6 +280,7 @@

[method:Box3 setFromObject]( [param:Object3D object] )

Computes the world-axis-aligned bounding box of an [page:Object3D] (including its children), accounting for the object's, and children's, world transforms. + The function may result in a larger box than strictly necessary.