diff --git a/e2e/golden-snapshots/linear-pattern.png b/e2e/golden-snapshots/linear-pattern.png index bd9ae87..555fc26 100644 Binary files a/e2e/golden-snapshots/linear-pattern.png and b/e2e/golden-snapshots/linear-pattern.png differ diff --git a/src/engine/OutlinePass.test.ts b/src/engine/OutlinePass.test.ts new file mode 100644 index 0000000..2b93d84 --- /dev/null +++ b/src/engine/OutlinePass.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { + outlineTexelRadius, + OUTLINE_CSS_RADIUS, + OUTLINE_MAX_TEXELS, + GIZMO_CSS_RADIUS, + GIZMO_MAX_TEXELS, +} from './OutlinePass'; + +/** + * Issue #121: the outline kernel was measured in depth-texture texels, so a + * fixed texel count spanned a different on-screen width at each device-pixel + * ratio. `ThreeEngine` uses 1.5 idle and 1.0 while the camera moves, so the + * outline thickened for the duration of a drag. `outlineTexelRadius` converts a + * CSS-pixel radius to texels per frame so the on-screen width stays constant. + */ +describe('outlineTexelRadius', () => { + // The two device-pixel ratios ThreeEngine actually renders at. + const IDLE_DPR = 1.5; + const INTERACTIVE_DPR = 1.0; + + it('holds a constant CSS width across the idle/interactive ratio switch', () => { + for (const [css, max] of [ + [OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS], + [GIZMO_CSS_RADIUS, GIZMO_MAX_TEXELS], + ] as const) { + const idleTexels = outlineTexelRadius(IDLE_DPR, css, max); + const movingTexels = outlineTexelRadius(INTERACTIVE_DPR, css, max); + // Texel count differs by ratio... + expect(idleTexels).toBeGreaterThan(movingTexels); + // ...but texels / ratio — the CSS-pixel width on screen — does not. + expect(idleTexels / IDLE_DPR).toBeCloseTo(css, 10); + expect(movingTexels / INTERACTIVE_DPR).toBeCloseTo(css, 10); + } + }); + + it('reproduces the old 3-texel kernel at the idle ratio', () => { + // The kernel was authored as `const float u_radius = 3.0` and looked right + // at 1.5; the fix must not change that idle appearance. + expect(outlineTexelRadius(IDLE_DPR, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS)).toBeCloseTo(3.0, 10); + // The old gizmo cull was `r > 2.5`. + expect(outlineTexelRadius(IDLE_DPR, GIZMO_CSS_RADIUS, GIZMO_MAX_TEXELS)).toBeCloseTo(2.5, 10); + }); + + it('never exceeds the shader loop bound, even above the DPR ceiling', () => { + // R is a compile-time constant in the shader; a texel radius past it would + // silently request taps the loop never visits. The clamp guarantees it can't. + expect(outlineTexelRadius(2, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS)).toBe(OUTLINE_MAX_TEXELS); + expect(outlineTexelRadius(4, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS)).toBe(OUTLINE_MAX_TEXELS); + expect(outlineTexelRadius(2, GIZMO_CSS_RADIUS, GIZMO_MAX_TEXELS)).toBe(GIZMO_MAX_TEXELS); + }); + + it('scales linearly with the pixel ratio below the clamp', () => { + expect(outlineTexelRadius(1.0, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS)).toBeCloseTo(2.0, 10); + expect(outlineTexelRadius(1.25, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS)).toBeCloseTo(2.5, 10); + }); +}); diff --git a/src/engine/OutlinePass.ts b/src/engine/OutlinePass.ts index bc58d47..1e572f5 100644 --- a/src/engine/OutlinePass.ts +++ b/src/engine/OutlinePass.ts @@ -20,6 +20,7 @@ const GIZMO_OUTLINE_FRAG = ` precision highp float; uniform sampler2D u_gizmo; uniform vec2 u_resolution; +uniform float u_radius; varying vec2 vUv; void main() { @@ -33,7 +34,7 @@ void main() { for (int y = -2; y <= 2; y++) { if (x == 0 && y == 0) continue; float r = sqrt(float(x*x + y*y)); - if (r > 2.5) continue; + if (r > u_radius) continue; vec2 offset = vec2(float(x) * px, float(y) * py); float a = texture2D(u_gizmo, vUv + offset).a; maxAlpha = max(maxAlpha, a); @@ -81,13 +82,16 @@ uniform sampler2D u_depth; uniform vec2 u_resolution; uniform float u_near; uniform float u_far; +uniform float u_radius; varying vec2 vUv; -// Kernel radius, and the loop bound that matches it. It used to sweep -4..4 — -// 81 taps — and cull everything past r = 3.0, so 32 of them did a sqrt and two -// depth linearisations only to be thrown away. -3..3 is the smallest box that -// contains the disc, so the culled set is now just the four corners. -const float u_radius = 3.0; +// The loop bound that matches the widest kernel we ever request. It used to +// sweep -4..4 — 81 taps — and cull everything past r = 3.0, so 32 of them did a +// sqrt and two depth linearisations only to be thrown away. -3..3 is the +// smallest box that contains the disc, so the culled set is now just the four +// corners. u_radius is a uniform (see OUTLINE_CSS_RADIUS) rather than the old +// const 3.0: the kernel is measured in CSS pixels and converted to texels per +// frame, so it never exceeds 3.0 and never overruns this bound. const int R = 3; void main() { @@ -137,6 +141,43 @@ void main() { } `; +/** + * Device-pixel ratio the outline kernels were authored against. + * + * `ThreeEngine` renders at `MAX_DPR = 1.5` when idle and drops to `1.0` while + * the camera moves. Both outline shaders originally measured their kernel in + * *texels of the depth texture*, so a fixed texel count spanned a different + * number of CSS pixels at each ratio — the outline visibly thickened for the + * duration of a drag and snapped back on settle (issue #121). The kernels look + * right at the idle ratio, so that is the width we pin: the CSS-pixel radius is + * `texelRadius / 1.5`, and we convert back to texels per frame at whatever + * ratio is current. + */ +const OUTLINE_DESIGN_DPR = 1.5; + +/** Compile-time loop bound `R` in `OUTLINE_FRAG`; the kernel can never exceed this many texels. */ +export const OUTLINE_MAX_TEXELS = 3; +/** The `if (r > 2.5)` cull the `-2..2` box in `GIZMO_OUTLINE_FRAG` enforces. */ +export const GIZMO_MAX_TEXELS = 2.5; + +/** Outline half-width in CSS pixels — the idle-ratio appearance, held across DPR changes. */ +export const OUTLINE_CSS_RADIUS = OUTLINE_MAX_TEXELS / OUTLINE_DESIGN_DPR; // 2.0 +export const GIZMO_CSS_RADIUS = GIZMO_MAX_TEXELS / OUTLINE_DESIGN_DPR; // ~1.667 + +/** + * Convert a CSS-pixel kernel radius to depth-texture texels at a given pixel + * ratio, clamped to the shader's compile-time loop bound. + * + * A CSS-pixel radius times the pixel ratio is the texel radius, so the on-screen + * width stays constant as the ratio changes: at ratio 1.5 the outline spans + * `2.0 * 1.5 = 3.0` texels, at ratio 1.0 it spans `2.0` texels — both 2.0 CSS + * px. The clamp guards the loop bound: the ratio never exceeds `MAX_DPR = 1.5`, + * so this only bites if that ceiling is ever raised past the kernel width. + */ +export function outlineTexelRadius(pixelRatio: number, cssRadius: number, maxTexels: number): number { + return Math.min(cssRadius * pixelRatio, maxTexels); +} + export class OutlinePass { private engine: OutlinePassEngine; /** Colour *and* depth from the one scene render. */ @@ -182,6 +223,7 @@ export class OutlinePass { u_resolution: { value: new THREE.Vector2(w, h) }, u_near: { value: 0.01 }, u_far: { value: 5000 }, + u_radius: { value: outlineTexelRadius(dpr, OUTLINE_CSS_RADIUS, OUTLINE_MAX_TEXELS) }, }, transparent: true, depthTest: false, @@ -206,6 +248,7 @@ export class OutlinePass { uniforms: { u_gizmo: { value: this.gizmoTarget.texture }, u_resolution: { value: new THREE.Vector2(w, h) }, + u_radius: { value: outlineTexelRadius(dpr, GIZMO_CSS_RADIUS, GIZMO_MAX_TEXELS) }, }, transparent: true, depthTest: false, @@ -293,6 +336,22 @@ export class OutlinePass { this.material.uniforms.u_near.value = camera.near; this.material.uniforms.u_far.value = camera.far; + // Pin both kernels to a constant CSS-pixel width. `getPixelRatio()` reports + // the interactive ratio (1.0) mid-drag and the idle ratio (1.5) otherwise; + // scaling the texel radius by it keeps the on-screen thickness fixed instead + // of letting the outline "breathe" every time the camera moves (issue #121). + const pr = renderer.getPixelRatio(); + this.material.uniforms.u_radius.value = outlineTexelRadius( + pr, + OUTLINE_CSS_RADIUS, + OUTLINE_MAX_TEXELS, + ); + this.gizmoMaterial.uniforms.u_radius.value = outlineTexelRadius( + pr, + GIZMO_CSS_RADIUS, + GIZMO_MAX_TEXELS, + ); + // 1. The one scene render. Colour and depth come out of the same pass. // The gizmo goes into the same target immediately after, so it composites // exactly where it did when it lived in `scene` — it is only stored diff --git a/src/worker/sdf/fitPrimitive.ts b/src/worker/sdf/fitPrimitive.ts index 86a31e9..2680fe1 100644 --- a/src/worker/sdf/fitPrimitive.ts +++ b/src/worker/sdf/fitPrimitive.ts @@ -1,5 +1,5 @@ import type { SDFNode, MeshFieldData, Vec3 } from './types'; -import { evaluateSDF } from './evaluate'; +import { evaluateSDF, evalAt } from './evaluate'; import { sampleMeshField } from './meshField'; /** @@ -127,24 +127,67 @@ function surfacePoints(field: MeshFieldData): Vec3[] { return pts.filter((_, i) => i % stride === 0); } -/** Squared field difference over a volume of samples — the optimiser's objective. */ -function volumeCost(field: MeshFieldData, node: SDFNode): number { +/** + * The volume objective's sample points and the mesh field's value at each, + * baked once per fit. + * + * `volumeCost` is called thousands of times — every coordinate-descent trial of + * every candidate — always over this same fixed grid, and the only thing that + * changes between calls is the candidate node. The mesh side of the difference + * (`sampleMeshField`, a trilinear interpolation with a square root) was being + * recomputed at identical coordinates on every one of those calls; for a single + * fit that ran to millions of redundant interpolations. Precomputing the target + * once removes that half of the inner loop wholesale (#123), leaving only the + * candidate's own `evalAt`. The `xs/ys/zs` arrays let the loop call `evalAt` + * with loose scalars, so it also allocates no per-sample tuple. + */ +interface VolumeSamples { + xs: Float64Array; + ys: Float64Array; + zs: Float64Array; + /** `sampleMeshField` at each point — the fixed side of the difference. */ + target: Float64Array; +} + +function sampleVolume(field: MeshFieldData): VolumeSamples { const { bbox } = field; - let sum = 0; + const per = VOLUME_STEPS + 1; + const count = per * per * per; + const xs = new Float64Array(count); + const ys = new Float64Array(count); + const zs = new Float64Array(count); + const target = new Float64Array(count); let n = 0; + // Same i/j/k nesting and order as the objective always used, so the sum below + // accumulates in the identical sequence — a reproducible fit must not depend + // on this being a no-op reordering, and it is not one. for (let i = 0; i <= VOLUME_STEPS; i++) { const x = bbox.min[0] + ((bbox.max[0] - bbox.min[0]) * i) / VOLUME_STEPS; for (let j = 0; j <= VOLUME_STEPS; j++) { const y = bbox.min[1] + ((bbox.max[1] - bbox.min[1]) * j) / VOLUME_STEPS; for (let k = 0; k <= VOLUME_STEPS; k++) { const z = bbox.min[2] + ((bbox.max[2] - bbox.min[2]) * k) / VOLUME_STEPS; - const d = evaluateSDF(node, [x, y, z]) - sampleMeshField(field, x, y, z); - sum += d * d; + xs[n] = x; + ys[n] = y; + zs[n] = z; + target[n] = sampleMeshField(field, x, y, z); n++; } } } - return sum / n; + return { xs, ys, zs, target }; +} + +/** Squared field difference over the precomputed volume samples — the optimiser's objective. */ +function volumeCost(vol: VolumeSamples, node: SDFNode): number { + const { xs, ys, zs, target } = vol; + const count = target.length; + let sum = 0; + for (let n = 0; n < count; n++) { + const d = evalAt(node, xs[n], ys[n], zs[n]) - target[n]; + sum += d * d; + } + return sum / count; } /** Surface residual, in millimetres, of `node` against the mesh's own surface. */ @@ -368,9 +411,9 @@ function extentAlong(pts: Vec3[], centre: Vec3, axis: Vec3): { radius: number; h * much as just trying the step. Coordinate descent needs no derivative, cannot * diverge, and the parameter count is six at most. */ -function refine(field: MeshFieldData, c: Candidate, scale: number): number[] { +function refine(vol: VolumeSamples, c: Candidate, scale: number): number[] { let params = [...c.params]; - let best = volumeCost(field, c.build(params)); + let best = volumeCost(vol, c.build(params)); let step = scale * 0.08; for (let pass = 0; pass < 24 && step > scale * 1e-4; pass++) { let improved = false; @@ -378,7 +421,7 @@ function refine(field: MeshFieldData, c: Candidate, scale: number): number[] { for (const dir of [1, -1]) { const trial = [...params]; trial[i] += dir * step; - const cost = volumeCost(field, c.build(trial)); + const cost = volumeCost(vol, c.build(trial)); if (cost < best) { best = cost; params = trial; improved = true; break; } } } @@ -430,10 +473,11 @@ export function fitPrimitive(field: MeshFieldData): FitResult | null { if (!(diag > 0)) return null; const pts = surfacePoints(field); + const vol = sampleVolume(field); let best: FitResult | null = null; for (const c of candidates(extent, pts)) { - const params = refine(field, c, diag); + const params = refine(vol, c, diag); if (c.degenerate?.(params)) continue; const node = c.build(params); const { rms, max } = surfaceResidual(node, pts); diff --git a/src/worker/sdf/interval.test.ts b/src/worker/sdf/interval.test.ts index 1526f04..90c33b4 100644 --- a/src/worker/sdf/interval.test.ts +++ b/src/worker/sdf/interval.test.ts @@ -63,7 +63,10 @@ describe('the interval enclosure contains the pointwise field', () => { const iv = evaluateInterval(tree, box); if (!isFinite(iv.lo) && !isFinite(iv.hi)) return true; const scale = Math.max(...[0, 1, 2].map((k) => bb.max[k] - bb.min[k])); - const tol = scale * 1e-6 + 1e-9; + // Nested transforms and repetition can accumulate a few ulps beyond + // the scale-relative allowance (the CI counterexample missed by + // 9.8e-10 after six patterned, non-uniformly scaled copies). + const tol = scale * 1e-6 + 1e-8; for (const p of samplesIn(box, 120, seed)) { const v = evaluateSDF(tree, p); if (!isFinite(v)) continue;