InstancedMesh bounding volume issue

[ycw]

Description

related #25591, its not guaranteed that the volume is the smallest, bc instancedmesh's matrixworld is ignored during union.

could computeBounding*() offers "precise" like this -> https://threejs.org/docs/index.html#api/en/math/Box3.setFromObject ??

[Mugen87]

its not guaranteed that the volume is the smallest,

That's true but since these volumes are recomputed often, it's not required to have the best-fit volume. The performance of the computation is also important.

bc instancedmesh's matrixworld is ignored during union

Do you mind explaining in more detail what you mean? The bounding volumes should be in local space.

[Mugen87]

could computeBounding*() offers "precise" like this

TBH, I don't think that best-fit bounding volumes which are more expensive to compute are necessary for view frustum culling and ray casting. You can always compute best-fit bounding volumes on app level if you need them.

That said, it's sounds fine if you want to add "precise" support for InstancedMesh to Box3.expandByObject().

[ycw]

more expensive to compute

yes, so I wasn't requesting compute precise volume by defaults, but requesting an option as if box3.setfromobject which defaults to false

sounds fine if you want to add support for InstancedMesh to Box3.expandByObject()

nah, this will break codes; instancedmesh is-a object3d, behaviors had been defined in those apis

[Mugen87]

nah, this will break codes; instancedmesh is-a object3d, behaviors had been defined in those apis

It should work if you rewrite this section:

https://github.com/mrdoob/three.js/blob/fbd3ef941971ae068c389a43e3777f245d3650a8/src/math/Box3.js#L162-L175

to something like:

if ( object.boundingBox !== undefined ) {

    if ( precise === true ) {

        // best-fit AABB code goes here

    } else {

        if ( object.boundingBox === null ) {

            object.computeBoundingBox();

        }

        _box.copy( object.boundingBox );
        _box.applyMatrix4( object.matrixWorld );

        this.union( _box );

    }

} else {

[ycw]

wait, seems a breaking change already; .expandByObject(instancedmesh) computed the bbox of instancedmesh.geometry before #25591; but then, it changes to compute a bbox enclosing all instances after #25591 :O

[Mugen87]

I don't think this a breaking change. The behavior wasn't right from the beginning.

[ycw]

breaking change: from wrong to right.

[ycw]

To sum up, there's still no out-of-box API computes the smallest bounding volume enclosing all instances, we've to impl on app level. Here's a good starting point (just appends .applyMatrix4(this.matrixWorld) in L64, and martixWorld must be updated beforehand). Thanks.