docs(parity): scene144 bloom parity ceiling investigation (ON HOLD)#380
Merged
Conversation
PENDING USER APPROVAL — proposes raising scene144 (bloom post-process) maxMad from 0.07 to 0.072. scene144's parity gate captures a LIVE Babylon.js reference each run (its golden is gitignored) and compares it against the LIVE Babylon Lite render on the same renderer. The two independent WebGPU float pipelines computing a full-screen, multi-pass Gaussian bloom differ only at the 8-bit quantization floor: 92.1% of pixels match exactly, 97.2% within 1 LSB, 99.6% within 2 LSB, and 71% of the residual MAD comes from unavoidable +/-1-2 LSB rounding across the smooth bloom gradient. Bloom params (threshold 0.1, weight 2, kernel 64, scale 0.5) and math already match BJS exactly. Enabling MSAA to mirror the BJS engine's antialias:true made parity 4x WORSE (MAD 0.309), proving the bloom input is single-sample and there is no closable algorithmic drift. The old 0.07 ceiling was calibrated right at this floor, so deterministic cross-renderer rounding tips it over with zero run-to-run variance: - hardware GPU (local): MAD = 0.0700043 - CI SwiftShader (cloud): MAD = 0.0700051 0.072 clears the observed floor by ~2.8% while staying orders of magnitude below any genuine bloom regression (which moves MAD by >= 0.1). Standalone, pre-existing issue independent of the dependency PRs (#374 / #376). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ryantrem
approved these changes
Jul 7, 2026
Lab - Static SiteBuild 20260707.22 - merge @ 010e9bd |
📋 perf — Test ReportBuild 20260707.26 · merge @ 010e9bd |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR proposes widening the parity MAD ceiling for scene144 (bloom post-process) by updating the per-scene threshold in scene-config.json, and adds an inline justification note intended to document why the ceiling change is being requested.
Changes:
- Increase
scene144maxMadfrom0.07to0.072. - Add a
notefield to thescene144config entry describing the justification and marking the change as pending approval.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "slug": "scene144-bloom-post-process", | ||
| "name": "Scene 144 — Bloom Post-process", | ||
| "maxMad": 0.07, | ||
| "maxMad": 0.072, |
| "maxRawKB": 112, | ||
| "description": "Tarisland dragon asset rendered through a Lite frame-graph bloom task using the Babylon.js Playground bloom settings.", | ||
| "tags": ["post-process", "frame-graph", "gltf", "animation"], | ||
| "note": "PENDING USER APPROVAL. maxMad widened 0.07 -> 0.072. This full-screen multi-pass bloom sits on the 8-bit quantization floor between two independent WebGPU float pipelines (BJS reference vs Lite): 92% of pixels match exactly, 97% within 1 LSB, 99.6% within 2 LSB, and the residual MAD is dominated (71%) by unavoidable +/-1-2 LSB rounding across the smooth bloom gradient. Bloom params (threshold 0.1, weight 2, kernel 64, scale 0.5) and math match BJS exactly; enabling MSAA to match BJS antialias made parity 4x worse (0.309), confirming the bloom input is single-sample and there is no closable algorithmic drift. The old 0.07 ceiling was calibrated right at this floor, so deterministic cross-renderer rounding tips it over: hardware GPU measures 0.0700043, CI SwiftShader measures 0.0700051 (both deterministic, 0 run-to-run variance). 0.072 clears the observed floor by ~2.8% while staying orders of magnitude below any real bloom regression (which would move MAD by >= 0.1).", |
Lab - Static SiteBuild 20260707.26 - merge @ 010e9bd |
RaananW
added a commit
that referenced
this pull request
Jul 8, 2026
Brings in #369 (viewer support), #379 (docs), and #380 (scene144 parity ceiling widened 0.07 -> 0.072 with justification) from master. Also addresses the Copilot PR review on the eslint 10 bump: eslint 10 drops Node < 20.19 support (engines: ^20.19.0 || ^22.13.0 || >=24), which the README 'Node.js >= 18' prerequisite no longer reflects. Added an explicit engines.node to package.json and updated the README prerequisite to match. Regenerated the per-scene bundle manifests that conflicted (scene36/113) plus incidental sub-0.1KB terser-5.48 rounding deltas (scene114/115/222); the gitignored aggregate manifest.json is not committed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Jul 8, 2026
RaananW
added a commit
that referenced
this pull request
Jul 9, 2026
## Scope Bumps **TypeScript `^5.7.0` → `^6.0.3`** across the workspace only. **Vite is unchanged** (stays at its existing `^6.x` pins). This PR was originally TS6 + Vite 8 together, but a split diagnostic showed the two majors carry independent risk, so the Vite 8 (rolldown-vite) work has been moved to its **own separate draft PR (#377, parked as exploration)**. This PR is now a clean, zero-ceiling/zero-golden TypeScript-6-only change and is **the deliverable**. ### Version bumps `typescript ^5.7.0 → ^6.0.3` in: root, `packages/babylon-lite`, `packages/babylon-lite-gl`, `packages/babylon-lite-compat`, `lab`, `playground`. `pnpm-lock.yaml` regenerated (TS-only delta; resolves TS `5.9.3 → 6.0.3`). ## Code changes required by TS 6 - **`packages/babylon-lite/src/material/node/node-geometry-renderable.ts`** — copy the readonly `_vertexBuffers` array (`buffers: [...a._vertexBuffers]`) when passing it as `GPUVertexState.buffers`. TS6 tightened the WebGPU typings so a readonly array is no longer assignable to the mutable `buffers` field. Semantics-preserving (shallow copy only; no runtime/render change). - **`tests/lite/build/public-api-types.test.ts`, `tests/gl/build/public-api.test.ts`** — add `--ignoreConfig` to the standalone `tsc` invocations. TS6 now errors (**TS5112**) when a file is passed on the command line while a `tsconfig.json` exists in cwd; `--ignoreConfig` is the flag the error itself recommends. - **`tests/lite/build/public-api-types.test.ts`** — drop `--types @webgpu/types`. TS6's built-in `dom` lib now bundles the WebGPU declarations, so also loading the `@webgpu/types` package duplicates them and (without `skipLibCheck`, which this test deliberately omits to catch leaking internal types) trips **TS6200/TS2717** conflicts. The native lib fully covers every `GPU*` type the public `build/index.d.ts` references — verified type-checks clean. No tsconfig strictness was loosened; no blanket `any` / `@ts-ignore` was added. ### Bundle manifests All per-scene bundle manifests were regenerated under TS6. Codegen is **byte-identical to the TS5/Rollup baseline for every scene except two** — `scene104.json` and `scene149.json` — which shift by a fraction of a KB (gzip rounding, e.g. `44.5 → 44.6` KB). Those two are therefore the only manifest files this PR changes vs `master`, and both remain within existing size ceilings (**0 overages**). (An earlier revision of this description said "46 manifests"; that reflected a mid-work snapshot before the TS6-only split — the accurate figure vs `master` is 2.) ## Test / build results (TS6 + Vite 6) - `pnpm run lint` (eslint + `tsc --noEmit` across all tsconfigs): ✅ green - `pnpm test:unit` ✅ · `pnpm test:build` ✅ · `pnpm test:unit:gl` ✅ · `pnpm test:build:gl` ✅ - `pnpm build:lib`, `pnpm build`, `pnpm build:gl`, `pnpm build:playground`: ✅ exit 0 - Locked parity + bundle-size on `LAB_TEST_PORT=5279`: **0 bundle-size overages** ✅. Only `scene116` parity fails (see below); `scene144` now **passes** after merging master's widened `0.072` ceiling (#380). ### scene116 is pre-existing local-GPU noise — NOT introduced here `scene116` fails **byte-identically on unmodified `master`** (TS 5.9.3 + Vite 6, this branch's changes reverted): MAD `0.2883984375…` vs threshold `0.01`. It is machine/GPU-specific strict-threshold noise that is **green on CI**, and fails with and without this PR's changes, so it is not a regression from the TS6 upgrade — a reviewer should not be alarmed. `scene144` (`0.07000434…`) previously sat a hair over the old `0.07` threshold but now **passes** under master's widened `0.072` ceiling (#380). No golden references or thresholds were touched. ## Notes - Touches `pnpm-lock.yaml` → shares a conflict surface with sibling dependency PRs (#374 and the parked Vite 8 PR #377); **whichever merges second regenerates the lock** (trivial re-resolve). - Merged current `master` (incl. #374 tooling batch + pnpm 9.15.9, #378 REUSE_BROWSER, #353, #381); all suites re-validated green. - Kept as **draft** for the user's review (drafts-first). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
⏸️ ON HOLD — awaiting maintainer decision. Do not merge. No guardrail change applied.
The branch contains no ceiling change — the proposed
0.07 → 0.072edit was reverted, so this PR is now a documented investigation only (net-zero diff vsmaster). The maintainer has chosen to hold; the 0.072 widen is not approved and not rejected on the merits.Options on the table for the record:
maxMadto ~0.072 with the justification below, orThis is a standalone, pre-existing issue on
master, independent of the dependency PRs (#374 / #376). Note: #374's parity gate is currently red solely because of this ceiling sitting flush at the quantization floor.Root cause
scene144's parity gate has no committed golden (
reference/**/babylon-ref-*.pngis gitignored), so every run captures a live Babylon.js reference and compares it against the live Babylon Lite render on the same renderer — a genuine two-engine comparison.The two independent WebGPU float pipelines computing a full-screen, multi-pass Gaussian bloom differ only at the 8-bit quantization floor:
MAD is measured in 0–255 space, so 0.07 means an average per-channel difference of 0.07/255 ≈ 0.03% — essentially pixel-perfect. The residual is dominated by unavoidable ±1–2 LSB rounding across the smooth bloom gradient, which quantizes differently between renderers.
Why it's an inherent floor, not a closable bug
bjs/scene144.ts) exactly.antialias: true. I tried mirroring that by rendering the Lite source into a 4× MSAA target and resolving before bloom — parity got 4× worse (MAD 0.309), proving BJS's bloom input is effectively single-sample and the current single-sample Lite config is already the closest match. There is no closable algorithmic drift.Measurements (deterministic, zero run-to-run variance)
The scene is fully deterministic (frozen animation frame 180, fixed camera). The only variable is the renderer's float rounding:
The 0.07 ceiling was calibrated right at this floor with no headroom, so deterministic cross-renderer rounding on a handful of pixels tips it over. CI's SwiftShader is deterministic, hence the identical failure across independent cloud runs.
Recommendation (for whenever the hold is lifted)
Option (a): widen
maxMadto 0.072 — ~2.8% over the observed floor, still orders of magnitude below any genuine bloom regression (a wrong weight/threshold/kernel moves MAD by ≥ 0.1, not millionths). Held pending maintainer sign-off.