Camera3d: distance fog - #1632
Merged
Merged
Conversation
`camera.setFog({ mode, near, far, density, color })` fades mesh geometry
toward a colour with distance — "linear" between two distances or "exp2"
from a single density, the two parameterisations inherited from
fixed-function graphics pipelines.
Every parameter is optional and the omitted ones resolve LIVE rather than
being captured at the call. The distances track the camera's own clip
planes, which is the whole reason fog belongs to the camera: a snapshot
would go out of step the moment `setClipPlanes` was called, and the
symptom — geometry clipping before it finished fading — reads as a fog
bug rather than a stale-copy bug. The colour tracks
`renderer.backgroundColor` for the same reason: a day/night fade must not
leave a band at the horizon. Pass `color` only when the fog should
deliberately differ from the backdrop.
Measured radially rather than from view-space z, so fog holds steady as
the camera turns, and applied per fragment, so it does not band across
large triangles.
Two details worth naming:
The fog blend is `mix(fogColor * a, rgb, f)`, not `mix(fogColor, rgb, f)`.
`vColor` is premultiplied by the vertex stage, so the fog colour has to
be scaled by the fragment's own coverage; the naive form paints
full-strength fog onto near-transparent fragments and haloes every
alpha-cutout leaf.
Fog is a COMPILED VARIANT on WebGL, not a runtime branch. A software
rasterizer predicates both sides of a branch, so an `exp()` behind a
runtime test still costs every fragment of every scene: with the branch
form the mesh benchmark blew its budget outright and took nine unrelated
specs down with it on timeouts. `#define FOG` means a scene that never
enables fog runs the shader it ran before fog existed.
Off by default. Per mesh, `fog: false` exempts an object that must stay
readable at any distance. Fog is per camera, so split-screen and minimap
views fog independently and a `Camera2d` clears it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
The `setFog` JSDoc listed calls without context. It now leads with the shape a game actually uses — set the sky, size the frustum, then let fog take its distances and colour from both — followed by the exp2 form, an animated explicit colour held by reference, and the argument-free form. `Mesh#fog` gains one too: enabling fog on the world while a beacon opts out is the case the property exists for, and it reads better as six lines than as a sentence. Routing, so the section is findable from where people start: the root skill's 3D row and the glTF/assets skill both name distance fog now (an outdoor imported scene almost always wants it, and it needs no per-node work because it lives on the camera), and the docs landing page lists it among the 3D features. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
All three skills that mentioned `InstancedMesh` carried the same one-liner — one draw call, a hundred trees versus a hundred thousand — and none of them answered the question a reader actually has, which is whether to use it INSTEAD of a `Mesh`, and what that costs. Four things move from per-object to per-group, and only the first is widely known: the set gets one depth sort key, one instanced ground shadow rather than a blob each, per-instance colour becomes opt-in, and `removeInstance` swaps the last instance into the hole — so any index the caller was holding silently points at a different object afterwards. That last one is the deciding factor more often than the count is. The useful question is not "how many are there" but "does the game address them individually": scenery instances cleanly, collision-tested props do too (the positions are yours either way), and anything removed one at a time — collectibles, enemies — usually costs more bookkeeping than the draw call saves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
The class JSDoc explained what an instanced mesh IS and what the instance buffer adds, but never the question a reader actually arrives with: when a plain `Mesh` each is the better answer. Four things move from per-object to per-group — a single depth sort key, one instanced ground shadow instead of a blob each, `removeInstance` swapping the last instance into the hole, and per-instance colour needing `instanceColors` declared up front. `removeInstance` already documented its own index instability; the class doc never connected that to the choice it forces. The deciding question is not the count but whether the game addresses the objects individually: scenery instances cleanly, collision-tested props do too since the positions are yours either way, and anything removed one at a time usually costs more bookkeeping than the draw call saves. Matches the guidance added to the melonjs-3d skill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
`drawRetainedMesh` picked its program per draw, because fog is a compiled variant. It did so unconditionally — and `WebGLRenderer.drawMesh` binds a renderable's custom shader immediately BEFORE calling in here. The re-bind threw that away: `setPlacementUniforms` then read the built-in program and the draw ran built-in shading. Nothing threw, the `finally` restored the default, and the mesh simply rendered wrong. It broke every `Mesh` carrying a `ShaderEffect` under a `Camera3d` on WebGL — the single-effect `customShader` fast path — and it did so with fog DISABLED, so "additive when unused" was not true after all. The instanced path was unaffected; it already warns and falls back. The swap now only ever replaces the batcher's own program. A custom mesh shader belongs to the author and has no fog variant to switch to. Also: the `uFogParams` guard tested `!== undefined`, but `extractUniforms` regexes the raw shader text without running the preprocessor, so the names inside the `#ifdef FOG` block are registered even in the program compiled WITHOUT fog — with a null location. The guard never skipped, and every unfogged mesh draw ran the fog block and wrote to a null location after each program swap. `!= null` is the fix, and it makes the "a scene without fog pays nothing" claim true on the CPU as well as the GPU. Two tests, both mutation-checked: a foreign program survives a retained draw, and the batcher's own program still swaps to the fog variant and back. Found by adversarial review, confirmed there by pixel probe against master before I touched it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
`setFog` retained the caller's options object and read four of its five
fields back every frame. So mutating `mode`, `near`, `far` or `density`
afterwards changed the fog AND bypassed every check `setFog` performs —
`options.mode = "banana"` was silently accepted where
`setFog({ mode: "banana" })` throws — while mutating `color` did nothing,
because that branch was decided once into an owned Color.
Four fields live, one not, and neither documented. Worse, the JSDoc
actively taught the wrong half: "a `Color` is kept by reference, so
mutating it animates the fog" trains a caller to treat the whole object
as live.
The scalars are now copied at the call, so the documented model is the
real one: a `Color` is live, everything else is settled. The `fog` getter
returns a fresh object rather than the retained one, so it cannot look
mutable while changing nothing.
Three tests, including the mutation-after-the-fact case that used to slip
past validation.
Found by adversarial review, which confirmed all four behaviours by probe.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
`mesh.fog = false` leaves the object unfogged but its ground shadow still fades, and both the JSDoc and the skill claimed the exemption without that qualifier. Keeping the behaviour rather than propagating the flag. A blob is a mark on the floor and fogs with the floor it lies on; one staying crisp under an object whose surroundings had dissolved would read as a fault rather than as emphasis. The blob quad is shared by every caster in the scene too, so it carries no per-object state to read — propagating would mean threading a flag through the deferred queue for a combination (marker, plus a ground shadow, plus fog) that is rare. Behaviour unchanged; only the two claims that overstated it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
WebGL compiles fog out with `#define FOG`, so a scene that never enables it runs the shader it ran before fog existed. WebGPU kept a runtime test, and the asymmetry was not a decision — the WebGL variant was forced on me by a benchmark that blew its budget, and nothing pushed back on the WebGPU side because this environment has no adapter to push back with. The output was already identical either way (mode 0 returns the colour untouched). The cost was not: the vertex stage computed `view * model * vertex` and its length for EVERY vertex of every mesh, fog or no fog, and three products per vertex per instance on the unlit instanced path. WGSL has no preprocessor, so the `#ifdef` trick does not port — but it has `override` declarations, which are the better tool anyway. The mesh modules now declare `override enable_fog : bool = false`, the vertex work and the fragment blend sit behind it, and the pipeline cache specializes it per pipeline with a matching key axis. One module, and the implementation folds the branch and drops the dead side. An override cannot remove an inter-stage variable, so `vFogDepth` keeps its location and interpolates either way. That is the remaining cost, and it is a slot rather than arithmetic. `meshState.fog` is left `undefined` rather than `false` when fog is off, so a scene without fog produces byte-identical mesh state and mints no new pipelines — the same convention the `depthWrite` axis already uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
The entry was written when the variant only existed on WebGL, so it said a scene without fog "renders exactly the shader it did before" without saying why that is true. With the WebGPU override constant in, it holds on both backends and is worth stating as the mechanism rather than as a claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Fog gave this batcher a third way to store a compiled program: a `fogShader` field, a `shadowFogShader` field, and a fog bit folded into `instancedShaderFor`'s numeric key. Three mechanisms for one axis. The cost is not tidiness, it is LIFETIME. Every program has to be released in both `init()` — the context-loss path — and `destroy()`, and a missed one leaks a program that later tries to recompile against a dead context. Two axes had already grown that to six release sites, and each had to be checked individually during review. They are now one `shaderVariants` map keyed by a namespaced string: `mesh|fog`, `instanced|<bits>`, `shadow`, `shadow|fog`. One release loop, in each of the two places, however many axes are added later. The next feature costs a key rather than another field plus its two teardowns. Behaviour is unchanged — same programs, same defines, same laziness. The specs that reached into `instancedShaders` follow the rename, and their count assertions now filter on the key prefix rather than trusting the map's total size, since the cache is shared. Prompted by comparing this against how an established engine handles GL shader variants: a bitmask over one cache rather than a field per feature. The remaining differences there — background compilation and a persistent cache — do not port to the web, where program binaries are not exposed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Two things the fog work left undocumented, both of which a reader would act on. A shader you supply is bound as written — the engine never substitutes a fogged variant of someone else's program — so a mesh carrying a `ShaderEffect` keeps full contrast while the scene around it recedes. That reads as a bug unless you know it is the contract, and the fix is either to fold the fog term into your own shader or to leave that mesh on the built-in shading. The same fact is why fog costs nothing when unused, which is worth stating positively: with no camera fog the mesh programs are compiled without any of it on both backends. Not a branch skipped at runtime — the code is not there. Also fixes a dangling pointer: the effects skill sent readers to `melonjs-3d` for "custom mesh shaders", which had no such section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
| throw new Error("Camera3d.setFog: density must be greater than zero"); | ||
| } | ||
|
|
||
| this._fogOptions = options; |
A mesh carrying a `ShaderEffect` under a fogged camera is safe: the custom program stays bound, nothing throws, no fog variant is minted on its behalf, and the mesh simply renders unfogged. Pinned by a test rather than left to reasoning, since this is exactly where the custom-shader regression hid. The more useful half is that it is an opt-in rather than a limitation. The fog uniforms are pushed to any mesh program that DECLARES them — the guard keys on the uniform being present, not on which shader it is — so a custom shader can take part by declaring `uFogColor` and `uFogParams`. Documented with the two things an author would otherwise get wrong: compute the distance radially, or fog swims as the camera turns; and scale the fog colour by the fragment's alpha, because `vColor` arrives premultiplied and mixing toward the unscaled colour haloes every alpha-cutout edge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
`renderer._fog3d` was written once per camera by `Camera2d.draw` and never reset, so anything drawn outside a camera bracket inherited whichever camera drew last — and, since nothing cleared it between frames, kept inheriting it after that camera was gone. No sequence in the engine reaches a mesh that way today, which is why it never showed. It is also why it would be miserable to find later: the symptom would be fog on geometry no camera asked to fog, appearing only in whatever order the cameras happened to draw. Clearing at frame start makes the default explicit — a frame begins with none, and each camera installs its own. Closes the last open item from the review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/melonjs/src/camera/camera3d.ts:399
setFogassignsthis._fogOptions = options, which retains the caller’s object. That makes it possible to mutateoptions.colorafter the call (e.g. swap in a differentColor) and affect fog without going throughsetFog, contradicting the documented/ tested “options object is not retained” behavior. Store a copy instead (keepingColorby reference is still fine).
this._fogOptions = options;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/melonjs/src/camera/camera3d.ts:418
Camera3d.setFog()commits internal fog state (_fogOptions, scalar copies) before parsing a CSScolor. SinceColor.parseCSS()can throw on invalid strings, a rejected call can leave fog partially enabled (contradicting the "rejected call leaves fog off" behavior tested for other failures). Parse/prepare the color first, then commit the new fog state only after all operations that can throw succeed.
this._fogOptions = options;
this._fogMode = mode;
this._fogNear = options.near;
this._fogFar = options.far;
this._fogDensity = options.density;
`setFog` copies the scalars at the call, so mutating the object passed in afterwards does nothing — and would have bypassed the validation if it did. That was the reviewer's finding, and the fix landed in the code and in an internal comment, but the note aimed at the public docs targeted text in `camera3d.ts` that had already moved to `fog.ts`. The edit matched nothing and said so to nobody. The one live handle is a `Color` passed as `color`, which stays by reference deliberately; the field already documents that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/melonjs/src/camera/camera3d.ts:399
setFogassignsthis._fogOptions = options, and_fog3dState()later readsoptions.colorevery frame. That means mutating the original options object after callingsetFog(e.g. setting/changingoptions.color) can change fog without re-validation, contradicting the documented/tested contract that the options object is not retained (except for aColorpassed by reference). A shallow copy at call-time avoids this while still keeping aColorreference live.
this._fogOptions = options;
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.
Closes #1622.
Fog fades mesh geometry toward a colour with distance. It is the cheapest single thing that stops a 3D scene reading as flat cut-outs, and it hides the far plane so props can appear without a visible edge.
Design
Owned by the camera, next to
setClipPlanes. Fog distances have to agree with the clip planes or geometry clips before it finishes fading, and per-camera ownership also gives split-screen and minimap views independent fog for free. ACamera2dresolves to none, so a 2D minimap sharing a stage with a fogged 3D camera renders clean.Optional parameters resolve live, not at the call. Distances default to the camera's own
near/far; the colour defaults torenderer.backgroundColor. A snapshot would go out of step the momentsetClipPlanesran, or the moment a day/night fade moved the background — and both failures look like fog bugs rather than stale-copy bugs.coloris there for when the fog should deliberately differ from the backdrop; aColoris held by reference so mutating it animates the fog.Radial and per-fragment. Radial (
length(viewPos.xyz)) rather than view-space z, or fog slides as the camera turns. Per fragment rather than per vertex, or it bands across large triangles — which is exactly what the CPU-side stand-in it replaces did.Two things worth reviewing closely
The blend is
mix(fogColor * a, rgb, f), notmix(fogColor, rgb, f).vColoris premultiplied by the vertex stage, so the fog colour must be scaled by the fragment's own coverage. The naive form writes full-strength fog onto near-transparent fragments — a grey halo around every alpha-cutout leaf. There is a pixel test whose bounds exclude the naive value specifically.Fog is a compiled variant on WebGL (
#define FOG), not a runtime branch. This one was measured, not assumed. A software rasterizer predicates both sides of a branch, so anexp()behind a runtime test still costs every fragment of every scene: with the branch form the mesh benchmark blew its budget outright and took nine unrelated specs down with it on timeouts. Compiling it out means a scene that never enables fog runs the shader it ran before fog existed, instruction for instruction — which is also what makes this fully additive.Scope
setFogis unchangedfog: falseexempts a marker that must stay readable at any distancefloatingrenderables — they never reach the mesh shadersOut of scope for a first version: height fog, sun inscattering, fog on the 2D tier.
Tests
camera3d_fog.spec.js(19) — the API, live-tracking defaults, colour ownership, rejected input, a default that goes degenerate later, per-camera independence, zero per-frame allocationwebgl_mesh_fog.spec.js(10) — pixel-level: both curves, the radial guard, the premultiplied case, the per-mesh opt-out, lit/unlit agreement, and a "never enabled → exact mesh colour" regression guardwebgpu_mesh_fog.spec.js(8) — the uniform block grew 208 → 240, fog lands at floats 52-59, every float is zero with no fog, and the derived instanced modules write the new varyingMutation-checked: the naive premultiplied mix and a view-space-z distance each fail exactly the test written for them.
Re-pinned deliberately:
webgpu_mesh_batcher.spec.jsandwebgpu_mtl_material.spec.jsassert the uniform block size.270 files / 6519 testspass; lint, types and biome clean. Verified on screen against a chase-camera scene with terrain, instanced props, particles, ground shadows and a floating HUD.🤖 Generated with Claude Code
https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N