Stop bundling luma.gl into @spatialdata/layers; cover labels colour-by in CI

.changeset/layers-external-luma.md

@@ -0,0 +1,22 @@
+---
+'@spatialdata/layers': patch
+---
+
+Stop bundling luma.gl into the published `@spatialdata/layers` artifact.
+
+The build externalized only the specifiers this package imports directly, so
+`@luma.gl/core`, `/engine` and `/shadertools` (plus `@probe.gl/*`) were pulled in
+transitively and shipped inside `dist/index.js` — 238 kB down to 92 kB now that they
+are not.
+
+Size was the least of it. deck.gl, Viv and this package must share ONE luma runtime.
+A consumer that also loads deck.gl got two `ShaderAssembler` classes, and
+`ShaderAssembler.getDefaultShaderAssembler()` is a static — so "the default shader
+assembler" meant different objects to deck and to Viv. Viv's `VivShaderAssembler`
+builds itself by copying that default's modules and hook functions, so it could copy
+from an assembler deck had never registered anything on, and every Viv-derived layer —
+labels included — then failed to compile its vertex shader for want of deck's
+`DECKGL_FILTER_*` hooks.
+
+The externals are now whole families by regex rather than a list of today's imports,
+matching what `@spatialdata/vis` has always done.

packages/layers/vite.config.ts

@@ -19,11 +19,28 @@ export default defineConfig({
       formats: ['es'],
     },
     rollupOptions: {
+      // Whole families, by regex, rather than the handful of specifiers this
+      // package happens to import today.
+      //
+      // deck.gl, Viv and this package must share ONE luma.gl runtime. The list
+      // named `@deck.gl/core` but no luma at all, so `@luma.gl/core`, `/engine`
+      // and `/shadertools` came in through the layers that build their own `Model`
+      // and were bundled into `dist/index.js`: a consumer that also loads deck.gl
+      // then had two `ShaderAssembler` classes — and `ShaderAssembler.getDefault…()`
+      // is a static, so "the default assembler" then means different objects to
+      // deck and to Viv. Viv's `VivShaderAssembler` copies deck's registered
+      // modules and hooks off that default, so it can copy from an assembler deck
+      // never touched and lose `DECKGL_FILTER_GL_POSITION` entirely.
+      //
+      // Mirrors `packages/vis`, which has externalized both families all along.
       external: [
-        '@deck.gl/core',
+        /^@deck\.gl\/.+$/,
+        /^@luma\.gl\/.+$/,
+        /^@math\.gl\/.+$/,
+        /^@probe\.gl\/.+$/,
+        /^@spatialdata\/[^/]+$/,
+        /^@vivjs\/.+$/,
         '@hms-dbmi/viv',
-        '@math.gl/core',
-        '@spatialdata/core',
         'deck.gl',
         'zod',
       ],

tests/production/browser/labels-color-by.spec.ts

@@ -0,0 +1,79 @@
+import { expect, test } from '@playwright/test';
+import {
+  CHANNEL_COLOR,
+  LABEL_1_COLOR,
+  LABEL_2_COLOR,
+  type LabelsColorBySamples,
+} from './labelsColorByContract';
+
+/**
+ * SwiftShader is exact for this scenario (flat fills, no filtering, no AA at the
+ * sample points), but a byte of slack costs nothing and keeps the test from
+ * pinning a rounding path rather than the behaviour.
+ */
+const CHANNEL_TOLERANCE = 4;
+
+function describeColor(color: readonly number[]): string {
+  return `rgba(${color.join(', ')})`;
+}
+
+function expectColor(actual: readonly number[], expected: readonly number[], label: string) {
+  const maxDrift = Math.max(
+    ...expected.map((channel, index) => Math.abs(channel - (actual[index] ?? 0)))
+  );
+  expect(
+    maxDrift,
+    `${label}: expected ${describeColor(expected)}, got ${describeColor(actual)}`
+  ).toBeLessThanOrEqual(CHANNEL_TOLERANCE);
+}
+
+test('labels feature colouring reaches the GPU in the built layers artifact', async ({
+  page,
+}, testInfo) => {
+  const consoleErrors: string[] = [];
+  const pageErrors: string[] = [];
+
+  page.on('console', (message) => {
+    if (message.type() === 'error') consoleErrors.push(message.text());
+  });
+  page.on('pageerror', (error) => pageErrors.push(error.message));
+
+  await page.goto('/?scenario=labels-color-by', { waitUntil: 'networkidle' });
+  await expect
+    .poll(() => page.evaluate(() => Boolean(document.createElement('canvas').getContext('webgl2'))))
+    .toBe(true);
+  await expect(page.getByTestId('labels-ready')).toBeAttached();
+
+  // Wait for the synthetic raster to have loaded and drawn: until then the
+  // sampled pixels are the empty canvas, which would fail for the wrong reason.
+  await expect
+    .poll(() => page.evaluate(() => window.labelsColorBySamples?.label1[3] ?? 0), {
+      timeout: 15_000,
+    })
+    .toBeGreaterThan(0);
+
+  await page.screenshot({ path: testInfo.outputPath('labels-color-by.png') });
+
+  const runtime = await page.evaluate(() => ({
+    samples: window.labelsColorBySamples as LabelsColorBySamples,
+    deckErrors: window.labelsColorByDeckErrors,
+    frames: window.labelsColorByRenderFrames,
+  }));
+
+  expect(runtime.deckErrors).toEqual([]);
+  expect(runtime.frames).toBeGreaterThan(0);
+
+  // The regression this pins: both bands come back in the CHANNEL colour when the
+  // feature LUT does not reach the shader, which is indistinguishable from
+  // "colour-by does nothing" in the app.
+  expect(
+    describeColor(runtime.samples.label1),
+    'label 1 drew in the channel colour — feature colouring did not reach the shader'
+  ).not.toBe(describeColor([...CHANNEL_COLOR, 255]));
+
+  expectColor(runtime.samples.label1, LABEL_1_COLOR, 'label 1');
+  expectColor(runtime.samples.label2, LABEL_2_COLOR, 'label 2');
+
+  expect(pageErrors).toEqual([]);
+  expect(consoleErrors).toEqual([]);
+});

tests/production/browser/labelsColorByContract.ts

@@ -0,0 +1,20 @@
+/**
+ * The colours and readback shape shared by the labels colour-by scenario and its
+ * spec.
+ *
+ * Deliberately free of any `@spatialdata/*` import: Playwright loads the spec in
+ * Node, and the scenario module pulls in the browser-only built layers bundle, so
+ * a spec that imported the scenario directly would fail to collect at all.
+ */
+
+export const LABEL_1_COLOR = [255, 0, 0, 255] as const;
+export const LABEL_2_COLOR = [0, 128, 255, 255] as const;
+/** Deliberately neither feature colour, so a fallback to it is unmistakable. */
+export const CHANNEL_COLOR = [255, 255, 255] as const;
+
+export type SampledPixel = [number, number, number, number];
+
+export interface LabelsColorBySamples {
+  label1: SampledPixel;
+  label2: SampledPixel;
+}

tests/production/browser/labelsColorByScenario.tsx

@@ -0,0 +1,169 @@
+import { Deck, OrthographicView } from '@deck.gl/core';
+import { type LabelFeatureState, LabelsLayer } from '@spatialdata/layers';
+import { useEffect, useRef } from 'react';
+import {
+  CHANNEL_COLOR,
+  LABEL_1_COLOR,
+  LABEL_2_COLOR,
+  type LabelsColorBySamples,
+  type SampledPixel,
+} from './labelsColorByContract';
+
+/**
+ * Labels feature colouring, read back off the GPU.
+ *
+ * The raster is synthetic rather than a fixture: what is under test is the path
+ * from `featureState` to the colour a fragment ends up with — the LUT build, its
+ * texture upload, the props reaching the bitmask sublayer, and the shader's
+ * `useFeatureColors` branch. A real store adds loading, tiling and coordinate
+ * transforms in front of all of that, none of which can fail in a way this
+ * scenario would attribute correctly.
+ *
+ * Everything is arranged so the expected pixel is EXACTLY the feature colour:
+ * full channel opacity, filled, and zero stroke width (which short-circuits the
+ * outline mask, whose colour is mixed toward white). A pixel that comes back as
+ * the channel colour instead means feature colouring did not reach the shader.
+ */
+
+const RASTER_SIZE = 64;
+const CANVAS_SIZE = 512;
+
+/** Left half is label 1, right half is label 2; label 0 (background) is never drawn. */
+function buildSyntheticLabels(): Uint32Array {
+  const data = new Uint32Array(RASTER_SIZE * RASTER_SIZE);
+  for (let y = 0; y < RASTER_SIZE; y += 1) {
+    for (let x = 0; x < RASTER_SIZE; x += 1) {
+      data[y * RASTER_SIZE + x] = x < RASTER_SIZE / 2 ? 1 : 2;
+    }
+  }
+  return data;
+}
+
+// Hoisted so their identity is stable across the forced re-renders below: the
+// layer memoises its LUT by `featureState` identity, and a fresh object every
+// frame would rebuild and re-upload the table instead of exercising the steady
+// state this scenario is about.
+const syntheticRaster = {
+  data: buildSyntheticLabels(),
+  width: RASTER_SIZE,
+  height: RASTER_SIZE,
+};
+
+/** The single-scale labels path asks its loader for exactly this. */
+const syntheticLoader = {
+  getRaster: async () => syntheticRaster,
+};
+
+const featureState: LabelFeatureState = {
+  fillColorByFeatureId: {
+    '1': [...LABEL_1_COLOR],
+    '2': [...LABEL_2_COLOR],
+  },
+};
+
+/** Band centres, far enough from the label boundary to be unambiguous interior. */
+const samplePoints = {
+  label1: [CANVAS_SIZE * 0.25, CANVAS_SIZE * 0.5],
+  label2: [CANVAS_SIZE * 0.75, CANVAS_SIZE * 0.5],
+} as const;
+
+declare global {
+  interface Window {
+    labelsColorByDeckErrors: string[];
+    labelsColorByRenderFrames: number;
+    labelsColorBySamples: LabelsColorBySamples | null;
+  }
+}
+
+window.labelsColorByDeckErrors = [];
+window.labelsColorByRenderFrames = 0;
+window.labelsColorBySamples = null;
+
+/**
+ * Sample the drawing buffer.
+ *
+ * Called from `onAfterRender`, which is the only point at which this is possible
+ * without `preserveDrawingBuffer`: the WebGL back buffer is still readable inside
+ * the frame that drew it, and is discarded once control returns to the browser.
+ */
+function sampleCanvas(canvas: HTMLCanvasElement): LabelsColorBySamples | null {
+  const readback = document.createElement('canvas');
+  readback.width = canvas.width;
+  readback.height = canvas.height;
+  const context = readback.getContext('2d', { willReadFrequently: true });
+  if (!context) return null;
+  context.drawImage(canvas, 0, 0);
+  const at = ([x, y]: readonly [number, number]): SampledPixel => {
+    const { data } = context.getImageData(Math.round(x), Math.round(y), 1, 1);
+    return [data[0], data[1], data[2], data[3]];
+  };
+  return { label1: at(samplePoints.label1), label2: at(samplePoints.label2) };
+}
+
+function buildLayer() {
+  return new LabelsLayer({
+    id: 'labels:synthetic',
+    loader: syntheticLoader,
+    selections: [{}],
+    visible: true,
+    opacity: 1,
+    channelColors: [[...CHANNEL_COLOR] as [number, number, number]],
+    channelsVisible: [true],
+    // Full fill opacity and no outline: the sampled pixel is then the feature
+    // colour itself rather than something blended with the channel colour.
+    channelOpacities: [1],
+    channelOutlineOpacities: [1],
+    channelsFilled: [true],
+    channelStrokeWidths: [0],
+    featureState,
+  });
+}
+
+export function LabelsColorByConsumer() {
+  const container = useRef<HTMLDivElement>(null);
+
+  useEffect(() => {
+    if (!container.current) return;
+
+    const canvas = document.createElement('canvas');
+    canvas.width = CANVAS_SIZE;
+    canvas.height = CANVAS_SIZE;
+    canvas.style.width = `${CANVAS_SIZE}px`;
+    canvas.style.height = `${CANVAS_SIZE}px`;
+    container.current.appendChild(canvas);
+
+    const deck = new Deck({
+      canvas,
+      width: CANVAS_SIZE,
+      height: CANVAS_SIZE,
+      // Keep drawing-buffer pixels and CSS pixels one to one, so the sample
+      // coordinates above are the ones actually read.
+      useDevicePixels: false,
+      views: new OrthographicView({ id: 'labels' }),
+      // zoom 3 scales the 64-unit raster to the full 512px canvas.
+      initialViewState: { target: [RASTER_SIZE / 2, RASTER_SIZE / 2, 0], zoom: 3 },
+      controller: false,
+      layers: [buildLayer()],
+      onAfterRender: () => {
+        window.labelsColorByRenderFrames += 1;
+        window.labelsColorBySamples = sampleCanvas(canvas);
+      },
+      onError: (error) => {
+        window.labelsColorByDeckErrors.push(error.message);
+        console.error(`Labels colour-by deck error: ${error.message}`);
+      },
+    });
+
+    // The raster arrives asynchronously and deck only draws when it has a reason
+    // to. Nudging it keeps frames coming after the load settles, so the sample
+    // above is taken from a steady frame rather than whichever one happened last.
+    const interval = window.setInterval(() => deck.setProps({ layers: [buildLayer()] }), 100);
+
+    return () => {
+      window.clearInterval(interval);
+      deck.finalize();
+    };
+  }, []);
+
+  return <div ref={container} data-testid="labels-ready" />;
+}