Add WireRenderer for wireframe debug shapes, and a debug examples category - #9288
Conversation
…egory Resolves #6024. `AppBase` grew a set of `@ignore` debug drawing methods over time - `drawWireSphere` and `drawWireAlignedBox` among them - which are used by a good number of examples despite never being public. This moves that functionality into extras as `WireRenderer`, and fills it out into the shape set that debugging actually calls for. ```javascript const wire = new WireRenderer(app); wire.color = Color.RED; wire.sphere(entity.getPosition(), 2); wire.frustum(otherCamera.camera); ``` ## API State lives on the renderer - `color`, `layer`, `depthTest`, `segments`, `transform` - rather than in a per-call options object, so drawing a thousand shapes with the same settings allocates nothing. A second set of state is simply a second instance: instances hold no GPU resources, and those sharing a layer and depth test mode submit into the same batch, so using several costs nothing extra. This mirrors how `Immediate` already keys its batches on (layer, depthTest). Shapes: `line`, `lines`, `linesPacked`, `polyline`, `loop`, `box`, `boxMinMax`, `sphere`, `circle`, `cylinder`, `capsule`, `cone`, `plane`, `point`, `arrow`, `axes`, `frustum`, `light`. Everything is thin lines. Solid shapes and arbitrary mesh submission are left out deliberately: they need the mesh path, which allocates a MeshInstance and a GraphNode per call, and they belong with whatever replaces `drawMesh` rather than here. `WideLineRenderer` remains the home for thick production line geometry, and both classes now cross-reference the other. `frustum` accepts a camera or a view-projection matrix. It derives the view matrix from the camera's entity transform rather than reading `CameraComponent#viewMatrix`, which is only refreshed for a camera that is actually rendering - and visualizing a camera you are not looking through is the whole point. `light` follows the light's own axis. A light shines along the negative y-axis of its entity, not the negative z-axis that `lookAt` aims, so anything using `forward` here is 90 degrees wrong. ## Writing straight into the batch Shapes are generated directly into the immediate batch's storage rather than built in an array and copied in. `Immediate#allocateLines` claims an exact number of vertices, accounts for them up front and returns a reused `LineWriter` cursor. Because the space is accounted for immediately there is no commit call, and because the cursor is reused there is no allocation per shape. It is therefore only valid inside the function that allocated it. `_arc` writes the storage itself instead of going through `LineWriter#segment`. It backs sphere, circle, cylinder, capsule, cone and arrow, so nearly every segment passes through it. The other shapes use the checked `segment` and `vertex` methods. Measured on 1000 spheres, 60000 segments: 2.4ms to 1.4ms. Most of that came from hoisting the center and tangent reads out of the arc loop rather than from avoiding the call, which was worth only about 14 percent once the hoisting was in. Stacked on #9286 the same workload went from 13.6ms to 1.4ms. ## Examples Adds a `debug` category, since debug visualization examples were scattered: - `debug/wire-shapes` - every shape, animated across the range of values it takes, with segments, depth test and animation controls - `debug/frustum-culling` - an orbiting camera whose volume is drawn with `frustum`, every object's bounds colored by `Frustum#containsAabb`, and arrows showing direction of travel - `debug/lines` - moved from `graphics`, and rewritten onto WireRenderer. Uses all five line functions at once - `debug/mini-stats` - moved from `misc` `graphics/clustered-lighting` gains a toggle that draws every light's shape in its own color. ## Tests 76 new tests. `WireRenderer` is covered for exact allocation per shape, the degenerate input paths, real geometry rather than just counts, the color and transform paths, passthrough versus copy, batch routing, and `light` aiming along the correct axis. `LineWriter` is covered for offsets, bounds asserts and seeing reallocated storage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Build size reportThis PR changes the size of the minified bundles.
|
Public API reportThis PR changes the public API surface (+25 / −0), per the docs' rules (@ignore / @Private / undocumented are excluded). Show API diff+WireRenderer.arrow(from: Vec3, to: Vec3): void
+WireRenderer.axes(matrix: Mat4, size: number): void
+WireRenderer.box(box: BoundingBox | OrientedBox): void
+WireRenderer.boxMinMax(min: Vec3, max: Vec3): void
+WireRenderer.capsule(start: Vec3, end: Vec3, radius: number): void
+WireRenderer.circle(center: Vec3, normal: Vec3, radius: number): void
+WireRenderer.color: Color
+WireRenderer.cone(apex: Vec3, direction: Vec3, angle: number, length: number): void
+WireRenderer.constructor(app: AppBase)
+WireRenderer.cylinder(start: Vec3, end: Vec3, radius: number): void
+WireRenderer.depthTest: boolean
+WireRenderer.frustum(source: Mat4 | CameraComponent): void
+WireRenderer.layer: Layer | null
+WireRenderer.light(light: LightComponent, size?: number): void
+WireRenderer.line(start: Vec3, end: Vec3): void
+WireRenderer.lines(positions: Vec3[], colors?: Color[]): void
+WireRenderer.linesPacked(positions: number[] | Float32Array<ArrayBufferLike>, colors?: number[] | Float32Array<ArrayBufferLike>): void
+WireRenderer.loop(positions: Vec3[], colors?: Color[]): void
+WireRenderer.plane(center: Vec3, normal: Vec3, size: number): void
+WireRenderer.point(position: Vec3, size: number): void
+WireRenderer.polyline(positions: Vec3[], colors?: Color[]): void
+WireRenderer.segments: number
+WireRenderer.sphere(center: Vec3, radius: number): void
+WireRenderer.transform: Mat4 | null
+class WireRendererInformational only — this never fails the build. |
There was a problem hiding this comment.
🟡 Changes recommended
One critical type-resolution issue and two moderate geometry correctness issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an efficient extras WireRenderer for single-frame debug geometry, supported by direct immediate-batch line writing and dedicated debug examples.
Changes:
- Adds and publicly exports wireframe shape rendering.
- Adds direct line allocation/writing with comprehensive tests.
- Adds debug examples and clustered-lighting visualization controls.
Required fixes:
- Critical: Resolve the missing
LineWritertype import. - Moderate: Validate that packed positions contain complete point pairs.
- Moderate: Derive frustum matrices using the renderer’s override-aware, scale-free camera path.
File summaries
| File | Description |
|---|---|
test/scene/immediate/line-writer.test.mjs |
Tests cursor writes, bounds, and storage growth. |
test/extras/renderers/wire-renderer.test.mjs |
Tests geometry, routing, transforms, and edge cases. |
src/scene/immediate/line-writer.js |
Adds direct batch-storage cursor writing. |
src/scene/immediate/immediate.js |
Exposes exact line allocation. |
src/scene/immediate/immediate-batch.js |
Adds direct vertex-range allocation. |
src/extras/renderers/wire-renderer.js |
Implements wireframe shapes and renderer state. |
src/extras/index.js |
Exports WireRenderer. |
examples/thumbnails/debug_mini-stats_small.webp |
Adds the small MiniStats thumbnail. |
examples/thumbnails/debug_mini-stats_large.webp |
Adds the large MiniStats thumbnail. |
examples/src/examples/graphics/lines.example.mjs |
Removes the superseded lines example. |
examples/src/examples/graphics/clustered-lighting.example.mjs |
Adds optional light-shape visualization. |
examples/src/examples/graphics/clustered-lighting.controls.jsx |
Adds the light-shape toggle. |
examples/src/examples/debug/wire-shapes.example.mjs |
Demonstrates all wireframe primitives. |
examples/src/examples/debug/wire-shapes.controls.jsx |
Adds wire-shape controls. |
examples/src/examples/debug/mini-stats.example.mjs |
Adds MiniStats to the debug category. |
examples/src/examples/debug/lines.example.mjs |
Demonstrates line APIs. |
examples/src/examples/debug/frustum-culling.example.mjs |
Demonstrates frustums, bounds, and culling. |
examples/src/examples/debug/frustum-culling.controls.jsx |
Adds culling visualization controls. |
Review details
- Files reviewed: 15/26 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /** | ||
| * Cursor for the shape currently being written, valid only for the duration of one call. | ||
| * | ||
| * @type {LineWriter|null} |
| Debug.assert(colors === undefined || colors.length === (positions.length / 3) * 4, | ||
| 'WireRenderer#linesPacked colors must hold four values per position, or be undefined.'); |
| // the view matrix is derived from the entity transform rather than read from | ||
| // camera.viewMatrix, which is only refreshed for a camera that is being rendered - | ||
| // and visualizing a camera you are not looking through is the point of this function | ||
| _view.copy(camera.entity.getWorldTransform()).invert(); | ||
| _mat.mul2(camera.projectionMatrix, _view); |
…r type `frustum` now builds a camera's matrices the way the renderer does, so the shape drawn is the volume the camera would actually render. The view transform is scale-free, and calculateProjection and calculateTransform are both honored. Previously the full entity world transform was inverted, so a camera under a scaled parent, or an oblique or reflection camera, drew the wrong frustum. `linesPacked` asserts its position count is a multiple of six. Anything else either leaves a partial position, which made the vertex count fractional and corrupted the batch, or an unpaired vertex, which mispairs every later line. Imports the `LineWriter` type, which was annotated but never declared. Adds tests for all three, and both frustum tests fail against the previous derivation.
mvaligursky
left a comment
There was a problem hiding this comment.
Automated PR review — Codex (GPT-5)
Reviewed the complete diff at bac7358087e15c9129b1d3c03901ad8f7d8e2d29, including the follow-up commit and existing review discussion. I found no remaining actionable issues.
The follow-up correctly resolves all three earlier findings: the LineWriter type is declared, linesPacked rejects incomplete segment data, and camera frustums now follow the renderer's scale-free, override-aware projection/transform path. I also checked the direct-write allocator and cursor lifecycle across buffer growth and frame reuse; exact vertex counts and all shape-generation paths; transforms, degenerate geometry, colors, layers and depth routing; light orientation; the public API/JSDoc/type surface; example moves and route references; and the WebGL/WebGPU abstraction boundary.
Verification performed:
- Focused
WireRenderer/LineWritertests: 60 passing - Full unit suite: passing
npm run lint: passingnpm run build:typesandnpm run test:types: passinggit diff --check: clean- GitHub CI, docs, examples-browser build, API report and deployment checks: green
The reported bundle increase (~8.2 KB minified / ~2.3–2.9 KB compressed) is noticeable but proportionate to the substantial public debug-rendering API and examples added here.
`AppBase#drawWireSphere` and `#drawWireAlignedBox` are replaced by `WireRenderer#sphere` and `#boxMinMax` from #9288, so their bodies and docs are gone and they report themselves through `Debug.removed`. Both were `@ignore` and absent from the type definitions, so this is not a public API change, but they were used widely enough that a silent removal would be unkind. `Debug.removed` is dropped from release builds, so a caller in production gets a no-op and the message only in development. `Color` is now only referenced from JSDoc in this file, so it becomes a type import. The other five `@ignore` draw methods stay. `drawMesh`, `drawMeshInstance`, `drawQuad`, `drawTexture` and `drawDepthTexture` have no replacement yet, and belong with whatever replaces the mesh submission path. `Immediate#drawWireSphere` and `#drawWireAlignedBox` both stay as well. The aligned box has three callers inside the engine - gsplat-data, gsplat-manager and gsplat-octree-instance - which cannot use WireRenderer because core does not depend on extras. The sphere has no callers left, but is worth keeping alongside it. Migrates the four examples that used the removed methods: - gaussian-splatting/clipping and gaussian-splatting/editor draw a single colored box, so the color moves to the renderer - gaussian-splatting/wind colors each handle as it is drawn, and its explicit segment count of 20 was already the default - physics/offset-collision reached into `scene.immediate` directly and passed a non default segment count and an explicit layer. Those are renderer settings, so they are applied once outside the update loop and the call itself becomes `wire.sphere(position, 0.3)` Verified in the browser: all four still draw as before, wind keeps its two handle colors, and the stubs report the expected messages. Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
|
Based on API, we often have to render some primitive shapes (for physics for example) that have transform (rotation and position). For example Capsule that is rotating, currently it seems that capsule will only be able to be one of the axes, and not be rotated. Also very simple shape "square", but again with rotation, not really a normal vector. |
use this engine/src/extras/renderers/wire-renderer.js Lines 140 to 146 in 2ef56a2
sure, can be added as needed, create PRs as needed. |
Resolves #6024.
AppBaseaccumulated a set of@ignoredebug drawing methods over time —drawWireSphereanddrawWireAlignedBoxamong them — used by a fair number of examples despite never being public API. This moves that into extras asWireRendererand fills it out into the shape set debugging actually needs.Examples
API
State lives on the renderer —
color,layer,depthTest,segments,transform— rather than a per-call options object, so drawing a thousand shapes with the same settings allocates nothing. A second set of state is just a second instance: instances hold no GPU resources, and those sharing a layer and depth test mode submit into the same batch, so using several costs nothing extra. That mirrors howImmediatealready keys batches on (layer, depthTest).Shapes:
line,lines,linesPacked,polyline,loop,box,boxMinMax,sphere,circle,cylinder,capsule,cone,plane,point,arrow,axes,frustum,light.Everything is thin lines. Solid shapes and arbitrary mesh submission are deliberately out: they need the mesh path, which allocates a
MeshInstanceandGraphNodeper call, and they belong with whatever replacesdrawMesh.WideLineRendererstays the home for thick production line geometry, and the two classes now cross-reference each other.Two details worth calling out:
frustumaccepts a camera or a view-projection matrix, and derives the view matrix from the camera's entity transform rather than readingCameraComponent#viewMatrix— that is only refreshed for a camera actually being rendered, and visualizing a camera you are not looking through is the whole point.lightfollows the light's own axis. A light shines along the negative y-axis of its entity, not the negative z thatlookAtaims, so anything usingforwardis 90 degrees wrong.Writing straight into the batch
Shapes generate directly into the immediate batch's storage instead of being built in an array and copied in.
Immediate#allocateLines(@ignore, and onImmediaterather thanAppBase, since this issue is about removing draw methods from AppBase) claims an exact vertex count, accounts for it up front, and returns a reusedLineWritercursor. Because the space is accounted for immediately there is no commit call, and because the cursor is reused there is no per-shape allocation — so it is only valid inside the function that allocated it._arcwrites the storage itself rather than going throughLineWriter#segment, as it backs sphere, circle, cylinder, capsule, cone and arrow and so carries nearly every segment. The other shapes use the bounds-checkedsegment/vertexmethods.Measured, 1000 spheres / 60000 segments: 2.4ms → 1.4ms. Most of that came from hoisting the center and tangent reads out of the arc loop, not from avoiding the call — which was worth only ~14% once the hoisting was in. Stacked on #9286, the same workload went 13.6ms → 1.4ms.
Tests
76 new.
WireRenderercovers exact allocation per shape (an under-fill would silently leave zeroed vertices, so each case asserts both the count and that the allocation was filled exactly), the degenerate input paths, real geometry rather than just counts (every sphere vertex on the sphere, frustum corners unprojected to±1 @ z=-1and±10 @ z=-10), the color and transform paths, passthrough versus copy, batch routing, andlightaiming along the correct axis.LineWritercovers offsets, bounds asserts, and seeing reallocated storage after the batch grows.Notes for review
allocateLines,LineWriter,ImmediateBatch#allocate) are in this PR rather than split out, because they have no consumer other thanWireRenderer— splitting would land an unused API. Happy to separate them if you would rather.AppBase's existingdrawWireSphere/drawWireAlignedBoxare not deprecated here. That is a follow-up, and their generation code should leaveImmediatetoo rather than being fronted by a facade.🤖 Generated with Claude Code