Normalise the four views with no renderer onto the planet's terrain (#227)

src/noise/preview/elevationRenderRequest.ts

@@ -92,7 +92,8 @@ export interface ElevationRenderRequest {
    * versus ocean, skipping the eight-way land argmax whose answer the finder
    * discards. It is deliberately absent from `ElevationPreviewPanel`'s own view
    * union, so no dev-mode toggle can select it. On any planet without a port it
-   * falls back to that planet's terrain, like every other view here.
+   * falls back to that planet's terrain, like every other view here - see
+   * `servedView`, which makes that fall-back explicit rather than incidental.
    */
   view?:
     | "elevation"
@@ -491,10 +492,50 @@ function renderNauvisThroughWasm(
   return { id: req.id, buffer: owned.buffer, width: req.width, height: req.height };
 }
 
+/**
+ * The view this request actually renders, which is not always the one it asks
+ * for.
+ *
+ * Four `(planet, view)` pairs have no renderer of their own: Vulcanus has no
+ * enemy bases, no trees and no ocean, and Nauvis has no land mask. Asking for
+ * one has always produced the planet's plain terrain, because the overlay
+ * blocks below simply never match and the land-mask branch is Fulgora's alone.
+ * The request rendered; it just rendered terrain.
+ *
+ * That silence is the problem. The Rust engine refuses all four outright - see
+ * the `supported` match in `crates/fmw-wasm/src/render.rs`, which pins
+ * `(Vulcanus, landmask)` as unsupported in its own test - so the fall-through
+ * has to become explicit BEFORE #227 deletes the TypeScript terrain renderers
+ * it lands on. Left alone, those four requests would stop rendering and start
+ * throwing, and Trap 2's error path would discard the reason.
+ *
+ * Normalising onto `"terrain"` rather than widening the engine's gate is what
+ * keeps the pixels identical: `"terrain"` is precisely what these four already
+ * draw. Widening the gate would ask the module for a render it has deliberately
+ * decided is meaningless.
+ *
+ * Nothing in the app can reach any of the four - `ElevationPreviewPanel`'s
+ * `effectiveView` emits only `terrain|resources|cliffs|rocks|all` on Vulcanus,
+ * and `"landmask"` is absent from its view union on every planet - so this
+ * closes a hole in the type surface rather than in anything a user sees. It is
+ * still worth closing: the type permits all four, and `findIslands` already
+ * posts a hand-built request rather than one the panel produced.
+ */
+function servedView(
+  planet: Planet,
+  view: ElevationRenderRequest["view"],
+): ElevationRenderRequest["view"] {
+  if (planet === "vulcanus" && (view === "enemies" || view === "trees" || view === "landmask")) {
+    return "terrain";
+  }
+  if (planet === "nauvis" && view === "landmask") return "terrain";
+  return view;
+}
+
 /**
  * Pure render step shared by the worker and its tests: run renderElevation or
- * renderTerrain (per `req.view`) and hand back the transferable RGBA buffer. No
- * Worker or DOM canvas involved.
+ * renderTerrain (per `req.view`, once `servedView` has normalised it) and hand
+ * back the transferable RGBA buffer. No Worker or DOM canvas involved.
  *
  * `engine` is optional and opt-in. When a caller supplies an instantiated Rust
  * engine AND the request is Fulgora's land mask - the one path #223 ports - the
@@ -511,23 +552,24 @@ export function runRenderRequest(
   engine?: EngineExports,
 ): ElevationRenderResult {
   const planet = req.planet ?? "nauvis";
+  const view = servedView(planet, req.view);
   let image: ImageData;
   if (
-    req.view === "terrain" ||
-    req.view === "resources" ||
-    req.view === "enemies" ||
-    req.view === "cliffs" ||
-    req.view === "trees" ||
-    req.view === "rocks" ||
-    req.view === "all" ||
-    req.view === "landmask"
+    view === "terrain" ||
+    view === "resources" ||
+    view === "enemies" ||
+    view === "cliffs" ||
+    view === "trees" ||
+    view === "rocks" ||
+    view === "all" ||
+    view === "landmask"
   ) {
     if (planet === "vulcanus") {
       // Vulcanus has its own resource and cliff overlays. The remaining three
       // Nauvis overlays (enemies, trees, rocks) have no Vulcanus port, so a
       // terrain-family view that asks for one still gets plain terrain rather
       // than a Nauvis field composited onto Vulcanus colors.
-      const wantsResources = req.view === "resources" || req.view === "all";
+      const wantsResources = view === "resources" || view === "all";
       // Checked BEFORE the TypeScript stack is built, for the reason the
       // Fulgora branch gives: `makeVulcanusStack` derives seed tables for the
       // whole biome, crack, climate and elevation chain, and building them only
@@ -539,13 +581,13 @@ export function runRenderRequest(
       // failed to load is slower and never wrong.
       if (
         engine !== undefined &&
-        (req.view === "terrain" ||
-          req.view === "cliffs" ||
-          req.view === "rocks" ||
-          req.view === "resources" ||
-          req.view === "all")
+        (view === "terrain" ||
+          view === "cliffs" ||
+          view === "rocks" ||
+          view === "resources" ||
+          view === "all")
       ) {
-        return renderVulcanusThroughWasm(req, engine, req.view);
+        return renderVulcanusThroughWasm(req, engine, view);
       }
       // ONE stack for the whole composite. Two things make this pay, and both
       // are needed: the overlays reuse the field objects terrain built, and
@@ -598,7 +640,7 @@ export function runRenderRequest(
           stack,
         });
       }
-      if (req.view === "rocks" || req.view === "all") {
+      if (view === "rocks" || view === "all") {
         renderVulcanusRocks(image, {
           seed0: req.seed0,
           originX: req.originX,
@@ -609,7 +651,7 @@ export function runRenderRequest(
           sharedStack: stack,
         });
       }
-      if (req.view === "cliffs" || req.view === "all") {
+      if (view === "cliffs" || view === "all") {
         renderVulcanusCliffs(image, {
           seed0: req.seed0,
           originX: req.originX,
@@ -627,8 +669,8 @@ export function runRenderRequest(
       // TypeScript stack is built, because `makeFulgoraStack` derives seed
       // tables for eight multioctave fields, and building them only to throw
       // them away would be most of the saving.
-      if (engine !== undefined && (req.view === "landmask" || req.view === "terrain")) {
-        return renderFulgoraThroughWasm(req, engine, req.view);
+      if (engine !== undefined && (view === "landmask" || view === "terrain")) {
+        return renderFulgoraThroughWasm(req, engine, view);
       }
       // Fulgora has a resources overlay now; it still has no cliffs and no
       // rocks, so those views fall back to plain terrain - the same fallback
@@ -655,12 +697,12 @@ export function runRenderRequest(
       };
       // Returns straight away: a land mask takes no overlays, and compositing
       // resources onto it would paint over the very bit the caller wants.
-      if (req.view === "landmask") {
+      if (view === "landmask") {
         const mask = renderFulgoraLandMask(fulgoraRender);
         return { id: req.id, buffer: mask.data.buffer, width: req.width, height: req.height };
       }
       image = renderFulgoraTerrain(fulgoraRender);
-      if (req.view === "resources" || req.view === "all") {
+      if (view === "resources" || view === "all") {
         renderFulgoraResources(image, {
           seed0: req.seed0,
           originX: req.originX,
@@ -691,17 +733,17 @@ export function runRenderRequest(
     // would be a wrong answer rather than a slow one. The app never sets it.
     if (
       engine !== undefined &&
-      (req.view === "terrain" ||
-        req.view === "trees" ||
-        req.view === "rocks" ||
-        req.view === "enemies" ||
-        req.view === "cliffs" ||
-        req.view === "resources" ||
-        req.view === "all") &&
+      (view === "terrain" ||
+        view === "trees" ||
+        view === "rocks" ||
+        view === "enemies" ||
+        view === "cliffs" ||
+        view === "resources" ||
+        view === "all") &&
       req.startingLakePositions === undefined &&
       req.startingPositions.length <= NAUVIS_MAX_STARTING_POINTS
     ) {
-      return renderNauvisThroughWasm(req, engine, req.view);
+      return renderNauvisThroughWasm(req, engine, view);
     }
     image = renderTerrain({
       seed0: req.seed0,
@@ -721,7 +763,7 @@ export function runRenderRequest(
         startingAreaMoistureFrequency: req.startingAreaMoistureFrequency,
       },
     });
-    if (req.view === "trees" || req.view === "all") {
+    if (view === "trees" || view === "all") {
       renderTrees(image, {
         seed0: req.seed0,
         originX: req.originX,
@@ -739,7 +781,7 @@ export function runRenderRequest(
         startingPositions: req.startingPositions,
       });
     }
-    if (req.view === "resources" || req.view === "all") {
+    if (view === "resources" || view === "all") {
       renderResources(image, {
         seed0: req.seed0,
         originX: req.originX,
@@ -763,7 +805,7 @@ export function runRenderRequest(
     // crossing an ore patch reads as the obstruction - same order as the
     // Vulcanus branch above. Trees stay under resources: a forest is cleared,
     // not an obstacle you route around.
-    if (req.view === "rocks" || req.view === "all") {
+    if (view === "rocks" || view === "all") {
       renderRocks(image, {
         seed0: req.seed0,
         originX: req.originX,
@@ -781,7 +823,7 @@ export function runRenderRequest(
         sweepBox: placementMarkSweepBox(req),
       });
     }
-    if (req.view === "enemies" || req.view === "all") {
+    if (view === "enemies" || view === "all") {
       renderEnemies(image, {
         seed0: req.seed0,
         originX: req.originX,
@@ -799,7 +841,7 @@ export function runRenderRequest(
         sweepBox: placementMarkSweepBox(req),
       });
     }
-    if (req.view === "cliffs" || req.view === "all") {
+    if (view === "cliffs" || view === "all") {
       renderCliffs(image, {
         seed0: req.seed0,
         originX: req.originX,

test/viewNormalisation.spec.ts

@@ -0,0 +1,154 @@
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import { describe, expect, it } from "vite-plus/test";
+
+import {
+  runRenderRequest,
+  type ElevationRenderRequest,
+} from "../src/noise/preview/elevationRenderRequest";
+import { compileEngine, instantiateEngine } from "../src/noise/wasm/engine";
+
+/**
+ * The four `(planet, view)` pairs that have no renderer of their own.
+ *
+ * Vulcanus has no enemy bases, no trees and no ocean; Nauvis has no land mask.
+ * Asking for one of those has always produced the planet's plain terrain -
+ * the overlay blocks never match, and the land-mask branch is Fulgora's alone -
+ * and until this file **nothing asserted that in either direction**. The
+ * closest thing, `test/wasmVulcanusRenderParity.spec.ts`'s "routes every view
+ * the planet has through the engine", iterates the five ported Vulcanus views
+ * and documents the hole in a comment without covering it.
+ *
+ * That mattered as soon as #227 came to delete the TypeScript terrain
+ * renderers those four land on. The Rust engine refuses all four pairings
+ * outright, so the fall-through had to become explicit - `servedView` - and the
+ * whole risk of doing that is moving pixels that nothing was watching. This
+ * file is the watch.
+ *
+ * Both arms are asserted deliberately. The engine arm is what survives the
+ * deletion; the no-engine arm is what proves the normalisation preserved
+ * today's answer rather than merely producing a self-consistent new one.
+ */
+const wasmPath = join(import.meta.dirname, "..", "src", "noise", "wasm", "engine.wasm");
+
+let compiled: WebAssembly.Module | undefined;
+async function engine() {
+  compiled ??= await compileEngine(readFileSync(wasmPath));
+  return instantiateEngine(compiled);
+}
+
+/**
+ * Small and off-origin on purpose: big enough that the terrain carries several
+ * colours (the anti-vacuity block below pins that), small enough that four
+ * pairs times three renders stays quick.
+ */
+const BASE = {
+  id: 1,
+  seed0: 123456,
+  width: 48,
+  height: 48,
+  originX: -96,
+  originY: -96,
+  tilesPerPixel: 4,
+  waterLevel: 0,
+  segmentationMultiplier: 1,
+  startingPositions: [{ x: 0, y: 0 }],
+} satisfies Omit<ElevationRenderRequest, "planet" | "view">;
+
+/** Every pair `servedView` normalises, with the label used in failures. */
+const FALL_THROUGH = [
+  { label: "vulcanus enemies", planet: "vulcanus", view: "enemies" },
+  { label: "vulcanus trees", planet: "vulcanus", view: "trees" },
+  { label: "vulcanus landmask", planet: "vulcanus", view: "landmask" },
+  { label: "nauvis landmask", planet: "nauvis", view: "landmask" },
+] as const;
+
+function pixels(
+  planet: ElevationRenderRequest["planet"],
+  view: ElevationRenderRequest["view"],
+  e?: Awaited<ReturnType<typeof engine>>,
+): Uint8ClampedArray {
+  return new Uint8ClampedArray(runRenderRequest({ ...BASE, planet, view }, e).buffer);
+}
+
+/** The set of distinct `r,g,b` triples in a render, packed one per number. */
+function colours(px: Uint8ClampedArray): Set<number> {
+  const seen = new Set<number>();
+  for (let i = 0; i < px.length; i += 4) seen.add((px[i] << 16) | (px[i + 1] << 8) | px[i + 2]);
+  return seen;
+}
+
+describe("the four views with no renderer of their own render that planet's terrain", () => {
+  it("is byte-identical to terrain through the engine, for all four", async () => {
+    const e = await engine();
+    for (const c of FALL_THROUGH) {
+      const got = pixels(c.planet, c.view, e);
+      const terrain = pixels(c.planet, "terrain", e);
+      expect(got.length, `${c.label}: length`).toBe(BASE.width * BASE.height * 4);
+      expect(Array.from(got), `${c.label}: pixels`).toEqual(Array.from(terrain));
+    }
+  }, 120000);
+
+  /**
+   * Without this the block above would pass against a blank image, which is
+   * exactly the failure a normalisation bug produces: route the request
+   * somewhere that paints nothing and every equality still holds.
+   */
+  it("is not vacuous - each planet's terrain carries several colours", async () => {
+    const e = await engine();
+    for (const c of FALL_THROUGH) {
+      const distinct = colours(pixels(c.planet, c.view, e));
+      expect(distinct.size, `${c.label}: distinct colours`).toBeGreaterThan(1);
+    }
+  }, 120000);
+
+  /**
+   * That the pair reaches the MODULE, not merely that it reaches terrain.
+   *
+   * Planted while writing this file: dropping the Nauvis `landmask` arm from
+   * `servedView` altogether left the block above green, because an
+   * un-normalised pair falls through to `renderTerrain` and paints the same
+   * bytes. Equality against terrain therefore grades the normalisation TARGET
+   * and not its PRESENCE - it would go quietly green again the day someone
+   * deleted an arm.
+   *
+   * The module's output buffer settles it. A request the engine serves writes
+   * there; a request that fell through to TypeScript cannot have. This is the
+   * idiom `test/wasmIslandFinderParity.spec.ts` uses for the same question.
+   *
+   * A fresh instance per pair, so a previous render cannot be mistaken for
+   * this one.
+   */
+  it("reaches the engine rather than falling through, for all four", async () => {
+    for (const c of FALL_THROUGH) {
+      const e = await engine();
+      const view = () => new Uint8Array(e.memory.buffer, e.render_ptr(), 64).slice();
+      const before = view();
+      runRenderRequest({ ...BASE, planet: c.planet, view: c.view }, e);
+      expect(Array.from(view()), `${c.label}: module buffer written`).not.toEqual(
+        Array.from(before),
+      );
+    }
+  }, 120000);
+
+  /**
+   * **The arm #227 deletes.**
+   *
+   * While the TypeScript terrain renderers still exist, each of the four can be
+   * rendered with no engine at all, and that render is the one this change had
+   * to preserve - it is what these pairs drew before `servedView` existed. Once
+   * `renderTerrain` and `renderVulcanusTerrain` are deleted there is no second
+   * arm and this block goes with them, which is why the engine-side assertions
+   * are kept above rather than folded in here.
+   */
+  it("renders identically with and without the engine, for all four", async () => {
+    const e = await engine();
+    for (const c of FALL_THROUGH) {
+      const withEngine = pixels(c.planet, c.view, e);
+      const withoutEngine = pixels(c.planet, c.view);
+      expect(Array.from(withEngine), `${c.label}: engine vs none`).toEqual(
+        Array.from(withoutEngine),
+      );
+    }
+  }, 120000);
+});