From 88ec367ab0ae5060dbe761cf10545b4d1be1d925 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 9 Sep 2026 15:55:15 +0200 Subject: [PATCH] Offer the pictures the server already holds when adding a texture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "+ → Texture" opened the machine's file dialog, so a picture that was uploaded once but lost its asset – an undone import, a deleted asset – was unreachable from the editor while still sitting on the server. Nothing collects those files, by design. It now opens a grid of every picture the server holds, with thumbnails: pick one and "Use image" makes a texture asset on that file, "Upload…" is the way in for bytes the server does not have yet and lands in the same import as a drop. A tile captioned with an asset name is one this project already wears – picking it selects that asset instead of making a second one, since two assets on one file would fight over `filter`. - `GET ?r=textures` now reports `width`/`height`; a texture asset holds the pixel size, and reading it off the GPU texture would tie the asset's correctness to load timing. - `TextureLibrary.load(file)` loads without touching `scaleMode` – a thumbnail must not restage the filter of a picture the scene is drawing with. - The grid geometry lives in `texture/imageGrid.ts` as pure functions; inside the widget no test could reach it. - The part after an upload is now one function shared by the drop import and the picker, so "one file = one asset" is stated in a single place. 12 new E2E checks; the strongest one undoes an import, leaving the file on the server with no asset naming it, and picks exactly that file back out of the dialog. --- README.md | 21 +- api/index.php | 2 +- api/lib/TextureStore.php | 19 +- src/core/store/TextureStore.ts | 4 + src/editor/debugHook.ts | 4 + src/editor/panels/AssetPanel.ts | 8 +- src/editor/texture/ImagePicker.ts | 294 ++++++++++++++++++++ src/editor/texture/imageGrid.ts | 68 +++++ src/editor/textureImport.ts | 68 +++-- src/viewport/TextureLibrary.ts | 21 +- tests/core/http-store.test.ts | 6 +- tests/editor/image-grid.test.ts | 91 ++++++ tests/editor/texture-import.test.ts | 44 +++ todo-notes/done/unreferenced-image-files.md | 40 +++ todo-notes/unreferenced-image-files.md | 14 - tools/e2e/12-textures.mjs | 126 +++++++++ 16 files changed, 774 insertions(+), 56 deletions(-) create mode 100644 src/editor/texture/ImagePicker.ts create mode 100644 src/editor/texture/imageGrid.ts create mode 100644 tests/editor/image-grid.test.ts create mode 100644 todo-notes/done/unreferenced-image-files.md delete mode 100644 todo-notes/unreferenced-image-files.md diff --git a/README.md b/README.md index a869f2b2..0c38e740 100644 --- a/README.md +++ b/README.md @@ -215,9 +215,17 @@ design: because it is an ordinary property, instance overrides, **Apply to conta One coin instance can wear a different picture than its nine siblings, and swapping a texture at runtime is a graph node that already exists. -**Getting an image in.** Three ways, all ending in the same import: **+ → Texture** in the -asset panel, **dropping image files onto the canvas**, or **Ctrl+V** pasting one. Several files -at once are imported one after another; a file that fails does not stop the rest. +**Getting an image in.** **Dropping image files onto the canvas** or **Ctrl+V** pasting one +uploads it and makes the asset in one go; several files at once are imported one after another, +and a file that fails does not stop the rest. + +**+ → Texture** asks a different question and so opens a different dialog: not the machine's +file system, but **every picture the server already holds**, as a grid of thumbnails. Files are +never collected (below), so a picture one project dropped is still there for the next to pick +up, and uploading the same bytes twice buys nothing. Picking one and pressing **Use image** +makes a texture asset on that file; **Upload…** is the way in for a picture the server does not +have yet, and lands in the same import as a drop. A tile captioned with an asset name is one +this project already wears – picking it selects that asset rather than making a second one. **Where it goes.** The image is uploaded as raw bytes to `POST ?r=textures` and stored as `data/textures/{sha256}.{ext}` — the file is named after its own content. That name buys four @@ -440,7 +448,7 @@ npm run format # Prettier over src, tests and tools npm run check # typecheck + lint + format:check + tests, what CI would run npm run build # writes dist/ and regenerates index.html export GM_REMOTE=user@host # where the browser runs; see below -node tools/seed-e2e.mjs && node tools/e2e.mjs # 318 editor checks, screenshots in /tmp/gm-e2e-*.png +node tools/seed-e2e.mjs && node tools/e2e.mjs # 330 editor checks, screenshots in /tmp/gm-e2e-*.png node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs # 24 checks: the example game, played end to end ``` @@ -516,7 +524,7 @@ All routes go through `api/index.php?r=…` (the site's nginx does not pass `PAT | GET | `projects/{id}` | `{id,name,revision,updatedAt,project}` | | PUT | `projects/{id}` | `{revision, project}` → `{revision,updatedAt}` or **409** `{revision}` when stale | | DELETE | `projects/{id}` | 204 | -| GET | `textures` | `[{file,bytes,updatedAt}]` | +| GET | `textures` | `[{file,width,height,bytes,updatedAt}]` – what the image picker offers | | POST | `textures` | **raw image bytes** → `{file,width,height,bytes}` (201) | | DELETE | `textures/{file}` | 204 | @@ -545,4 +553,5 @@ Several scenes in one run (a `Load scene` node – screens are containers today), sound, auth, touch-optimised layout, graph comments/groups. A frame is always stretched to the sprite's `width` × `height`: no animation (nothing steps through a sheet's frames yet) and no nine-slice. Slicing cuts a regular grid only — no detection of where the sprites actually -sit on the sheet. Unreferenced image files are never collected. +sit on the sheet. Unreferenced image files are never collected – deliberately, since the image +picker offers them again; nothing deletes a stored image except `DELETE ?r=textures/{file}`. diff --git a/api/index.php b/api/index.php index 9cbbc6b2..211acfd3 100644 --- a/api/index.php +++ b/api/index.php @@ -13,7 +13,7 @@ declare(strict_types=1); * PUT ?r=projects/{id} {revision, project} → {revision,updatedAt} | 409 {error,revision} * DELETE ?r=projects/{id} → 204 * - * GET ?r=textures → [{file,bytes,updatedAt}] + * GET ?r=textures → [{file,width,height,bytes,updatedAt}] * POST ?r=textures raw image bytes → {file,width,height,bytes} * DELETE ?r=textures/{file} → 204 * diff --git a/api/lib/TextureStore.php b/api/lib/TextureStore.php index e88d861c..1c96c57b 100644 --- a/api/lib/TextureStore.php +++ b/api/lib/TextureStore.php @@ -10,8 +10,9 @@ declare(strict_types=1); * of embedding megabytes of base64, and deleting a texture asset is undoable for free * because nothing here is removed when it stops being referenced. * - * The flip side is that files outlive the projects that used them; `tools/gc-textures.mjs` - * collects the unreferenced ones. + * The flip side is that files outlive the projects that used them: nothing collects the + * unreferenced ones. They stay pickable instead – `GET ?r=textures` is what the editor's + * image picker offers, so a picture one project dropped is still there for the next. */ final class TextureStore { @@ -91,8 +92,15 @@ final class TextureStore return ['file' => $file, 'width' => $width, 'height' => $height, 'bytes' => strlen($bytes)]; } - /** Every stored file, newest first – the input of the garbage collector. */ - /** @return list */ + /** + * Every stored file, newest first – what the image picker offers. + * + * The pixel size is read from each header rather than left to the client: a texture asset + * records the size, so a picker that only knew the file name could not create one without + * decoding the picture first. An unreadable file reports 0 × 0 and the picker skips it. + * + * @return list + */ public function list(): array { $items = []; @@ -101,8 +109,11 @@ final class TextureStore if (!self::isValidName($file)) { continue; } + $size = @getimagesize($path); $items[] = [ 'file' => $file, + 'width' => (int) ($size[0] ?? 0), + 'height' => (int) ($size[1] ?? 0), 'bytes' => (int) filesize($path), 'updatedAt' => gmdate('c', (int) filemtime($path)), ]; diff --git a/src/core/store/TextureStore.ts b/src/core/store/TextureStore.ts index 8790e5cc..d62e3168 100644 --- a/src/core/store/TextureStore.ts +++ b/src/core/store/TextureStore.ts @@ -6,8 +6,12 @@ export interface UploadedTexture { bytes: number; } +/** A file the server holds, whether any project references it or not. */ export interface StoredTexture { file: string; + /** 0 × 0 when the server could not read the header; such a file cannot become an asset. */ + width: number; + height: number; bytes: number; updatedAt: string; } diff --git a/src/editor/debugHook.ts b/src/editor/debugHook.ts index 8eb23bd9..f8495a72 100644 --- a/src/editor/debugHook.ts +++ b/src/editor/debugHook.ts @@ -4,6 +4,7 @@ import { findObjectHolder } from '../core/model/Project'; import { Button, ui, Widget } from '../ui'; import { textureLibrary } from '../viewport/TextureLibrary'; import type { CentreTab, Editor } from './Editor'; +import { currentImagePicker } from './texture/ImagePicker'; import type { EditorState } from './state'; /** @@ -168,6 +169,9 @@ export function installDebugHook(app: Application, state: EditorState, editor: E center: () => centreOf(graphEditor.canvas), pickerWidth: () => graphEditor.pickerWidth, }, + /** The open image picker: what it offers, what is picked, and where a tile sits. */ + imagePicker: () => currentImagePicker()?.debugInfo() ?? null, + imagePickerTile: (file: string) => currentImagePicker()?.tileGlobal(file) ?? null, texture: { current: () => textureEditor.currentAssetId, pickerWidth: () => textureEditor.pickerWidth, diff --git a/src/editor/panels/AssetPanel.ts b/src/editor/panels/AssetPanel.ts index 339b339e..66095be0 100644 --- a/src/editor/panels/AssetPanel.ts +++ b/src/editor/panels/AssetPanel.ts @@ -13,7 +13,7 @@ import type { TextureStore } from '../../core/store/TextureStore'; import { Button, ListView, Menu, type MenuItem, Panel, ScrollContainer, startDrag } from '../../ui'; import { confirmDeleteAsset } from '../actions'; import type { EditorState } from '../state'; -import { pickAndImport } from '../textureImport'; +import { openImagePicker } from '../texture/ImagePicker'; /** * Lists all assets of the project; creates and deletes them. Placing is a drag into a view. @@ -83,15 +83,15 @@ export class AssetPanel extends Panel { } /** - * A texture is an uploaded picture, so its menu entry opens the file dialog instead of - * creating an empty asset. + * A texture is a picture, so its menu entry offers the ones the server already holds instead of + * creating an empty asset. Uploading a new one is a button inside that dialog. */ private importTextures(): void { if (!this.textures) { this.state.setNotice('Image upload is not available.'); return; } - pickAndImport({ state: this.state, textures: this.textures }); + openImagePicker({ state: this.state, textures: this.textures }); } /** Double-click / Enter on a scene makes it the active one. Placing is a drag, not a click. */ diff --git a/src/editor/texture/ImagePicker.ts b/src/editor/texture/ImagePicker.ts new file mode 100644 index 00000000..88e6fe0f --- /dev/null +++ b/src/editor/texture/ImagePicker.ts @@ -0,0 +1,294 @@ +import { type FederatedPointerEvent, Graphics, Sprite, Text } from 'pixi.js'; +import type { StoredTexture } from '../../core/store/TextureStore'; +import { + Button, + drawControlBox, + HStack, + Label, + Popup, + ScrollContainer, + type Size, + Spacer, + Theme, + textStyle, + truncate, + Widget, +} from '../../ui'; +import { pressCount } from '../../ui/clicks'; +import { textureLibrary } from '../../viewport/TextureLibrary'; +import { importStoredImage, pickAndImport, type TextureImportDeps } from '../textureImport'; +import { CAPTION_HEIGHT, containRect, gridMetrics, type GridMetrics, tileAt, tileRect } from './imageGrid'; + +/** The dialog keeps this size whatever the list turns out to be, so it does not jump on opening. */ +const GRID_WIDTH = 560; +const GRID_HEIGHT = 340; +/** Inset of the picture inside its tile, so the selection frame stays visible around it. */ +const TILE_PAD = 4; + +/** One offered picture, with the asset that already wears it (if any). */ +interface PickerItem extends StoredTexture { + assetName: string | null; +} + +/** + * The tiles. Pictures come from the shared `textureLibrary`, which keys by file name, so a + * thumbnail of a picture the scene already draws costs no second upload to the GPU. + */ +class ImageGrid extends Widget { + private items: PickerItem[] = []; + private tiles: { box: Graphics; sprite: Sprite; caption: Text }[] = []; + private metrics: GridMetrics = gridMetrics(GRID_WIDTH, 0); + private selectedFile: string | null = null; + private readonly empty = new Text({ text: '', style: textStyle('muted') }); + private readonly stopWatching: () => void; + onPick?: (file: string) => void; + onActivate?: (file: string) => void; + emptyText = ''; + + constructor() { + super(); + this.addChild(this.empty); + this.on('pointerdown', (e: FederatedPointerEvent) => this.onPress(e)); + this.stopWatching = textureLibrary.onChange(() => this.invalidate()); + } + + setItems(items: PickerItem[]): void { + this.items = items; + for (const tile of this.tiles) tile.box.destroy({ children: true }); + this.tiles = items.map(() => { + const box = new Graphics(); + const sprite = new Sprite(); + const caption = new Text({ style: textStyle('small') }); + box.addChild(sprite, caption); + this.addChild(box); + return { box, sprite, caption }; + }); + if (!items.some((item) => item.file === this.selectedFile)) this.selectedFile = null; + this.layout(); + this.redraw(); + } + + get selected(): PickerItem | null { + return this.items.find((item) => item.file === this.selectedFile) ?? null; + } + + get fileList(): string[] { + return this.items.map((item) => item.file); + } + + get usedFiles(): string[] { + return this.items.filter((item) => item.assetName !== null).map((item) => item.file); + } + + select(file: string | null): void { + if (file === this.selectedFile) return; + this.selectedFile = file; + this.redraw(); + if (file) this.onPick?.(file); + } + + /** Global centre of a tile; null when the file is not offered (test hook). */ + tileGlobal(file: string): { x: number; y: number } | null { + const index = this.items.findIndex((item) => item.file === file); + const tile = this.tiles[index]; + if (!tile) return null; + const rect = tileRect(index, this.metrics); + const p = tile.box.getGlobalPosition(); + return { x: p.x + rect.w / 2, y: p.y + this.metrics.tile / 2 }; + } + + override measure(): Size { + // The very first measure happens before the scroll container has sized us, so the declared + // width stands in – it is the width the dialog gives the grid anyway. + const width = this.w || this.minWidth; + return { w: this.minWidth, h: Math.max(this.minHeight, gridMetrics(width, this.items.length).height) }; + } + + override layout(): void { + this.metrics = gridMetrics(this.w, this.items.length); + this.tiles.forEach((tile, index) => { + const rect = tileRect(index, this.metrics); + tile.box.position.set(rect.x, rect.y); + }); + } + + override redraw(): void { + const { tile: side } = this.metrics; + const inner = side - TILE_PAD * 2; + this.empty.text = this.items.length === 0 ? this.emptyText : ''; + this.empty.position.set(0, 0); + + this.tiles.forEach((node, index) => { + const item = this.items[index]; + if (!item) return; + const selected = item.file === this.selectedFile; + node.box.clear(); + drawControlBox(node.box, side, side, { + fill: selected ? Theme.colors.selection : Theme.colors.control, + border: selected ? Theme.colors.accent : Theme.colors.border, + }); + + const texture = item.width > 0 ? textureLibrary.load(item.file) : null; + node.sprite.visible = texture !== null; + if (texture) { + const fitted = containRect(item.width, item.height, inner, inner); + node.sprite.texture = texture; + node.sprite.position.set(TILE_PAD + fitted.x, TILE_PAD + fitted.y); + node.sprite.width = fitted.w; + node.sprite.height = fitted.h; + } + + // The asset name is the useful caption where there is one; a bare hash tells nobody anything. + const text = item.width > 0 ? (item.assetName ?? `${item.width} × ${item.height}`) : 'unreadable'; + node.caption.style = textStyle(item.assetName ? 'accent' : 'muted'); + node.caption.text = truncate(text, node.caption.style, side); + node.caption.position.set(0, side + 1); + }); + } + + override destroy(): void { + this.stopWatching(); + super.destroy({ children: true }); + } + + private onPress(e: FederatedPointerEvent): void { + const local = this.toLocal(e.global); + const index = tileAt(local.x, local.y, this.items.length, this.metrics); + const item = this.items[index]; + if (!item) return; + this.select(item.file); + if (pressCount(e, item.file) === 2) this.onActivate?.(item.file); + } +} + +/** + * Offers every picture the server holds, rather than the machine's file system: uploading the + * same image twice is wasteful, and nothing collects the files a project stopped referencing. + * Uploading is still one button away, for a picture the server does not have yet. + */ +export class ImagePicker extends Popup { + private readonly grid = new ImageGrid(); + private readonly scroll = new ScrollContainer(); + private readonly status: Label; + private readonly detail: Label; + private readonly use: Button; + + constructor(private readonly deps: TextureImportDeps) { + super({ modal: true, padding: Theme.padding * 2, gap: Theme.gap }); + this.content.add(new Label('Choose an image', { style: 'bodyBold' })); + this.status = new Label('Loading images…', { style: 'muted' }); + this.content.add(this.status); + + this.grid.minWidth = GRID_WIDTH; + this.grid.emptyText = 'No images on the server yet – upload one.'; + this.grid.onPick = () => this.refreshSelection(); + this.grid.onActivate = () => this.confirm(); + this.scroll.minWidth = GRID_WIDTH; + this.scroll.minHeight = GRID_HEIGHT; + this.scroll.setContent(this.grid); + this.content.add(this.scroll); + + this.detail = new Label('', { style: 'muted' }); + this.content.add(this.detail); + + const buttons = new HStack({ gap: Theme.gap, align: 'center' }); + const upload = new Button('Upload…'); + upload.onClick = this.closeThen(() => pickAndImport(this.deps)); + buttons.add(upload); + buttons.add(new Spacer(), 1); + const cancel = new Button('Cancel'); + cancel.onClick = this.closeThen(); + buttons.add(cancel); + this.use = new Button('Use image', { variant: 'primary' }); + this.use.onClick = () => this.confirm(); + this.use.setEnabled(false); + buttons.add(this.use); + this.content.add(buttons); + + void this.loadList(); + } + + /** What the dialog offers, which of it the project already wears, and what is picked (test hook). */ + debugInfo(): { files: string[]; used: string[]; selected: string | null } { + return { + files: this.grid.fileList, + used: this.grid.usedFiles, + selected: this.grid.selected?.file ?? null, + }; + } + + tileGlobal(file: string): { x: number; y: number } | null { + return this.grid.tileGlobal(file); + } + + override close(): void { + super.close(); + if (current === this) current = null; + // Escape, Cancel and a confirmed pick all land here, and `Popup.close` tolerates a second + // call; the grid's listener must be dropped exactly once. + if (!this.grid.destroyed) this.grid.destroy(); + } + + private async loadList(): Promise { + try { + const stored = await this.deps.textures.list(); + const named = new Map(); + for (const asset of this.deps.state.project.assets) { + if (asset.type === 'texture') named.set(asset.file, asset.name); + } + this.grid.setItems(stored.map((item) => ({ ...item, assetName: named.get(item.file) ?? null }))); + // Only as tall as the tiles need, up to the cap: a handful of pictures should not leave + // half a dialog of empty box under them. + const metrics = gridMetrics(GRID_WIDTH, stored.length); + this.scroll.minHeight = Math.min(GRID_HEIGHT, Math.max(metrics.tile + CAPTION_HEIGHT, metrics.height)); + this.status.setText( + stored.length === 1 ? '1 image on the server' : `${stored.length} images on the server`, + ); + } catch (error) { + this.status.setText( + `Could not list the images: ${error instanceof Error ? error.message : String(error)}`, + ); + } + this.refreshSelection(); + } + + private refreshSelection(): void { + const item = this.grid.selected; + this.use.setEnabled(item !== null && item.width > 0); + this.detail.setText(item ? describe(item) : ''); + this.refreshLayout(); + } + + private confirm(): void { + const item = this.grid.selected; + if (!item || item.width <= 0) return; + this.close(); + importStoredImage(item, this.deps); + } +} + +function describe(item: PickerItem): string { + if (item.width <= 0) return 'The server cannot read this file as an image.'; + const size = `${item.width} × ${item.height} px, ${formatBytes(item.bytes)}`; + return item.assetName ? `${size} – already in the project as "${item.assetName}"` : size; +} + +function formatBytes(bytes: number): string { + if (bytes >= 1024 * 1024) return `${(bytes / 1024 / 1024).toFixed(1)} MB`; + // A small sprite is a few hundred bytes, and "0 KB" reads like a broken file. + return bytes >= 1024 ? `${Math.round(bytes / 1024)} KB` : `${bytes} B`; +} + +let current: ImagePicker | null = null; + +export function openImagePicker(deps: TextureImportDeps): ImagePicker { + current?.close(); + current = new ImagePicker(deps); + current.openCentered(); + return current; +} + +/** The open dialog, for the test hook. */ +export function currentImagePicker(): ImagePicker | null { + return current; +} diff --git a/src/editor/texture/imageGrid.ts b/src/editor/texture/imageGrid.ts new file mode 100644 index 00000000..ebcd09f9 --- /dev/null +++ b/src/editor/texture/imageGrid.ts @@ -0,0 +1,68 @@ +/** + * Geometry of the image picker's tile grid: how many tiles fit a width, where each one sits, and + * which one a point hits. Pure, so the arithmetic is testable without a canvas. + */ + +export interface Rect { + x: number; + y: number; + w: number; + h: number; +} + +/** Narrowest a tile may get before a column is dropped, and widest it may grow. */ +export const TILE_MIN = 92; +export const TILE_MAX = 132; +export const TILE_GAP = 8; +/** Strip under the picture holding the caption. */ +export const CAPTION_HEIGHT = 16; + +export interface GridMetrics { + columns: number; + /** Side of a tile's picture area; a tile is this plus the caption strip. */ + tile: number; + rows: number; + /** Total height of the grid, which is what the scroll container has to give it. */ + height: number; +} + +export function gridMetrics(width: number, count: number, gap = TILE_GAP): GridMetrics { + const columns = Math.max(1, Math.floor((width + gap) / (TILE_MIN + gap))); + const spread = (width - gap * (columns - 1)) / columns; + const tile = Math.max(TILE_MIN, Math.min(TILE_MAX, Math.floor(spread))); + const rows = Math.ceil(count / columns); + const height = rows * (tile + CAPTION_HEIGHT) + Math.max(0, rows - 1) * gap; + return { columns, tile, rows, height }; +} + +export function tileRect(index: number, metrics: GridMetrics, gap = TILE_GAP): Rect { + const column = index % metrics.columns; + const row = Math.floor(index / metrics.columns); + const h = metrics.tile + CAPTION_HEIGHT; + return { x: column * (metrics.tile + gap), y: row * (h + gap), w: metrics.tile, h }; +} + +/** Index of the tile under a point, or -1 for the gaps between them and everything past the last. */ +export function tileAt(x: number, y: number, count: number, metrics: GridMetrics, gap = TILE_GAP): number { + if (x < 0 || y < 0) return -1; + const h = metrics.tile + CAPTION_HEIGHT; + const column = Math.floor(x / (metrics.tile + gap)); + const row = Math.floor(y / (h + gap)); + if (column >= metrics.columns) return -1; + if (x - column * (metrics.tile + gap) > metrics.tile) return -1; + if (y - row * (h + gap) > h) return -1; + const index = row * metrics.columns + column; + return index < count ? index : -1; +} + +/** + * The picture centred in the box, scaled to fit either way. Small pictures are blown up: an icon + * shown at its own 16 px would be unrecognisable next to a full-size sheet. + */ +export function containRect(imageW: number, imageH: number, boxW: number, boxH: number): Rect { + if (imageW <= 0 || imageH <= 0) return { x: 0, y: 0, w: 0, h: 0 }; + const scale = Math.min(boxW / imageW, boxH / imageH); + const w = Math.max(1, Math.round(imageW * scale)); + const h = Math.max(1, Math.round(imageH * scale)); + return { x: Math.round((boxW - w) / 2), y: Math.round((boxH - h) / 2), w, h }; +} diff --git a/src/editor/textureImport.ts b/src/editor/textureImport.ts index c240ee34..dd0028bd 100644 --- a/src/editor/textureImport.ts +++ b/src/editor/textureImport.ts @@ -1,7 +1,7 @@ import { CreateTextureCommand } from '../core/commands/assetCommands'; import { nextAssetName } from '../core/commands/naming'; import type { TextureAsset } from '../core/model/Asset'; -import type { TextureStore } from '../core/store/TextureStore'; +import type { TextureStore, UploadedTexture } from '../core/store/TextureStore'; import type { EditorState } from './state'; /** What the picker offers and a drop accepts; the server rejects everything else anyway. */ @@ -110,10 +110,7 @@ export async function importImages( if (files.length === 0) return []; const outcomes: ImportOutcome[] = []; for (const file of files) outcomes.push(await importImage(file, deps)); - - const created = outcomes.filter((outcome) => outcome.assetId !== null); - const last = created[created.length - 1]; - if (last?.assetId) deps.state.select({ kind: 'asset', assetId: last.assetId }); + selectLast(outcomes, deps); deps.state.setNotice(importNotice(outcomes)); return outcomes; } @@ -122,26 +119,57 @@ async function importImage(file: File, deps: TextureImportDeps): Promise asset.type === 'texture' && asset.file === uploaded.file, - ); - if (existing) { - return { name: file.name, assetId: existing.id, error: null, reused: true }; - } - - const name = nextAssetName(deps.state.project, assetNameFor(file)); - const command = new CreateTextureCommand(name, uploaded); - // A rejected command has already put its reason into the notice; that is the message here. - if (!deps.state.execute(command)) return { name: file.name, assetId: null, error: deps.state.notice }; - return { name: file.name, assetId: command.asset.id, error: null, reused: false }; + return adopt(uploaded, assetNameFor(file), file.name, deps); } catch (error) { return { name: file.name, assetId: null, error: error instanceof Error ? error.message : String(error) }; } } +/** + * Makes an asset for a file the server holds. `label` names the source in messages, `nameBase` is + * what the asset is called. + */ +function adopt( + image: UploadedTexture, + nameBase: string, + label: string, + deps: TextureImportDeps, +): ImportOutcome { + // The server stores by content hash, so a re-imported picture comes back as the file the + // project already references; two assets on one file would disagree about `filter`. + const existing = deps.state.project.assets.find( + (asset): asset is TextureAsset => asset.type === 'texture' && asset.file === image.file, + ); + if (existing) return { name: label, assetId: existing.id, error: null, reused: true }; + + const name = nextAssetName(deps.state.project, nameBase); + const command = new CreateTextureCommand(name, image); + // A rejected command has already put its reason into the notice; that is the message here. + if (!deps.state.execute(command)) return { name: label, assetId: null, error: deps.state.notice }; + return { name: label, assetId: command.asset.id, error: null, reused: false }; +} + +/** + * Adopts a picture the server already holds – no upload, the bytes are there. Nothing collects + * unreferenced files, so a picture one project dropped is still there for the next to pick up. + */ +export function importStoredImage(image: UploadedTexture, deps: TextureImportDeps): ImportOutcome { + const outcome = adopt(image, 'Texture', image.file, deps); + selectLast([outcome], deps); + deps.state.setNotice( + outcome.error ?? + (outcome.reused ? 'That image is already in the project.' : 'Added the image to the project.'), + ); + return outcome; +} + +/** Selects the asset the batch created last, so the panel shows what just arrived. */ +function selectLast(outcomes: readonly ImportOutcome[], deps: TextureImportDeps): void { + const created = outcomes.filter((outcome) => outcome.assetId !== null); + const last = created[created.length - 1]; + if (last?.assetId) deps.state.select({ kind: 'asset', assetId: last.assetId }); +} + /** One line for the whole batch; reuse is worded apart, since "Imported 1 image" would be a lie. */ export function importNotice(outcomes: readonly ImportOutcome[]): string { const added = outcomes.filter((outcome) => outcome.assetId !== null && !outcome.reused).length; diff --git a/src/viewport/TextureLibrary.ts b/src/viewport/TextureLibrary.ts index 8f917da2..4cbdb15a 100644 --- a/src/viewport/TextureLibrary.ts +++ b/src/viewport/TextureLibrary.ts @@ -27,11 +27,8 @@ export class TextureLibrary { * the last one drawn wins; upload avoids that by reusing the existing asset for a known file. */ get(ref: TextureRef): Texture | null { - const texture = this.loaded.get(ref.file); - if (!texture) { - this.startLoading(ref.file); - return null; - } + const texture = this.load(ref.file); + if (!texture) return null; const mode = ref.smooth ? 'linear' : 'nearest'; if (texture.source.scaleMode !== mode) { texture.source.scaleMode = mode; @@ -42,6 +39,20 @@ export class TextureLibrary { return texture; } + /** + * The picture if it is in memory, otherwise null while it loads – without touching the scale + * mode, which belongs to whoever wears the texture. A thumbnail must not decide the filter of + * a picture the scene is drawing with. + */ + load(file: string): Texture | null { + const texture = this.loaded.get(file); + if (!texture) { + this.startLoading(file); + return null; + } + return texture; + } + /** The picture if it is in memory, without starting a load or touching the scale mode. */ peek(file: string): Texture | null { return this.loaded.get(file) ?? null; diff --git a/tests/core/http-store.test.ts b/tests/core/http-store.test.ts index a7dc0be4..4d25c0ed 100644 --- a/tests/core/http-store.test.ts +++ b/tests/core/http-store.test.ts @@ -70,9 +70,11 @@ describe('HttpTextureStore', () => { }); it('lists and deletes', async () => { - stubFetch(JSON.stringify([{ file: FILE, bytes: 12, updatedAt: 'now' }]), { status: 200 }); + stubFetch(JSON.stringify([{ file: FILE, width: 8, height: 4, bytes: 12, updatedAt: 'now' }]), { + status: 200, + }); await expect(new HttpTextureStore('/api').list()).resolves.toEqual([ - { file: FILE, bytes: 12, updatedAt: 'now' }, + { file: FILE, width: 8, height: 4, bytes: 12, updatedAt: 'now' }, ]); const mock = stubFetch(null, { status: 204 }); diff --git a/tests/editor/image-grid.test.ts b/tests/editor/image-grid.test.ts new file mode 100644 index 00000000..7197dec8 --- /dev/null +++ b/tests/editor/image-grid.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from 'vitest'; +import { + CAPTION_HEIGHT, + containRect, + gridMetrics, + TILE_GAP, + TILE_MAX, + TILE_MIN, + tileAt, + tileRect, +} from '../../src/editor/texture/imageGrid'; + +/** The width the picker gives its grid. */ +const WIDTH = 560; + +describe('gridMetrics', () => { + it('fits as many columns as the width holds, and spreads the leftover across them', () => { + const m = gridMetrics(WIDTH, 7); + expect(m.columns).toBe(5); + expect(m.tile).toBeGreaterThanOrEqual(TILE_MIN); + expect(m.columns * m.tile + (m.columns - 1) * TILE_GAP).toBeLessThanOrEqual(WIDTH); + }); + + it('keeps one column in a width too narrow for even that', () => { + expect(gridMetrics(40, 3).columns).toBe(1); + }); + + it('caps a lone tile instead of blowing one picture up over the whole dialog', () => { + expect(gridMetrics(140, 1).tile).toBe(TILE_MAX); + }); + + it('reports the height the scroll container has to give it', () => { + const m = gridMetrics(WIDTH, 7); + expect(m.rows).toBe(2); + expect(m.height).toBe(2 * (m.tile + CAPTION_HEIGHT) + TILE_GAP); + expect(gridMetrics(WIDTH, 0).height).toBe(0); + }); +}); + +describe('tileRect', () => { + it('lays the tiles out row by row', () => { + const m = gridMetrics(WIDTH, 7); + expect(tileRect(0, m)).toEqual({ x: 0, y: 0, w: m.tile, h: m.tile + CAPTION_HEIGHT }); + expect(tileRect(1, m).x).toBe(m.tile + TILE_GAP); + expect(tileRect(1, m).y).toBe(0); + expect(tileRect(m.columns, m)).toEqual({ + x: 0, + y: m.tile + CAPTION_HEIGHT + TILE_GAP, + w: m.tile, + h: m.tile + CAPTION_HEIGHT, + }); + }); +}); + +describe('tileAt', () => { + const m = gridMetrics(WIDTH, 7); + + it('finds the tile a point is on', () => { + expect(tileAt(2, 2, 7, m)).toBe(0); + expect(tileAt(m.tile + TILE_GAP + 2, 2, 7, m)).toBe(1); + expect(tileAt(2, m.tile + CAPTION_HEIGHT + TILE_GAP + 2, 7, m)).toBe(m.columns); + }); + + it('takes nothing in the gaps between tiles', () => { + expect(tileAt(m.tile + 2, 2, 7, m)).toBe(-1); + expect(tileAt(2, m.tile + CAPTION_HEIGHT + 2, 7, m)).toBe(-1); + }); + + it('takes nothing past the last tile – the grid is filled to the box height', () => { + // Seven pictures in five columns fill the second row up to place 6; 7 is the first empty one. + expect(tileAt(tileRect(6, m).x + 2, tileRect(6, m).y + 2, 7, m)).toBe(6); + expect(tileAt(tileRect(7, m).x + 2, tileRect(7, m).y + 2, 7, m)).toBe(-1); + expect(tileAt(2, 4000, 7, m)).toBe(-1); + expect(tileAt(-2, 2, 7, m)).toBe(-1); + }); +}); + +describe('containRect', () => { + it('centres a picture and keeps its ratio', () => { + expect(containRect(64, 32, 100, 100)).toEqual({ x: 0, y: 25, w: 100, h: 50 }); + expect(containRect(32, 64, 100, 100)).toEqual({ x: 25, y: 0, w: 50, h: 100 }); + }); + + it('blows a small icon up to the box, so it is recognisable next to a sheet', () => { + expect(containRect(16, 16, 100, 100)).toEqual({ x: 0, y: 0, w: 100, h: 100 }); + }); + + it('has nothing to draw for a picture the server could not read', () => { + expect(containRect(0, 0, 100, 100)).toEqual({ x: 0, y: 0, w: 0, h: 0 }); + }); +}); diff --git a/tests/editor/texture-import.test.ts b/tests/editor/texture-import.test.ts index 6f72fd36..f5d35a16 100644 --- a/tests/editor/texture-import.test.ts +++ b/tests/editor/texture-import.test.ts @@ -1,8 +1,13 @@ import { describe, expect, it, vi } from 'vitest'; +import type { TextureAsset } from '../../src/core/model/Asset'; +import { createEmptyProject } from '../../src/core/model/Project'; +import type { TextureStore } from '../../src/core/store/TextureStore'; +import { EditorState } from '../../src/editor/state'; import { encodeWithinBudget, type ImportOutcome, importNotice, + importStoredImage, MAX_TEXTURE_SIDE, MAX_UPLOAD_BYTES, planImageFit, @@ -106,3 +111,42 @@ describe('importNotice', () => { ); }); }); + +describe('importStoredImage', () => { + const FILE = `${'a'.repeat(64)}.png`; + const image = { file: FILE, width: 64, height: 48, bytes: 900 }; + /** The picker takes the image from the list; nothing here ever uploads. */ + const textures = { upload: vi.fn(), list: vi.fn(), delete: vi.fn() } as unknown as TextureStore; + + function setup(): EditorState { + const state = new EditorState(); + state.loadProject(createEmptyProject('p', 'P'), 1); + return state; + } + + it('makes an asset out of a picture the server already holds', () => { + const state = setup(); + const outcome = importStoredImage(image, { state, textures }); + + const asset = state.project.assets.find((a): a is TextureAsset => a.type === 'texture'); + expect(asset).toMatchObject({ file: FILE, width: 64, height: 48, name: 'Texture' }); + expect(outcome).toMatchObject({ assetId: asset?.id, error: null, reused: false }); + expect(state.selection).toEqual({ kind: 'asset', assetId: asset?.id }); + expect(state.notice).toBe('Added the image to the project.'); + // Adopting is a command like any other, so it can be taken back. + state.undo(); + expect(state.project.assets.filter((a) => a.type === 'texture')).toEqual([]); + }); + + it('reuses the asset that already names the file instead of making a second one', () => { + const state = setup(); + const first = importStoredImage(image, { state, textures }); + const again = importStoredImage(image, { state, textures }); + + expect(again.assetId).toBe(first.assetId); + expect(again.reused).toBe(true); + // Two assets on one file would disagree about `filter`, which is a property of the picture. + expect(state.project.assets.filter((a) => a.type === 'texture')).toHaveLength(1); + expect(state.notice).toBe('That image is already in the project.'); + }); +}); diff --git a/todo-notes/done/unreferenced-image-files.md b/todo-notes/done/unreferenced-image-files.md new file mode 100644 index 00000000..99c99773 --- /dev/null +++ b/todo-notes/done/unreferenced-image-files.md @@ -0,0 +1,40 @@ +Zitat aus der readme: "Unreferenced image files are never collected." + +Ich würde sagen dass soll vorerst auch so bleiben. + +Was passt: + drag and drop file in den editor => textur wird hochgeladen und passende asset wird erstellt => das soll so bleiben, das passt + +Was wir auch haben aber ändern: + klickt man auf das `+` => `Texture`, dann öffnet sich der File System Browser. + +Anstelle des file system browser => ein dialog mit allen images die am server existieren. + - auswählen => ok, dann wird es wiederverwendet => dialog schließt sich => texture asset mit ausgewählten image wird erstellt. + - oder hochladen button => file system browser => OK => dialog schließt sich => texture asset mit hochgeladenen image wird erstellt (im prinzip das was wir jetzt auch haben) + +--- + +**Umgesetzt (2026-09-09).** Der Weg ueber `+` → `Texture` oeffnet jetzt einen Bild-Dialog statt +des File-Browsers; Drag&Drop und Ctrl+V bleiben unveraendert, und gesammelt wird weiterhin nichts. + +**Der Dialog** (`src/editor/texture/ImagePicker.ts`) zeigt jede Datei, die der Server haelt, als +Kachel mit Vorschaubild. Auswaehlen und **Use image** legt ein Texture-Asset auf diese Datei an — +ohne Upload, die Bytes liegen ja schon da. **Upload…** oeffnet den File-Browser und muendet in +genau denselben Import wie ein Drop. Doppelklick auf eine Kachel ist die Kurzform von *Use image*. + +Kacheln, die das Projekt schon traegt, sind mit dem **Asset-Namen** beschriftet (statt mit der +Pixelgroesse); eine davon zu waehlen legt kein zweites Asset an, sondern selektiert das +vorhandene — zwei Assets auf einer Datei wuerden sich sonst ueber `filter` streiten. + +**Zwei Aenderungen, die dazu noetig waren:** +- `GET ?r=textures` liefert jetzt auch `width`/`height` (aus dem Datei-Header). Ein Texture-Asset + haelt die Pixelgroesse, und der Dialog haette sie sonst nur ueber das geladene Vorschaubild + gehabt — dann haenge die Richtigkeit des Assets am Ladezustand der GPU-Textur. +- `TextureLibrary.load(file)` neben `get(ref)`: laedt, **ohne** den `scaleMode` anzufassen. Ein + Vorschaubild darf den Filter eines Bildes nicht umstellen, das die Szene gerade zeichnet. + +**Belegt** (`tools/e2e/12-textures.mjs`, Block 19e): ein zweites Bild wird eingefuegt, das Asset +per Undo wieder entfernt — die Datei bleibt auf dem Server, das Projekt kennt sie nicht mehr. +Genau die taucht dann im Dialog auf, ist *nicht* als "im Projekt" markiert, und ueber *Use image* +entsteht daraus wieder ein Asset mit 24 × 24 aus dem Server-Datensatz. Zusaetzlich geprueft: der +Dialog zeigt exakt die Dateiliste von `GET ?r=textures`. diff --git a/todo-notes/unreferenced-image-files.md b/todo-notes/unreferenced-image-files.md deleted file mode 100644 index 42fdfe22..00000000 --- a/todo-notes/unreferenced-image-files.md +++ /dev/null @@ -1,14 +0,0 @@ -Zitat aus der readme: "Unreferenced image files are never collected." - -Ich würde sagen dass soll vorerst auch so bleiben. - -Was passt: - drag and drop file in den editor => textur wird hochgeladen und passende asset wird erstellt => das soll so bleiben, das passt - -Was wir auch haben aber ändern: - klickt man auf das `+` => `Texture`, dann öffnet sich der File System Browser. - -Anstelle des file system browser => ein dialog mit allen images die am server existieren. - - auswählen => ok, dann wird es wiederverwendet => dialog schließt sich => texture asset mit ausgewählten image wird erstellt. - - oder hochladen button => file system browser => OK => dialog schließt sich => texture asset mit hochgeladenen image wird erstellt (im prinzip das was wir jetzt auch haben) - \ No newline at end of file diff --git a/tools/e2e/12-textures.mjs b/tools/e2e/12-textures.mjs index d1a9d6ec..d785b640 100644 --- a/tools/e2e/12-textures.mjs +++ b/tools/e2e/12-textures.mjs @@ -19,6 +19,23 @@ const PASTE_IMAGE = `(async () => { window.dispatchEvent(new ClipboardEvent('paste', { clipboardData: transfer, bubbles: true })); })()`; +/** + * A second, plainly different picture. Its only job is to exist on the server under a hash of + * its own, so the picker has something to offer that the project does not already wear. + */ +const PASTE_SPARE = `(async () => { + const canvas = document.createElement('canvas'); + canvas.width = 24; + canvas.height = 24; + const g = canvas.getContext('2d'); + g.fillStyle = '#ffcc00'; g.fillRect(0, 0, 24, 24); + g.fillStyle = '#202020'; g.fillRect(6, 6, 12, 12); + const blob = await new Promise((resolve) => canvas.toBlob(resolve, 'image/png')); + const transfer = new DataTransfer(); + transfer.items.add(new File([blob], 'spare.png', { type: 'image/png' })); + window.dispatchEvent(new ClipboardEvent('paste', { clipboardData: transfer, bubbles: true })); +})()`; + function colourDistance(a, b) { const channels = (hex) => [1, 3, 5].map((i) => parseInt(hex.slice(i, i + 2), 16)); const [ar, ag, ab] = channels(a); @@ -28,6 +45,7 @@ function colourDistance(a, b) { export async function textures(t) { const { check, sleep, json, evaluate, shot, mutate, press } = t; + const { api, click, clickText, doubleClick, locate, undo } = t; // The previous step leaves a picker open and a text field focused, and paste is deliberately // ignored while an input has focus, so that Ctrl+V inside a field still edits text. @@ -191,4 +209,112 @@ export async function textures(t) { JSON.stringify({ fallback }), ); } + + // 19e. "+ → Texture" offers the pictures the server already holds instead of the machine's + // file system. Nothing collects unreferenced files, so a picture one project dropped is + // still there for the next – which is exactly what this drives: paste a second picture, + // undo the asset (the file stays), then pick that file out of the dialog. + await evaluate(PASTE_SPARE); + await sleep(2500); + const two = await json("__gm.state.project.assets.filter((a) => a.type === 'texture').map((a) => a.file)"); + const spare = two[1] ?? ''; + check( + 'a second picture is stored under a hash of its own', + two.length === 2 && spare !== two[0], + JSON.stringify(two), + ); + + await undo(); + await sleep(300); + const afterUndo = await json( + "__gm.state.project.assets.filter((a) => a.type === 'texture').map((a) => a.file)", + ); + check( + 'undoing the import leaves the file on the server with no asset naming it', + afterUndo.length === 1 && !afterUndo.includes(spare), + JSON.stringify(afterUndo), + ); + + await clickText('+'); + // By position, not by text: the menu entry "Texture" shares its label with the centre tab, and + // `locate` would hand back whichever it walked into first. + const entry = await locate('Texture', true); + await click(entry.x, entry.y); + await sleep(1500); + const picker = await json('__gm.imagePicker()'); + const stored = (await api('textures')).map((item) => item.file); + check( + 'the Texture entry opens the image picker instead of the file dialog', + picker !== null, + JSON.stringify(picker), + ); + check( + 'it offers exactly the files the server holds', + JSON.stringify(picker?.files ?? []) === JSON.stringify(stored), + JSON.stringify({ shown: picker?.files.length, stored: stored.length }), + ); + check( + 'the picture the project wears is marked as already in it, the dropped one is not', + (picker?.used ?? []).includes(texture.file) && !(picker?.used ?? []).includes(spare), + JSON.stringify(picker?.used), + ); + await shot('34-image-picker'); + + const tile = await json(`__gm.imagePickerTile(${JSON.stringify(spare)})`); + await click(tile.x, tile.y); + check( + 'clicking a tile picks that image', + (await json('__gm.imagePicker()'))?.selected === spare, + JSON.stringify(await json('__gm.imagePicker()')), + ); + await shot('35-image-picked'); + + await clickText('Use image'); + await sleep(600); + const adopted = await json( + "__gm.state.project.assets.filter((a) => a.type === 'texture').map((a) => ({ name: a.name, file: a.file, w: a.width, h: a.height }))", + ); + check('the dialog closes on the pick', (await json('__gm.imagePicker()')) === null); + check( + 'the picked file becomes a texture asset, sized from the server record', + adopted.length === 2 && adopted[1]?.file === spare && adopted[1]?.w === 24 && adopted[1]?.h === 24, + JSON.stringify(adopted), + ); + check( + 'and the notice says it was added rather than imported', + (await json('__gm.state.notice')) === 'Added the image to the project.', + JSON.stringify(await json('__gm.state.notice')), + ); + + // Reusing a file already in the project must not make a second asset on it – two assets on one + // picture would disagree about its filter. + await clickText('+'); + const entryAgain = await locate('Texture', true); + await click(entryAgain.x, entryAgain.y); + await sleep(1500); + const reopened = await json('__gm.imagePicker()'); + check( + 'the adopted file is marked as in the project when the dialog is opened again', + (reopened?.used ?? []).includes(spare), + JSON.stringify(reopened?.used), + ); + const sameTile = await json(`__gm.imagePickerTile(${JSON.stringify(spare)})`); + await doubleClick(sameTile.x, sameTile.y); + await sleep(600); + const afterSecondPick = await json( + `__gm.state.project.assets.filter((a) => a.file === ${JSON.stringify(spare)}).length`, + ); + check( + 'double-clicking a picture the project already has keeps it at one asset', + afterSecondPick === 1 && (await json('__gm.state.notice')) === 'That image is already in the project.', + JSON.stringify({ assets: afterSecondPick, notice: await json('__gm.state.notice') }), + ); + + // Back to one texture asset, the way the later steps expect the fixture. + await undo(); + await sleep(300); + check( + 'undo takes the adopted asset back out', + (await json("__gm.state.project.assets.filter((a) => a.type === 'texture').length")) === 1, + ); }