[BREAKING] Store frustum planes in a typed array - #9196
Conversation
Frustum kept its six planes as Plane objects, so every plane access walked from the frustum to a Plane to a Vec3 to its components. The planes now live in a single Float32Array, packed as four floats per plane - the normal's x, y and z followed by the distance - in the existing right, left, bottom, top, far, near order. This is a breaking change: the public planes array is replaced by getPlane and setPlane. setPlane normalizes what it stores, as containsSphere requires unit length normals. The packed array is available as planeData for internal use. Measured in a browser on a scene of 1542 mesh instances, per call rather than per frame: MeshInstance#_isVisible drops from 54 to 42 ns, and setFromMat4 from 320 to 55 ns - the latter mostly from dropping the Plane.set and Vec3.normalize call chain, and it runs once per frustum per frame, so it scales with the number of shadow casting lights. Float32 storage measured faster than Float64 in the browser (42 vs 61 ns), and costs no precision, as Mat4 is itself backed by a Float32Array and so the planes are already derived from float32 inputs. The two consumers of the plane data - the gsplat frustum culler and the gsplat shadow renderer - already kept their own Float32Array in exactly this packed layout for the cull compute shaders, so both now copy the array directly instead of unpacking plane objects.
Public API reportThis PR changes the public API surface (+2 / −1), per the docs' rules (@ignore / @Private / undocumented are excluded). Show API diff-Frustum.planes: Plane[]
+Frustum.getPlane(index: number, result: Plane): Plane
+Frustum.setPlane(index: number, plane: Plane): FrustumInformational only — this never fails the build. |
Build size reportThis PR changes the size of the minified bundles.
|
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
Automated PR review by Codex (GPT-5).
I found one blocking compatibility issue and one smaller public-API reporting issue. The API report also removes Frustum.constructor() even though runtime construction remains supported and the examples still show new Frustum(); please retain an explicit empty constructor so this unrelated documented API entry does not disappear.
The packed implementation itself looks correct: extraction order and normalization match the previous code, point/sphere tests use the same boundary conditions, copy remains independent, XR add retains its algorithm without per-call allocation, and both gsplat consumers make the required defensive copy.
Local verification on the exact head commit: 44 focused Frustum, Camera, and local-shadow-renderer tests passed; ESLint passed for all changed files; generated TypeScript declarations and test:types passed; and git diff --check passed. All current GitHub checks are green.
|
Updates of this scale can break a large number of components. Unfortunately, backward compatibility was not preserved at all. |
|
it's pretty much impossible to make original direct access to internal array backwards compatible. And yep, in a follow up PR I'm switching culling from spheres to aabbs, so there will be a test function. |
2 participants
Frustumkept its six planes asPlaneobjects, so reading a plane walked from the frustum to aPlaneto aVec3to its components. The planes now live in a singleFloat32Array, packed as four floats per plane - the normal's x, y and z followed by the distance - in the existing right, left, bottom, top, far, near order.API Changes:
The public
planesarray is gone, replaced by an accessor pair:getPlane(index, result)writes the plane atindexinto the suppliedPlaneand returns it.setPlane(index, plane)stores a plane, normalizing it on the way in -containsSpherecompares a plane distance against a sphere radius, so it requires unit length normals.setFromMat4already normalized, so the two entry points now agree.planeDatafor internal use, tagged@ignore.Changes:
containsPoint,containsSphere,copyandsetFromMat4read and write the packed array directly.addis unchanged in behaviour: it reads the other frustum's planes into scratchPlaneobjects and reuses the existing three-plane corner intersection. It only runs for stereo XR, a couple of times per frame, so the unpacking costs nothing there.Float32Array(24)in exactly this packed layout for their cull compute shaders, so both now copy the array in one go instead of unpacking six plane objects. They keep their own buffer rather than sharing the frustum's, since each may be handed a shared scratch frustum that a later call overwrites.test/core/shape/frustum.test.mjs, covering thegetPlane/setPlaneround trip and normalization,copyandcloneindependence,containsPoint,containsSphere, andaddcontainment over sampled points.Performance:
Measured in a browser on a scene of 5072 mesh instances lit by 10 shadow casting omni lights, so 61 frustums are built per frame - one for the camera and one per shadow map face. Timings are of the culling work itself rather than whole frames, which in this scene it is far too small a share of to resolve. Each figure is the best of nine rounds, and the runs were stable to within a couple of percent.
The frustum build is the larger relative win, mostly from dropping the
Plane.setplusVec3.normalizecall chain rather than from the storage itself, and it scales with the number of shadow casting lights.Float32 rather than Float64 storage costs no real precision here, since
Mat4is itself backed by aFloat32Arrayand so the planes were already derived from float32 inputs. The extra rounding works out at roughly 5e-6 of the camera's distance from the origin - tens of microns at this scene's scale. It is measurable at the frustum boundary: this scene reports 3550 mesh instances visible after the change and 3548 before, a difference of two objects sitting within that band of a plane. A smaller scene of 1542 mesh instances reported an identical count either way.