Box3 now supports computing minimal bounds for setFromObject

[arikwex]

This is a draft for implementing the "minimal bounding box" idea proposed here: #17940 (comment).

I'll need to update the docs and such still, but I'd like to know if everyone's cool with the direction before going further.

[arikwex]

P.S. This does not support morphAttributesPosition in the same manner that BufferGeometry's bounding box computation does. It could be added if necessary.

[arikwex]

@WestLangley Am I supposed to do something to nudge this PR? I'm not sure exactly what the expected PR lifecycle is for this ThreeJS.

[WestLangley]

@arikwex I added a milestone -- that should help prevent this PR from getting lost -- but I have no control over what gets merged.

[arikwex]

Dear @mrdoob, should I refer to you as

[@mrdoob]

or

[                   @mrdoob                                ]

?

With the help of this PR, I will be able to call you by EITHER name depending on your preference.

[WestLangley]

/ping @Mugen87 for opinion

[donmccurdy]

Following up on discussion in #17940, a few comments:

The behavior under minimal = false here considers morph targets; the behavior under minimal = true does not. I don't think that difference is captured in the name "minimal" ... should it consider morph targets? If so, perhaps it should consider the current state of the morph targets, rather than the maximum possible extent of the morph targets (as computeBoundingBox does)?

@mrdoob had made another suggestion in #17940 (comment), for creating a new method expandByObjectBounds. Under that naming, presumably the behavior would be:

• expandByObjectBounds / setFromObjectBounds: Expands the Box3 conservatively and efficiently, based on a cached geometry bounding box that considers the maximum possible extent of morph targets, if present.
• expandByObject / setFromObject: Expands the Box3 precisely and at greater expense, based on vertex data and the current state of morph targets, if present.

In terms of clarity, I might prefer that API. The downside is that it decreases performance on the existing method, with little way to communicate that to users. Brainstorming a few alternatives...

// names indicating use of `geometry.boundingBox` and maximum morph target extent...
expandByObject( object, useBounds = true )
expandByObject( object, boundingBox = true )
expandByObject( object, cached = true )
expandByObject( object, useMaximumExtent = true )

// names indicating a full scan of vertex data and current morph target state...
expandByObject( object, deep = false )
expandByObject( object, exact = false )
expandByObject( object, tight = false )

It's also possible that the geometry's bounding box was computed without considering morph targets, or at a much larger or smaller size (e.g. to account for skinning bind matrices). So I think it's probably good to be explicit about use of the bounding box rather than to use a word like "tight" or "minimal".

[mrdoob]

expandByObject( object, exact = false )

That one sounds good to me.

As well as this one:

expandByObject( object, precise = false )

[mrdoob]

@arikwex Sorry for the huge delay... 😬

Do you mind resolving the merge conflicts?

[arikwex]

Bones rattle in a deep and dark cave. A slumbering beast awakens, hungry for a bounding box. It rebases...

Everything should be up-to-date and the signature for the expandByObject + setFromObject methods have been adjusted with the name precise instead of minimal. However, I haven't changed the morphTargets. I couldn't tell from @donmccurdy's comment if that's a requirement to merge this change.

[mrdoob]

Thanks!