Skip to content

Conversation

mkeblx
Copy link
Contributor

@mkeblx mkeblx commented Apr 4, 2025

Description

The CapsuleGeometry was missing a parameter for controlling segments along the height, useful/critical for shaders. This adds a heightsParameter matching CylinderGeometry, etc.

Copy link

github-actions bot commented Apr 4, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.44
78.37
336.44
78.37
+0 B
+0 B
WebGPU 541.67
150.04
541.67
150.04
+0 B
+0 B
WebGPU Nodes 541.14
149.93
541.14
149.93
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.37
112.21
465.37
112.21
+0 B
+0 B
WebGPU 614.51
166.09
614.51
166.09
+0 B
+0 B
WebGPU Nodes 569.5
155.49
569.5
155.49
+0 B
+0 B

@Mugen87 Mugen87 added this to the r176 milestone Apr 5, 2025
@WestLangley
Copy link
Collaborator

UVs may not be correct. I would not expect the texture to appear differently when tessellations are increased.

Also, perhaps unrelated to this PR, the number of segments in the caps appears to be twice the number specified.

@mkeblx
Copy link
Contributor Author

mkeblx commented Apr 5, 2025

UVs may not be correct. I would not expect the texture to appear differently when tessellations are increased.

I agree with that, and will change that. Looking at UVs closer, there are issues prior to this patch with UVs (as you vary capSegments; and I would say overall UV layout)

Also, perhaps unrelated to this PR, the number of segments in the caps appears to be twice the number specified.

Yes that happens before this patch, but I can fix.

--

I do have a question of UV strategy before I implement. I think an even UV distribution along the total curve length makes the most sense, but other methods could work. Any objections to that?

@WestLangley
Copy link
Collaborator

WestLangley commented Apr 5, 2025

I do have a question of UV strategy before I implement. I think an even UV distribution along the total curve length makes the most sense, but other methods could work. Any objections to that?

It might be easier, and adequate, to make an even distribution of UV along the total height. (Also likely easier to create a custom texture in that case.)

@WestLangley
Copy link
Collaborator

Also, perhaps unrelated to this PR, the number of segments in the caps appears to be twice the number specified.

Yes that happens before this patch, but I can fix.

Ask @Mugen87. That would be a breaking change, and I am not sure it is worth it. Regardless, I will support his decision on that.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 6, 2025

Also, perhaps unrelated to this PR, the number of segments in the caps appears to be twice the number specified

Yes, I think that should be fixed even if it breaks a user setup. The migration task to restore the previous visual should be trivial. But let's do this with a different PR.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 6, 2025

I would also say the texture coordinate issue should be tackled with a different PR. Maybe a fix is required in LatheGeometry.

@Mugen87 Mugen87 merged commit 311af89 into mrdoob:dev Apr 7, 2025
12 checks passed
@Mugen87 Mugen87 changed the title CapsuleGeometry: Add heightSegments parameter CapsuleGeometry: Add heightSegments parameter Apr 7, 2025
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.

3 participants