fix: the sky meets the sea, and the places rail admits it scrolls
Two defects a photograph found and no gate could. **The sky seam.** A phone held upright drew a hard horizontal step across the top 10.5% of the frame — a flat duller slab above a warm gradient, meeting in one pixel. Painting the sky dome red proved the slab was the dome itself and everything below the line was sea; so this was never missing geometry, it was two halves of one join disagreeing about a colour. The sea is a flat plane, so however wide it is made the camera's own far plane cuts it at an elevation *below* the horizontal — about 12 degrees in a plan view of the state. atmosphere.ts has fogged it to exactly the horizon colour by then, which is the entire mechanism by which water dissolves into sky instead of ending in an edge. The dome at that same angle has to be the same number, and it was not: the below-horizon darkening ramped to its full 0.82 over the first 11.5 degrees. The sea arrived at the horizon colour and met a sky already a fifth darker. Measured 155,107,99 against 127,88,81 across one pixel. Holding the horizon's own colour for the first 30 degrees costs nothing the darkening was for — a floor or an ocean covers that band whenever there is one, and where there is not, matching is the whole point. Measured after: the worst single-row step anywhere in the sky falls from 15/255 to 8, and the 8 is water texture rather than a line. render/skyHorizonSeam.test.ts parses the shader text, which is a weaker instrument than a frame and the strongest one available with no GL context. It asserts the one number the picture proved wrong — how far down the dome still paints the horizon colour — and it caught a real off-by-a-hair while being written: 0.42 is sin(24.8 deg), just inside the 25 deg the sea can reach. It also guards the backtick-in-a-template-literal trap that has now broken this same shader three times. **The places rail.** Its cap is min(22rem, 42vh) and rows are variable height, so it always landed mid-row, and a row sliced through its own text reads as a button that failed to draw rather than as more list. markOverflow writes data-overflow and the stylesheet fades whichever end has list behind it. Every measurement in it is guarded, because ui/mount.ts is deliberately tested with no DOM at all: no geometry must mean no attribute, not data-overflow=NaN. A pixel of slack at each end, because fractional layout reports a fully-scrolled list a fraction short and a fade that never switches off dims a row over nothing. stylesheet.test.ts asserts the two files still speak the same four words — nothing else connects them and the failure is silent. 1,664 + 295 tests, all gates, budget 779,789 tri / 150 draws / p95 16.7 ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,110 @@
|
||||
/**
|
||||
* The join between the sea and the sky, asserted where it is decided.
|
||||
*
|
||||
* ## The defect
|
||||
*
|
||||
* A phone held upright, on the California board, drew a hard horizontal step
|
||||
* across the top **10.5%** of the frame: a flat, duller slab above a warm
|
||||
* gradient, meeting in one pixel. It survived every gate in the repo. It is a
|
||||
* shader constant, so no typecheck sees it; nothing here compiles GLSL, so no
|
||||
* test saw it; and it is above the terrain, so the performance budget and the
|
||||
* pixel-regression baselines were both unmoved.
|
||||
*
|
||||
* ## What causes it
|
||||
*
|
||||
* The sea is a flat plane. However wide it is made — and `SEA_SPAN_MULTIPLE`
|
||||
* puts its rim nine board spans out precisely so it never ends in a visible
|
||||
* edge — a camera standing off above it has its own **far plane** cut the water
|
||||
* at an elevation some degrees *below* the horizontal. In a plan view of the
|
||||
* state that angle is around twelve degrees. `atmosphere.ts` has fogged the
|
||||
* water to exactly the horizon colour by the time it gets there, which is the
|
||||
* whole mechanism by which water dissolves into sky instead of ending.
|
||||
*
|
||||
* So the dome, at that same angle, is the other half of that join. It has to be
|
||||
* painting the same number. It was not: the below-horizon darkening ramped to
|
||||
* its full 0.82 over the first 11.5 degrees, so the sea arrived at the horizon
|
||||
* colour and met a sky already a fifth darker.
|
||||
*
|
||||
* ## What is asserted
|
||||
*
|
||||
* The shader's own text, parsed. That is a weaker instrument than a rendered
|
||||
* frame and it is the strongest one available without a GL context, so what it
|
||||
* checks is the one number a picture proved wrong: how far below the horizontal
|
||||
* the dome still paints the horizon's own colour. A future edit is free to
|
||||
* change the curve, the depth, or the colour — it is not free to start
|
||||
* darkening inside the band where the sea is still being drawn.
|
||||
*/
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { describe, it } from "node:test";
|
||||
|
||||
const source = readFileSync(new URL("../../engine/scenekit.ts", import.meta.url), "utf8");
|
||||
|
||||
/**
|
||||
* The lowest elevation, as a sine, at which a board can still be drawing sea.
|
||||
*
|
||||
* The camera may retreat two spans from the scene origin, and the sea's rim is
|
||||
* nine spans out, so the shallowest sight-line to the far water is about
|
||||
* `atan(2 / 9)` = 12.5 degrees below the horizontal. Rounded out to 25 degrees,
|
||||
* because the number that matters is the one a *fog* far plane produces rather
|
||||
* than the geometry's rim, and the far plane moves with the stand-off.
|
||||
*/
|
||||
const SEA_MAY_REACH_SINE = Math.sin((25 * Math.PI) / 180);
|
||||
|
||||
describe("the sky below the horizon", () => {
|
||||
/** The one line that decides it, isolated so a failure names the right thing. */
|
||||
const line = /color = mix\( color, uHorizon \* [\d.]+, ([^;]+)\);/.exec(source)?.[1]?.trim();
|
||||
|
||||
it("still exists and is still a blend toward a darker horizon", () => {
|
||||
assert.notEqual(line, undefined, "the below-horizon term is gone from the sky shader");
|
||||
const factor = Number(/uHorizon \* ([\d.]+)/.exec(source)?.[1]);
|
||||
assert.ok(factor > 0 && factor <= 1, `below-horizon sky must not be brighter: ${factor}`);
|
||||
});
|
||||
|
||||
it("holds the horizon's own colour through every angle the sea can reach", () => {
|
||||
const edges = /smoothstep\(\s*([\d.]+),\s*([\d.]+),\s*- ?height\s*\)/.exec(line ?? "");
|
||||
assert.notEqual(
|
||||
edges,
|
||||
null,
|
||||
`the below-horizon ramp is no longer a smoothstep in -height: ${line}`,
|
||||
);
|
||||
const start = Number(edges![1]);
|
||||
assert.ok(
|
||||
start > SEA_MAY_REACH_SINE,
|
||||
`the sky starts darkening ${((Math.asin(start) * 180) / Math.PI).toFixed(1)}deg below ` +
|
||||
`the horizontal, inside the ${((Math.asin(SEA_MAY_REACH_SINE) * 180) / Math.PI).toFixed(0)}deg ` +
|
||||
"band where the sea is still drawn — that is the seam",
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The fix it replaced, named so it cannot be reintroduced as a tidy-up. A
|
||||
* `clamp(-height * k, 0, 1)` is a perfectly ordinary way to write this and it
|
||||
* is the exact shape that shipped the seam: at any k large enough to look
|
||||
* like a horizon it is fully dark before the sea has finished.
|
||||
*/
|
||||
it("does not go back to a clamp that saturates in the first few degrees", () => {
|
||||
assert.ok(
|
||||
!/clamp\(\s*- ?height \* [\d.]+/.test(source),
|
||||
"the below-horizon term is a clamp again; see this suite's header",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Not a shader assertion, and here because this file is where the trap is
|
||||
* documented: the sky shader is a **template literal**, and a backtick anywhere
|
||||
* inside it — including in a comment quoting a filename — terminates it and
|
||||
* turns the rest of the GLSL into JavaScript. It has cost this repo three
|
||||
* builds. `tsc` does catch it, so this is a faster and clearer message rather
|
||||
* than a new guarantee.
|
||||
*/
|
||||
describe("the sky shader's source text", () => {
|
||||
it("carries no backtick inside the shader template literals", () => {
|
||||
const shaders = [...source.matchAll(/(vertexShader|fragmentShader): `([\s\S]*?)`,\n/g)];
|
||||
assert.ok(shaders.length >= 2, "the sky shaders are no longer inline template literals");
|
||||
for (const [, which, body] of shaders) {
|
||||
assert.ok(!(body ?? "").includes("`"), `a backtick inside ${which} ends the literal early`);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -6,7 +6,7 @@ import type { DeviceCommand, DeviceDeclaration } from "../../devices/types.ts";
|
||||
import type { ControlMode } from "../../play/controlMode.ts";
|
||||
import { chromeState } from "../../ui/chromeState.ts";
|
||||
import type { ChromeInputs } from "../../ui/chromeState.ts";
|
||||
import { mountChrome } from "../../ui/mount.ts";
|
||||
import { markOverflow, mountChrome } from "../../ui/mount.ts";
|
||||
import type { ChromeMountOptions } from "../../ui/mount.ts";
|
||||
import { FakeDocument, type FakeElement, el, fakeStorage } from "./fakeDom.ts";
|
||||
|
||||
@@ -515,3 +515,68 @@ describe("mountChrome", () => {
|
||||
chrome.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
describe("the places rail's overflow edge", () => {
|
||||
/** The bare minimum a real element offers: three numbers and an attribute bag. */
|
||||
function rail(scrollTop: number, clientHeight: number, scrollHeight: number) {
|
||||
const attributes = new Map<string, string>();
|
||||
return {
|
||||
scrollTop,
|
||||
clientHeight,
|
||||
scrollHeight,
|
||||
attributes,
|
||||
setAttribute(name: string, value: string) {
|
||||
attributes.set(name, value);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function edgeOf(scrollTop: number, clientHeight: number, scrollHeight: number) {
|
||||
const node = rail(scrollTop, clientHeight, scrollHeight);
|
||||
markOverflow(node as unknown as HTMLElement);
|
||||
return node.attributes.get("data-overflow");
|
||||
}
|
||||
|
||||
it("says nothing is clipped when the list fits", () => {
|
||||
assert.equal(edgeOf(0, 400, 400), "none");
|
||||
});
|
||||
|
||||
it("fades the bottom when there are rows below the cap", () => {
|
||||
assert.equal(edgeOf(0, 352, 900), "bottom");
|
||||
});
|
||||
|
||||
it("fades the top once the reader has scrolled past the first row", () => {
|
||||
assert.equal(edgeOf(548, 352, 900), "top");
|
||||
});
|
||||
|
||||
it("fades both ends in the middle of a long list", () => {
|
||||
assert.equal(edgeOf(200, 352, 900), "both");
|
||||
});
|
||||
|
||||
/**
|
||||
* The bug this guards: fractional layout means a list scrolled fully down
|
||||
* reports a scrollTop a fraction of a pixel short, and a `<` with no slack
|
||||
* leaves the bottom permanently dimmed over nothing.
|
||||
*/
|
||||
it("treats a sub-pixel shortfall at either end as arrival", () => {
|
||||
assert.equal(edgeOf(0.4, 352, 352.4), "none");
|
||||
assert.equal(edgeOf(547.6, 352.2, 900), "top");
|
||||
});
|
||||
|
||||
/**
|
||||
* This module is tested with no DOM, and half the ways of running it — the
|
||||
* `FakeElement` harness, a node without layout, an element measured before
|
||||
* first paint — have no geometry to report. That must set no attribute at
|
||||
* all, rather than `data-overflow="NaN"` matching no rule and masking nothing
|
||||
* while looking like a live value in the inspector.
|
||||
*/
|
||||
it("sets nothing at all when there is no geometry to read", () => {
|
||||
const node = rail(Number.NaN, Number.NaN, Number.NaN);
|
||||
markOverflow(node as unknown as HTMLElement);
|
||||
assert.equal(node.attributes.has("data-overflow"), false);
|
||||
|
||||
const bare = { setAttribute() {} } as unknown as HTMLElement;
|
||||
assert.doesNotThrow(() => markOverflow(bare));
|
||||
assert.doesNotThrow(() => markOverflow(null));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -233,3 +233,51 @@ describe("the stylesheet", () => {
|
||||
assert.ok(DEVICE_PANEL_CSS.includes("@media (prefers-reduced-motion: reduce)"));
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The places rail's fade is a two-file mechanism: `markOverflow` in
|
||||
* `ui/mount.ts` writes `data-overflow`, and the stylesheet decides what that
|
||||
* looks like. Nothing else connects them, so either half can be renamed without
|
||||
* the other noticing and the failure is silent — a list that clips a row and
|
||||
* offers no sign it scrolls, which is exactly the fault the fade was added to
|
||||
* fix. This asserts the two halves still speak the same four words.
|
||||
*/
|
||||
describe("the places rail's overflow fade", () => {
|
||||
const mount = read("src/ui/mount.ts");
|
||||
|
||||
/** Every value `markOverflow` can assign, read out of the code itself. */
|
||||
const emitted = ["both", "top", "bottom", "none"];
|
||||
|
||||
it("emits exactly the values this test knows about", () => {
|
||||
const line = /const edge =[\s\S]*?;/.exec(mount)?.[0] ?? "";
|
||||
assert.notEqual(line, "", "markOverflow no longer computes an `edge`");
|
||||
for (const value of emitted) {
|
||||
assert.ok(line.includes(`"${value}"`), `markOverflow stopped emitting ${value}`);
|
||||
}
|
||||
const quoted = [...line.matchAll(/"([a-z]+)"/g)].map((match) => match[1]);
|
||||
assert.deepEqual(new Set(quoted), new Set(emitted), "markOverflow emits a new value");
|
||||
});
|
||||
|
||||
it("masks every clipped state and leaves the unclipped one alone", () => {
|
||||
for (const value of ["top", "bottom", "both"]) {
|
||||
assert.ok(
|
||||
css.includes(`.places[data-overflow="${value}"]`),
|
||||
`no fade rule for data-overflow="${value}"`,
|
||||
);
|
||||
}
|
||||
assert.ok(
|
||||
!css.includes('.places[data-overflow="none"]'),
|
||||
"a list that is not clipped must not be masked",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the unprefixed mask alongside the -webkit- one", () => {
|
||||
const rules = css.split(".places[data-overflow=").slice(1);
|
||||
assert.equal(rules.length, 3);
|
||||
for (const rule of rules) {
|
||||
const body = rule.slice(0, rule.indexOf("}"));
|
||||
assert.ok(body.includes("-webkit-mask-image:"), "missing the prefixed mask");
|
||||
assert.ok(/[^-]mask-image:/.test(body), "missing the standard mask");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user