-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#9663: Fix radius bug in CylinderBufferGeometry #9672
Conversation
@@ -153,7 +153,7 @@ function CylinderBufferGeometry( radiusTop, radiusBottom, height, radialSegments | |||
|
|||
// handle special case if radiusTop/radiusBottom is zero | |||
|
|||
if ( ( radiusTop === 0 && y === 0 ) || ( radiusBottom === 0 && y === heightSegments ) ) { | |||
if ( ( radiusTop < Number.EPSILON && y === 0 ) || ( radiusBottom < Number.EPSILON && y === heightSegments ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need this special case? I'm not sure you do; just don't multiply by radius
first?
@WestLangley Good point! What do you think about the last commit? No special cases any more... |
@Mugen87 Not tested, but something like: var u = x / radialSegments;
var theta = u * thetaLength + thetaStart;
var sinTheta = Math.sin( theta );
var cosTheta = Math.cos( theta );
// vertex
vertices.setXYZ( index, radius * sinTheta, - v * height + halfHeight, radius * cosTheta );
// normal
normal.set( sinTheta, tanTheta, cosTheta ).normalize(); // tanTheta should be renamed, IMO
normals.setXYZ( index, normal.x, normal.y, normal.z ); |
Would be something like |
Doesn't matter. It is just not the same |
Done! 😊 |
@WestLangley all good? |
OK. Thanks, @Mugen87. |
Yays! |
Thanks everyone, I really appreciate it! 😄 |
* mrdoob#9663: Fix radius bug in CylinderBufferGeometry * CylinderBufferGeometry: simplify normal generation * CylinderBufferGeometry: Cleanup * CylinderBufferGeometry: Rename tanTheta variable
See #9663.
The zero comparison does not work in some special cases with radius values very close to zero. Instead we now using a tolerance value (Number.EPSILON).