From de7926c0392b6c7a2b034a9788529645cb00dcd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 3 Oct 2026 09:56:35 +0000 Subject: [PATCH] fix(rendering): keep bloom's glow purely additive over lower layers The bloom composite raised the target's alpha to the halo's own, which made the glow partly cover whatever was presented beneath the camera's render target and replace it with glow color. Render targets are presented as premultiplied alpha, so the glow only needs to be added to the color: keep the scene's alpha, and drop the now-unused alpha from the threshold pass. Adds an e2e scene/spec that presents a bloomed foreground over a blue background and checks the yellow glow never lowers the blue channel. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016Ldg64ToB3cSB6sDGbpLt8 --- AGENTS.md | 9 +- CHANGELOG.md | 4 + .../docs/docs/rendering/bloom.md | 15 +- e2e/fixtures/scenes/bloom-over-background.ts | 202 ++++++++++++++++++ e2e/specs/bloom-over-background.spec.ts | 80 +++++++ .../post-process/bloom-composite.frag.glsl | 15 +- .../post-process/bloom-threshold.frag.glsl | 9 +- 7 files changed, 311 insertions(+), 23 deletions(-) create mode 100644 e2e/fixtures/scenes/bloom-over-background.ts create mode 100644 e2e/specs/bloom-over-background.spec.ts diff --git a/AGENTS.md b/AGENTS.md index d345f4bf..fc872ef9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -680,11 +680,16 @@ gl.ONE_MINUS_SRC_ALPHA)` (see `render-system.ts`). Never plain (see `present-system.ts`). - Clear colors are straight-alpha `Color`s and get premultiplied when written (`RenderContext.clear`, `createTerrainRenderEcsSystem`). +- Light that should only brighten what's beneath it (bloom's glow) is + added to the color and leaves alpha untouched (see + `bloom-composite.frag.glsl`). Giving it alpha of its own makes it cover, + and dim, the layers presented under it. - Leave `gl.BLEND` disabled when a system finishes drawing - it's global GL state, and the next system to draw would otherwise inherit it. -`e2e/specs/translucent-ui-compositing.spec.ts` checks this end to end on -a real canvas. +`e2e/specs/translucent-ui-compositing.spec.ts` and +`e2e/specs/bloom-over-background.spec.ts` check this end to end on a real +canvas. ### Device Pixels vs. CSS Pixels diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e46341a..12ce0b5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +#### Fixed + +- **rendering:** Bloom's glow now only adds light to whatever is presented beneath its camera's render target, such as a background camera's layer. It used to give the halo partial opacity of its own, so the glow covered part of the layer behind it and dimmed colors the glow lacks (a yellow glow over a blue background reduced the blue) + ## [0.25.6] - 2026-10-03 #### Added diff --git a/documentation-site/docs/docs/rendering/bloom.md b/documentation-site/docs/docs/rendering/bloom.md index a1545239..22f6bff3 100644 --- a/documentation-site/docs/docs/rendering/bloom.md +++ b/documentation-site/docs/docs/rendering/bloom.md @@ -40,15 +40,14 @@ well past a sprite's edges with a modest, cheap `passes` count. The composite pass upsamples it back implicitly, via the bloom texture's own linear-filtered sampling. -The thresholded buffer's alpha carries how strongly each pixel contributes -to the glow, not the source pixel's original transparency, so the blur can -spread the glow's own opacity out past a sprite's silhouette into -previously-transparent pixels. This matters if the camera's `renderTarget` -gets alpha-blended onto something else afterwards (for example a sharp +The glow is purely additive light: the composite adds it to the scene's +color and leaves the scene's alpha untouched. Because render targets hold +premultiplied alpha, that's enough for the halo to show past a sprite's +silhouette, over pixels that were fully transparent, when the camera's +`renderTarget` is presented over something else (for example a sharp foreground layered over a background, as in -[Layering multiple render targets](./multipass-rendering.md#layering-multiple-render-targets)): -without this, the glow would only ever brighten already-opaque pixels and -never show as a soft halo bleeding past their edges. +[Layering multiple render targets](./multipass-rendering.md#layering-multiple-render-targets)). +The glow only ever brightens whatever is beneath it; it never covers it. ## Wiring it up diff --git a/e2e/fixtures/scenes/bloom-over-background.ts b/e2e/fixtures/scenes/bloom-over-background.ts new file mode 100644 index 00000000..b234e878 --- /dev/null +++ b/e2e/fixtures/scenes/bloom-over-background.ts @@ -0,0 +1,202 @@ +import { + addPositionComponent, + createTransformEcsSystem, + Time, +} from '../../../src/common/index.js'; +import { EcsWorld } from '../../../src/ecs/index.js'; +import { + addBloomComponent, + addCameraComponent, + Color, + createBloomEcsSystem, + createCanvas, + createImageSprite, + createPresentEcsSystem, + createRenderContext, + createRenderEcsSystem, + createRenderTarget, + spriteId, +} from '../../../src/rendering/index.js'; +import { createWhiteSquareImage } from './create-white-square-image.js'; +import { CreateScene, SceneHandle } from './scene.js'; + +const defaultStepDeltaMilliseconds = 16.6666; + +const glowRenderCategory = 1 << 0; + +// A mostly-blue background and a yellow sprite: yellow has no blue at all, +// so a glow that only adds light can never lower the background's blue +// channel, while a glow that partly covers the background visibly does. +const backgroundColor = new Color(0.1, 0.3, 0.8); +const spriteColor = new Color(1, 1, 0); + +// One world unit is one CSS pixel (see `verticalWorldUnits` below), so the +// sprite is this many CSS pixels wide, centered on the canvas. +const spriteSizeInPixels = 32; + +// Strong enough that the glow clearly reaches well past the sprite's edge. +const bloomSettings = { threshold: 0.5, passes: 6, intensity: 3 }; + +// The part of the row through the sprite's center that `measure` scans for +// the halo, in CSS pixels right of the sprite's edge. +const haloScanRange = { start: 2, end: 64 }; + +/** A sampled pixel's color, `0`-`255` per channel. */ +export interface SampledColor { + r: number; + g: number; + b: number; +} + +/** Everything `bloom-over-background.spec.ts` asserts against, from one frame. */ +export interface BloomOverBackgroundMeasurement { + /** The background, sampled far from the sprite and its glow. */ + farBackground: SampledColor; + + /** The lowest blue found in the halo just outside the sprite's silhouette. */ + lowestHaloBlue: number; + + /** The highest red found in the halo just outside the sprite's silhouette. */ + highestHaloRed: number; +} + +/** The handle `bloom-over-background.spec.ts` drives and asserts against. */ +export interface BloomOverBackgroundSceneHandle extends SceneHandle { + /** + * Reads back the presented canvas: the far background, and the halo + * along the row through the sprite's center, right of its edge. Must be + * called in the same `page.evaluate` task as the preceding `step()` - see + * `SceneHandle.step` and `camera-pan-zoom.ts`'s `measureGreenSquareBounds` + * for why. + */ + measure(): BloomOverBackgroundMeasurement; +} + +/** + * Builds a minimal scene for bloom layered over another camera: a + * background camera cleared to opaque blue, and a foreground camera (with a + * transparent clear and a `BloomEcsComponent`) drawing one yellow sprite, + * each into its own render target. `bloom-over-background.spec.ts` checks + * the glow brightens the background past the sprite's silhouette without + * dimming any of its channels. + * @param container - The element to render the scene's canvas into. + * @returns The scene's handle. + */ +export const createScene: CreateScene = async ( + container: HTMLElement, +): Promise => { + const time = new Time(); + const world = new EcsWorld(); + const canvas = createCanvas(container); + + canvas.width = 400; + canvas.height = 300; + + // See `measureGreenSquareBounds` in `camera-pan-zoom.ts` for why this is + // required for a reliable same-run pixel readback. + const renderContext = createRenderContext(canvas, { + preserveDrawingBuffer: true, + }); + + const backgroundCameraEntity = world.createEntity(); + + addPositionComponent(world, backgroundCameraEntity); + addCameraComponent(world, backgroundCameraEntity, { + isStatic: true, + layer: 0, + clearColor: backgroundColor, + cullingMask: 0, + renderTarget: createRenderTarget( + renderContext.gl, + renderContext.width, + renderContext.height, + ), + }); + + const glowCameraEntity = world.createEntity(); + + addPositionComponent(world, glowCameraEntity); + addCameraComponent(world, glowCameraEntity, { + isStatic: true, + layer: 1, + clearColor: Color.transparent, + cullingMask: glowRenderCategory, + verticalWorldUnits: renderContext.cssHeight, + renderTarget: createRenderTarget( + renderContext.gl, + renderContext.width, + renderContext.height, + ), + }); + addBloomComponent(world, glowCameraEntity, bloomSettings); + + const squareImage = await createWhiteSquareImage(); + const sprite = createImageSprite(squareImage, renderContext, { + pixelsPerUnit: 1, + layer: glowRenderCategory, + }); + const spriteEntity = world.createEntity(); + + addPositionComponent(world, spriteEntity); + world.addComponent(spriteEntity, spriteId, { + ...sprite, + width: spriteSizeInPixels, + height: spriteSizeInPixels, + tintColor: spriteColor, + }); + + world.addSystem(createTransformEcsSystem()); + world.addSystem(createRenderEcsSystem(renderContext)); + world.addSystem(createBloomEcsSystem(renderContext)); + world.addSystem(createPresentEcsSystem(renderContext)); + + let clockInMilliseconds = 0; + + return { + step(deltaMilliseconds: number = defaultStepDeltaMilliseconds): void { + clockInMilliseconds += deltaMilliseconds; + time.update(clockInMilliseconds); + world.update(); + }, + + measure(): BloomOverBackgroundMeasurement { + const { gl, width, height } = renderContext; + const pixels = new Uint8Array(width * height * 4); + + gl.bindFramebuffer(gl.FRAMEBUFFER, null); + gl.readPixels(0, 0, width, height, gl.RGBA, gl.UNSIGNED_BYTE, pixels); + + const pixelAt = (x: number, y: number): SampledColor => { + const index = (Math.round(y) * width + Math.round(x)) * 4; + + return { r: pixels[index], g: pixels[index + 1], b: pixels[index + 2] }; + }; + + // The drawing buffer is `pixelRatio` times the canvas's CSS size, so + // scale the CSS-pixel distances above into device pixels. + const { pixelRatio } = renderContext; + const centerX = width / 2; + const centerY = height / 2; + const spriteEdgeX = centerX + (spriteSizeInPixels / 2) * pixelRatio; + let lowestHaloBlue = Number.POSITIVE_INFINITY; + let highestHaloRed = Number.NEGATIVE_INFINITY; + + for ( + let offset = haloScanRange.start * pixelRatio; + offset <= haloScanRange.end * pixelRatio; + offset++ + ) { + const { r, b } = pixelAt(spriteEdgeX + offset, centerY); + + lowestHaloBlue = Math.min(lowestHaloBlue, b); + highestHaloRed = Math.max(highestHaloRed, r); + } + + return { + farBackground: pixelAt(width * 0.05, height * 0.05), + lowestHaloBlue, + highestHaloRed, + }; + }, + }; +}; diff --git a/e2e/specs/bloom-over-background.spec.ts b/e2e/specs/bloom-over-background.spec.ts new file mode 100644 index 00000000..3c6b47d9 --- /dev/null +++ b/e2e/specs/bloom-over-background.spec.ts @@ -0,0 +1,80 @@ +import { expect, test } from '@playwright/test'; +import type { + BloomOverBackgroundMeasurement, + BloomOverBackgroundSceneHandle, +} from '../fixtures/scenes/bloom-over-background.js'; + +// See `translucent-ui-compositing.spec.ts` for why the hooks are cast inline +// rather than declared per scene. +type Hooks = BloomOverBackgroundSceneHandle; + +// How far a halo channel may fall below the background's own value and +// still count as unchanged: absorbs 8-bit rounding, while still catching a +// glow that partly covers the background, which drops blue next to the +// sprite to a fraction of its value here. +const quantizationTolerance = 1; + +// How much the glow must brighten the background's red channel just past +// the sprite's edge, to prove the halo actually shows there. +const minimumHaloRedGain = 20; + +/** + * Advances the scene by one frame and samples the presented canvas in the + * same task - see AGENTS.md's "Be wary of pixel-level rendering assertions" + * for why `step()` and any pixel read must happen in the same + * `page.evaluate` call. + */ +const captureMeasurement = ( + page: import('@playwright/test').Page, +): Promise => + page.evaluate(() => { + const scene = window.__forgeTestHooks as unknown as Hooks; + + scene.step(); + + return scene.measure(); + }); + +test.describe('bloom over a background layer', () => { + test.beforeEach(async ({ page }) => { + await test.step('load the bloom-over-background scene', async () => { + // See `translucent-ui-compositing.spec.ts` for why page errors are + // captured here. + let pageError: Error | undefined; + + page.once('pageerror', (error) => { + pageError = error; + }); + + await page.goto('/?scene=bloom-over-background'); + + try { + await page.waitForFunction(() => Boolean(window.__forgeTestHooks)); + } catch (timeoutError) { + throw pageError ?? timeoutError; + } + }); + }); + + test('adds the glow to the background without dimming any of its channels', async ({ + page, + }) => { + const { farBackground, lowestHaloBlue, highestHaloRed } = + await test.step('render one frame and read back the canvas', () => + captureMeasurement(page)); + + await test.step('assert the halo shows past the sprite', () => { + expect( + highestHaloRed, + `the yellow glow should brighten the background's red past the sprite's edge (far background ${JSON.stringify(farBackground)})`, + ).toBeGreaterThan(farBackground.r + minimumHaloRedGain); + }); + + await test.step("assert the halo doesn't dim the background's blue", () => { + expect( + lowestHaloBlue, + `a yellow glow has no blue, so it should leave the background's blue (${farBackground.b} far from the sprite) unchanged`, + ).toBeGreaterThanOrEqual(farBackground.b - quantizationTolerance); + }); + }); +}); diff --git a/src/rendering/shaders/post-process/bloom-composite.frag.glsl b/src/rendering/shaders/post-process/bloom-composite.frag.glsl index eb75558e..cd713d5d 100644 --- a/src/rendering/shaders/post-process/bloom-composite.frag.glsl +++ b/src/rendering/shaders/post-process/bloom-composite.frag.glsl @@ -15,13 +15,14 @@ void main() { vec4 scene = texture(u_sceneTexture, v_texCoord); vec4 bloom = texture(u_bloomTexture, v_texCoord); - // The blurred glow's own alpha (see bloom-threshold.frag) is folded - // into the result's alpha via max(), not scene.a alone: otherwise the - // halo would only ever appear on top of already-opaque scene pixels, - // and vanish the moment it blurs past the source sprite's silhouette - // into what was fully transparent. + // Render targets hold premultiplied alpha and are presented with + // ONE, ONE_MINUS_SRC_ALPHA, so the glow is just light added to the + // color: it reaches the screen past the source sprite's silhouette, + // over fully transparent pixels, without needing any coverage of its + // own. Keeping the scene's alpha means the halo only ever adds to + // whatever is presented beneath this target, instead of partly + // covering it and replacing it with glow color. vec3 color = scene.rgb + bloom.rgb * u_intensity; - float alpha = clamp(max(scene.a, bloom.a * u_intensity), 0.0, 1.0); - fragColor = vec4(color, alpha); + fragColor = vec4(color, scene.a); } diff --git a/src/rendering/shaders/post-process/bloom-threshold.frag.glsl b/src/rendering/shaders/post-process/bloom-threshold.frag.glsl index 6081833a..297b64dd 100644 --- a/src/rendering/shaders/post-process/bloom-threshold.frag.glsl +++ b/src/rendering/shaders/post-process/bloom-threshold.frag.glsl @@ -46,10 +46,7 @@ void main() { } } - // Alpha carries the averaged contribution itself, not the source pixels' - // original alpha: this buffer gets blurred next, and the glow needs to - // spread its own opacity outward past the source sprite's silhouette - // (into pixels that were fully transparent) for the halo to actually - // show up once it's composited back with alpha blending. - fragColor = vec4(accumulatedColor / sampleCount, accumulatedContribution / sampleCount); + // The composite pass only reads the glow's color, adding it to the scene + // as light; the glow has no coverage of its own, so alpha stays 0. + fragColor = vec4(accumulatedColor / sampleCount, 0.0); }