From 2d30ae50b1a1a38559631de2b6a16ddf59a7b33e Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 9 Nov 2026 17:06:00 +0100 Subject: [PATCH 1/2] Replace ImageMagick with Chrome Render screenshot contact sheets with the existing Chrome DevTools harness and remove ImageMagick setup from scripts, CI, and docs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 6 - .github/workflows/copilot-setup-steps.yml | 11 - .github/workflows/windows-verify.yml | 15 -- README.md | 2 +- docs/code.md | 16 +- scripts/shot-sheet.mjs | 241 +++++++++++++--------- scripts/shot.mjs | 2 - scripts/tools.mjs | 49 ----- tests/platform-tools.test.mjs | 47 ----- tests/shot-sheet.test.mjs | 24 ++- 10 files changed, 168 insertions(+), 245 deletions(-) delete mode 100644 scripts/tools.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19a4abb..0828992 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,12 +31,6 @@ jobs: node-version: 22 cache: npm - - name: Install screenshot tools - run: | - set -euo pipefail - sudo apt-get update - sudo apt-get install -y --no-install-recommends imagemagick fonts-dejavu-core - - name: Resolve Chrome run: | set -euo pipefail diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index adeeb2c..0913ee3 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -31,12 +31,6 @@ jobs: - name: Install dependencies run: npm ci - - name: Install screenshot tools - run: | - set -euo pipefail - sudo apt-get update - sudo apt-get install -y --no-install-recommends imagemagick fonts-dejavu-core - - name: Resolve Chrome run: | set -euo pipefail @@ -55,9 +49,4 @@ jobs: run: | set -euo pipefail node --version - if command -v magick >/dev/null 2>&1; then - magick --version | head -n 1 - else - convert --version | head -n 1 - fi "$CHROME_BIN" --version diff --git a/.github/workflows/windows-verify.yml b/.github/workflows/windows-verify.yml index 9832107..8b68375 100644 --- a/.github/workflows/windows-verify.yml +++ b/.github/workflows/windows-verify.yml @@ -37,21 +37,6 @@ jobs: } & $chrome --version - - name: Install ImageMagick - shell: pwsh - run: | - if (-not (Get-Command magick.exe -ErrorAction SilentlyContinue)) { - choco install imagemagick --yes --no-progress - $magick = Get-ChildItem $env:ProgramFiles -Directory -Filter 'ImageMagick*' | - Select-Object -First 1 - if (-not $magick) { - throw 'ImageMagick installed but magick.exe was not found.' - } - $env:PATH = "$($magick.FullName);$env:PATH" - $magick.FullName | Out-File -FilePath $env:GITHUB_PATH -Append -Encoding utf8 - } - magick -version - - name: Install dependencies run: npm ci diff --git a/README.md b/README.md index aacdbbf..88a0176 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ application services to set up. Run `npm run verify` to check the project, or `npm run build` to make its publishable files. Full verification works on Windows and Linux and needs -Chrome or Chromium plus ImageMagick; setup details are in the +Chrome or Chromium; setup details are in the [code guide](docs/code.md#local-and-ci-environments). GitHub also runs the checks for you when you make a change. The normal checks run on Linux even if you are working on Windows; maintainers can manually run Windows verification to diff --git a/docs/code.md b/docs/code.md index 433a0f8..8a61b9c 100644 --- a/docs/code.md +++ b/docs/code.md @@ -22,23 +22,21 @@ second catalogue here. ## Local and CI environments The project commands work from Windows PowerShell or a Linux shell. Full local -verification needs Chrome and ImageMagick on Windows, or Chromium and -ImageMagick on Linux. GitHub Actions runs the same verification on Ubuntu. +verification needs Chrome on Windows or Chromium on Linux. GitHub Actions runs +the same verification on Ubuntu. -On Debian or Ubuntu, install the screenshot tools with: +On Debian or Ubuntu, install Chromium with: ```sh -sudo apt-get install chromium imagemagick +sudo apt-get install chromium ``` -On Windows, install Google Chrome and ImageMagick, and make sure ImageMagick's -`magick.exe` is on `PATH`. Chrome is found in its standard installation -folders; set these PowerShell variables if either tool is installed elsewhere: -replace the example paths with the actual locations on your computer. +On Windows, install Google Chrome. Chrome is found in its standard installation +folders; set this PowerShell variable if it is installed elsewhere. Replace +the example path with its actual location on your computer. ```powershell $env:CHROME_BIN = 'C:\Program Files\Google\Chrome\Application\chrome.exe' -$env:IMAGEMAGICK_BIN = 'C:\Program Files\ImageMagick\magick.exe' npm run verify ``` diff --git a/scripts/shot-sheet.mjs b/scripts/shot-sheet.mjs index 78fc5c9..e93e6d6 100644 --- a/scripts/shot-sheet.mjs +++ b/scripts/shot-sheet.mjs @@ -12,11 +12,15 @@ * nothing in CI can do that looking on the reviewer's behalf - see * docs/decisions/Let the author attach the screenshots.md. */ -import { existsSync, readdirSync, rmSync } from "node:fs"; +import { createServer } from "node:http"; +import { existsSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import { rm } from "node:fs/promises"; +import { setTimeout as sleep } from "node:timers/promises"; -import { magick, haveImageMagick } from "./tools.mjs"; +import { openChrome } from "./chrome.mjs"; +import { freeDebugPort, listenOnFreePort } from "./concurrency.mjs"; const root = fileURLToPath(new URL("..", import.meta.url)); const shotsDir = join(root, ".art/shots"); @@ -25,132 +29,163 @@ const SHEET = "contact-sheet.png"; const CELL_HEIGHT = 300; const COLUMNS = 4; +function escapeHtml(text) { + return text + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">") + .replaceAll('"', """) + .replaceAll("'", "'"); +} + /** - * Whether ImageMagick can render text. Ubuntu's imagemagick package installed - * without recommends has no font, and `label:` then fails with "unable to read - * font (null)" - which is how the GitHub runner is set up. Probing once beats - * naming a font, since which fonts exist is exactly what varies. + * The browser handles sizing, labels, and image composition in one render. + * Grouping rows explicitly keeps the prior four-column, left-to-right order. */ -let labelsWork; -function canLabel() { - if (labelsWork === undefined) { - const probe = join(shotsDir, "_probe.png"); +export function createSheetHtml(shots) { + const rows = []; + const sortedShots = [...shots].sort(); + for (let i = 0; i < sortedShots.length; i += COLUMNS) { + const cells = sortedShots + .slice(i, i + COLUMNS) + .map((file) => { + const label = escapeHtml(file.replace(/\.png$/, "")); + return `
${label}
${label}
`; + }) + .join(""); + rows.push(`
${cells}
`); + } + return ` + + +Screenshot contact sheet + +
${rows.join("\n")}
+`; +} + +function serveSheet(shots, html) { + const files = new Set(shots); + return createServer((request, response) => { + let pathname; try { - magick(["-pointsize", "18", "label:probe", probe]); - labelsWork = true; + pathname = new URL(request.url, "http://localhost").pathname; } catch { - labelsWork = false; - console.warn("ImageMagick has no font available; the contact sheet will not be labelled."); + response.writeHead(400).end("invalid URL"); + return; + } + + if (pathname === "/") { + response.writeHead(200, { "content-type": "text/html; charset=utf-8" }).end(html); + return; + } + + let filename; + try { + filename = decodeURIComponent(pathname.slice("/shots/".length)); + } catch { + response.writeHead(400).end("invalid filename"); + return; + } + if (!pathname.startsWith("/shots/") || !files.has(filename)) { + response.writeHead(404).end("not found"); + return; + } + + response + .writeHead(200, { "content-type": "image/png", "cache-control": "no-store" }) + .end(readFileSync(join(shotsDir, filename))); + }); +} + +async function waitForImages(browser, expectedCount) { + const deadline = Date.now() + 10000; + for (;;) { + if ( + await browser.evaluate( + `document.readyState === 'complete' && document.images.length === ${expectedCount} && [...document.images].every(image => image.complete && image.naturalWidth > 0)`, + ) + ) { + await browser.evaluate("document.fonts.ready.then(() => true)"); + return; + } + if (Date.now() >= deadline) { + throw new Error("Chrome timed out loading the contact sheet images."); } - rmSync(probe, { force: true }); + await sleep(50); } - return labelsWork; } -/** - * Builds the sheet and returns its path, or null if there was nothing to pack - * or ImageMagick is not installed. Throws if ImageMagick fails. - * - * Falls back to an unlabelled sheet where text cannot be rendered: the pictures - * are the point, and they stay in filename order, left to right and top to - * bottom. - */ -export function buildSheet() { - if (!existsSync(shotsDir) || !haveImageMagick()) return null; +export async function buildSheet() { + if (!existsSync(shotsDir)) return null; const shots = readdirSync(shotsDir) - .filter((f) => f.endsWith(".png") && f !== SHEET) + .filter((file) => file.endsWith(".png") && file !== SHEET) .sort(); if (shots.length === 0) return null; - const temporaries = []; + const server = serveSheet(shots, createSheetHtml(shots)); + const profile = mkdtempSync(join(root, ".art/chrome-profile-sheet-")); + let browser; + let serverStarted = false; try { - const label = canLabel(); - const cells = shots.map((file) => { - const resized = join(shotsDir, `_fit-${file}`); - const cell = join(shotsDir, `_cell-${file}`); - temporaries.push(resized, cell); - // Portrait and landscape shots come out of the run at different sizes, so - // normalise on height and let the widths differ. The resize has to happen - // before the label joins the list, or the label is scaled to 300px tall too. - magick([ - join(shotsDir, file), - "-resize", - `x${CELL_HEIGHT}`, - "-depth", - "8", - "-strip", - resized, - ]); - magick([ - "-background", - "white", - ...(label - ? ["-fill", "#333", "-pointsize", "18", `label:${file.replace(/\.png$/, "")}`] - : []), - resized, - "-gravity", - "center", - "-append", - "-bordercolor", - "white", - "-border", - "8", - cell, - ]); - return cell; + const port = await listenOnFreePort(server); + serverStarted = true; + browser = await openChrome({ debugPort: freeDebugPort(), profileDir: profile }); + await browser.send("Page.enable"); + const navigation = await browser.send("Page.navigate", { + url: `http://127.0.0.1:${port}/`, }); + if (navigation.errorText) throw new Error(navigation.errorText); + await waitForImages(browser, shots.length); - const rows = []; - for (let i = 0; i < cells.length; i += COLUMNS) { - const row = join(shotsDir, `_row-${rows.length}.png`); - temporaries.push(row); - magick([ - ...cells.slice(i, i + COLUMNS), - "-background", - "white", - "-gravity", - "south", - "+append", - row, - ]); - rows.push(row); - } - + const { width, height } = await browser.evaluate( + "(() => { const { width, height } = document.querySelector('.sheet').getBoundingClientRect(); return { width: Math.ceil(width), height: Math.ceil(height) }; })()", + ); + await browser.send("Emulation.setDeviceMetricsOverride", { + width, + height, + deviceScaleFactor: 1, + mobile: false, + }); + const { data } = await browser.send("Page.captureScreenshot", { + format: "png", + captureBeyondViewport: true, + clip: { x: 0, y: 0, width, height, scale: 1 }, + }); const sheet = join(shotsDir, SHEET); - magick([ - ...rows, - "-background", - "white", - "-gravity", - "west", - "-append", - "-depth", - "8", - "-strip", - sheet, - ]); + writeFileSync(sheet, Buffer.from(data, "base64")); return sheet; } finally { - // Cleared even when a step failed, or the leftovers are picked up as - // screenshots by the next run and uploaded as if they were evidence. - for (const file of temporaries) rmSync(file, { force: true }); + try { + await browser?.close(); + } finally { + try { + if (serverStarted) { + server.closeAllConnections(); + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + } + } finally { + await rm(profile, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + } + } } } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - if (!haveImageMagick()) { - console.error( - "Cannot find ImageMagick.\n" + - "On Debian or Ubuntu: sudo apt-get install imagemagick.\n" + - "On Windows, install ImageMagick and make sure magick.exe is on PATH.", - ); - process.exit(1); - } let sheet; try { - sheet = buildSheet(); + sheet = await buildSheet(); } catch (error) { - console.error(`ImageMagick failed: ${error.message.split("\n")[0]}`); + console.error(`Chrome could not build the contact sheet: ${error.message.split("\n")[0]}`); process.exit(1); } if (!sheet) { diff --git a/scripts/shot.mjs b/scripts/shot.mjs index 35c8d63..0d5c8f2 100644 --- a/scripts/shot.mjs +++ b/scripts/shot.mjs @@ -22,7 +22,6 @@ import { setTimeout as sleep } from "node:timers/promises"; import { openChrome } from "./chrome.mjs"; import { browserSlots, cpuSlots, freeDebugPort, listenOnFreePort } from "./concurrency.mjs"; import { buildSheet } from "./shot-sheet.mjs"; -import { requireImageMagick } from "./tools.mjs"; const root = fileURLToPath(new URL("..", import.meta.url)); const dist = join(root, "dist"); @@ -55,7 +54,6 @@ export function preflight() { "Run npm run build.", ); } - requireImageMagick(); } const MIME = { diff --git a/scripts/tools.mjs b/scripts/tools.mjs deleted file mode 100644 index ed1a6c8..0000000 --- a/scripts/tools.mjs +++ /dev/null @@ -1,49 +0,0 @@ -/** Resolve ImageMagick 6/7 for the screenshot contact sheet. */ -import { execFileSync } from "node:child_process"; - -function isImageMagick(binary) { - try { - const version = execFileSync(binary, ["-version"], { encoding: "utf8" }); - return /ImageMagick/i.test(version); - } catch { - return false; - } -} - -export function resolveImageMagick({ - platform = process.platform, - env = process.env, - supports = isImageMagick, -} = {}) { - if (env.IMAGEMAGICK_BIN) return env.IMAGEMAGICK_BIN; - - const candidates = - platform === "win32" - ? ["magick.exe", "magick", "convert.exe", "convert"] - : ["magick", "convert"]; - return candidates.find(supports) ?? null; -} - -let binary; -function imagemagick() { - if (binary === undefined) { - binary = resolveImageMagick(); - } - return binary; -} - -export const haveImageMagick = () => imagemagick() !== null; - -export function requireImageMagick() { - if (!haveImageMagick()) { - throw new Error( - "Cannot find ImageMagick. On Debian or Ubuntu: sudo apt-get install imagemagick. " + - "On Windows, install ImageMagick and make sure magick.exe is on PATH, or set IMAGEMAGICK_BIN.", - ); - } -} - -export function magick(args) { - requireImageMagick(); - return execFileSync(imagemagick(), args); -} diff --git a/tests/platform-tools.test.mjs b/tests/platform-tools.test.mjs index 2c83a39..a29b5ff 100644 --- a/tests/platform-tools.test.mjs +++ b/tests/platform-tools.test.mjs @@ -1,7 +1,6 @@ import { describe, expect, it } from "vitest"; import { resolveChromeExecutable } from "../scripts/chrome.mjs"; -import { resolveImageMagick } from "../scripts/tools.mjs"; describe("browser executable discovery", () => { it("uses the configured browser path on either platform", () => { @@ -38,49 +37,3 @@ describe("browser executable discovery", () => { ); }); }); - -describe("ImageMagick discovery", () => { - it("checks Windows executable names and selects the first working command", () => { - const tried = []; - - const binary = resolveImageMagick({ - platform: "win32", - env: {}, - supports: (candidate) => { - tried.push(candidate); - return candidate === "magick"; - }, - }); - - expect(tried).toEqual(["magick.exe", "magick"]); - expect(binary).toBe("magick"); - }); - - it("checks Linux commands without requiring a shell", () => { - const tried = []; - - const binary = resolveImageMagick({ - platform: "linux", - env: {}, - supports: (candidate) => { - tried.push(candidate); - return candidate === "convert"; - }, - }); - - expect(tried).toEqual(["magick", "convert"]); - expect(binary).toBe("convert"); - }); - - it("honors an explicit ImageMagick path", () => { - expect( - resolveImageMagick({ - platform: "win32", - env: { IMAGEMAGICK_BIN: "C:\\ImageMagick\\magick.exe" }, - supports: () => { - throw new Error("should not probe the PATH"); - }, - }), - ).toBe("C:\\ImageMagick\\magick.exe"); - }); -}); diff --git a/tests/shot-sheet.test.mjs b/tests/shot-sheet.test.mjs index 33f529d..b0c032c 100644 --- a/tests/shot-sheet.test.mjs +++ b/tests/shot-sheet.test.mjs @@ -5,6 +5,7 @@ import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; +import { createSheetHtml } from "../scripts/shot-sheet.mjs"; describe("screenshot sheet command", () => { it("runs its entrypoint from a path with spaces and reports missing screenshots", () => { @@ -12,7 +13,7 @@ describe("screenshot sheet command", () => { const scripts = join(root, "scripts"); try { mkdirSync(scripts); - for (const name of ["shot-sheet.mjs", "tools.mjs"]) { + for (const name of ["shot-sheet.mjs", "chrome.mjs", "concurrency.mjs"]) { copyFileSync( fileURLToPath(new URL(`../scripts/${name}`, import.meta.url)), join(scripts, name), @@ -20,7 +21,6 @@ describe("screenshot sheet command", () => { } const result = spawnSync(process.execPath, [join(scripts, "shot-sheet.mjs")], { - env: { ...process.env, IMAGEMAGICK_BIN: process.execPath }, encoding: "utf8", }); @@ -31,4 +31,24 @@ describe("screenshot sheet command", () => { rmSync(root, { recursive: true, force: true }); } }); + + it("lays out sorted screenshots in four columns with escaped filename labels", () => { + const html = createSheetHtml([ + "extra.png", + 'root-wide"<&.png', + "subpath-wide.png", + "subpath-narrow.png", + "root-narrow.png", + ]); + + expect(html.match(/class="row"/g)).toHaveLength(2); + expect(html.indexOf("extra.png")).toBeLessThan(html.indexOf("root-narrow.png")); + expect(html).toContain("root-wide"<&"); + expect(html.indexOf("subpath-narrow.png")).toBeLessThan(html.indexOf("subpath-wide.png")); + expect(html.match(/
/g)).toHaveLength(5); + expect(html).toContain("height: 300px"); + expect(html).toContain("font: 18px sans-serif"); + expect(html).toContain("align-items: flex-end"); + expect(html).toContain('
'); + }); }); From e179294be428f592e171e9e6b6b6cbb032c8702e Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 9 Nov 2026 17:18:37 +0100 Subject: [PATCH 2/2] Await contact sheet generation Wait for the asynchronous sheet builder before asserting its result so screenshot failures reach the main error handler. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- scripts/shot.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/shot.mjs b/scripts/shot.mjs index 0d5c8f2..f156ec6 100644 --- a/scripts/shot.mjs +++ b/scripts/shot.mjs @@ -220,7 +220,7 @@ async function main() { ); const failures = results.filter((result) => result.status === "rejected"); if (failures.length) throw new AggregateError(failures.map((result) => result.reason)); - assert.ok(buildSheet(), "The screenshot contact sheet was not generated."); + assert.ok(await buildSheet(), "The screenshot contact sheet was not generated."); } finally { server.closeAllConnections(); await new Promise((resolve, reject) =>