Three.js: RoundedBoxBufferGeometry is buggy

[WestLangley]

RoundedBoxBufferGeometry should render correctly for the default argument values.

geometry = new RoundedBoxBufferGeometry(); // no args

//

RoundedBoxBufferGeometry should be a box with rounded edges. That means the six sides should have faces that are coplanar. That is, the vertex _positions_ are not correct.

geometry = new RoundedBoxBufferGeometry( 10, 10, 10, 6, 2 ); // number of segments is even

//

UVs are not correct.

//

The normals for the triangles in the six primary faces must be parallel to the coordinate axes. That is, the vertex _normals_ are not correct.

• Three.js version: [r.123dev]

/ping @gkjohnson If you want to have a look at this, that is fine with me. I am not going to pursue this.

[gkjohnson]

Yeah I noticed the normals weren't exactly right when I was working on it given how the box is generated but I think it's still a useful as a primitive. I don't think I'll be investing more time into this since I just wanted to get the UVs looking reasonable but thanks for bringing it up. If I think of a simple solution I may add it.

Regarding the particularly poor quality with an even segment number per side we could just enforce that the arguments be odd (add 1 if it's even) or document that it needs to be odd to get the expected look. If the rounded box is being generated from a segmented cube I'm not sure what the "right" thing to do for an even number of segments would be -- you want an even number of segments for the rounding corners on each side and one segment for the plane in the middle.

[gkjohnson]

@WestLangley thought of something 😁

20666

[mrdoob]

@gkjohnson Does #20666 fix all the issues WestLangley reported here?

[WestLangley]

Does #20666 fix all the issues WestLangley reported here?

It is better, but the default values are not fixed, and passing in 1 for segments is buggy.

Also, the bounding box is too small. It should be the same size as the equivalent BoxBufferGeometry. In other words, the vertex positions are still not correct.

I'd suggest reopening this, or creating a new issue.

[gkjohnson]

@mrdoob It should fix everything listed in the first post, yes.

@WestLangley

It is better, but the default values are not fixed, and passing in 1 for segments is buggy.

Can you explain a bit more? There is now special handling for when segments === 1 which will cause it to be exactly the same as a cube (also oops typo on "we'r" 😁):

https://github.com/mrdoob/three.js/blob/b19cc35a01267ae87ba4e013ec6eb2010076ca90/examples/jsm/geometries/RoundedBoxBufferGeometry.js#L47-L54

Here's what I see now when using default values new RoundedBoxBufferGeometry():

Also, the bounding box is too small. It should be the same size as the equivalent BoxBufferGeometry. In other words, the vertex positions are still not correct.

The bounding box is slightly smaller -- this is probably not difficult to fix either. I'll look into addressing that.

[gkjohnson]

Fixed the total size issue in #20672.

Now afaik the remaining issue is if radius is more than half the half of any of the given sides:

[WestLangley]

Can you explain a bit more?

I suggest you do this so the default looks like a box with rounded edges:

constructor( width = 1, height = 1, depth = 1, segments = 4, radius = 0.1 ) {

Also, I think setting 15 for segments in the example is overkill. Four to six should be adequate when using smooth normals.

[gkjohnson]

I've just changed the defaults in the other PR to what you suggested. I'll leave the segment count in the example up to @mrdoob -- lower segments counts can be noticeable in the box silhouette, though.

[mrdoob]

@gkjohnson instead of doing segments = segments % 2 === 0 ? segments + 1 : segments; would it make sense to just multiply segments * 2?

[gkjohnson]

@mrdoob

just multiply segments * 2

You'd have to do segments * 2 + 1 because it needs to be odd. I'm not sure if I feel that's better, though, and it would give less granular control over the segment count.

[mrdoob]

I see. Makes sense yes...
Anything left from this issue?

[gkjohnson]

Just what I mentioned in https://github.com/mrdoob/three.js/issues/20665#issuecomment-725532600 -- I don't know if you want to do anything about it or leave it as is. An easy way to address it is to force the radius to be at most half the width of any dimension:

radius = Math.min( width / 2, height / 2, depth / 2, radius );

I'm fine with leaving it as is, though.

[WestLangley]

I think segments should refer to the number of segments defining the curved corners of each of the primary faces. In that regard, it can take values of 1, 2, 3, ...

As the code is written, different inputs can generate the same rendered output.

and it would give less granular control over the segment count.

Consequently, I disagree with that statement.

[mrdoob]

As the code is written, different inputs can generate the same rendered output.

Yes, that's what I found confusing. That's what made me suggest multiplying by 2.

[gkjohnson]

@WestLangley

Consequently, I disagree with that statement.

You're right now that I've thought about it more I agree. That means the code should change to segments = segments * 2 + 1 and the lottie example should change to set the segments to 7. @mrdoob do you want to just make the change? Otherwise I can make another PR.

[mrdoob]

I'll do the change 👌

[mrdoob]

Fixed segments: https://github.com/mrdoob/three.js/commit/2d1723fd7ed98deecb9acd1484f9fe49154ddfcd

[mrdoob]

Fixed radius: https://github.com/mrdoob/three.js/commit/4c0e435e8f413378eeb0e22131e0a3508a6463b7