Pre-size geometry buffers instead of growing arrays and converting

[sophiedeziel]

Geometry buffers across the codebase are built by pushing onto plain number[], then handed to three.js — which copies each one into a typed array. Large models pay twice: the arrays grow and reallocate as they fill, then every element is copied.

Every one of these sizes is known before the loop starts, so the buffer can be allocated once and filled by cursor. Same change in both places it happens.

1. Line rendering — SceneManager.renderPathsAsLines

One position buffer, built per render.

	before	after
pack time (median of 10)	3.1 ms	0.5 ms
speedup	—	6.2×
transient boxed doubles	~5.0 MB	0

2. Tube rendering — ExtrusionGeometry

Four buffers — vertices, normals, uvs, indices — built per path. The sizes follow from the path topology:

vertices = (points + 1) * (radialSegments + 1) * 3
uvs      = points * (radialSegments + 1) * 2
indices  = (points - 1) * radialSegments * 6

buffer	count	push → convert	pre-sized
position	1,613,235	8.2 ms	1.1 ms
normal	1,613,235	7.7 ms	1.1 ms
uv	1,075,490	4.9 ms	0.8 ms
index	2,412,600	11.0 ms	3.1 ms
total	6.7M pushes	31.8 ms	6.1 ms

End to end, building every tube geometry in the model: 103 ms → 79 ms.

Benchmark setup

3.5 MB 3DBenchy — 163,474 lines → 7023 paths → 104,037 points, producing 652,818 line floats and 537,745 tube vertices / 2,412,600 indices. Chrome, median of 7–10 runs after warm-up.

Correctness

Both changes produce byte-identical output, verified rather than assumed. The previous implementations were reimplemented as references and compared element-by-element:

• line positions: identical across the full 652,818-float buffer
• tube position, normal, uv and index buffers: identical across 400 geometries, no tolerance

Model renders unchanged in lines and tubes mode.

New tests cover the parts that could silently drift:

• line packing: segment count, the nozzle-height offsets, empty buffer for a too-short path, and that three.js receives a Float32Array
• tube buffers: every buffer filled exactly with no trailing slack, and the index width widening from Uint16Array to Uint32Array past 65,535 vertices

npm run check passes: 12 test files, typecheck, lint.

Worth a reviewer's attention

Pre-sizing the index buffer means the index width is now chosen explicitly instead of inferred by setIndex. Indices only ever reference the geometry's own vertices, so Uint16Array is correct below 65,536 vertices and Uint32Array above — there's a test for the crossover, since no model in the repo reaches it (the Benchy's largest path is 1075 vertices).

Related

#350 is a separate optimization in the same ExtrusionGeometry loop — caching the ring of sines and cosines rather than recomputing it a million times. Different technique, so it is kept apart, but the two touch the same function and whichever merges second will need a small rebase. Together they take tube geometry from 103 ms to 39 ms.

Follow-up: zero-copy attributes

Per review feedback, Float32BufferAttribute was replaced with plain BufferAttribute for the position, normal and uv attributes. The buffers are already exact-size Float32Arrays, and Float32BufferAttribute's constructor does new Float32Array(array) — a full copy. BufferAttribute adopts the buffer as-is, eliminating 3 full-buffer copies per path (position + normal + uv).

Node bench, 200k-point spiral, new ExtrusionGeometry(points, 0.4, 0.2, 4), median of 5 constructions: ~65 ms → ~60 ms (~8% faster; the buffer fill dominates, the copies were the remaining overhead).

Assisted by Claude Code - Opus 5

[github-actions]

Visit the preview URL for this PR (updated for commit 533c51b):

https://gcode-preview--pr349-perf-pack-line-verti-0bv00xd9.web.app

_{(expires Mon, 21 Sep 2026 02:41:51 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 59bd114ae4847b32c2bba0b68620b9069a3e3531}

[remcoder]

Solid refactor. Verified locally (157/157 tests pass) and checked the sizing math against the fill loops for point counts 0–10 — all exact. Details:

• Buffer sizing is provably correct. accounts for the closing segment; and match the / loops exactly. I also confirmed the sizing is robust to the latent bug: both branches of produce exactly one extra ring, so is safe in both test and browser environments.
• Index width boundary is safe. selects Uint32; at exactly 65536 the max index is 65535 (still Uint16-addressable), so it's conservative by one — harmless, just one vertex earlier than strictly needed.
• ** is behavior-identical** to the inlined loop, including the Y offset and the Z offset. Segment-count formula matches the loop exactly.
• Good regression tests: exact-size assertions catch both oversizing and undersizing, the trailing-index check catches slack, and the Uint16/Uint32 boundary test is well chosen.

Non-blocking notes:

1. has a pre-existing quirk: for a path with not a multiple of 3 (e.g. 4), it reads past the end (, → NaN). Not introduced here, but worth a guard while you're in there.
2. The path means writes yield NaN silently — same as before, just noting typed arrays don't throw.
3. Consider describing the 6-floats-per-segment layout — already in the JSDoc, good.

[remcoder]

Solid refactor. Verified locally (157/157 tests pass) and checked the sizing math against the fill loops for point counts 0-10 — all exact. Details:

• Buffer sizing is provably correct. vertexCount = (points.length + 1) * ringSize accounts for the closing segment; uvCount and indexCount match the generateUVs/generateIndices loops exactly. Also confirmed the sizing is robust to the closed latent bug: both branches of generateSegment(closed === false ? ...) produce exactly one extra ring, so the +1 is safe in both test and browser environments.
• Index width boundary is safe. vertexCount > 65535 selects Uint32; at exactly 65536 the max index is 65535 (still Uint16-addressable), so it's conservative by one — harmless, just one vertex earlier than strictly needed.
• packLineVertices is behavior-identical to the inlined loop, including the -0.1 Y offset and the offset = -lineHeight/2 Z offset. The segment-count formula Math.ceil((len-3)/3) matches the loop exactly.
• Good regression tests: exact-size assertions catch both oversizing and undersizing, the trailing-index check catches slack, and the Uint16/Uint32 boundary test is well chosen.

Non-blocking notes:

1. packLineVertices has a pre-existing quirk: for a path with vertices.length not a multiple of 3 (e.g. 4), it reads past the end (vertices[4], vertices[5] -> NaN). Not introduced here, but worth a guard while you're in there.
2. NaN path means Float32Array writes yield NaN silently — same as before, just noting typed arrays don't throw.

[remcoder]

One thing I missed on the first pass: the copy this PR sets out to remove is still there.

Float32BufferAttribute's constructor is super(new Float32Array(array), itemSize, normalized), and new Float32Array(someFloat32Array) allocates and copies. Verified at runtime — with Float32BufferAttribute, attribute.array === src is false; with BufferAttribute it's true. So the array growth is gone (real win), but all three float buffers are still copied once each. The index got BufferAttribute and avoids it; the attributes didn't.

Since path.ts:117 builds one ExtrusionGeometry per Path, that's 3 full-buffer copies per path on a large model.

this.setIndex(new BufferAttribute(indices, 1));
this.setAttribute('position', new BufferAttribute(vertices, 3));
this.setAttribute('normal', new BufferAttribute(normals, 3));
this.setAttribute('uv', new BufferAttribute(uvs, 2));

Safe as a drop-in: these are already Float32Array of exactly the right length, so type and itemSize are unchanged. Float32BufferAttribute can come off the import.

I'd rather this landed here than as a follow-up — without it the PR's goal is half-delivered and the four lines are inconsistent with each other.

Not asking for a test: "didn't allocate a copy" isn't observable through the public surface without spying on the Float32Array constructor, and the exact-size tests already cover everything behavioral. A before/after measurement in the description — the way packLineVertices documents its ~6x — is the right artifact.

Everything else in my approval stands.

[sophiedeziel]

Your suggestion was a good catch. I checked (actually I told Claude to check for me)

That's what's litteraly in three.js:

// three.js source
class Float32BufferAttribute extends BufferAttribute {
  constructor(array, itemSize, normalized) {
    super(new Float32Array(array), itemSize, normalized);  // ← allocates + copies
  }
}

The last commit shaved off an additional 8% of the rendering time on a 200k-point spiral, median of 5 constructions, 3 runs.