Skip to content

Conversation

@Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Nov 28, 2025

Related issue: #32359

Description

I've revisited #32359 since I've wanted to get rid of the TODO in ExtrudeGeometry. After looking closer at the history of ExtrudeGeometry, it was easier to understand where the TODO came from.

Instead of having a straight line along the extrusion happens, an optional extrude path allows for more customization. I suspect when this feature was initially implemented, it was not designed to create a closed extruded shape though. Hence, the closed parameter of computeFrenetFrames() was always set to false.

When CatmullRomCurve3 came around, there was eventually one curve class that had a closed property. And indeed if closed is set to true on the curve, it is right to assign the same value when computing to Frenet Frames since the last set of data are more correct.

Compared to #32359, this PR does an explicit test on CatmullRomCurve3 so it's clear where the closed property comes from. I've also visually compared a few closed extruded shapes with this PR and r181 but I did not see any differences. Looking at the normal and binormal values from the Frenet Frames data, the numerical differences are so tiny that it's clear why no hard seam appears when rendering with a normal material. The fix is still worth it, imo.

@Mugen87 Mugen87 added this to the r182 milestone Nov 28, 2025
@github-actions
Copy link

github-actions bot commented Nov 28, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 350.26
83.06
350.26
83.06
+0 B
+0 B
WebGPU 613.65
170.41
613.65
170.41
+0 B
+0 B
WebGPU Nodes 612.26
170.14
612.26
170.14
+0 B
+0 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 482.25
117.84
482.25
117.84
+0 B
+0 B
WebGPU 684.79
186.19
684.79
186.19
+0 B
+0 B
WebGPU Nodes 634.63
173.37
634.63
173.37
+0 B
+0 B

@WestLangley
Copy link
Collaborator

I, too, tested this issue using MeshNormalMaterial and saw no visible seams with closed paths.

@Mugen87 Mugen87 merged commit 82c64b0 into mrdoob:dev Nov 28, 2025
8 of 9 checks passed
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