Skip to content

ExtrudeGeometry: Honor closed property of CatmullRomCurve3.#32410

Merged
Mugen87 merged 2 commits into
mrdoob:devfrom
Mugen87:dev3
Nov 28, 2025
Merged

ExtrudeGeometry: Honor closed property of CatmullRomCurve3.#32410
Mugen87 merged 2 commits into
mrdoob:devfrom
Mugen87:dev3

Conversation

@Mugen87

@Mugen87 Mugen87 commented Nov 28, 2025

Copy link
Copy Markdown
Collaborator

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

github-actions Bot commented Nov 28, 2025

Copy link
Copy Markdown

📦 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
Copy Markdown
Collaborator

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

Comment thread src/geometries/ExtrudeGeometry.js Outdated
@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