From e517571a93dcd1475a884d3123247353ee09f0a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 06:28:14 +0000 Subject: [PATCH] fix(rendering): let Color carry HDR values above 1 Color clamped r, g and b to [0, 1], so HDR tints and clear colors were silently capped at white before reaching an hdr render target. RGB is now only clamped to 0 from below; alpha stays in [0, 1]; non-finite channels throw; toRGBAString clamps its output to 255. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01FLZGR5YxNP6W4AW3coFP9h --- .cspell/project-words.txt | 1 + CHANGELOG.md | 4 ++ .../docs/docs/rendering/bloom.md | 18 ++++++- .../docs/docs/rendering/hdr-rendering.md | 10 ++++ src/rendering/color.test.ts | 39 +++++++++++++-- src/rendering/color.ts | 48 ++++++++++++++++--- src/ui/systems/ui-transition-system.ts | 3 ++ 7 files changed, 109 insertions(+), 14 deletions(-) diff --git a/.cspell/project-words.txt b/.cspell/project-words.txt index 0dfe15320..9fb3aa357 100644 --- a/.cspell/project-words.txt +++ b/.cspell/project-words.txt @@ -107,6 +107,7 @@ normalise notdef Oboro Okonjo +overbright perlin pillarbox pillarboxing diff --git a/CHANGELOG.md b/CHANGELOG.md index fe2f7e2ce..17fe309fc 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:** `Color` no longer caps `r`, `g` and `b` at `1`, so HDR tints and clear colors such as `new Color(4, 2, 0.5)` reach an `hdr` render target at full brightness for bloom and tone mapping instead of being silently clamped to white. An `ldr` render target or the canvas still saturates them to `1` on write, and `toRGBAString()` clamps them to `255`. Negative channels are still clamped to `0` and alpha to `[0, 1]`. If your code relied on the clamp (for example, darkening UI tints because brighter ones had no effect, or computing a color that can exceed `1`), clamp it yourself. A `NaN` or infinite channel now throws instead of being clamped + ## [0.25.8] - 2026-10-03 #### Fixed diff --git a/documentation-site/docs/docs/rendering/bloom.md b/documentation-site/docs/docs/rendering/bloom.md index c79328c58..7133db1b3 100644 --- a/documentation-site/docs/docs/rendering/bloom.md +++ b/documentation-site/docs/docs/rendering/bloom.md @@ -249,8 +249,7 @@ world.addSystem(createToneMapEcsSystem(renderContext)); world.addSystem(createPresentEcsSystem(renderContext)); ``` -Without the emissive map, `threshold` is the only way to make part of a -sprite glow more than the rest, and it can't distinguish "this part is +Without the emissive map, `threshold` alone can't distinguish "this part is meant to be a light source" from "this part happens to be pale" — both read as the same brightness once clamped to `[0, 1]`. The emissive map sidesteps that: its contribution is added _after_ the albedo sample, so it @@ -260,6 +259,21 @@ Rendering & Tone Mapping](./hdr-rendering.md) for how the `hdr` render target and `addToneMappingComponent` work together to make this look right once presented. +When the whole sprite should glow, an HDR tint is simpler than an emissive +map. `Color` channels aren't capped at `1`, so a tint like +`new Color(4, 2, 0.5)` multiplies the sprite's texture past white, and on +an `hdr` camera it blooms more than an untinted white sprite next to it: + +```ts +import { Color, createImageSprite } from '@forge-game-engine/forge/rendering'; + +const lamp = createImageSprite(lampImage, renderContext); +lamp.tintColor = new Color(4, 2, 0.5); +``` + +Unlike an emissive map, a tint scales the texture's own color, so its dark +pixels stay dark. + ### Authoring an emissive map An emissive map's color is added on top of the tinted albedo; its own diff --git a/documentation-site/docs/docs/rendering/hdr-rendering.md b/documentation-site/docs/docs/rendering/hdr-rendering.md index 2c36ea5cc..3f64f9197 100644 --- a/documentation-site/docs/docs/rendering/hdr-rendering.md +++ b/documentation-site/docs/docs/rendering/hdr-rendering.md @@ -51,6 +51,16 @@ target and any post-processing scratch buffers built from it, so only opt in where it's actually needed. ::: +## Brighter-than-white colors + +A [`Color`](/Forge/docs/api/classes/Color)'s red, green and blue channels +can go above `1`, so tints and clear colors can be HDR too. A sprite tinted +`new Color(4, 2, 0.5)`, or a camera whose `clearColor` is +`new Color(1.5, 1.5, 2)`, writes those values into an `hdr` render target +unchanged, where bloom and tone mapping see them. An `ldr` render target or +the canvas saturates them to `1` when they're written. Negative channels +are clamped to `0`, and alpha always stays in `[0, 1]`. + ## Tone mapping An `hdr` render target has to be compressed back into `[0, 1]` before it's diff --git a/src/rendering/color.test.ts b/src/rendering/color.test.ts index 05c5b631e..a98f54583 100644 --- a/src/rendering/color.test.ts +++ b/src/rendering/color.test.ts @@ -11,12 +11,41 @@ describe('Color', () => { expect(color.toRGBAString()).toBe('rgba(255, 0, 128, 1)'); }); - it('should clamp RGB values to the valid range (0-1)', () => { - const color = new Color(1.2, -0.2, 0.5); + it('should keep RGB values above 1 for HDR colors', () => { + const color = new Color(4, 2, 0.5); - expect(color.r).toBe(1); // Clamped to 1 - expect(color.g).toBe(0); // Clamped to 0 - expect(color.b).toBe(0.5); // Unchanged + expect(color.r).toBe(4); + expect(color.g).toBe(2); + expect(color.b).toBe(0.5); + expect(color.toFloat32Array()).toEqual(new Float32Array([4, 2, 0.5, 1])); + }); + + it('should clamp negative RGB values to 0', () => { + const color = new Color(-0.2, -3, 0.5); + + expect(color.r).toBe(0); + expect(color.g).toBe(0); + expect(color.b).toBe(0.5); + }); + + it('should clamp alpha to the range 0-1', () => { + expect(new Color(1, 1, 1, 1.5).a).toBe(1); + expect(new Color(1, 1, 1, -0.5).a).toBe(0); + }); + + it.each([ + ['r', [Number.NaN, 0, 0, 1]], + ['g', [0, Number.POSITIVE_INFINITY, 0, 1]], + ['b', [0, 0, Number.NEGATIVE_INFINITY, 1]], + ['a', [0, 0, 0, Number.NaN]], + ])('should throw when channel %s is not finite', (channel, [r, g, b, a]) => { + expect(() => new Color(r, g, b, a)).toThrow(`channel "${channel}"`); + }); + + it('should clamp overbright channels to 255 in the CSS string', () => { + const color = new Color(4, 0.5, 1.2, 0.5); + + expect(color.toRGBAString()).toBe('rgba(255, 128, 255, 0.5)'); }); it('should create a color using HSL values', () => { diff --git a/src/rendering/color.ts b/src/rendering/color.ts index c8b83046d..ee427f6a5 100644 --- a/src/rendering/color.ts +++ b/src/rendering/color.ts @@ -2,6 +2,13 @@ import { clamp } from '../math/index.js'; /** * The `Color` class represents a color that can be created using RGB(A) or HSL(A). + * + * Color channels are floating point. `r`, `g` and `b` have no upper bound: + * values above `1` are "overbright". An `ldr` render target (and the canvas) + * saturates them to `1` when they're written, while an `hdr` render target + * keeps them, so bloom and tone mapping see a tint or clear color that's + * brighter than white. Alpha is a coverage fraction and always lies in + * `[0, 1]`. */ export class Color { private readonly _r: number; @@ -24,15 +31,28 @@ export class Color { /** * Constructs a new `Color` instance using RGBA values. - * @param r - The red component (0-1). - * @param g - The green component (0-1). - * @param b - The blue component (0-1). + * + * `r`, `g` and `b` are clamped to `0` from below but not from above, so an + * HDR color such as `new Color(4, 2, 0.5)` keeps its brightness. Negative + * color isn't light: fed to the tone mapper's Reinhard curve `c / (c + 1)` + * it divides by zero at `-1`. Alpha is clamped to `[0, 1]`, since the + * straight-alpha blend (`SRC_ALPHA, ONE_MINUS_SRC_ALPHA`) would give the + * destination a negative weight for alpha above `1`. + * @param r - The red component (`0` and up; above `1` is overbright). + * @param g - The green component (`0` and up; above `1` is overbright). + * @param b - The blue component (`0` and up; above `1` is overbright). * @param a - The alpha component (0-1). Defaults to 1 (fully opaque). + * @throws An error if any component is `NaN` or infinite. */ constructor(r: number, g: number, b: number, a: number = 1) { - this._r = clamp(r, 0, 1); - this._g = clamp(g, 0, 1); - this._b = clamp(b, 0, 1); + Color._assertFinite('r', r); + Color._assertFinite('g', g); + Color._assertFinite('b', b); + Color._assertFinite('a', a); + + this._r = Math.max(r, 0); + this._g = Math.max(g, 0); + this._b = Math.max(b, 0); this._a = clamp(a, 0, 1); } @@ -73,6 +93,14 @@ export class Color { return new Color(r, g, b, a); } + private static _assertFinite(channel: string, value: number): void { + if (!Number.isFinite(value)) { + throw new Error( + `Unable to create a Color: channel "${channel}" is ${value}, but every channel must be a finite number.`, + ); + } + } + private static _hueToRGB(p: number, q: number, t: number): number { const wrappedTValue = ((t % 1) + 1) % 1; @@ -121,10 +149,16 @@ export class Color { /** * Converts the color to a CSS-compatible RGBA string. + * + * CSS `rgba()` can't express overbright color, so channels above `1` are + * clamped to `255`. * @returns The RGBA string (e.g., `rgba(255, 0, 0, 1)`). */ public toRGBAString(): string { - return `rgba(${Math.round(this._r * 255)}, ${Math.round(this._g * 255)}, ${Math.round(this._b * 255)}, ${this._a})`; + const toByte = (channel: number): number => + Math.min(Math.round(channel * 255), 255); + + return `rgba(${toByte(this._r)}, ${toByte(this._g)}, ${toByte(this._b)}, ${this._a})`; } /** diff --git a/src/ui/systems/ui-transition-system.ts b/src/ui/systems/ui-transition-system.ts index 9d0efad73..471c7fd18 100644 --- a/src/ui/systems/ui-transition-system.ts +++ b/src/ui/systems/ui-transition-system.ts @@ -29,6 +29,9 @@ function colorForState( return colorsByState[state]; } +// A back or elastic easing takes `t` outside `[0, 1]`, so the tint briefly +// overshoots past either end color. `Color` keeps an overbright result and +// only clamps negative channels and alpha. function lerpColor(from: Color, to: Color, t: number): Color { return new Color( from.r + (to.r - from.r) * t,