Skip to content
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

SphereGeometry: Increased default widthSegments 32 and heightSegment 16 #22141

Merged
merged 1 commit into from
Jul 16, 2021

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Jul 16, 2021

Description

The default values were set in the CanvasRenderer days.
I think we can increase them today and align them to Blender's default values.

Before:

Screenshot 2021-07-16 at 13 05 06

After:

Screenshot 2021-07-16 at 13 05 12

@mrdoob mrdoob added this to the r131 milestone Jul 16, 2021
@mrdoob mrdoob merged commit 96fa820 into dev Jul 16, 2021
@mrdoob mrdoob deleted the sphere branch July 16, 2021 13:19
@Dannie226
Copy link
Contributor

I know this specific pull request has already been merged into three.js, but with the change of the sphere geometry to match blender's defaults, should the other primitives (cylinder, cone, and torus) also have their defaults changed to match blender's defaults? Or has this already been discussed and decided against?

@mrdoob
Copy link
Owner Author

mrdoob commented Nov 28, 2022

Hmm, maybe we could do that yeah...

What geometries would we have to change?

@Dannie226
Copy link
Contributor

As far as I know, just cylinder, cone, and torus, but a more in-depth search would probably be in order.

@mrdoob
Copy link
Owner Author

mrdoob commented Nov 29, 2022

Would you like to do that in-depth search?

@Dannie226
Copy link
Contributor

Dannie226 commented Nov 29, 2022

From what I see, all blender primitives are as follows: Plane, Cube, Circle, Sphere, Icosphere, Cylinder, Cone, Torus, Grid, and Monkey. The plane and cube are fine, and the plane takes care of the grid. Monkey doesn't by default exist in three.js. The sphere was already taken care of. That leaves the circle, the cylinder, the cone, and the torus. The circle will probably need to update both ring geometry and circle geometry. Through looking at the blender nodes page, I found mesh nodes, and if the side bar image is to be trusted for the default values, then the correct defaults would be as follows. Circle: 32, Cylinder:32, and Cone:32. Couldn't find a torus node, so the defaults for that might have to be found out by you guys.

Edit:
Can confirm after opening up blender, that those defaults are correct, and torus has major segments as 48, and minor segments as 12
I will make a new fork, and pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants