New LatheBufferGeometry

[Mugen87]

This PR introduces LatheBufferGeometry. Here's the test fiddle

• I've added a little enhancement to BufferGeometry.normalizeNormals to avoid NaN values with zero vectors. This is done in a separate commit.
• I'm not quite sure about the generation of the normals with .computeVertexNormals. If the geometry is closed, the implementation of LatheBufferGeometry must manually merge some normals to avoid a faulty seam (see screenshot). Duplicated vertices (which are necessary because of their unique UV coordinates) are causing this effect. Does anyone know an alternative approach to generate the normals of the geometry?

[WestLangley]

// if the geometry is closed, we need to average the normals along the seam.

[WestLangley]

I think the way you have handled normals along the seam is fine.

[WestLangley]

I've added a little enhancement to BufferGeometry.normalizeNormals to avoid NaN values with zero vectors. This is done in a separate commit.

For future reference, that should probably have been in a separate PR. In any event, why were you getting zero vectors?

[Mugen87]

I've discovered this problem with my mentioned jsfiddle example. The algorithm in BufferGeometry .computeVertexNormals selects two zero vectors for computing the face normal, which leads to a zero normal vector.

pA.fromArray( positions, vA ); // zero vector
pB.fromArray( positions, vB ); // zero vector
pC.fromArray( positions, vC );

The result looks then quite faulty (see example screenshot). I think this is caused by the first point of the LatheBufferGeometry points parameter, which is THREE.Vector2( 0, 0 ).

[Mugen87]

For future reference, that should probably have been in a separate PR.

Ok, i will keep that in mind! 👍

[WestLangley]

The geometry could form a point at the top, like a spear. Is that problematic?

Also, I llke your goblet. : - )

[Mugen87]

Also, I like your goblet. : - )

I have the points data from the three.js editor 😉. Indeed, it's a really cool shape!

The geometry could form a point at the top, like a spear. Is that problematic?

Um, i'm afraid that i can't imagine what it is about. Do you mean i need to adjust my points array for LatheBufferGeometry? Or do you suggest a different index generation?

[WestLangley]

The geometry could form a point at the top, like a spear. Is that problematic?

Sorry, I could have stated that better. I was wondering if there is a problem with the normals at the tip of a cone-shaped lathe geometry. In hindsight, I do not think that is an issue.

[Mugen87]

BTW, where does the term LatheGeometry originally come from? I wonder if this expressions is an allusion to the corresponding tool (with the meaning of "engine lathe")...

[mrdoob]

I know it from 3D Studio. Not sure where they got it from.

[WestLangley]

I wonder if this expressions is an allusion to the corresponding tool

Yes, that was my understanding.

[mrdoob]

Is this good to merge now?

[Mugen87]

I think there are no outstanding issues to resolve. Should be ready for merge...

[WestLangley]

@mrdoob Yes, but please look at the changes to BufferGeometry.js and see if you agree. I do not understand @Mugen87's explanation as to why his normals had zero length.

[MasterJames]

Didn't you ever get to use a lathe in both wood working and/or metal shop? Great fun! https://en.wikipedia.org/wiki/Lathe?wprov=sfla1

[Mugen87]

The changes to BufferGeometry should prevent NaN values analogous to eg. Vector3.multiplyScalar. In my example i had NaN values because normalizeNormals tried to normalize a normal with zero values.

[mrdoob]

Hmmm, maybe it would be better to understand first why the normals have Infinity before adding a check blindly.

[Mugen87]

Sorry, i hope this is more clear. I mean it like this...

From normalizeNormals:

x = normals[ i ]; // 0
y = normals[ i + 1 ]; // 0
z = normals[ i + 2 ]; // 0

n = 1.0 / Math.sqrt( x * x + y * y + z * z ); // n = Infinity

normals[ i ] *= n; // NaN
normals[ i + 1 ] *= n; // NaN
normals[ i + 2 ] *= n; // NaN

The normal with zero values occurs, because .computeVertexNormals computes a face normal with zero length. The geometry in my example had a face with two vertex positions with ( 0, 0, 0 ) values.

pA.fromArray( positions, vA ); // zero vector
pB.fromArray( positions, vB ); // zero vector
pC.fromArray( positions, vC );

cb.subVectors( pC, pB );
ab.subVectors( pA, pB );
cb.cross( ab );

normals[ i ] = cb.x; // 0
normals[ i + 1 ] = cb.y; // 0
normals[ i + 2 ] = cb.z; // 0

[WestLangley]

Hmmm, maybe it would be better to understand first why the normals have Infinity before adding a check blindly.

Exactly. And since this was a user error, I do not think the check is needed. We assume the geometry is a valid one.

[mrdoob]

The normal with zero values occurs, because .computeVertexNormals computes a face normal with zero length. The geometry in my example had a face with two vertex positions with ( 0, 0, 0 ) values.

So if I understand it correctly, it was a temporal error that is no longer there?

[Mugen87]

No. If i remove the fix from .normalizeNormals, i get NaN values again. I've provided here a jsfiddle with a three.js build without the fix. When you look at the bottom of the goblet, you should see the following visual variance.

If you set a breakpoint to BufferGeometry.computeVertexNormals, you will see that the calculation of the first face normal leads to a vector with zero length. Then .normalizeNormals will produce NaNvalues.

I thought, we can avoid this problem by adding the mentioned fix. But if you think that the geometry isn't valid, we maybe need to adjust LatheBufferGeometry.

I used the following points array to produce the LatheBufferGeometry. (I borrowed the data from the three.js editor)

var points = [
    new THREE.Vector2( 0, 0 ),
    new THREE.Vector2( 4, 0 ),
    new THREE.Vector2( 3.5, 0.5 ),
    new THREE.Vector2( 1, 0.75 ),
    new THREE.Vector2( 0.8, 1 ),
    new THREE.Vector2( 0.8, 4 ),
    new THREE.Vector2( 1, 4.2 ),
    new THREE.Vector2( 1.4, 4.8 ),
    new THREE.Vector2( 2, 5 ),
    new THREE.Vector2( 2.5, 5.4 ),
    new THREE.Vector2( 3, 12 )
];

The former version of LatheGeometry can use this array to create a shape without visual errors (or without NaN values). But if i use this array with LatheBufferGeometry, i get the mentioned problem when i generate the normals with .computeVertexNormals.

[WestLangley]

This is caused by the problem I was concerned about above. You are creating degenerate triangles in the base. The cross-product will be problematic in that case. Try changing the first point and inspect the geometry in wireframe.

var points = [
    new THREE.Vector2( 1, 0 ),

This modification will produce a hole in the bottom of the base of the goblet.

In production, you could use

var points = [
    new THREE.Vector2( 0.001, 0 ), // avoid zero value in turned radius

[Mugen87]

@WestLangley Yeah, this approach works. Ok, i will undo the commit, so there will be no changes to BufferGeometry.

Besides, do you think there is a way to avoid the generation of degenerate triangles in LatheBufferGeometry? Or is this something we just can't avoid...

[WestLangley]

The x-coordinate of each point must be greater than zero. Perhaps that should be noted in the docs.

(Or allow zero, if you can figure out a way. Experiment with a cone shape.)

It is unfortunate that the function is defined with the y-axis as the abscissa, and x-axis as the ordinate.

I think it would be more natural to rotate around the x-axis -- as a lathe does, but it is probably not worth changing at this point.

[Mugen87]

I see!

The x-coordinate of each point must be greater than zero. Perhaps that should be noted in the docs.

That's a good point! I will put this into the docu...

[WestLangley]

@mrdoob This is OK to merge.

@Mugen87 Thank you.

[mrdoob]

Great! Thanks guys!