Box3 expandByObject fails if attribute.itemSize is 2.

[johnshaughnessy]

Box3's expandByObject fails for buffer geometry whose itemSize !== 3.

If you call expandByObject(obj) where obj has a child with (for example) a TextGeometry, this line will attempt to read past the end of an array, then call applyMatrix4 so that v1 is {x:NaN, y:NaN, z:NaN}: https://github.com/mrdoob/three.js/blob/master/src/math/Box3.js#L255 This means that the box's min and max both become NaN, NaN, NaN.

One (obviously not optimized) solution is to replace that line with:

if (attribute.itemSize === 2) {
  var v2 = new Vector2();
  v2.fromBufferAttribute(attribute, i);
  v1.x = v2.x;
  v1.y = v2.y;
  v1.z = 0;
} else {
   v1.fromBufferAttribute(attribute, i);
}
v1.applyMatrix4(node.matrixWorld);

Of course, this would fail if attribute.itemSize === 4. (I'm not sure ever happens in practice.) You can fix that by checking if itemSize ===3 before reading from it with a Vector3.

This is not a problem with setFromBufferAttribute because that does not rely on Math.min or Math.max, which favor NaN over any numbers. (Instead it uses < and > which favor numbers over NaN.)

Three.js version

• [x] Dev
• [x] r93
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

Hardware Requirements (graphics card, VR Device, ...)

[Mugen87]

Box3 assumes a 3D vector space. Why do you want to calculate a 3D bounding volume for 2D input data?

If you call expandByObject(obj) where obj has a child with (for example) a TextGeometry, this line will attempt to read past the end of an array,

Can you please demonstrate the issue with a live example?

[johnshaughnessy]

Why do you want to calculate a 3D bounding volume for 2D input data?

Perhaps there's a better way to go about this, but I wanted to pass the root of an arbitrary object (sometimes loaded from an external source) into setFromObject in order to get a bounding box to use as a box collider so I can drive the object's movement with a physics system. (I don't need the collisions to be very accurate, so a box collider will do fine.)

a child with (for example) a TextGeometry,

This was a bad example. I noticed this issue when using https://github.com/Jam3/three-bmfont-text, which uses 2D buffer attributes, but this is not the general case for text geometry in three js.

Can you please demonstrate the issue with a live example?

Here is a live example: https://jsfiddle.net/gy1p4q86/34/ edit: I initially posted a broken fiddle, but this one should work. In it you should see a cube with a flat rectangle coming out of the center. The flat rectangle uses a buffer geometry with a 2D buffer attribute and is at a child leaf of the object hierarchy. I'd like to be able to compute a bounding box for an object like this (that is a combination of 2D and 3D shapes) using Box3.setFromObject, especially if this kind of object can be created from a loader (like GLTFLoader). I don't know actually know if those loaders would ever create a 2D buffer attribute, but I don't see why not.

In this fiddle example you'll also notice an error where the BufferGeometry I created fails to compute a bounding sphere. This is because of the same assumption on this line: https://github.com/mrdoob/three.js/blob/master/src/core/BufferGeometry.js#L648 Is it wrong to create BufferGeometry with 2D BufferAttributes?

[Usnul]

@johnshaughnessy

Is it wrong to create BufferGeometry with 2D BufferAttributes?

Not at all. It's just an attribute, if it happens to have 1d, 2d, or whatever other data - that's up to you as a programmer to decide - no wrong asnwers here, everyone is a winner!

@Mugen87 I think that's our fail. The API is Box3.expandByObject(object : Object3D) : Box3, so there is actually nothing illegal in having funky attributes on your geometry, perhaps it was too ambitious to offer such a broad API, but it doesn't say anything about geometry or restrictions on such.

Possible ways to address this:

1. Update documentation of expandByObject to reflect that for BufferGeometry position attribute must be 3D
2. Patch expandByObject to expand only in available dimensions (i.e. x, xy, xyz)
3. Deprecate the API and come up with a more restricted one to avoid confusion in the future.

[Mugen87]

In this case I vote to clarify the documentation. Users have to ensure their geometry data is expressed in 3D space when using Box3.

[Mugen87]

See #6288 and the conclusion. I vote to close the issue and mark it as a duplicate.

[WestLangley]

@Usnul @Mugen87

Is it wrong to create BufferGeometry with 2D BufferAttributes?

Not at all. It's just an attribute, if it happens to have 1d, 2d, or whatever other data - that's up to you as a programmer to decide - no wrong asnwers here, everyone is a winner!

Not true. position is a vec3.

// default vertex attributes provided by Geometry and BufferGeometry
attribute vec3 position;
attribute vec3 normal;
attribute vec2 uv;

[Usnul]

@WestLangley

Not true. position is a vec3.

Please show me where, i can't see it. https://github.com/mrdoob/three.js/blob/a866c59735b2fa0816009d1fdaae41a95a254ba0/src/core/BufferGeometry.js#L20-L43

[mrdoob]

Please show me where, i can't see it.

https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLProgram.js#L385-L387

[Usnul]

I see. So WebGL renderer makes that assumption. Okay, I won't argue further.