feat(video): backend-neutral vertex formats and draw topologies (#1551) - #1558
Merged
Conversation
The batcher's vertex layout has been a portable descriptor since #1509 — except `type` held a raw GLenum and draw modes held `gl.TRIANGLES`. Those were the last GL-specific values in it, so a second backend would have to translate them on every path instead of reading the record. An attribute can now be declared with a single format token, and a draw with a topology name: { name: "aColor", format: "unorm8x4", offset: 20 } batcher.mode = "triangle-list" The GL form is accepted indefinitely and unchanged. `addAttribute` takes three shapes — a descriptor object, `(name, format, offset)`, and the existing `(name, size, glType, normalized, offset)` — and records carry both spellings, so every existing reader of `size`/`type`/`normalized` is untouched and `vertexstate.js` needed no change at all. The spine plugin still declares its layout in GLenum and was deliberately left alone as the live back-compat test. The format vocabulary is the canonical one because translation is only lossless downward: `"unorm8x4"` yields 4 + UNSIGNED_BYTE + normalized, while UNSIGNED_BYTE alone cannot say how many components or whether it is normalized. Inferring the format from the typed array instead — the other approach in use elsewhere — cannot work here: the interleaved buffer is a Float32Array while `aColor` is four normalized bytes smuggled through it, so the data cannot describe itself. `Batcher.mode` becomes an accessor pair over one private canonical topology. This is not cosmetic: a plain field cannot intercept `batcher.mode = "triangle-list"`, and WebIDL coerces that string to 0 — which is GL_POINTS. The batch would render as points with no error, and `PrimitiveBatcher`'s topology switch falls through to POINTS as well, so two independent paths would agree on the same wrong answer. Reading `mode` still returns the GLenum, so every internal comparison and the switch are untouched; `topology` is the new portable spelling. Two behaviour changes on the legacy path, both fixes, both in the changelog: an omitted `offset` now packs after the previous attribute rather than silently defaulting to byte 0, and a stride that is not a multiple of 4 now throws instead of building a fractional vertex size that discards every write. The five built-in layouts are migrated, which is what runs the format table through the whole rendering suite — the hardcoded stride table and the 19-pointer count catch any byte-size error immediately. `mesh`'s `aColor` stays `float32x4`, not `unorm8x4`; the Metal NaN-canonicalization rationale for that is unchanged. Tests: 214 new, 100% line/branch/function coverage on all four new modules. Every one of the 36 formats and 7 topologies is pinned against a longhand table rather than a derivation, all 7 GL types x 4 component counts x both normalization settings are covered, and legacy and format spellings are asserted to produce byte-identical records. Verified visually across ten examples covering all five migrated layouts, the primitive topology paths and the untouched plugin. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
CI failed with two `beforeAll` timeouts and 12 skipped tests — the shared WebGL context stopped coming up partway through the run. The table-driven cases in `webgl_attribute_vocabulary.spec.js` each built a whole `Batcher`, and a `Batcher` compiles a shader program and allocates buffers plus a vertex array. At roughly 84 constructions per run, none of them freed, that is enough live GL state to starve a software renderer late in a session — and the casualty is whichever spec happens to ask for a context next, not the one responsible. `addAttribute` needs a real `Batcher` only because it calls private methods, so the tables now reuse one and reset its layout between cases. Measured: 84 shader programs down to 13. Also fast-paths the topology normalization added in this branch. `Batcher.flush`, `QuadBatcher.flush` and `PrimitiveBatcher.drawVertices` were resolving on every call, and resolving a number now costs a WeakMap plus a Map lookup — per draw, on the hottest path in the renderer. Only strings need resolving: `this.mode` is normalized by its setter, so the numeric path is back to what it cost before this branch. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adopts a backend-neutral vocabulary for vertex layouts and draw topologies, closing #1551. Groundwork for the WebGPU backend (#1184).
Since #1509 each
Batcherhas owned an immutable vertex descriptor that is already shaped like a GPU vertex-buffer layout — excepttypeheld a rawGLenumand draw modes heldgl.TRIANGLES. Those were the last GL-specific values in it, so a second backend would have to translate on every path rather than read the record.An attribute can now be declared with a single format token, and a draw with a topology name:
The GL form is accepted indefinitely and behaves exactly as before.
addAttributetakes three shapes — a descriptor object,(name, format, offset), and the existing(name, size, glType, normalized, offset)— and records carry both spellings, so every existing reader ofsize/type/normalizedis untouched andvertexstate.jsneeded no change at all.packages/spine-pluginstill declares its layout in GLenum and was deliberately left alone as the live back-compat test.Why formats rather than inferring from the data
Translation is only lossless downward:
"unorm8x4"yields 4 +UNSIGNED_BYTE+ normalized, whileUNSIGNED_BYTEalone cannot say how many components or whether it is normalized. Inferring the format from the typed array — the other approach in use elsewhere — cannot work here: the interleaved buffer is aFloat32ArraywhileaColoris four normalized bytes smuggled through it (hence theUint8Array-view upload). The data cannot describe itself, so the format has to be declared.A format-declared layout also needs no live rendering context, which is what lets one layout serve two backends.
Batcher.modebecomes an accessor pairNot cosmetic. A plain field cannot intercept
batcher.mode = "triangle-list", and WebIDL coerces that string to0— which isGL_POINTS. The batch would render as points with no error anywhere, andPrimitiveBatcher's topology switch falls through to POINTS as well, so two independent paths would agree on the same wrong answer. Readingmodestill returns the GLenum, so every internal comparison and the switch are untouched;topologyis the new portable spelling.Two behaviour changes on the legacy path
Both are fixes, both are in the changelog:
offsetnow packs after the previous attribute instead of silently defaulting to byte 0 (which overlapped every attribute)Scope
36 vertex formats and 7 topologies.
float16*is excluded because no vertex buffer view can currently populate it; packed and BGRA formats because they break thecomponents × sizestride formula or have no WebGL equivalent."line-loop"and"triangle-fan"ship as documented engine extensions — modern GPU APIs have neither, and the primitive batcher uses both.The five built-in layouts are migrated, which is what runs the format table through the whole rendering suite: the hardcoded stride table and the 19-pointer count catch any byte-size error immediately.
mesh'saColorstaysfloat32x4, notunorm8x4— the Metal NaN-canonicalization rationale for that is unchanged.Type of change
Checklist
pnpm lintpasses)pnpm testpasses)pnpm build)214 new tests, 100% line/branch/function coverage on all four new modules. Every one of the 36 formats and 7 topologies is pinned against a longhand table rather than a derivation of the implementation; all 7 GL types × 4 component counts × both normalization settings are covered; legacy and format spellings are asserted to produce byte-identical records.
Verified visually across ten examples covering all five migrated layouts, the primitive topology paths (dashed lines, polygon fills, arcs, thick-stroke expansion) and the untouched plugin — each compared against its own same-code noise floor.
An adversarial review before merge caught a blocker that would have shipped: the emitted declarations typed the setters as
number, makingbatcher.mode = "line-list"— the headline affordance — a compile error for TypeScript consumers. It also found a surviving mutant inQuadBatcher.flushand two assertions that could not fail. All fixed and mutation-verified.Related issues
Closes #1551. Largely supersedes #1492, whose declarative-descriptor ask was delivered by #1509 — what remained was the vocabulary. Groundwork for #1184; #1552 and #1555 build the uniform-buffer machinery on top.
🤖 Generated with Claude Code