Src: add CapsuleGeometry #23586

gsimone · 2022-02-24T20:47:01Z

Related issue: #23579

Preview

https://rawcdn.githack.com/gsimone/three.js/feature/capsule-geometry/docs/index.html?q=caps#api/en/geometries/CapsuleGeometry

Description

Adds a simple CapsuleGeometry class that extends LatheGeometry.

Some Notes

the naming for the arguments and the descriptions were inspired by other geometries, suggestions and improvements are welcome.
there are combinations of radii/height that don't produce a geometry, should we try to prevent them or leave it to users?

Checklist

src/geometries/CapsuleGeometry.js

gsimone · 2022-02-24T20:48:34Z

src/geometries/CapsuleGeometry.js

+		// max here prevents failing when height is 0
+		const a = Math.asin( R1 / Math.max( Number.EPSILON, height ) );


I added this check to avoid a void geometry.
Problems still arise when passing certain combinations of radii and height, should we add more checks/failsafes?

gsimone · 2022-02-24T21:22:28Z

Not sure what's wrong with the unit tests, maybe I missed something but the action message doesn't help

mrdoob · 2022-02-24T21:48:07Z

Could you remove the builds from the PR?

gsimone · 2022-02-25T07:58:35Z

✅ Removed them

src/geometries/CapsuleGeometry.js

docs/api/en/geometries/CapsuleGeometry.html

gsimone · 2022-02-25T11:04:03Z

@mrdoob @WestLangley

2 separate radii or 1? I agree that radius/length is less clumsy to use, and the change is not a big effort 🤷 Let me know which one you prefer

Mugen87 · 2022-02-25T12:04:13Z

I vote for a single radius value.

mrdoob · 2022-02-25T12:24:38Z

Yes, radius and length 👍

src/geometries/CapsuleGeometry.js

gsimone · 2022-02-25T18:30:45Z

Removed the double radius situation, only thing missing is this suggestion #23586 (comment)

gsimone · 2022-02-25T19:20:00Z

✅ simplified code accounting for single radius

Mugen87 · 2022-02-25T19:23:48Z

You have to add the new unit test file to this section:

three.js/test/unit/three.source.unit.js

Lines 85 to 104 in 5d81b27

    
           import './src/geometries/BoxGeometry.tests.js'; 
        
           import './src/geometries/CircleGeometry.tests.js'; 
        
           import './src/geometries/ConeGeometry.tests.js'; 
        
           import './src/geometries/CylinderGeometry.tests.js'; 
        
           import './src/geometries/DodecahedronGeometry.tests.js'; 
        
           import './src/geometries/EdgesGeometry.tests.js'; 
        
           import './src/geometries/ExtrudeGeometry.tests.js'; 
        
           import './src/geometries/IcosahedronGeometry.tests.js'; 
        
           import './src/geometries/LatheGeometry.tests.js'; 
        
           import './src/geometries/OctahedronGeometry.tests.js'; 
        
           import './src/geometries/PlaneGeometry.tests.js'; 
        
           import './src/geometries/PolyhedronGeometry.tests.js'; 
        
           import './src/geometries/RingGeometry.tests.js'; 
        
           import './src/geometries/ShapeGeometry.tests.js'; 
        
           import './src/geometries/SphereGeometry.tests.js'; 
        
           import './src/geometries/TetrahedronGeometry.tests.js'; 
        
           import './src/geometries/TorusGeometry.tests.js'; 
        
           import './src/geometries/TorusKnotGeometry.tests.js'; 
        
           import './src/geometries/TubeGeometry.tests.js'; 
        
           import './src/geometries/WireframeGeometry.tests.js';

…s into feature/capsule-geometry

mrdoob · 2022-02-25T21:05:18Z

src/geometries/CapsuleGeometry.js

+
+		super( path.getPoints( capSegments ), radialSegments );
+
+		this.translate( 0, - length / 2, 0 );


Is there a way of doing this offset in the Path instead? 🤔

4ed3391 LGTY?

mrdoob · 2022-02-26T06:35:15Z

Thanks!

mrdoob · 2022-02-28T20:25:11Z

@gsimone It seems to me that SphereGeometry could be simplified by adopt the approach used here?

gsimone · 2022-03-01T08:58:38Z

@mrdoob This is what it would look like, I didn't check if it generates the same vertices though, is that something we'd care about 🤔

Screen.Recording.2022-03-01.at.09.55.46.mov

Try here 👉 https://codesandbox.io/s/three-js-lathe-spheregeometry-vg4bvz?file=/src/LatheSphereGeometry.js

mrdoob · 2022-03-01T19:00:28Z

That's great!

I didn't check if it generates the same vertices though, is that something we'd care about 🤔

Ideally yes. Including the phiStart, phiLength, thetaStart and thetaLength properties 😬

gsimone · 2022-03-02T10:58:29Z

I think it would be an interesting experiment, but maybe not worth it? I'll play with adding the angles and see how it differs from the sphere we have now.

It could be good as long as it's not a huge breaking change (eg. UV and normals are close enough to not cause differences if people are relying on constructing spheres with the old geo)

* feat: adds CapsuleGeometry * . * Update src/geometries/CapsuleGeometry.js * Update docs/api/en/geometries/CapsuleGeometry.html * removes radiusBottom * Update src/geometries/CapsuleGeometry.js * adds test import * fixes test file name typo * . * fixes imports in capsuleGeometry module * . * pre-translate path * lint

feat: adds CapsuleGeometry

4f1368a

gsimone commented Feb 24, 2022

View reviewed changes

src/geometries/CapsuleGeometry.js Outdated Show resolved Hide resolved

gsimone commented Feb 24, 2022

View reviewed changes

mrdoob added this to the r139 milestone Feb 24, 2022

.

2798126

gsimone commented Feb 25, 2022

View reviewed changes

src/geometries/CapsuleGeometry.js Outdated Show resolved Hide resolved

Update src/geometries/CapsuleGeometry.js

f7b4ab6

Mugen87 reviewed Feb 25, 2022

View reviewed changes

docs/api/en/geometries/CapsuleGeometry.html Outdated Show resolved Hide resolved

gsimone commented Feb 25, 2022

View reviewed changes

docs/api/en/geometries/CapsuleGeometry.html Outdated Show resolved Hide resolved

Update docs/api/en/geometries/CapsuleGeometry.html

a6ea558

removes radiusBottom

702963a

gsimone commented Feb 25, 2022

View reviewed changes

src/geometries/CapsuleGeometry.js Outdated Show resolved Hide resolved

Update src/geometries/CapsuleGeometry.js

89e9514

gsimone added 6 commits February 25, 2022 21:01

adds test import

e176f72

Merge branch 'feature/capsule-geometry' of github.com:gsimone/three.j…

264364e

…s into feature/capsule-geometry

fixes test file name typo

1474b5c

.

38588e5

fixes imports in capsuleGeometry module

0ba9820

.

638b767

gsimone mentioned this pull request Feb 25, 2022

RFC: Additional linting rule for import extensions #23591

Closed

mrdoob reviewed Feb 25, 2022

View reviewed changes

gsimone added 2 commits February 25, 2022 22:58

pre-translate path

d26fdac

lint

9ce5234

mrdoob merged commit 3d24599 into mrdoob:dev Feb 26, 2022

gsimone deleted the feature/capsule-geometry branch February 26, 2022 09:05

WestLangley changed the title ~~feat: adds CapsuleGeometry~~ Src: add CapsuleGeometry Feb 26, 2022

joshuaellis mentioned this pull request Mar 1, 2022

r139 three-types/three-ts-types#192

Closed

10 tasks

0b5vr mentioned this pull request Mar 3, 2022

feat: adds CapsuleGeometry definitions three-types/three-ts-types#194

Merged

3 tasks

arpu mentioned this pull request Mar 22, 2022

Support Capsule Shape pmndrs/cannon-es#132

Open

wasabia mentioned this pull request Apr 4, 2022

three.js r139 add CapsuleGeometry TODO wasabia/three_dart#34

Open

linbingquan mentioned this pull request Apr 19, 2022

Editor: Add capsule geometry. #23914

Merged

This was referenced Jun 2, 2022

Bump three version meshcat-dev/meshcat#116

Merged

[meshcat] Add capsule support RobotLocomotion/drake#17319

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Src: add CapsuleGeometry #23586

Src: add CapsuleGeometry #23586

gsimone commented Feb 24, 2022 •

edited

gsimone Feb 24, 2022

gsimone commented Feb 24, 2022

mrdoob commented Feb 24, 2022

gsimone commented Feb 25, 2022

gsimone commented Feb 25, 2022

Mugen87 commented Feb 25, 2022

mrdoob commented Feb 25, 2022

gsimone commented Feb 25, 2022

gsimone commented Feb 25, 2022

Mugen87 commented Feb 25, 2022

mrdoob Feb 25, 2022

gsimone Feb 25, 2022

mrdoob Feb 26, 2022

mrdoob commented Feb 26, 2022

mrdoob commented Feb 28, 2022

gsimone commented Mar 1, 2022

mrdoob commented Mar 1, 2022

gsimone commented Mar 2, 2022

		// max here prevents failing when height is 0
		const a = Math.asin( R1 / Math.max( Number.EPSILON, height ) );


		super( path.getPoints( capSegments ), radialSegments );

		this.translate( 0, - length / 2, 0 );

Src: add CapsuleGeometry #23586

Src: add CapsuleGeometry #23586

Conversation

gsimone commented Feb 24, 2022 • edited

gsimone Feb 24, 2022

Choose a reason for hiding this comment

gsimone commented Feb 24, 2022

mrdoob commented Feb 24, 2022

gsimone commented Feb 25, 2022

gsimone commented Feb 25, 2022

Mugen87 commented Feb 25, 2022

mrdoob commented Feb 25, 2022

gsimone commented Feb 25, 2022

gsimone commented Feb 25, 2022

Mugen87 commented Feb 25, 2022

mrdoob Feb 25, 2022

Choose a reason for hiding this comment

gsimone Feb 25, 2022

Choose a reason for hiding this comment

mrdoob Feb 26, 2022

Choose a reason for hiding this comment

mrdoob commented Feb 26, 2022

mrdoob commented Feb 28, 2022

gsimone commented Mar 1, 2022

mrdoob commented Mar 1, 2022

gsimone commented Mar 2, 2022

gsimone commented Feb 24, 2022 •

edited