perf(particles): compute particle bounds lazily - #1607
Merged
Conversation
A `Renderable` refreshes its bounds from a callback fired on every `pos`
assignment. That is the right trade for a scene object and the wrong one for a
particle: `Particle.update` writes `pos.x` and `pos.y` separately, so the
callback fired TWICE per particle per frame — and both runs happened before
`currentTransform` was rebuilt, deriving bounds from the previous frame's
matrix and then discarding the result. `accurateBounds: true`, which exists to
buy an accurate hitbox, added a third pass on top. So the cheap setting did the
work twice and the accurate one did it three times; neither did it once.
A particle now swaps its own position callback for one that marks the bounds
stale, and recomputes on read. A particle nothing looks at costs nothing, and
the bounds a reader gets are current rather than a frame behind.
Measured on WebGL with a burst emitter, every particle on screen and verified
actually drawn:
20,000 particles 20.1ms -> 14.9ms ceiling ~16,500 -> ~22,000
4x CPU throttle 19.8ms -> 14.5ms ceiling ~4,200 -> ~5,700
About a third off the update loop, and the same relative gain on throttled
hardware — which is the case that matters, since the absolute numbers above
come from a Mac Studio and are optimistic for anyone's players.
`accurateBounds` is deprecated as a result: it existed to trade hitbox accuracy
for speed, and there is no longer a trade to make. Still accepted, now inert.
Two things worth knowing for anyone touching this again. `updateBounds()` keeps
its eager contract because `Container.updateBounds` aggregates child bounds
through its RETURN value under `enableChildBoundsUpdate` — deferring there
would feed it stale data, which is why the callback rather than the method is
the lever. And the dirty flag is cleared at the TOP of `updateBounds()`:
`getBounds()` calls it, and it calls `getBounds()`, so clearing late recurses
until the stack blows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
None of those three entries were API changes. Two were consequences of what the release ADDED — a scene that already set one of the six modes now renders it — and the third records that 3D mesh rendering is UNCHANGED, which is the opposite of a change. The two substantive caveats, the per-draw capture and composite cost and the drawMesh fallback, move onto the Added entry they belong to. `### Changed` is for user-facing API changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
# Conflicts: # packages/melonjs/CHANGELOG.md
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.
Particles recomputed their bounding box twice per frame, from a stale transform, and threw both results away. Fixing that is worth about a third of the particle update loop.
The bug
Renderablerefreshes bounds from a callback fired on everyposassignment:That is the right trade for a scene object. For a particle it is the wrong one three times over —
Particle.updatedoes:currentTransformis rebuilt, so they derive bounds from the previous frame's matrixaccurateBounds: truemakes it threeSo the setting meant to be cheap did the work twice, and the setting meant to be accurate did it three times. Neither did it once.
The fix
A particle swaps its own position callback for one that just marks the bounds stale, and
getBounds()recomputes if needed. A particle nothing looks at costs nothing, and the bounds a reader gets are current rather than a frame behind.Contained entirely to
particle.ts. NoRenderablechange, so #817 stays untouched — the onesetCallbackline needs a local cast for exactly that reason, with a comment pointing at the ticket.Measurements
WebGL, burst emitter, every particle on screen and verified actually drawn (the harness counts
drawImagecalls — an early version of it droveemitter.update()directly, which skips the visibility pass, leaving every particle culled and reporting draw cost as ~0 regardless of count).Roughly a third off the update loop, and the same relative gain throttled — which is the case that counts. The absolute numbers come from a Mac Studio and are optimistic for real players; at 4x throttle the practical ceiling was ~1,400 particles before this, which games do reach.
accurateBoundsis deprecatedIt existed to trade hitbox accuracy for speed. There is no longer a trade: bounds are always current and cost one refresh per read. Still accepted, now inert, documented as such.
Two things for whoever touches this next
updateBounds()keeps its eager contract deliberately.Container.updateBoundsaggregates child bounds through its return value underenableChildBoundsUpdate(container.js:715), so deferring there would have fed it stale data. That is why the callback, not the method, is the lever — and there is a test for it.The dirty flag is cleared at the TOP of
updateBounds().getBounds()callsupdateBounds(), which callsgetBounds(); clearing late recurses until the stack blows. The mutation run confirms it — that ordering change takes all 10 tests down with a stack overflow.And the flag is not a
#privatefield.updateBounds()is reached from the base constructor chain viaPolygon.setVertices, before a subclass's field initializers exist, and writing an undeclared private field throws.Tests
12 new in
tests/particle-bounds.spec.js, covering both halves — that the laziness is real, and that nothing reading bounds can tell the difference. Includes theenableChildBoundsUpdateaggregation path, pooled-particle callback reinstallation, culling correctness, and the construction-ordering case.Three mutations, each breaking its own tests: restoring the eager callback (5 failures),
getBounds()no longer refreshing (3), and not clearing the flag first (10, stack overflow).Full suite 6341 passing / 263 files, root lint 0 errors.
🤖 Generated with Claude Code
https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N