Skip to content

perf(mesh): retained-mode mesh rendering (#1507) - #1557

Merged
obiot merged 15 commits into
masterfrom
feat/1507-retained-mesh
Jul 31, 2026
Merged

perf(mesh): retained-mode mesh rendering (#1507)#1557
obiot merged 15 commits into
masterfrom
feat/1507-retained-mesh

Conversation

@obiot

@obiot obiot commented Jul 31, 2026

Copy link
Copy Markdown
Member

Description

Converts mesh rendering from immediate mode to retained mode, closing #1507.

Previously every mesh baked its transform into every vertex on the CPU each frame, the batcher rebuilt the interleaved stream applying the view matrix and tint per vertex, and the whole thing was re-uploaded with STREAM_DRAW. A model that never changed paid all of that sixty times a second.

Geometry now lives on the GPU in model space, and placement is carried by uniforms (uModelMatrix / uViewMatrix / uTint). Moving, rotating, scaling or re-tinting a mesh therefore touches no buffer at all. Buffers are rewritten only when the mesh says its geometry changed, via the new public Mesh.needsUpdate.

There is no "static" flag, which is where this departs from the ticket's proposal. The ticket asked for mesh.static = true plus dirty tracking; making placement a uniform removes the thing there was to opt into. For the same reason the ticket's "animated meshes stay on the immediate path" no longer applies — a changing transform is a uniform write, so animated glTF models are retained too. Only the Camera2d path stays immediate, because its CPU perspective divide cannot fold into a mat4.

Two improvements fall out as deletions rather than additions:

  • indices upload as authored, and the reflection bridge is corrected with frontFace(CW) instead of maintaining a winding-reversed copy of the index array
  • getBounds3d() bounds the model-space geometry through the model matrix, so it is correct before the first draw instead of reflecting the last one (it previously had no test coverage at all)

A mesh past 65 535 vertices is also a single drawElements call rather than N chunks, since WebGL 2 draws 32-bit indices natively.

Performance

Measured on an Apple M4 Max (ANGLE Metal), draw-phase CPU per frame, at the vertex counts from the ticket:

vertices/frame before after draw calls
158k 3.6ms 0.20ms 47 → 9
376k 6.0ms 0.20ms 110 → 14
718k 9.6ms 0.27ms 206 → 19
1.16M 13.8ms 0.29ms 349 → 23

The old path scales linearly with vertex count; the new one is flat, because no per-vertex work happens. At 1.16M vertices, submitting the scene went from 83% of a 60fps frame budget to under 2%. This is the CPU cost of submitting a frame — GPU work is not waited on, so a scene limited by fill rate rather than geometry submission will see less of it back.

Also included

  • Application.updateAverageDeltalastUpdateDelta (deprecated alias kept, non-breaking). It holds the measured cost of the most recent logic step and has never been an average — the smoothing was removed in 2015, a day after it was added. It is a different quantity from updateDelta, which is simulated time.
  • debug-plugin 16.1.1 — the frame-time readouts were both wrong. draw printed two decimals on a clock clamped to 100µs, so the second decimal could never move; it is now a 30-frame mean. update subtracted two values that are not a matched pair (GAME_AFTER_UPDATE carries lastUpdate, assigned only inside the fixed-step loop, hence stale on frames that run no step) and went negative; it now reads the engine's own measurement. The panel also now reads the application it was registered against rather than the global game singleton, and derives its minimum-engine gate from peerDependencies.

Review findings

An adversarial pre-merge review caught five defects that are fixed here: lit meshes could render NaN (the CPU path substituted a unit vector for zero-length normals; the retained builder uploaded them raw), toPolygon() returned a hull collapsed onto the origin on a multi-camera stage, GPU geometry leaked when a mesh was removed with keepalive or recycled through the pool, drawElements ignored batcher.mode, and frontFace was never restored.

It also found nine tests that passed vacuously — the worst asserted pixel[3] > 0 to prove state restoration, which a cleared framebuffer satisfies. All are repaired and mutation-verified.

Type of change

  • Bug fix
  • Performance improvement
  • Refactoring (no functional changes)

Checklist

  • I have read the Contributing Guide
  • My code follows the existing code style (pnpm lint passes)
  • I have tested my changes locally (pnpm test passes)
  • I have added tests that cover my changes (if applicable)
  • The build succeeds (pnpm build)

4931 engine tests + 16 debug-plugin tests, 0 lint errors, tsc and biome clean. The debug-plugin package had no test setup before this; one is added following the physics-adapter pattern, and wired into CI.

Related issues

Closes #1507

Unblocks #1508 (instancing), which now needs only a per-instance transform attribute plus drawElementsInstanced rather than a redesign.

🤖 Generated with Claude Code

obiot and others added 7 commits July 30, 2026 10:35
Mesh drawing was immediate-mode: every frame, every mesh baked its
transform into every vertex on the CPU, the batcher rebuilt the
interleaved stream applying the view matrix and tint per vertex, and
the whole thing was re-uploaded with STREAM_DRAW. A model that never
changed paid all of that sixty times a second.

Geometry now lives on the GPU in model space and placement is carried
by uniforms (uModelMatrix / uViewMatrix / uTint), so moving, rotating,
scaling or re-tinting a mesh touches no buffer at all. Buffers are
rewritten only when the mesh says its geometry changed, via the new
public `Mesh.needsUpdate` signal. There is no "static" flag to set:
placement stopped being part of the geometry, so nothing needs to
opt in.

Two things fall out of the model-space move rather than being added:

- indices upload as authored and the reflection bridge is corrected
  with frontFace(CW) instead of a reversed index copy
- getBounds3d() bounds the model-space geometry through the model
  matrix, so it is correct before the first draw rather than
  reflecting the last one

A mesh past 65 535 vertices is also one drawElements call now instead
of N chunks, since WebGL 2 draws 32-bit indices natively.

The Camera2d path is unchanged: its CPU perspective divide cannot fold
into a mat4, so it keeps chunked accumulation. _projectVerticesWorld
and friends are kept as the CPU reference for toPolygon and the Canvas
renderer.

Also fixes a latent bug in WebGLVertexState.build(): a vertex state
that rebuilt while it was itself the bound one restored the binding to
the handle release() had just deleted, raising INVALID_OPERATION and
leaving no vertex array bound. That is the path a batcher takes through
reset() and context restore.

Verification: 4919 tests green. Two new specs pin the claim by counting
GL calls, which is exact and reproducible in CI where headless frame
times are not — zero bufferData calls and zero CPU vertex projections
on every frame after the first, proven non-vacuous by mutation-testing
the retained path off. Winding is pinned as retained/legacy parity
across both bridges and both authored windings. Examples verified
byte-identical between the two paths (camera-3d, billboard, gltf); the
animated ones differ only within their measured same-code noise floor.

Measured on an Apple M4 Max (ANGLE Metal) across the vertex counts from
the ticket, draw-phase CPU per frame falls from 3.6ms to 0.20ms at 158k
verts and from 13.8ms to 0.29ms at 1.16M — linear before, flat after,
83% of a 60fps budget down to under 2%.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
…g (16.1.1)

The update/draw readouts printed `toFixed(2)` on a raw per-frame
`performance.now()` delta. Browsers clamp that clock to 100µs unless the
page is cross-origin isolated, so a single frame's timing can only land
on a multiple of 0.1ms and the second decimal was structurally always
zero — the panel was advertising precision the clock never provided.

Worse, a sub-millisecond phase straddling a tick boundary flickered
between neighbouring values (a ~0.5ms draw reading as 0.00 / 1.00)
rather than showing the figure in between, which invites reading a
floor as a measurement.

Both readouts now show a mean over the last 30 frames. Quantization
error is what averages out, so the effective resolution drops well
under the tick. Samples are collected on every frame rather than inside
`_updatePanel`, so the average is already warm when the panel is first
opened, and the mean is summed fresh each call so no floating point
drift accumulates over a long session. The sparkline keeps plotting raw
per-frame values, so spikes stay visible.

Verified in-browser on the multi-material-mesh example: before reads
0.20 / 0.30 / 0.40 / 0.10 (always a multiple of 0.1, hundredths pinned
at zero), after reads 0.10 / 0.11 / 0.12 / 0.15 / 0.18 / 0.19.

Patch release: a presentation change with no melonJS API surface and no
new runtime dependencies, so the `melonjs >=19.8.0` peer requirement is
unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
The field holds the measured wall-clock cost of the most recent logic
step. It has not been an average since 2015: `08215501a` introduced it
with exponential smoothing, and `2847f9bdb` removed the smoothing the
next day. The name survived 11 years, through the game -> Application
refactor and the TypeScript conversion.

The name actively misleads, because `updateDelta` sits two lines away
and is a different kind of quantity: simulated time a step advances
(fixed at 1000 / world.fps), versus real time that step cost. They meet
in the accumulator's spiral-of-death guard, where reading them as the
same kind of thing is precisely the mistake to avoid.

`updateAverageDelta` stays as a deprecated accessor forwarding to the
new field, for removal in 21.0.0 — a silent `undefined` is a poor
failure mode for something public since 2015. Both fields also gained
the JSDoc explaining which is which.

Tests: previously untested. Adds coverage that the value is a real
elapsed measurement (a child that burns 3ms in the update phase must
show up, which mutation-testing confirms a hardcoded zero fails), that
it is distinct from `updateDelta`, that it is what the accumulator
drains by when it exceeds the step size, and that the deprecated alias
reads and writes through. The measurement test asserts `lastUpdateStart`
left null first — without that the assertions passed vacuously on a
value that never left zero, which is how the first draft was wrong.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
…ne names

The update readout now prefers `lastUpdateDelta` (melonJS 20.0.0) and
falls back to `updateAverageDelta`, so this release keeps working across
the whole supported range — the package declares `melonjs >=19.8.0`, and
19.x only has the old name.

This package had no test setup at all, so adds one alongside the
existing pattern in the physics adapters: a browser-mode vitest config,
a `test` script, and the job in CI (without that last part the specs
would never run, since the workflow invokes each package's tests
explicitly).

Nine tests over the two things 16.1.1 fixes: that the mean divides by
the sample count rather than the buffer capacity, that it returns 0
instead of NaN before the first sample, that it recovers a mid-tick
value from readings quantised to a coarse clock (the actual symptom),
that a bounded window ages old frames out, that the update readout
reads the engine's measurement and can never go negative, that the 19.x
fallback works, and that the new name wins when both are present.

Mutation-tested both directions: removing the fallback fails the 19.x
test, and dividing by capacity fails the sample-count test.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Five defects found by an adversarial review of the retained-mode work,
plus the test repairs that would have caught them.

- Lit meshes could render NaN. `_projectNormalsWorld` substituted a unit
  +Y vector for zero-length source normals, precisely because the shader
  does `normalize(vNormal)` and `normalize(vec3(0))` is NaN. The retained
  builder uploaded them raw, so a degenerate triangle in an OBJ/glTF
  asset turned every fragment touching that vertex to garbage. The whole
  lit retained path had no test; it now has two.

- `toPolygon()` guarded on the activation-time `_useWorldSpace` while
  `draw()` chooses from the live viewport. On a multi-camera stage, or a
  mesh drawn without ever being parented, the projection was skipped and
  the hull was read out of a `vertices` array the retained path never
  writes — collapsing it onto the origin. Now keyed off `_worldSpace`.
  `toPolygon()` had no test anywhere; it does now.

- GPU geometry leaked whenever a mesh left the world without being
  destroyed: `removeChild(child, keepalive)` and pool recycling both skip
  `destroy()`, so nothing evicted the retained entry and its VBO/IBO/VAO
  survived until the next renderer reset. Released on `onDeactivateEvent`
  instead, which also means a recycled instance rebuilds from scratch
  rather than inheriting the previous occupant's geometry.

- `drawRetainedMesh` hardcoded `gl.TRIANGLES`, ignoring `batcher.mode`,
  so the same mesh drew as triangles retained and as lines accumulated.

- `frontFace(CW)` was never restored, unlike the `CULL_FACE` toggle
  beside it, leaking a non-default winding to any later consumer of the
  context.

`_setupWorldSpace` also no longer builds the winding-reversed index copy
on the retained path, which corrects winding with `frontFace` and never
reads it — that copy is the size of the whole index array, megabytes on
a dense mesh.

Test repairs (each verified by mutation): the state-restore test asserted
`pixel[3] > 0`, but a cleared framebuffer is already [0,0,0,255], so it
passed with the restore deleted entirely — it now asserts the binding
invariant, since the dynamic path's fixed z makes a colour check
depth-confounded. The >65k test could not see indices being narrowed to
16 bits. The context-loss test asserted only "did not throw". The reset
test checked map size while every GL object leaked. The billboard test
could not tell "no uploads" from "nothing drawn".

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Review follow-ups.

`typeof x !== "undefined"` passed for `null` and had no terminal
fallback, so an engine exposing neither name would write `undefined`
into the Float32Array sample ring — one `undefined` makes it NaN, and
the panel then prints "NaNms" for the next 30 frames while the graph's
bars silently vanish (`NaN > 0` is false). `??` chained to 0 degrades
instead. Unreachable inside the declared peer range, but the failure
mode was silent and permanent.

More importantly the averaging — the actual fix in 16.1.1 — had no
wiring test: every test poked the sample buffers directly and none
called `_onAfterDraw`, so deleting the whole ring-buffer collection, or
dropping the `_timeSampleCount` cap, or swapping the update and draw
buffers all left the suite green. Three tests now drive real frames
through the handler and cover the wrap, the cap, and buffer separation;
all three mutations now fail.

Also dropped two tests that could not fail: one asserted a non-negative
value from a plain field copy, and "prefers the new name" set only the
field, which the deprecated alias reads through — so both names returned
the same number and the assertion held whichever the code picked. It now
shadows the old name with a different value.

README updated: the readouts are averaged, which a reader otherwise
interprets as this frame's cost.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
… gate

Two wrong-reference bugs, both pre-existing.

The panel read the global `game` singleton in seven places while
`BasePlugin` already hands it the registering application as `this.app`.
`game` is whichever Application was constructed last and the frame
events are broadcast by every one of them, so with more than one
application on a page the panel silently reported another's numbers. It
could not even be bound to a non-default app: the constructor called a
bare `super()`. It now takes and forwards one — `plugin.register` passes
its extra arguments through, so this is additive and omitting it keeps
the previous behaviour. The `game` import is gone entirely.

The minimum-engine gate was a hardcoded "19.5.0" while the package's
peer range said ">=19.8.0". Both state the same fact and had drifted:
19.5 is what 16.0.0's physics overlay needed, 19.8 is what 16.1.0's 3D
bounding-box overlay needs, and raising the peer range didn't raise the
gate. So melonJS 19.6/19.7 loaded the plugin and then failed inside the
mesh overlay. The gate is the enforcing side — `plugin.register` throws,
whereas the peer range only warns at install and not at all for CDN or
pre-bundled use. It is now derived from `peerDependencies.melonjs`, with
the comparator stripped because `checkVersion` compares numeric parts
and would read a leading ">=" as 0.

Tests: cross-application isolation (a panel bound to app A reports A's
timing while the global points at B — writing that test surfaced that
constructing an Application repoints `game`, which is the hazard
itself), and two that pin the gate to the declared range so it cannot
drift again. Mutation-verified: hardcoding the gate back, and leaving
the comparator in, each fail.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 00:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

CI failed with two `beforeAll` timeouts at 30s, in
`webgl_retained_geometry.spec.js` (new here) and
`texture-resource.spec.js` (pre-existing, untouched by this branch).
Both hooks do `boot()` + `video.init(WEBGL)`; 5282 tests passed, so
nothing was actually broken. Master is green, so the two spec files
added here pushed an already-marginal runner over.

Every spec file that calls `video.init` pays a WebGL context creation,
which on a container with no GPU goes through a software rasterizer.
The two specs added for #1507 needed one context between them, not two,
so `webgl_retained_geometry.spec.js` is folded into
`webgl_mesh_retained.spec.js` as a nested describe. Same 21 tests, one
context.

It has to sit before the context-loss test, not after: that test leaves
the context unusable, and running the unit tests behind it returned
CONTEXT_LOST_WEBGL from every `getError`. Same ordering trap the file
already documents.

`hookTimeout` also raised to 90s. The default fails a correct suite
whenever the container is loaded — this hook is genuinely slow, not
hanging, and `texture-resource.spec.js` timing out shows the margin was
already thin without this branch.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 00:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Second CI failure, and this time both timeouts were in specs this branch
never touches (`texture-resource`, `texturecache-batcher-reset`) — at
the 90s limit raised in the previous commit. 5281 tests passed. So it is
not slowness, and raising the ceiling further would not help.

The suite forces a WebGL context in ~34 spec files and essentially none
release it. Browsers cap live contexts and force-lose the oldest once
past that cap, after which creating another stalls — which is exactly
the observed shape: a `beforeAll` somewhere late in the run times out
with nothing wrong in it, and which spec draws the short straw moves
around as files are added or reordered. Master sits just under the
threshold; the one file added here crosses it.

This spec now hands its context back in `afterAll` before restoring, so
the branch stops consuming a slot the baseline did not.

That is a down-payment, not the fix. The suite will hit this again the
next time a WebGL spec is added, and the durable answer is a shared
context fixture — a test-infrastructure change too broad to fold into
this PR.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 00:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Third CI failure, identical to the second: the same two pre-existing
specs timing out in `beforeAll` at 90s, 5281 tests passing. Neither the
spec merge nor the context release moved it, because both were treating
a symptom.

The actual cause is in the config. `pnpm test` runs
`vitest --config packages/melonjs/vitest.config.ts` from the repo ROOT,
and the config's `include` was unanchored — so it globbed every
workspace package, not just this one. CI reports 180 test files where
running from `packages/melonjs` reports 173: the extra 7 are the two
physics adapters' specs and the debug-plugin spec added on this branch,
all of which CI *also* runs through their own `pnpm -F ... test` jobs.

So those files were executing twice, and more to the point each one that
calls `video.init` adds another WebGL context to the single shared
browser session. Browsers cap live contexts; past the cap a later
`beforeAll` stalls, which is why the victim was always some unrelated
spec late in the run.

Anchoring `root` to the package and scoping `include` to `tests/` takes
the shared session from 180 files to 173 — five fewer than master's 178,
where CI is green. No coverage is lost: every package with specs has its
own CI job, verified.

Local runs from the repo root and from `packages/melonjs` now agree at
173 files, which they did not before — that divergence is what hid this.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Found the actual CI cause, which none of the previous three attempts
addressed.

`webgl_mesh_retained.spec.js` deliberately loses the WebGL context, then
called `restoreContext()` and asserted immediately — never waiting for
the restore to complete. The file therefore ended with the context still
coming back. The two pre-existing specs that do the same cycle
(`webgl_vao_recreation`, `webgl_vao_adversarial`) both await
`ONCONTEXT_RESTORED` first; this one didn't, and the previous commit
made it worse by losing the context a second time in `afterAll`.

On a machine with a real GPU the restore is instant, which is why it
never reproduced locally. On a GPU-less CI container it is not, and
whatever spec ran next stalled waiting on a context that had not come
back — always some unrelated file, which is why the casualty moved
around and why merging specs and scoping the config both failed to fix
it.

The test now follows the house pattern: yield so `webglcontextlost` is
dispatched, restore, await `ONCONTEXT_RESTORED`, then assert. Doing that
also made it a real test rather than a "did not throw" — it now pins
that the geometry is rebuilt from scratch, exactly one upload and one
draw, rather than drawing through handles from the dead context.

It surfaced something else worth recording: the engine's own restore
sequence leaves an INVALID_OPERATION pending before any mesh code runs.
Pre-existing and out of scope here, so the test drains it explicitly and
says why, rather than silently absorbing it.

Also adds `tests/helpers/webgl-context.js`: one WebGL renderer shared by
every spec that asks for one, instead of a fresh context per spec file.
`video.init` builds a new canvas and GL context on each call, browsers
cap how many they keep alive, and past that cap creation stalls —
producing exactly this class of misattributed timeout. Three specs are
migrated as proof, including the two that kept failing; the remaining
~18 are mechanical follow-up.

`texture-resource.spec.js` gains something in the move: it never passed
`failIfMajorPerformanceCaveat: false`, so on a software-GL runner its
WebGL init is rejected outright and the whole block skipped. Through the
fixture it actually runs there.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 01:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

27 spec files now borrow one WebGL renderer instead of creating their
own; 34 did before. `video.init` builds a fresh canvas and GL context on
every call, browsers cap how many they keep alive, and past that cap
creation stalls in a spec that has nothing to do with the cause.

Seven keep their own context, each for a reason:

  webgl_vao_teardown        destroys its renderer on purpose
  webgl_required_check      asserts init throws when WebGL is absent
  webgl_vao_adversarial     manages renderer lifetime itself
  glcore-audit              needs `transparent: true` for alpha readback
  depth                     switches renderer six times
  toframetexture            Canvas/WebGL parity in one file
  shadereffect-settexture   same

Also fixes `RetainedGeometry.destroy()` deleting buffers whose context
had already been lost, which raises INVALID_OPERATION — the handles die
with the context, so it now checks `isBuffer` first, the same guard
`WebGLVertexState.release()` already had. I had attributed that error to
the engine's restore path; it was mine. The context-loss test asserts a
clean `getError()` rather than draining it, and mutation-testing
confirms removing the guard fails it.

`timer.spec.js`'s two interval tests get an explicit 8s `vi.waitFor`
budget. They assert engine-driven timers, which advance only as fast as
the browser schedules the game loop, so the default 1s budget fails on a
loaded machine — verified flaky on a clean baseline before any of this
branch's changes. The assertions are unchanged, only the patience.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 03:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

CI failed on `renderTargetPool.spec.js` with a 15s *test* timeout. The
migration left its three WebGL cases calling `getWebGLRenderer()` inside
their own `it()` bodies, where they are bounded by the per-test budget
rather than the much larger hook budget — and creating the very first
WebGL context on a runner without a GPU can exceed it. Locally, where
that context comes up instantly, it passed.

Acquisition moves to a `beforeAll` in the enclosing describe, matching
every other migrated spec, with the per-test boilerplate (`boot()`, the
try/catch around init, the `video.renderer?.gl` probe) collapsing into a
single guard. Checked that no other spec has the same shape: every
`getWebGLRenderer` call in the suite is now inside a hook.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 04:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Every WebGL spec guards itself with `ctx.skip()`, which is right for one
test and wrong for the suite: a runner that silently loses WebGL turns
hundreds of tests into skips and still reports green. The engine's
primary backend would be entirely unexercised with nothing to say so.

That is not hypothetical. `texture-resource.spec.js` omitted
`failIfMajorPerformanceCaveat: false`, so on a software-GL runner its
whole WebGL block skipped — unnoticed until this branch. CI reported 13
skipped tests where a local run reported 1, and nobody had reason to
look.

Adds a tripwire that asserts a usable WebGL 2 context: defined renderer,
`WebGL 2` version string, non-zero MAX_TEXTURE_SIZE, context not lost.
It fails rather than skips, with a message naming the likely causes.
`VITE_ALLOW_NO_WEBGL=1` downgrades it to a skip for genuinely GL-less
hardware.

Mutation-verified: stubbing the fixture to return no renderer fails it.

Matters more as the WebGL/WebGPU backends become the primary path and
Canvas the secondary one — a green build has to mean the primary backend
actually ran.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 04:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

As first written the tripwire failed anywhere WebGL was missing, which
punishes the wrong party: a contributor on hardware without a usable GL
stack would get a red suite for an environment problem, not a code one.

It now distinguishes the two cases. In CI the container image is pinned
and WebGL must be present, so its absence is a hard failure — that is
the case worth catching, because it means every WebGL spec skipped and
the run went green with the primary backend untested. Anywhere else a
missing context is a visible skip, so local work is not blocked.
`VITE_ALLOW_NO_WEBGL=1` forces the skip in CI too, if a runner ever
genuinely loses its GL stack and the build should not be held hostage
while that is sorted out.

Vitest's `env` does not reach the browser side, so the config exposes
`process.env.CI` through a vite `define`.

Note this changes nothing about the engine: `video.AUTO` still falls
back to Canvas when WebGL is unavailable, and `video.WEBGL` still throws
(#1509). This is only about whether the suite is allowed to report
success without having exercised WebGL.

Verified all three paths: no-WebGL outside CI skips, no-WebGL in CI
fails with an actionable message, WebGL present in CI passes.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvxaXQRgQn5AoR8DNkv6Wg
Copilot AI review requested due to automatic review settings July 31, 2026 05:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@obiot
obiot merged commit 8de1d03 into master Jul 31, 2026
6 checks passed
@obiot
obiot deleted the feat/1507-retained-mesh branch July 31, 2026 05:16
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.

3D: retained-mode mesh rendering (upload geometry once, place with uniforms)

2 participants