Skip to content

Use Geometry#calculateTangents in procedural geometry subclasses - #9253

Merged
willeastcott merged 2 commits into
mainfrom
simplify-geometry-tangents
Aug 29, 2026
Merged

Use Geometry#calculateTangents in procedural geometry subclasses#9253
willeastcott merged 2 commits into
mainfrom
simplify-geometry-tangents

Conversation

@willeastcott

Copy link
Copy Markdown
Contributor

Description

All eight procedural geometry classes (BoxGeometry, CircleGeometry, SphereGeometry, PlaneGeometry, TorusGeometry, CapsuleGeometry, ConeGeometry, CylinderGeometry) inline the calculateTangents utility call in their constructors:

if (opts.calculateTangents) {
    this.tangents = calculateTangents(positions, normals, uvs, indices);
}

But the base class already exposes exactly this as Geometry#calculateTangents(), which reads the same positions/normals/uvs/indices fields each constructor assigns immediately beforehand. Each subclass now calls the inherited method and drops its geometry-utils.js import:

if (opts.calculateTangents) {
    this.calculateTangents();
}

Net −8 lines and eight fewer import bindings; the standalone calculateTangents/calculateNormals exports are unchanged. The base method's Debug.assert guards are satisfied at every call site (fields assigned just above, or by the ConeBaseGeometry super constructor) and are stripped from release builds anyway.

Verified byte-identical output: all eight geometries constructed with calculateTangents: true and non-default segment counts produce identical positions, normals, uvs, uvs1, indices and tangents streams before and after the change (SHA-256 over every stream). Lint clean; full unit suite at 2337 passing with failures identical to the main baseline (pre-existing jsdom resource-loading timeouts).

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

🤖 Generated with Claude Code

All eight procedural geometry classes inlined the calculateTangents
utility call even though their base class already exposes exactly that
as Geometry#calculateTangents, reading the same positions, normals, uvs
and indices fields the constructors assign immediately beforehand. Each
subclass now calls the inherited method and drops its geometry-utils
import.

Verified byte-identical output: all eight geometries constructed with
calculateTangents enabled and non-default segment counts produce
identical positions, normals, uvs, uvs1, indices and tangents streams
before and after the change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Public API report

This PR changes the public API surface (+9 / −0), per the docs' rules (@ignore / @Private / undocumented are excluded).

Show API diff
+BoxGeometry.tangents: ArrayLike<number> | undefined
+CapsuleGeometry.tangents: ArrayLike<number> | undefined
+CircleGeometry.tangents: ArrayLike<number> | undefined
+ConeGeometry.tangents: ArrayLike<number> | undefined
+CylinderGeometry.tangents: ArrayLike<number> | undefined
+DomeGeometry.tangents: ArrayLike<number> | undefined
+PlaneGeometry.tangents: ArrayLike<number> | undefined
+SphereGeometry.tangents: ArrayLike<number> | undefined
+TorusGeometry.tangents: ArrayLike<number> | undefined

Informational only — this never fails the build.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

Build size report

This PR changes the size of the minified bundles.

Bundle Minified Gzip Brotli
playcanvas.min.js 2390.3 KB (−0.1 KB, −0.01%) 614.6 KB (−0.1 KB, −0.01%) 476.7 KB (−0.2 KB, −0.04%)
playcanvas.min.mjs 2387.6 KB (−0.1 KB, −0.01%) 613.4 KB (−0.0 KB, −0.00%) 476.4 KB (+0.0 KB, +0.01%)

@willeastcott
willeastcott merged commit c64d927 into main Aug 29, 2026
10 checks passed
@willeastcott
willeastcott deleted the simplify-geometry-tangents branch August 29, 2026 08:43
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.

1 participant