From a9c8111847e48962bc2fe56cfb9abf22502695e7 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Sun, 12 Jul 2026 20:52:14 +0100 Subject: [PATCH 1/2] Add axis mapping authoring model --- Cargo.lock | 27 +- ROADMAP.md | 2 + .../components/variation/CreateAxisDialog.tsx | 19 +- .../src/lib/interpolation/interpolate.ts | 7 +- .../src/renderer/src/lib/model/Font.test.ts | 99 +++- .../src/renderer/src/lib/model/Font.ts | 65 ++- .../renderer/src/lib/model/FontStore.test.ts | 1 + .../src/renderer/src/lib/model/FontStore.ts | 3 +- .../renderer/src/lib/model/variation.test.ts | 23 +- .../src/lib/workspace/WorkspaceClient.ts | 17 +- .../WorkspaceEditCoordinator.test.ts | 19 +- .../lib/workspace/WorkspaceEditCoordinator.ts | 12 +- apps/desktop/src/shared/workspace/protocol.ts | 5 + .../src/utility/workspace/WorkspaceHost.ts | 3 + crates/shift-backends/Cargo.toml | 2 +- crates/shift-backends/docs/DOCS.md | 4 +- .../src/designspace/axis_labels.rs | 225 ++++++++++ .../shift-backends/src/designspace/error.rs | 3 + crates/shift-backends/src/designspace/mod.rs | 119 ++++- .../shift-backends/src/designspace/reader.rs | 150 ++++++- .../shift-backends/src/designspace/writer.rs | 105 ++++- crates/shift-bridge/docs/DOCS.md | 1 + crates/shift-bridge/dts-header.d.ts | 1 + crates/shift-bridge/index.d.ts | 65 ++- crates/shift-bridge/index.js | 2 + crates/shift-bridge/src/bridge.rs | 201 +++++++-- crates/shift-bridge/src/input.rs | 7 +- crates/shift-cli/README.md | 2 + crates/shift-cli/src/inspect.rs | 37 +- crates/shift-cli/src/inspect/render.rs | 54 ++- crates/shift-cli/src/inspect/report.rs | 168 ++++++- crates/shift-font/docs/DOCS.md | 7 +- crates/shift-font/src/changes.rs | 44 +- crates/shift-font/src/error.rs | 19 +- crates/shift-font/src/intents.rs | 42 +- crates/shift-font/src/ir/axis.rs | 422 +++++++++++++++++- crates/shift-font/src/ir/entity.rs | 1 + crates/shift-font/src/ir/font.rs | 117 ++++- crates/shift-font/src/ir/mod.rs | 8 +- crates/shift-font/src/ir/variation.rs | 225 +++++++++- crates/shift-source/docs/DOCS.md | 9 +- crates/shift-source/src/lib.rs | 8 +- crates/shift-source/src/package.rs | 337 ++++++++++++-- crates/shift-source/tests/package_test.rs | 82 +++- crates/shift-store/src/change_set.rs | 80 +++- crates/shift-store/src/font_state.rs | 47 +- crates/shift-store/src/schema.rs | 9 + crates/shift-store/src/source.rs | 4 +- crates/shift-store/tests/store_test.rs | 41 ++ crates/shift-wire/src/bridges/napi/mod.rs | 135 +++++- crates/shift-wire/src/lib.rs | 91 +++- crates/shift-workspace/src/ledger.rs | 6 +- crates/shift-workspace/src/workspace.rs | 62 ++- .../shift-workspace/tests/workspace_test.rs | 2 +- packages/types/docs/DOCS.md | 3 +- packages/types/src/bridge/generated.ts | 59 ++- packages/types/src/bridge/index.ts | 7 + packages/types/src/ids.ts | 22 + packages/types/src/index.ts | 11 + scripts/generate-bridge-types.mjs | 1 + 60 files changed, 3026 insertions(+), 323 deletions(-) create mode 100644 crates/shift-backends/src/designspace/axis_labels.rs diff --git a/Cargo.lock b/Cargo.lock index b546c36a..b27ff273 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -938,7 +938,7 @@ dependencies = [ "kurbo 0.11.2", "log", "ordered-float 4.6.0", - "quick-xml", + "quick-xml 0.37.5", "regex", "serde", "smol_str 0.2.2", @@ -1398,7 +1398,7 @@ dependencies = [ "close_already", "indexmap", "plist", - "quick-xml", + "quick-xml 0.37.5", "serde", "serde_derive", "serde_repr", @@ -1407,15 +1407,16 @@ dependencies = [ [[package]] name = "norad" -version = "0.16.0" +version = "0.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e914290e267484b1d18e8d9c7bcee24433c264ae42c12240add5c710a4bf042c" +checksum = "d223261353c1d60c5e00c3b669c6936c47b2e9dc860b5121bc57b964389c47bb" dependencies = [ "base64", "close_already", "indexmap", + "log", "plist", - "quick-xml", + "quick-xml 0.38.4", "serde", "serde_derive", "serde_repr", @@ -1549,7 +1550,7 @@ checksum = "3d77244ce2d584cd84f6a15f86195b8c9b2a0dfbfd817c09e0464244091a58ed" dependencies = [ "base64", "indexmap", - "quick-xml", + "quick-xml 0.37.5", "serde", "time", ] @@ -1612,6 +1613,16 @@ dependencies = [ "serde", ] +[[package]] +name = "quick-xml" +version = "0.38.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66c2058c55a409d601666cffe35f04333cf1013010882cec174a7467cd4e21c" +dependencies = [ + "memchr", + "serde", +] + [[package]] name = "quote" version = "1.0.40" @@ -1884,9 +1895,9 @@ dependencies = [ "fontc", "glyphs-reader", "libc", - "norad 0.16.0", + "norad 0.18.4", "plist", - "quick-xml", + "quick-xml 0.37.5", "serde", "shift-font", "shift-source", diff --git a/ROADMAP.md b/ROADMAP.md index 9707a821..fec2a763 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -582,6 +582,8 @@ These are allowed to jump around when energy is high, but they should not silent - [x] Load `.designspace` files - [x] Parse axis definitions (wght, wdth, ital, custom) +- [x] Persist continuous/discrete axes and axis value labels +- [x] Round-trip independent and cross-axis mappings - [ ] Named instances **Masters Editing** diff --git a/apps/desktop/src/renderer/src/components/variation/CreateAxisDialog.tsx b/apps/desktop/src/renderer/src/components/variation/CreateAxisDialog.tsx index 1d64161d..3bcbaf89 100644 --- a/apps/desktop/src/renderer/src/components/variation/CreateAxisDialog.tsx +++ b/apps/desktop/src/renderer/src/components/variation/CreateAxisDialog.tsx @@ -48,14 +48,17 @@ export const CreateAxisDialog = ({ open, onOpenChange }: CreateAxisDialogProps) const handleSubmit = (event: FormEvent) => { event.preventDefault(); - font.createAxis( - values.name, - values.tag, - Number(values.min), - Number(values.default), - Number(values.max), - values.hidden, - ); + font.createAxis({ + name: values.name, + tag: values.tag, + role: "external", + axisType: "continuous", + minimum: Number(values.min), + default: Number(values.default), + maximum: Number(values.max), + labels: [], + hidden: values.hidden, + }); onOpenChange(false); }; diff --git a/apps/desktop/src/renderer/src/lib/interpolation/interpolate.ts b/apps/desktop/src/renderer/src/lib/interpolation/interpolate.ts index a4ef3625..810d1350 100644 --- a/apps/desktop/src/renderer/src/lib/interpolation/interpolate.ts +++ b/apps/desktop/src/renderer/src/lib/interpolation/interpolate.ts @@ -18,12 +18,15 @@ export type NormalizedLocation = Record; /** Asymmetric normalize — mirrors `Axis::normalize` in `shift-font`. */ export function normalizeAxis(value: number, axis: Axis): number { + const minimum = axis.minimum ?? Math.min(...(axis.values ?? [axis.default])); + const maximum = axis.maximum ?? Math.max(...(axis.values ?? [axis.default])); + if (value < axis.default) { - const range = axis.default - axis.minimum; + const range = axis.default - minimum; return range === 0 ? 0 : (value - axis.default) / range; } if (value > axis.default) { - const range = axis.maximum - axis.default; + const range = maximum - axis.default; return range === 0 ? 0 : (value - axis.default) / range; } return 0; diff --git a/apps/desktop/src/renderer/src/lib/model/Font.test.ts b/apps/desktop/src/renderer/src/lib/model/Font.test.ts index 62c8a8fe..9c1b3d3d 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { mintAxisId, + mintAxisMappingId, mintGlyphId, mintLayerId, mintSourceId, @@ -37,6 +38,7 @@ const SNAPSHOT: WorkspaceSnapshot = { }, ], axes: [], + axisMappings: [], }; describe("Font projects the workspace snapshot", () => { @@ -121,13 +123,7 @@ describe("font-level intents make the font variable", () => { { kind: "createAxis", createAxis: { - axisId: weightAxisId, - tag: "wght", - name: "Weight", - min: 100, - default: 400, - max: 900, - hidden: false, + axis: continuousAxis(weightAxisId), }, }, ]); @@ -176,6 +172,57 @@ describe("font-level intents make the font variable", () => { expect(stack.font.glyph(glyphId)?.layers).toEqual([{ id: layerId, sourceId }]); }); + it("projects mapping edits and evaluates them in Rust", async () => { + const stack = createWorkspaceStack(); + await stack.createWorkspace(); + const axisId = stack.font.createAxis({ + tag: "wght", + name: "Weight", + role: "external", + axisType: "continuous", + minimum: 100, + default: 400, + maximum: 900, + labels: [], + hidden: false, + }); + await stack.editCoordinator.settled(); + + const mappingId = mintAxisMappingId(); + stack.font.setAxisMappings([ + { + id: mappingId, + name: "Weight curve", + inputs: [axisId], + outputs: [axisId], + points: [ + mappingPoint(axisId, 100, 100), + mappingPoint(axisId, 400, 400), + mappingPoint(axisId, 900, 800), + ], + }, + ]); + await stack.editCoordinator.settled(); + + expect(stack.font.getAxisMappings().map((mapping) => mapping.id)).toEqual([mappingId]); + const mapped = await stack.font.mapLocation({ + values: { [axisId]: 900 } as Record, + }); + expect(mapped.values[axisId]).toBeCloseTo(800); + + stack.font.deleteAxis(axisId); + await stack.editCoordinator.settled(); + expect(stack.font.getAxes()).toEqual([]); + expect(stack.font.getAxisMappings()).toEqual([]); + + await stack.editCoordinator.undo(); + expect(stack.font.getAxes().map((axis) => axis.id)).toEqual([axisId]); + expect(stack.font.getAxisMappings().map((mapping) => mapping.id)).toEqual([mappingId]); + + await stack.editCoordinator.undo(); + expect(stack.font.getAxisMappings()).toEqual([]); + }); + it("createGlyph authors a default layer for fresh glyphs", async () => { const stack = createWorkspaceStack(); await stack.createWorkspace(); @@ -246,13 +293,7 @@ describe("font-level intents make the font variable", () => { { kind: "createAxis", createAxis: { - axisId, - tag: "wght", - name: "Weight", - min: 100, - default: 400, - max: 900, - hidden: false, + axis: continuousAxis(axisId), }, }, ]); @@ -304,13 +345,7 @@ describe("font-level intents make the font variable", () => { { kind: "createAxis", createAxis: { - axisId, - tag: "wght", - name: "Weight", - min: 100, - default: 400, - max: 900, - hidden: false, + axis: continuousAxis(axisId), }, }, ]); @@ -360,3 +395,25 @@ describe("font-level intents make the font variable", () => { expect(glyphs.map((glyph) => glyph.id)).toEqual([b.id, a.id, b.id]); }); }); + +function continuousAxis(axisId: AxisId) { + return { + id: axisId, + tag: "wght", + name: "Weight", + role: "external", + axisType: "continuous", + minimum: 100, + default: 400, + maximum: 900, + labels: [], + hidden: false, + }; +} + +function mappingPoint(axisId: AxisId, input: number, output: number) { + return { + input: { values: { [axisId]: input } as Record }, + output: { values: { [axisId]: output } as Record }, + }; +} diff --git a/apps/desktop/src/renderer/src/lib/model/Font.ts b/apps/desktop/src/renderer/src/lib/model/Font.ts index ef3ac2ca..b8f7c387 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.ts @@ -2,6 +2,7 @@ import type { FontMetrics, FontMetadata, Axis, + AxisMapping, Source, GlyphId, GlyphRecord, @@ -330,6 +331,7 @@ export class Font { readonly #metadataCell: Signal; readonly #sourcesCell: Signal; readonly #axesCell: Signal; + readonly #axisMappingsCell: Signal; readonly #unicodesCell: Signal; readonly #glyphRecordsCell: Signal; @@ -352,6 +354,7 @@ export class Font { this.#metadataCell = computed(() => workspaceCell.value?.metadata ?? {}); this.#sourcesCell = computed(() => workspaceCell.value?.sources ?? []); this.#axesCell = computed(() => workspaceCell.value?.axes ?? []); + this.#axisMappingsCell = computed(() => workspaceCell.value?.axisMappings ?? []); this.#directoryCell = computed(() => GlyphDirectory.fromRecords(workspaceCell.value?.glyphs ?? []), ); @@ -398,6 +401,11 @@ export class Font { return this.#axesCell; } + /** Reactive font-owned independent and cross-axis mappings. */ + get axisMappingsCell(): Signal { + return this.#axisMappingsCell; + } + /** Reactive committed variation sources for sidebar controls. */ get sourcesCell(): Signal { return this.#sourcesCell; @@ -1211,24 +1219,46 @@ export class Font { return sourceId; } - /** @knipclassignore — used by VariationPanel component */ - createAxis( - name: string, - tag: string, - min: number, - def: number, - max: number, - hidden: boolean = false, - ): AxisId { + /** + * Creates one variation axis with renderer-minted stable identity. + * + * @param axis - Complete axis definition apart from the id assigned here. + * @returns The id submitted to the workspace. + */ + createAxis(axis: Omit): AxisId { const axisId = mintAxisId(); this.editCoordinator.push({ kind: "createAxis", - createAxis: { axisId, name, tag, min, default: def, max, hidden }, + createAxis: { axis: { id: axisId, ...axis } }, }); return axisId; } + /** + * Replaces an existing axis definition while preserving its stable id. + * + * @param axis - Complete replacement axis returned from the current font model. + */ + updateAxis(axis: Axis): void { + this.editCoordinator.push({ + kind: "updateAxis", + updateAxis: { axis }, + }); + } + + /** + * Replaces the font-owned mapping collection as one undoable edit. + * + * @param mappings - Complete ordered mapping collection; Rust validates all axis references. + */ + setAxisMappings(mappings: readonly AxisMapping[]): void { + this.editCoordinator.push({ + kind: "setAxisMappings", + setAxisMappings: { mappings: [...mappings] }, + }); + } + /** @knipclassignore — used by VariationPanel component */ deleteAxis(axisId: AxisId): void { this.editCoordinator.push({ @@ -1249,6 +1279,21 @@ export class Font { return this.#axesCell.peek(); } + /** Returns the current font-owned mapping collection. */ + getAxisMappings(): AxisMapping[] { + return this.#axisMappingsCell.peek(); + } + + /** + * Evaluates independent mappings followed by cross-axis mappings in Rust. + * + * @param location - External location keyed by stable axis id; omitted axes use defaults. + * @returns The mapped location after pending workspace edits have settled. + */ + mapLocation(location: Location): Promise { + return this.editCoordinator.mapLocation(location); + } + /** @knipclassignore — used by VariationPanel component */ get sources(): Source[] { return this.#sourcesCell.peek(); diff --git a/apps/desktop/src/renderer/src/lib/model/FontStore.test.ts b/apps/desktop/src/renderer/src/lib/model/FontStore.test.ts index e068a188..7457c82b 100644 --- a/apps/desktop/src/renderer/src/lib/model/FontStore.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/FontStore.test.ts @@ -198,6 +198,7 @@ function snapshot(documentId: string, layerId: LayerId): WorkspaceSnapshot { }, ], axes: [], + axisMappings: [], }; } diff --git a/apps/desktop/src/renderer/src/lib/model/FontStore.ts b/apps/desktop/src/renderer/src/lib/model/FontStore.ts index 93bde877..357847e1 100644 --- a/apps/desktop/src/renderer/src/lib/model/FontStore.ts +++ b/apps/desktop/src/renderer/src/lib/model/FontStore.ts @@ -122,11 +122,12 @@ export class FontStore { batch(() => { const nextWorkspace = - applied.glyphs || applied.axes || applied.sources + applied.glyphs || applied.axes || applied.axisMappings || applied.sources ? { ...current, glyphs: applied.glyphs ?? current.glyphs, axes: applied.axes ?? current.axes, + axisMappings: applied.axisMappings ?? current.axisMappings, sources: applied.sources ?? current.sources, } : current; diff --git a/apps/desktop/src/renderer/src/lib/model/variation.test.ts b/apps/desktop/src/renderer/src/lib/model/variation.test.ts index 27089a40..49f4ada3 100644 --- a/apps/desktop/src/renderer/src/lib/model/variation.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/variation.test.ts @@ -87,13 +87,7 @@ async function variableFont(): Promise<{ { kind: "createAxis", createAxis: { - axisId: weightAxisId, - tag: "wght", - name: "Weight", - min: 100, - default: 400, - max: 900, - hidden: false, + axis: continuousAxis(weightAxisId), }, }, ]); @@ -134,6 +128,21 @@ async function variableFont(): Promise<{ return { stack, glyphId, regularLayerId, boldLayerId, bold }; } +function continuousAxis(axisId: AxisId) { + return { + id: axisId, + tag: "wght", + name: "Weight", + role: "external", + axisType: "continuous", + minimum: 100, + default: 400, + maximum: 900, + labels: [], + hidden: false, + }; +} + async function loadGlyph(stack: WorkspaceStack, glyphId: GlyphId) { return stack.font.loadGlyph(glyphId); } diff --git a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceClient.ts b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceClient.ts index f7935dc6..a6c2bc54 100644 --- a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceClient.ts +++ b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceClient.ts @@ -8,7 +8,7 @@ import type { WorkspaceSnapshot, } from "@shared/workspace/protocol"; import type { ShiftHost } from "@shared/host/ShiftHost"; -import type { AppliedChange, FontIntent } from "@shift/types"; +import type { AppliedChange, FontIntent, Location } from "@shift/types"; import { signal } from "@/lib/signals/signal"; /** @@ -129,15 +129,28 @@ export class WorkspaceClient { return this.#require().call("workspace.glyphSnapshots", { requests: [...requests] }); } + /** + * Evaluates the current font's axis mappings in the utility process. + * + * @param location - External location keyed by stable axis id. + * @returns The mapped location, with omitted axes filled from their defaults. + */ + async mapLocation(location: Location): Promise { + await this.connect(); + + return this.#require().call("workspace.mapLocation", location); + } + #fold(applied: AppliedChange): AppliedChange { const current = this.workspaceCell.peek(); if (!current) return applied; - if (applied.glyphs || applied.axes || applied.sources) { + if (applied.glyphs || applied.axes || applied.axisMappings || applied.sources) { this.workspaceCell.set({ ...current, glyphs: applied.glyphs ?? current.glyphs, axes: applied.axes ?? current.axes, + axisMappings: applied.axisMappings ?? current.axisMappings, sources: applied.sources ?? current.sources, }); } diff --git a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.test.ts b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.test.ts index 94595455..88773ad9 100644 --- a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.test.ts +++ b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.test.ts @@ -129,13 +129,18 @@ describe("WorkspaceEditCoordinator issues save on the committed-op lane", () => editCoordinator.push({ kind: "createAxis", createAxis: { - axisId, - tag: "wght", - name: "Weight", - min: 100, - default: 400, - max: 900, - hidden: false, + axis: { + id: axisId, + tag: "wght", + name: "Weight", + role: "external", + axisType: "continuous", + minimum: 100, + default: 400, + maximum: 900, + labels: [], + hidden: false, + }, }, }); diff --git a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.ts b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.ts index 03f2af71..4029aaba 100644 --- a/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.ts +++ b/apps/desktop/src/renderer/src/lib/workspace/WorkspaceEditCoordinator.ts @@ -1,4 +1,4 @@ -import type { AppliedChange, FontIntent } from "@shift/types"; +import type { AppliedChange, FontIntent, Location } from "@shift/types"; import type { WorkspaceDocumentState, WorkspaceGlyphSnapshot, @@ -153,6 +153,16 @@ export class WorkspaceEditCoordinator { return this.#withFlush(() => this.#workspace.glyphSnapshots(requests)); } + /** + * Evaluates a location behind all pending axis and mapping edits. + * + * @param location - External location keyed by stable axis id. + * @returns The mapped location after queued edits have settled. + */ + mapLocation(location: Location): Promise { + return this.#withFlush(() => this.#workspace.mapLocation(location)); + } + /** Replays the latest redo entry after pending pushes flush. */ redo(): Promise { return this.#withFlush(async () => { diff --git a/apps/desktop/src/shared/workspace/protocol.ts b/apps/desktop/src/shared/workspace/protocol.ts index 714c847d..0fbed82d 100644 --- a/apps/desktop/src/shared/workspace/protocol.ts +++ b/apps/desktop/src/shared/workspace/protocol.ts @@ -1,6 +1,7 @@ import type { AppliedChange, Axis, + AxisMapping, FontIntent, FontMetadata, FontMetrics, @@ -8,6 +9,7 @@ import type { GlyphRecord, GlyphState, GlyphVariationData, + Location, Source, SourceId, } from "@shift/types"; @@ -22,6 +24,7 @@ export type WorkspaceSnapshot = { glyphs: GlyphRecord[]; sources: Source[]; axes: Axis[]; + axisMappings: AxisMapping[]; }; export type WorkspaceGlyphLayerSnapshot = { @@ -134,6 +137,8 @@ export type SyncCallMap = { request: { requests: WorkspaceGlyphSnapshotRequest[] }; response: WorkspaceGlyphSnapshot[]; }; + /** Evaluates font-owned independent and cross-axis mappings in Rust. */ + "workspace.mapLocation": { request: Location; response: Location }; }; export type SyncEventMap = { diff --git a/apps/desktop/src/utility/workspace/WorkspaceHost.ts b/apps/desktop/src/utility/workspace/WorkspaceHost.ts index e1d4006d..bc54d97f 100644 --- a/apps/desktop/src/utility/workspace/WorkspaceHost.ts +++ b/apps/desktop/src/utility/workspace/WorkspaceHost.ts @@ -112,6 +112,8 @@ export class WorkspaceHost { "workspace.saveAs": ({ path }) => this.#serialize(() => this.#saveAs(path)), "workspace.glyphSnapshots": ({ requests }) => this.#serialize(() => this.#bridge.getGlyphSnapshots(requests) as WorkspaceGlyphSnapshot[]), + "workspace.mapLocation": (location) => + this.#serialize(() => this.#bridge.mapLocation(location)), }); } @@ -143,6 +145,7 @@ export class WorkspaceHost { glyphs: this.#bridge.getGlyphs(), sources: this.#bridge.getSources(), axes: this.#bridge.getAxes(), + axisMappings: this.#bridge.getAxisMappings(), }; } diff --git a/crates/shift-backends/Cargo.toml b/crates/shift-backends/Cargo.toml index 892bfb85..089d5d6a 100644 --- a/crates/shift-backends/Cargo.toml +++ b/crates/shift-backends/Cargo.toml @@ -12,7 +12,7 @@ crate-type = ["rlib"] shift-font = { workspace = true } shift-source = { workspace = true } -norad = "0.16.0" +norad = "0.18.4" skrifa = "0.32.0" fontc = "0.2.0" glyphs-reader = "0.2.0" diff --git a/crates/shift-backends/docs/DOCS.md b/crates/shift-backends/docs/DOCS.md index 8c76b3d7..a8ed5ccb 100644 --- a/crates/shift-backends/docs/DOCS.md +++ b/crates/shift-backends/docs/DOCS.md @@ -40,7 +40,7 @@ src/ - `FontBackend` -- auto-implemented marker trait for types implementing both `FontReader` + `FontWriter` - `UfoReader` -- loads `.ufo` bundles via `norad` - `UfoWriter` -- writes `.ufo` bundles via `norad`; rounds coordinates to integers -- `DesignspaceWriter` -- writes a `.designspace` file plus a companion `.ufo`; additional Shift sources are represented as layers in that companion UFO and referenced by source layer names +- `DesignspaceReader` / `DesignspaceWriter` -- read and atomically write `.designspace` projects plus companion UFOs, including continuous/discrete axes, axis value labels, per-axis maps, and cross-axis mappings - `UfoBackend` -- unit struct implementing `FontBackend` by delegating to `UfoReader`/`UfoWriter` - `GlyphsReader` -- loads `.glyphs` and `.glyphspackage` files via `glyphs-reader`; read-only (no writer) @@ -54,6 +54,8 @@ src/ **Glyphs-format specifics:** `GlyphsReader` also extracts axes, sources, and per-master locations -- data that UFO does not natively represent. Kerning group membership is derived from per-glyph `right_kern`/`left_kern` fields and normalized to `public.kern1.*`/`public.kern2.*` conventions. +**Designspace mapping:** Per-axis `` entries become independent `AxisMapping` values. Designspace 5.1+ `` entries become the font's single cross-axis mapping group. Axis value labels use the standard Designspace 5.0 `` representation. + **Saving:** Only UFO write is supported. `UfoWriter` builds a `norad::Font`, populates metadata/metrics/kerning/groups/guidelines/lib, then converts each glyph per layer. `features.fea` is written as a standalone file before calling `norad::Font::save`. ## Workflow recipes diff --git a/crates/shift-backends/src/designspace/axis_labels.rs b/crates/shift-backends/src/designspace/axis_labels.rs new file mode 100644 index 00000000..38c87779 --- /dev/null +++ b/crates/shift-backends/src/designspace/axis_labels.rs @@ -0,0 +1,225 @@ +use super::error::{DesignspaceError, DesignspaceResult}; +use quick_xml::events::{BytesEnd, BytesStart, Event}; +use quick_xml::{Reader, Writer}; +use shift_font::{Axis, AxisLabel, AxisLabelRange}; +use std::collections::HashMap; + +pub(super) fn parse(xml: &str) -> DesignspaceResult>> { + let mut reader = Reader::from_str(xml); + reader.config_mut().trim_text(true); + let mut current_axis = None; + let mut in_axis_labels = false; + let mut labels = HashMap::>::new(); + + loop { + match reader.read_event() { + Ok(Event::Start(event)) => match event.name().as_ref() { + b"axis" => current_axis = attribute(&reader, &event, b"name")?, + b"labels" if current_axis.is_some() => in_axis_labels = true, + b"label" if in_axis_labels => { + push_label(&reader, &event, current_axis.as_deref(), &mut labels)?; + } + _ => {} + }, + Ok(Event::Empty(event)) if event.name().as_ref() == b"label" && in_axis_labels => { + push_label(&reader, &event, current_axis.as_deref(), &mut labels)?; + } + Ok(Event::End(event)) => match event.name().as_ref() { + b"labels" if current_axis.is_some() => in_axis_labels = false, + b"axis" => { + current_axis = None; + in_axis_labels = false; + } + _ => {} + }, + Ok(Event::Eof) => break, + Err(error) => return Err(parse_error(error)), + _ => {} + } + } + + Ok(labels) +} + +pub(super) fn insert(xml: &str, axes: &[Axis]) -> DesignspaceResult> { + let labels = axes + .iter() + .filter(|axis| !axis.labels().is_empty()) + .map(|axis| (axis.name(), axis.labels())) + .collect::>(); + if labels.is_empty() { + return Ok(xml.as_bytes().to_vec()); + } + + let mut reader = Reader::from_str(xml); + let mut writer = Writer::new_with_indent(Vec::new(), b' ', 2); + let mut current_labels = None; + + loop { + match reader.read_event() { + Ok(Event::Start(event)) if event.name().as_ref() == b"axis" => { + current_labels = attribute(&reader, &event, b"name")? + .and_then(|name| labels.get(name.as_str()).copied()); + write_event(&mut writer, Event::Start(event))?; + } + Ok(Event::Empty(event)) if event.name().as_ref() == b"axis" => { + let axis_labels = attribute(&reader, &event, b"name")? + .and_then(|name| labels.get(name.as_str()).copied()); + if let Some(axis_labels) = axis_labels { + write_event(&mut writer, Event::Start(event.into_owned()))?; + write_labels(&mut writer, axis_labels)?; + write_event(&mut writer, Event::End(BytesEnd::new("axis")))?; + } else { + write_event(&mut writer, Event::Empty(event))?; + } + } + Ok(Event::End(event)) if event.name().as_ref() == b"axis" => { + if let Some(axis_labels) = current_labels.take() { + write_labels(&mut writer, axis_labels)?; + } + write_event(&mut writer, Event::End(event))?; + } + Ok(Event::Eof) => break, + Ok(event) => write_event(&mut writer, event)?, + Err(error) => return Err(parse_error(error)), + } + } + + Ok(writer.into_inner()) +} + +fn push_label( + reader: &Reader<&[u8]>, + event: &BytesStart, + axis_name: Option<&str>, + labels: &mut HashMap>, +) -> DesignspaceResult<()> { + let axis_name = axis_name.ok_or_else(|| DesignspaceError::ParseDesignspaceXml { + details: "axis label is not nested in an axis".to_string(), + })?; + let name = required_attribute(reader, event, b"name")?; + let value = number_attribute(reader, event, b"uservalue")?.ok_or_else(|| { + DesignspaceError::ParseDesignspaceXml { + details: format!("axis label {name:?} has no uservalue"), + } + })?; + let minimum = number_attribute(reader, event, b"userminimum")?; + let maximum = number_attribute(reader, event, b"usermaximum")?; + let range = match (minimum, maximum) { + (None, None) => None, + (Some(minimum), Some(maximum)) => Some(AxisLabelRange { minimum, maximum }), + _ => { + return Err(DesignspaceError::ParseDesignspaceXml { + details: format!( + "axis label {name:?} must provide both userminimum and usermaximum" + ), + }) + } + }; + let linked_value = number_attribute(reader, event, b"linkeduservalue")?; + let elidable = attribute(reader, event, b"elidable")? + .is_some_and(|value| matches!(value.as_str(), "true" | "1")); + + labels + .entry(axis_name.to_string()) + .or_default() + .push(AxisLabel { + name, + value, + range, + linked_value, + elidable, + }); + Ok(()) +} + +fn write_labels(writer: &mut Writer>, labels: &[AxisLabel]) -> DesignspaceResult<()> { + write_event(writer, Event::Start(BytesStart::new("labels")))?; + for label in labels { + let mut event = BytesStart::new("label"); + let value = label.value.to_string(); + event.push_attribute(("name", label.name.as_str())); + event.push_attribute(("uservalue", value.as_str())); + + let range_values = label + .range + .as_ref() + .map(|range| (range.minimum.to_string(), range.maximum.to_string())); + if let Some((minimum, maximum)) = &range_values { + event.push_attribute(("userminimum", minimum.as_str())); + event.push_attribute(("usermaximum", maximum.as_str())); + } + + let linked_value = label.linked_value.map(|value| value.to_string()); + if let Some(linked_value) = &linked_value { + event.push_attribute(("linkeduservalue", linked_value.as_str())); + } + if label.elidable { + event.push_attribute(("elidable", "true")); + } + + write_event(writer, Event::Empty(event))?; + } + write_event(writer, Event::End(BytesEnd::new("labels"))) +} + +fn required_attribute( + reader: &Reader<&[u8]>, + event: &BytesStart, + name: &[u8], +) -> DesignspaceResult { + attribute(reader, event, name)?.ok_or_else(|| DesignspaceError::ParseDesignspaceXml { + details: format!( + "{} element is missing {}", + String::from_utf8_lossy(event.name().as_ref()), + String::from_utf8_lossy(name) + ), + }) +} + +fn number_attribute( + reader: &Reader<&[u8]>, + event: &BytesStart, + name: &[u8], +) -> DesignspaceResult> { + attribute(reader, event, name)? + .map(|value| { + value + .parse::() + .map_err(|error| DesignspaceError::ParseDesignspaceXml { + details: format!( + "invalid {} value {value:?}: {error}", + String::from_utf8_lossy(name) + ), + }) + }) + .transpose() +} + +fn attribute( + reader: &Reader<&[u8]>, + event: &BytesStart, + name: &[u8], +) -> DesignspaceResult> { + for attribute in event.attributes() { + let attribute = attribute.map_err(parse_error)?; + if attribute.key.as_ref() == name { + return attribute + .decode_and_unescape_value(reader.decoder()) + .map(|value| Some(value.into_owned())) + .map_err(parse_error); + } + } + + Ok(None) +} + +fn write_event(writer: &mut Writer>, event: Event) -> DesignspaceResult<()> { + writer.write_event(event).map_err(parse_error) +} + +fn parse_error(error: impl ToString) -> DesignspaceError { + DesignspaceError::ParseDesignspaceXml { + details: error.to_string(), + } +} diff --git a/crates/shift-backends/src/designspace/error.rs b/crates/shift-backends/src/designspace/error.rs index 4d6c0992..6c09463e 100644 --- a/crates/shift-backends/src/designspace/error.rs +++ b/crates/shift-backends/src/designspace/error.rs @@ -58,6 +58,9 @@ pub enum DesignspaceError { #[error("failed to parse axisless designspace XML: {details}")] ParseAxislessXml { details: String }, + #[error("failed to parse designspace XML: {details}")] + ParseDesignspaceXml { details: String }, + #[error(transparent)] Font(#[from] shift_font::CoreError), } diff --git a/crates/shift-backends/src/designspace/mod.rs b/crates/shift-backends/src/designspace/mod.rs index 5f119886..d8e52e90 100644 --- a/crates/shift-backends/src/designspace/mod.rs +++ b/crates/shift-backends/src/designspace/mod.rs @@ -1,3 +1,4 @@ +mod axis_labels; mod error; mod reader; mod writer; @@ -10,7 +11,10 @@ pub use writer::DesignspaceWriter; mod tests { use super::*; use crate::traits::{FontReader, FontWriter}; - use shift_font::{Contour, Font, Glyph, GlyphLayer, LayerId, PointType}; + use shift_font::{ + Axis, AxisKind, AxisLabel, AxisLabelRange, AxisMapping, AxisMappingPoint, Contour, Font, + Glyph, GlyphLayer, LayerId, Location, PointType, + }; use std::fs; fn test_font() -> Font { @@ -69,4 +73,117 @@ mod tests { let _ = fs::remove_dir_all(&temp_dir); } + + #[test] + fn round_trips_axis_kinds_labels_and_mappings() { + let temp_dir = tempfile::tempdir().unwrap(); + let designspace_path = temp_dir.path().join("Mapped.designspace"); + let mut font = test_font(); + + let mut weight = Axis::weight(); + weight.set_labels(vec![AxisLabel { + name: "Regular".to_string(), + value: 400.0, + range: Some(AxisLabelRange { + minimum: 350.0, + maximum: 450.0, + }), + linked_value: None, + elidable: true, + }]); + let weight_id = weight.id(); + font.add_axis(weight); + + let italic = Axis::discrete_with_id( + shift_font::AxisId::new(), + "ital".to_string(), + "Italic".to_string(), + vec![0.0, 1.0], + 0.0, + ); + let italic_id = italic.id(); + font.add_axis(italic); + + let independent = AxisMapping::new( + "Weight curve".to_string(), + vec![weight_id.clone()], + vec![weight_id.clone()], + vec![ + mapping_point(&[(weight_id.clone(), 100.0)], &[(weight_id.clone(), 80.0)]), + mapping_point(&[(weight_id.clone(), 400.0)], &[(weight_id.clone(), 400.0)]), + mapping_point(&[(weight_id.clone(), 900.0)], &[(weight_id.clone(), 850.0)]), + ], + ); + let mut cross = AxisMapping::new( + "Weight by italic".to_string(), + vec![weight_id.clone(), italic_id.clone()], + vec![weight_id.clone()], + vec![mapping_point( + &[(weight_id.clone(), 850.0), (italic_id.clone(), 1.0)], + &[(weight_id.clone(), 800.0)], + )], + ); + cross.set_description(Some("Italic masters become lighter".to_string())); + font.set_axis_mappings(vec![independent, cross]).unwrap(); + + DesignspaceWriter::new() + .save(&font, designspace_path.to_str().unwrap()) + .unwrap(); + + let xml = fs::read_to_string(&designspace_path).unwrap(); + assert!(xml.contains("format=\"5.2\"")); + assert!(xml.contains("values=\"0 1\"")); + assert!(xml.contains(" AxisMappingPoint { + AxisMappingPoint { + description: None, + input: location(input), + output: location(output), + } + } + + fn location(values: &[(shift_font::AxisId, f64)]) -> Location { + let mut location = Location::new(); + for (axis_id, value) in values { + location.set(axis_id.clone(), *value); + } + location + } } diff --git a/crates/shift-backends/src/designspace/reader.rs b/crates/shift-backends/src/designspace/reader.rs index 967e0d69..0deba3bd 100644 --- a/crates/shift-backends/src/designspace/reader.rs +++ b/crates/shift-backends/src/designspace/reader.rs @@ -1,3 +1,4 @@ +use super::axis_labels; use super::error::{DesignspaceError, DesignspaceResult}; use crate::errors::{FormatBackendError, FormatBackendResult}; use crate::traits::FontReader; @@ -5,8 +6,11 @@ use crate::ufo::UfoReader; use norad::designspace::DesignSpaceDocument; use quick_xml::events::{BytesStart, Event}; use quick_xml::Reader; -use shift_font::{Axis, Component, Font, GlyphLayer, LayerId, Location, Source, SourceId}; -use std::collections::HashMap; +use shift_font::{ + Axis, AxisId, AxisMapping, AxisMappingId, AxisMappingPoint, Component, Font, GlyphLayer, + LayerId, Location, Source, SourceId, +}; +use std::collections::{HashMap, HashSet}; use std::fs; use std::path::Path; @@ -39,6 +43,11 @@ impl DesignspaceReader { .ok_or_else(|| DesignspaceError::MissingParent { path: ds_path.to_path_buf(), })?; + let xml = fs::read_to_string(ds_path).map_err(|source| DesignspaceError::ReadFile { + path: ds_path.to_path_buf(), + source, + })?; + let mut axis_labels = axis_labels::parse(&xml)?; let doc = match DesignSpaceDocument::load(ds_path) { Ok(doc) => doc, @@ -88,17 +97,37 @@ impl DesignspaceReader { // Add axes. for ds_axis in &doc.axes { - let (minimum, maximum) = derive_axis_range(ds_axis); - let mut axis = Axis::new( - ds_axis.tag.clone(), - ds_axis.name.clone(), - minimum, - ds_axis.default as f64, - maximum, - ); + let mut axis = if let Some(values) = &ds_axis.values { + let mut values = values.iter().map(|value| *value as f64).collect::>(); + values.sort_by(f64::total_cmp); + values.dedup(); + Axis::discrete_with_id( + AxisId::new(), + ds_axis.tag.clone(), + ds_axis.name.clone(), + values, + ds_axis.default as f64, + ) + } else { + let (minimum, maximum) = derive_axis_range(ds_axis); + Axis::new( + ds_axis.tag.clone(), + ds_axis.name.clone(), + minimum, + ds_axis.default as f64, + maximum, + ) + }; axis.set_hidden(ds_axis.hidden); + axis.set_labels( + axis_labels + .remove(ds_axis.name.as_str()) + .unwrap_or_default(), + ); + axis.validate()?; font.add_axis(axis); } + font.set_axis_mappings(axis_mappings_from_designspace(&doc, font.axes())?)?; // Register the default source. let default_location = @@ -186,6 +215,107 @@ impl DesignspaceReader { } } +fn axis_mappings_from_designspace( + doc: &DesignSpaceDocument, + axes: &[Axis], +) -> DesignspaceResult> { + let axes_by_name = axes + .iter() + .map(|axis| (axis.name(), axis.id())) + .collect::>(); + let mut mappings = Vec::new(); + + for ds_axis in &doc.axes { + let Some(ds_points) = &ds_axis.map else { + continue; + }; + let Some(axis_id) = axes_by_name.get(ds_axis.name.as_str()).cloned() else { + continue; + }; + let points = ds_points + .iter() + .map(|point| AxisMappingPoint { + description: None, + input: singleton_location(axis_id.clone(), point.input as f64), + output: singleton_location(axis_id.clone(), point.output as f64), + }) + .collect(); + mappings.push(AxisMapping::with_id( + AxisMappingId::new(), + format!("{} mapping", ds_axis.name), + vec![axis_id.clone()], + vec![axis_id], + points, + )); + } + + if let Some(group) = &doc.axis_mappings { + let mut inputs = Vec::new(); + let mut outputs = Vec::new(); + let mut points = Vec::new(); + for entry in &group.mappings { + let input = mapping_location_from_dimensions(&entry.input, &axes_by_name)?; + let output = mapping_location_from_dimensions(&entry.output, &axes_by_name)?; + extend_axis_ids(&mut inputs, &input); + extend_axis_ids(&mut outputs, &output); + points.push(AxisMappingPoint { + description: entry.description.clone(), + input, + output, + }); + } + + if !points.is_empty() { + let name = group + .description + .clone() + .unwrap_or_else(|| "Cross-axis mapping".to_string()); + let mut mapping = + AxisMapping::with_id(AxisMappingId::new(), name, inputs, outputs, points); + mapping.set_description(group.description.clone()); + mappings.push(mapping); + } + } + + Ok(mappings) +} + +fn mapping_location_from_dimensions( + dimensions: &[norad::designspace::Dimension], + axes_by_name: &HashMap<&str, AxisId>, +) -> DesignspaceResult { + let mut location = Location::new(); + for dimension in dimensions { + let Some(axis_id) = axes_by_name.get(dimension.name.as_str()) else { + return Err(DesignspaceError::LoadDesignspace { + path: std::path::PathBuf::new(), + details: format!("mapping references unknown axis {:?}", dimension.name), + }); + }; + let Some(value) = dimension.xvalue.or(dimension.uservalue) else { + continue; + }; + location.set(axis_id.clone(), value as f64); + } + Ok(location) +} + +fn singleton_location(axis_id: AxisId, value: f64) -> Location { + let mut location = Location::new(); + location.set(axis_id, value); + location +} + +fn extend_axis_ids(target: &mut Vec, location: &Location) { + let existing = target.iter().cloned().collect::>(); + target.extend( + location + .iter() + .map(|(axis_id, _)| axis_id.clone()) + .filter(|axis_id| !existing.contains(axis_id)), + ); +} + #[derive(Clone, Debug)] struct AxislessSource { filename: String, diff --git a/crates/shift-backends/src/designspace/writer.rs b/crates/shift-backends/src/designspace/writer.rs index 8cb8fe7d..93147a87 100644 --- a/crates/shift-backends/src/designspace/writer.rs +++ b/crates/shift-backends/src/designspace/writer.rs @@ -1,15 +1,19 @@ +use super::axis_labels; use super::error::{DesignspaceError, DesignspaceResult}; use crate::atomic::write_file_atomic; use crate::errors::{FormatBackendError, FormatBackendResult}; use crate::traits::{FontView, FontWriter}; use crate::ufo::UfoWriter; -use norad::designspace::{Axis as DsAxis, DesignSpaceDocument, Dimension, Source as DsSource}; +use norad::designspace::{ + Axis as DsAxis, AxisMapping as DsAxisMapping, AxisMappingEntry as DsAxisMappingEntry, + AxisMappings as DsAxisMappings, DesignSpaceDocument, Dimension, Source as DsSource, +}; use quick_xml::events::{BytesDecl, BytesEnd, BytesStart, Event}; use quick_xml::Writer; use serde::Serialize; use shift_font::{ - Axis, BinaryData, FeatureData, Font, FontMetadata, FontMetrics, Glyph, Guideline, KerningData, - LibData, Location, Source, SourceId, + Axis, AxisKind, AxisMapping, BinaryData, FeatureData, Font, FontMetadata, FontMetrics, Glyph, + Guideline, KerningData, LibData, Location, Source, SourceId, }; use std::collections::HashSet; use std::fs; @@ -233,18 +237,86 @@ impl DesignspaceWriter { } } - fn axis(axis: &Axis) -> DsAxis { + fn axis(axis: &Axis, mappings: &[AxisMapping]) -> DsAxis { + let map = mappings + .iter() + .find(|mapping| { + mapping.is_independent() && mapping.inputs().first() == Some(&axis.id()) + }) + .map(|mapping| { + mapping + .points() + .iter() + .filter_map(|point| { + Some(DsAxisMapping { + input: point.input.get(&axis.id())? as f32, + output: point.output.get(&axis.id())? as f32, + }) + }) + .collect() + }); + + let (minimum, maximum, values) = match axis.kind() { + AxisKind::Continuous { + minimum, maximum, .. + } => (Some(*minimum as f32), Some(*maximum as f32), None), + AxisKind::Discrete { values, .. } => ( + None, + None, + Some(values.iter().map(|value| *value as f32).collect()), + ), + }; + DsAxis { name: axis.name().to_string(), tag: axis.tag().to_string(), - minimum: Some(axis.minimum() as f32), + minimum, default: axis.default() as f32, - maximum: Some(axis.maximum() as f32), + maximum, + values, + map, hidden: axis.is_hidden(), ..Default::default() } } + fn mapping_location(location: &Location, axes: &[Axis]) -> Vec { + location + .iter() + .filter_map(|(axis_id, value)| { + let axis = axes.iter().find(|axis| axis.id() == *axis_id)?; + Some(Dimension { + name: axis.name().to_string(), + xvalue: Some(*value as f32), + ..Default::default() + }) + }) + .collect() + } + + fn cross_axis_mappings(mappings: &[AxisMapping], axes: &[Axis]) -> Option { + let mapping = mappings.iter().find(|mapping| !mapping.is_independent())?; + let mappings = mapping + .points() + .iter() + .map(|point| DsAxisMappingEntry { + description: point.description.clone(), + input: Self::mapping_location(&point.input, axes), + output: Self::mapping_location(&point.output, axes), + }) + .collect::>(); + + Some(DsAxisMappings { + description: Some( + mapping + .description() + .unwrap_or_else(|| mapping.name()) + .to_string(), + ), + mappings, + }) + } + fn location(location: &Location, axes: &[Axis]) -> Vec { axes.iter() .map(|axis| Dimension { @@ -365,7 +437,11 @@ impl DesignspaceWriter { /// Serializes exactly like norad's `DesignSpaceDocument::save`, but /// routed through a temp-file + fsync + rename so a failed save never /// truncates the existing designspace. - fn save_document_atomic(document: &DesignSpaceDocument, path: &Path) -> DesignspaceResult<()> { + fn save_document_atomic( + document: &DesignSpaceDocument, + axes: &[Axis], + path: &Path, + ) -> DesignspaceResult<()> { let mut xml = String::from("\n"); let mut serializer = quick_xml::se::Serializer::new(&mut xml); serializer.indent(' ', 2); @@ -376,8 +452,9 @@ impl DesignspaceWriter { details: error.to_string(), })?; xml.push('\n'); + let xml = axis_labels::insert(&xml, axes)?; - write_file_atomic(path, xml.as_bytes()).map_err(|source| DesignspaceError::WriteFile { + write_file_atomic(path, &xml).map_err(|source| DesignspaceError::WriteFile { path: path.to_path_buf(), source, }) @@ -451,14 +528,20 @@ impl DesignspaceWriter { }); } + let axis_mappings = Self::cross_axis_mappings(font.axis_mappings(), axes); + let format = if axis_mappings.is_some() { 5.2 } else { 5.0 }; let document = DesignSpaceDocument { - format: 5.0, - axes: axes.iter().map(Self::axis).collect(), + format, + axes: axes + .iter() + .map(|axis| Self::axis(axis, font.axis_mappings())) + .collect(), + axis_mappings, sources, ..Default::default() }; - Self::save_document_atomic(&document, path) + Self::save_document_atomic(&document, axes, path) } } diff --git a/crates/shift-bridge/docs/DOCS.md b/crates/shift-bridge/docs/DOCS.md index 45cf3961..d25384aa 100644 --- a/crates/shift-bridge/docs/DOCS.md +++ b/crates/shift-bridge/docs/DOCS.md @@ -38,6 +38,7 @@ crates/shift-bridge/ - `BridgeError` -- typed bridge error enum converted once at the NAPI boundary. - `NapiAppliedChange` -- replace-grade mutation response returned by apply/undo/redo. - `NapiLayerReplaced` -- NAPI adapter for one replaced glyph layer in an applied change. +- `NapiAxis` / `NapiAxisMapping` -- authoring DTOs used by axis create/update, mapping replacement, and mapped-location queries. ## How it works diff --git a/crates/shift-bridge/dts-header.d.ts b/crates/shift-bridge/dts-header.d.ts index acbceb85..4252ceab 100644 --- a/crates/shift-bridge/dts-header.d.ts +++ b/crates/shift-bridge/dts-header.d.ts @@ -3,6 +3,7 @@ import type { PointId, AnchorId, AxisId, + AxisMappingId, ComponentId, GuidelineId, GlyphId, diff --git a/crates/shift-bridge/index.d.ts b/crates/shift-bridge/index.d.ts index 784b4d16..465191c6 100644 --- a/crates/shift-bridge/index.d.ts +++ b/crates/shift-bridge/index.d.ts @@ -3,6 +3,7 @@ import type { PointId, AnchorId, AxisId, + AxisMappingId, ComponentId, GuidelineId, GlyphId, @@ -48,6 +49,8 @@ export declare class Bridge { getGlyphSnapshots(requests: Array): Array isVariable(): boolean getAxes(): Array + getAxisMappings(): Array + mapLocation(location: NapiLocation): NapiLocation getSources(): Array } @@ -129,6 +132,8 @@ export interface NapiAppliedChange { glyphs?: Array /** Full axes list when font-level axis structure changed; absent otherwise. */ axes?: Array + /** Full mapping list when font-level axis mappings changed; absent otherwise. */ + axisMappings?: Array /** * Full sources list when font-level source structure changed (createAxis * reshapes locations, createSource adds one); absent otherwise. @@ -142,12 +147,45 @@ export interface NapiAxis { id: AxisId tag: string name: string - minimum: number + role: NapiAxisRole + axisType: NapiAxisType + minimum?: number default: number - maximum: number + maximum?: number + values?: Array + labels: Array hidden: boolean } +export interface NapiAxisLabel { + name: string + value: number + minimum?: number + maximum?: number + linkedValue?: number + elidable: boolean +} + +export interface NapiAxisMapping { + id: AxisMappingId + name: string + description?: string + inputs: Array + outputs: Array + points: Array +} + +export interface NapiAxisMappingPoint { + description?: string + input: NapiLocation + output: NapiLocation +} + +export declare const enum NapiAxisRole { + External = 'external', + Internal = 'internal' +} + export interface NapiAxisTent { axisTag: string lower: number @@ -155,6 +193,11 @@ export interface NapiAxisTent { upper: number } +export declare const enum NapiAxisType { + Continuous = 'continuous', + Discrete = 'discrete' +} + export interface NapiBooleanOpIntent { layerId: LayerId contourIdA: ContourId @@ -188,13 +231,7 @@ export interface NapiContourData { * OpenType label and must be unique within the font. */ export interface NapiCreateAxisIntent { - axisId: AxisId - tag: string - name: string - min: number - default: number - max: number - hidden: boolean + axis: NapiAxis } /** @@ -268,7 +305,9 @@ export interface NapiFontIntent { createGlyph?: NapiCreateGlyphIntent updateGlyph?: NapiUpdateGlyphIntent createAxis?: NapiCreateAxisIntent + updateAxis?: NapiUpdateAxisIntent deleteAxis?: NapiDeleteAxisIntent + setAxisMappings?: NapiSetAxisMappingsIntent createSource?: NapiCreateSourceIntent deleteSource?: NapiDeleteSourceIntent createGlyphLayer?: NapiCreateGlyphLayerIntent @@ -438,6 +477,10 @@ export interface NapiReverseContourIntent { contourId: ContourId } +export interface NapiSetAxisMappingsIntent { + mappings: Array +} + export interface NapiSetContourClosedIntent { layerId: LayerId contourId: ContourId @@ -470,6 +513,10 @@ export interface NapiTranslatePointsIntent { dy: number } +export interface NapiUpdateAxisIntent { + axis: NapiAxis +} + /** * Font-level glyph update. The glyph id targets an existing committed glyph; * names are user-editable labels and are not stable identity. diff --git a/crates/shift-bridge/index.js b/crates/shift-bridge/index.js index 9baf0343..2b7da1de 100644 --- a/crates/shift-bridge/index.js +++ b/crates/shift-bridge/index.js @@ -577,4 +577,6 @@ if (!nativeBinding) { module.exports = nativeBinding module.exports.Bridge = nativeBinding.Bridge +module.exports.NapiAxisRole = nativeBinding.NapiAxisRole +module.exports.NapiAxisType = nativeBinding.NapiAxisType module.exports.NapiPointType = nativeBinding.NapiPointType diff --git a/crates/shift-bridge/src/bridge.rs b/crates/shift-bridge/src/bridge.rs index e72b71e1..423548ee 100644 --- a/crates/shift-bridge/src/bridge.rs +++ b/crates/shift-bridge/src/bridge.rs @@ -5,19 +5,20 @@ use napi::{Error, Status}; use napi_derive::napi; use shift_backends::{ExportFormat, FontExportRequest, FontExportResult, FontExporter, FontView}; use shift_font::{ - AnchorId, AnchorSeed, Axis as FontAxis, AxisId, BooleanOp, ContourId, Font, FontChange, - FontIntent, FontIntentSet, Glyph, GlyphId, LayerId, Location as FontLocation, PointId, PointSeed, - SourceId, + AnchorId, AnchorSeed, Axis as FontAxis, AxisId, AxisLabel, AxisLabelRange, + AxisMapping as FontAxisMapping, AxisMappingId, AxisMappingPoint as FontAxisMappingPoint, + AxisRole, BooleanOp, ContourId, Font, FontChange, FontIntent, FontIntentSet, Glyph, GlyphId, + LayerId, Location as FontLocation, PointId, PointSeed, SourceId, }; use shift_wire::{ bridges::napi::{ - NapiAnchorSeed, NapiAppliedChange, NapiAxis, NapiFontIntent, NapiFontMetadata, NapiFontMetrics, - NapiGlyphRecord, NapiGlyphSnapshot, NapiGlyphSnapshotRequest, NapiLayerReplaced, NapiLocation, - NapiPointSeed, NapiSource, + NapiAnchorSeed, NapiAppliedChange, NapiAxis, NapiAxisMapping, NapiAxisRole, NapiAxisType, + NapiFontIntent, NapiFontMetadata, NapiFontMetrics, NapiGlyphRecord, NapiGlyphSnapshot, + NapiGlyphSnapshotRequest, NapiLayerReplaced, NapiLocation, NapiPointSeed, NapiSource, }, interpolation::{build_glyph_variation_data, build_masters, GlyphVariationBuild}, - Axis, FontMetadata, FontMetrics, GlyphChangedEntities, GlyphLayerSnapshot, GlyphRecord, - GlyphSnapshot, GlyphSnapshotRequest, GlyphState, GlyphStructure, Source, + Axis, AxisMapping, FontMetadata, FontMetrics, GlyphChangedEntities, GlyphLayerSnapshot, + GlyphRecord, GlyphSnapshot, GlyphSnapshotRequest, GlyphState, GlyphStructure, Source, }; use shift_workspace::{ FontWorkspace, NewWorkspace, PackageDraft, PackageIdentity, WorkspaceError, WorkspaceSource, @@ -399,6 +400,7 @@ impl Bridge { layers: Vec::new(), glyphs: None, axes: None, + axis_mappings: None, sources: None, dependents: Vec::new(), }); @@ -422,6 +424,7 @@ impl Bridge { fn applied_echo(&self, outcome: shift_font::AppliedIntents) -> errors::Result { let mut glyphs_changed = false; let mut axes_changed = false; + let mut axis_mappings_changed = false; let mut sources_changed = false; for change in &outcome.changes.changes { match change { @@ -431,10 +434,11 @@ impl Bridge { | FontChange::GlyphLayerCreated(_) | FontChange::GlyphLayerDeleted(_) => glyphs_changed = true, // Axis structure reshapes every source location's design space. - FontChange::AxisCreated(_) | FontChange::AxisDeleted(_) => { + FontChange::AxisCreated(_) | FontChange::AxisUpdated(_) | FontChange::AxisDeleted(_) => { axes_changed = true; sources_changed = true; } + FontChange::AxisMappingsUpdated(_) => axis_mappings_changed = true, FontChange::SourceCreated(_) | FontChange::SourceDeleted(_) => sources_changed = true, _ => {} } @@ -469,6 +473,9 @@ impl Bridge { layers, glyphs: glyphs_changed.then(|| self.get_glyphs()).transpose()?, axes: axes_changed.then(|| self.get_axes()).transpose()?, + axis_mappings: axis_mappings_changed + .then(|| self.get_axis_mappings()) + .transpose()?, sources: sources_changed.then(|| self.get_sources()).transpose()?, dependents, }) @@ -557,6 +564,26 @@ impl Bridge { ) } + #[napi] + pub fn get_axis_mappings(&self) -> errors::Result> { + Ok( + self + .font()? + .axis_mappings() + .iter() + .map(AxisMapping::from) + .map(Into::into) + .collect(), + ) + } + + #[napi] + pub fn map_location(&self, location: NapiLocation) -> errors::Result { + let external = map_location(location)?; + let mapped = self.font()?.mapped_location(&external)?; + Ok(shift_wire::Location::from(&mapped).into()) + } + #[napi] pub fn get_sources(&self) -> errors::Result> { Ok( @@ -810,16 +837,15 @@ fn map_intent(intent: NapiFontIntent) -> errors::Result { } "createAxis" => { let payload = intent.create_axis.ok_or_else(|| missing("createAxis"))?; - let mut axis = FontAxis::with_id( - parse::(&payload.axis_id)?, - payload.tag, - payload.name, - payload.min, - payload.default, - payload.max, - ); - axis.set_hidden(payload.hidden); - Ok(FontIntent::CreateAxis { axis }) + Ok(FontIntent::CreateAxis { + axis: map_axis(payload.axis)?, + }) + } + "updateAxis" => { + let payload = intent.update_axis.ok_or_else(|| missing("updateAxis"))?; + Ok(FontIntent::UpdateAxis { + axis: map_axis(payload.axis)?, + }) } "deleteAxis" => { let payload = intent.delete_axis.ok_or_else(|| missing("deleteAxis"))?; @@ -827,6 +853,18 @@ fn map_intent(intent: NapiFontIntent) -> errors::Result { axis_id: parse::(&payload.axis_id)?, }) } + "setAxisMappings" => { + let payload = intent + .set_axis_mappings + .ok_or_else(|| missing("setAxisMappings"))?; + Ok(FontIntent::SetAxisMappings { + mappings: payload + .mappings + .into_iter() + .map(map_axis_mapping) + .collect::>>()?, + }) + } "deleteSource" => { let payload = intent .delete_source @@ -914,15 +952,104 @@ fn map_location(location: NapiLocation) -> errors::Result { Ok(FontLocation::from_map(values)) } +fn map_axis(axis: NapiAxis) -> errors::Result { + let axis_id = parse::(&axis.id)?; + let mut mapped = match axis.axis_type { + NapiAxisType::Continuous => FontAxis::continuous_with_id( + axis_id, + axis.tag, + axis.name, + axis.minimum.ok_or_else(|| BridgeError::InvalidInput { + kind: "continuous axis minimum", + value: "missing".to_string(), + })?, + axis.default, + axis.maximum.ok_or_else(|| BridgeError::InvalidInput { + kind: "continuous axis maximum", + value: "missing".to_string(), + })?, + ), + NapiAxisType::Discrete => FontAxis::discrete_with_id( + axis_id, + axis.tag, + axis.name, + axis.values.ok_or_else(|| BridgeError::InvalidInput { + kind: "discrete axis values", + value: "missing".to_string(), + })?, + axis.default, + ), + }; + mapped.set_role(match axis.role { + NapiAxisRole::External => AxisRole::External, + NapiAxisRole::Internal => AxisRole::Internal, + }); + let mut labels = Vec::new(); + for label in axis.labels { + let range = match (label.minimum, label.maximum) { + (None, None) => None, + (Some(minimum), Some(maximum)) => Some(AxisLabelRange { minimum, maximum }), + _ => { + return Err(BridgeError::InvalidInput { + kind: "axis label range", + value: "minimum and maximum must be provided together".to_string(), + }) + } + }; + labels.push(AxisLabel { + name: label.name, + value: label.value, + range, + linked_value: label.linked_value, + elidable: label.elidable, + }); + } + mapped.set_labels(labels); + mapped.set_hidden(axis.hidden); + mapped.validate()?; + Ok(mapped) +} + +fn map_axis_mapping(mapping: NapiAxisMapping) -> errors::Result { + let mut mapped = FontAxisMapping::with_id( + parse::(&mapping.id)?, + mapping.name, + mapping + .inputs + .iter() + .map(|id| parse::(id)) + .collect::>>()?, + mapping + .outputs + .iter() + .map(|id| parse::(id)) + .collect::>>()?, + mapping + .points + .into_iter() + .map(|point| { + Ok(FontAxisMappingPoint { + description: point.description, + input: map_location(point.input)?, + output: map_location(point.output)?, + }) + }) + .collect::>>()?, + ); + mapped.set_description(mapping.description); + Ok(mapped) +} + #[cfg(test)] mod tests { use super::*; use shift_wire::bridges::napi::{ - NapiAddAnchorsIntent, NapiAddContourIntent, NapiAddPointsIntent, NapiCloneGlyphLayerIntent, - NapiCreateAxisIntent, NapiCreateGlyphIntent, NapiCreateGlyphLayerIntent, - NapiCreateSourceIntent, NapiDeleteAxisIntent, NapiDeleteSourceIntent, NapiGlyphSnapshotRequest, - NapiGlyphState, NapiLocation, NapiMoveAnchorsIntent, NapiMovePointsIntent, NapiPointSeed, - NapiPointType, NapiRemoveAnchorsIntent, NapiRemovePointsIntent, NapiReverseContourIntent, + NapiAddAnchorsIntent, NapiAddContourIntent, NapiAddPointsIntent, NapiAxis, NapiAxisRole, + NapiAxisType, NapiCloneGlyphLayerIntent, NapiCreateAxisIntent, NapiCreateGlyphIntent, + NapiCreateGlyphLayerIntent, NapiCreateSourceIntent, NapiDeleteAxisIntent, + NapiDeleteSourceIntent, NapiGlyphSnapshotRequest, NapiGlyphState, NapiLocation, + NapiMoveAnchorsIntent, NapiMovePointsIntent, NapiPointSeed, NapiPointType, + NapiRemoveAnchorsIntent, NapiRemovePointsIntent, NapiReverseContourIntent, NapiSetContourClosedIntent, NapiSetPointSmoothIntent, NapiSetXAdvanceIntent, NapiTranslatePointsIntent, }; @@ -949,7 +1076,9 @@ mod tests { create_glyph: None, update_glyph: None, create_axis: None, + update_axis: None, delete_axis: None, + set_axis_mappings: None, create_source: None, delete_source: None, create_glyph_layer: None, @@ -1875,13 +2004,19 @@ mod tests { ) -> NapiFontIntent { NapiFontIntent { create_axis: Some(NapiCreateAxisIntent { - axis_id: axis_id.to_string(), - tag: tag.to_string(), - name: name.to_string(), - min, - default, - max, - hidden: false, + axis: NapiAxis { + id: axis_id.to_string(), + tag: tag.to_string(), + name: name.to_string(), + role: NapiAxisRole::External, + axis_type: NapiAxisType::Continuous, + minimum: Some(min), + default, + maximum: Some(max), + values: None, + labels: Vec::new(), + hidden: false, + }, }), ..skeleton_intent("createAxis") } @@ -1935,9 +2070,9 @@ mod tests { assert_eq!(axes.len(), 1); assert_eq!(axes[0].tag, "wght"); assert_eq!(axes[0].name, "Weight"); - assert_eq!(axes[0].minimum, 100.0); + assert_eq!(axes[0].minimum, Some(100.0)); assert_eq!(axes[0].default, 400.0); - assert_eq!(axes[0].maximum, 900.0); + assert_eq!(axes[0].maximum, Some(900.0)); // locations may change shape, so sources ride along assert!(applied.sources.is_some()); assert!(applied.glyphs.is_none()); diff --git a/crates/shift-bridge/src/input.rs b/crates/shift-bridge/src/input.rs index 9d1e10fa..ba244e35 100644 --- a/crates/shift-bridge/src/input.rs +++ b/crates/shift-bridge/src/input.rs @@ -1,7 +1,8 @@ use std::str::FromStr; use shift_font::{ - AnchorId, AxisId, ComponentId, ContourId, GlyphId, GuidelineId, LayerId, PointId, SourceId, + AnchorId, AxisId, AxisMappingId, ComponentId, ContourId, GlyphId, GuidelineId, LayerId, PointId, + SourceId, }; use crate::errors::{BridgeError, BridgeResult}; @@ -41,6 +42,10 @@ impl BridgeParse for AxisId { const KIND: &'static str = "axis ID"; } +impl BridgeParse for AxisMappingId { + const KIND: &'static str = "axis mapping ID"; +} + impl BridgeParse for ComponentId { const KIND: &'static str = "component ID"; } diff --git a/crates/shift-cli/README.md b/crates/shift-cli/README.md index 76f85068..af41dbfd 100644 --- a/crates/shift-cli/README.md +++ b/crates/shift-cli/README.md @@ -9,6 +9,7 @@ The crate builds the `shift-cli` binary. Its first command is `inspect`, which o ```sh cargo run -p shift-cli -- inspect path/to/Family.shift cargo run -p shift-cli -- inspect --view axes path/to/Family.shift +cargo run -p shift-cli -- inspect --view mappings path/to/Family.shift cargo run -p shift-cli -- inspect --view sources path/to/Family.shift cargo run -p shift-cli -- inspect --view layers path/to/Family.shift cargo run -p shift-cli -- inspect --json path/to/Family.shift @@ -20,6 +21,7 @@ Available views: - `summary`: package metadata, counts, and sources - `axes`: variable font axes +- `mappings`: independent and cross-axis mappings - `sources`: design sources and locations - `glyphs`: glyph names, Unicode values, and layer counts - `layers`: glyph layer source bindings and geometry counts diff --git a/crates/shift-cli/src/inspect.rs b/crates/shift-cli/src/inspect.rs index 4b92269f..39c65824 100644 --- a/crates/shift-cli/src/inspect.rs +++ b/crates/shift-cli/src/inspect.rs @@ -10,6 +10,7 @@ pub use report::InspectReport; pub enum InspectView { Summary, Axes, + Mappings, Sources, Glyphs, Layers, @@ -21,7 +22,8 @@ mod tests { use serde_json::json; use shift_font::{ - Axis, AxisId, Contour, Font, Glyph, GlyphLayer, LayerId, Location, Point, Source, SourceId, + Axis, AxisId, AxisMapping, AxisMappingPoint, Contour, Font, Glyph, GlyphLayer, LayerId, + Location, Point, Source, SourceId, }; use shift_source::ShiftSourcePackage; @@ -85,6 +87,38 @@ mod tests { ); } + #[test] + fn mappings_view_reports_mapping_axes_and_points() { + let mut font = sample_font(); + let axis_id = font.axes()[0].id(); + let mut input = Location::new(); + input.set(axis_id.clone(), 900.0); + let mut output = Location::new(); + output.set(axis_id.clone(), 800.0); + font.set_axis_mappings(vec![AxisMapping::new( + "Weight curve".to_string(), + vec![axis_id.clone()], + vec![axis_id], + vec![AxisMappingPoint { + description: None, + input, + output, + }], + )]) + .unwrap(); + let report = InspectReport::from_font(Path::new("/tmp/Dogfood.shift"), &font); + + assert_eq!(report.axis_mappings.len(), 1); + assert_eq!(report.axis_mappings[0].kind, "independent"); + assert_eq!(report.axis_mappings[0].inputs[0].tag, "wght"); + assert_eq!(report.axis_mappings[0].points[0].output[0].value, 800.0); + assert!( + report + .render(InspectView::Mappings, RenderMode::Plain) + .contains("Weight curve") + ); + } + #[test] fn json_output_includes_stable_sections() { let report = InspectReport::from_font(Path::new("/tmp/Dogfood.shift"), &sample_font()); @@ -92,6 +126,7 @@ mod tests { assert_eq!(json["manifest"]["format"], "shift-source"); assert_eq!(json["axes"][0]["tag"], "wght"); + assert_eq!(json["axisMappings"], json!([])); assert_eq!(json["sources"][1]["location"][0]["axisTag"], "wght"); assert_eq!(json["sources"][1]["location"][0]["value"], json!(700.0)); assert_eq!(json["glyphs"][0]["unicodes"][0], "U+0041"); diff --git a/crates/shift-cli/src/inspect/render.rs b/crates/shift-cli/src/inspect/render.rs index 8bcddfd4..3c3fa073 100644 --- a/crates/shift-cli/src/inspect/render.rs +++ b/crates/shift-cli/src/inspect/render.rs @@ -3,7 +3,7 @@ use comfy_table::presets::NOTHING; use comfy_table::{Attribute, Cell, CellAlignment, Color, ContentArrangement, Table}; use super::InspectView; -use super::report::{InspectReport, LocationValue}; +use super::report::{AxisReference, InspectReport, LocationValue}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum RenderMode { @@ -16,6 +16,7 @@ impl InspectReport { match view { InspectView::Summary => self.render_summary(mode), InspectView::Axes => self.render_axes(mode), + InspectView::Mappings => self.render_mappings(mode), InspectView::Sources => self.render_sources(mode), InspectView::Glyphs => self.render_glyphs(mode), InspectView::Layers => self.render_layers(mode), @@ -29,6 +30,7 @@ impl InspectReport { format_kv("schema", &self.manifest.schema_version.to_string(), mode), String::new(), format_count("axes", self.axes.len(), mode), + format_count("mappings", self.axis_mappings.len(), mode), format_count("sources", self.sources.len(), mode), format_count("glyphs", self.glyph_count, mode), ]; @@ -52,9 +54,13 @@ impl InspectReport { header("tag", mode), header("name", mode), header("id", mode), + header("role", mode), + header("kind", mode), header("min", mode), header("default", mode), header("max", mode), + header("values", mode), + header("labels", mode), header("hidden", mode), ]); for axis in &self.axes { @@ -62,17 +68,50 @@ impl InspectReport { accent(&axis.tag, mode), Cell::new(axis.name.clone()), muted(compact_id(&axis.id), mode), + Cell::new(axis.role.clone()), + Cell::new(axis.kind.clone()), number(axis.minimum), number(axis.default), number(axis.maximum), + right(axis.values.as_ref().map_or(0, Vec::len)), + right(axis.labels.len()), right(axis.hidden), ]); } - align_right(&mut table, &[3, 4, 5, 6]); + align_right(&mut table, &[5, 6, 7, 8, 9, 10]); section_with_table("Axes", table, mode) } + fn render_mappings(&self, mode: RenderMode) -> String { + if self.axis_mappings.is_empty() { + return empty_section("Mappings", "No axis mappings", mode); + } + + let mut table = base_table(); + table.set_header(vec![ + header("name", mode), + header("kind", mode), + header("inputs", mode), + header("outputs", mode), + header("points", mode), + header("id", mode), + ]); + for mapping in &self.axis_mappings { + table.add_row(vec![ + Cell::new(mapping.name.clone()), + Cell::new(mapping.kind.clone()), + Cell::new(format_axis_references(&mapping.inputs)), + Cell::new(format_axis_references(&mapping.outputs)), + right(mapping.points.len()), + muted(compact_id(&mapping.id), mode), + ]); + } + align_right(&mut table, &[4]); + + section_with_table("Mappings", table, mode) + } + fn render_sources(&self, mode: RenderMode) -> String { if self.sources.is_empty() { return empty_section("Sources", "No sources", mode); @@ -319,6 +358,17 @@ fn format_location(location: &[LocationValue]) -> String { } } +fn format_axis_references(axes: &[AxisReference]) -> String { + if axes.is_empty() { + return "-".to_string(); + } + + axes.iter() + .map(|axis| axis.tag.as_str()) + .collect::>() + .join(" ") +} + fn format_number(value: f64) -> String { if value.fract().abs() < f64::EPSILON { return format!("{value:.0}"); diff --git a/crates/shift-cli/src/inspect/report.rs b/crates/shift-cli/src/inspect/report.rs index 85cdee1b..2d83b756 100644 --- a/crates/shift-cli/src/inspect/report.rs +++ b/crates/shift-cli/src/inspect/report.rs @@ -4,7 +4,10 @@ use std::path::{Path, PathBuf}; use miette::Diagnostic; use serde::Serialize; -use shift_font::{Axis, AxisId, Font, Glyph, GlyphLayer, Source, SourceId}; +use shift_font::{ + Axis, AxisId, AxisKind, AxisLabel, AxisMapping, AxisMappingPoint, AxisRole, Font, Glyph, + GlyphLayer, Location, Source, SourceId, +}; use shift_source::{FORMAT_ID, SCHEMA_VERSION, ShiftSourcePackage, SourcePackageError}; use thiserror::Error; @@ -49,12 +52,54 @@ pub struct AxisSummary { pub id: String, pub tag: String, pub name: String, + pub role: String, + pub kind: String, pub minimum: f64, pub default: f64, pub maximum: f64, + pub values: Option>, + pub labels: Vec, pub hidden: bool, } +#[derive(Clone, Debug, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisLabelSummary { + pub name: String, + pub value: f64, + pub minimum: Option, + pub maximum: Option, + pub linked_value: Option, + pub elidable: bool, +} + +#[derive(Clone, Debug, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMappingSummary { + pub id: String, + pub name: String, + pub description: Option, + pub kind: String, + pub inputs: Vec, + pub outputs: Vec, + pub points: Vec, +} + +#[derive(Clone, Debug, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMappingPointSummary { + pub description: Option, + pub input: Vec, + pub output: Vec, +} + +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisReference { + pub id: String, + pub tag: String, +} + #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct SourceSummary { @@ -105,6 +150,7 @@ pub struct InspectReport { pub manifest: ManifestSummary, pub metadata: MetadataSummary, pub axes: Vec, + pub axis_mappings: Vec, pub sources: Vec, pub glyph_count: usize, pub glyphs: Vec, @@ -143,6 +189,11 @@ impl InspectReport { display_name: font.metadata().display_name(), }, axes: font.axes().iter().map(AxisSummary::from).collect(), + axis_mappings: font + .axis_mappings() + .iter() + .map(|mapping| AxisMappingSummary::from_mapping(mapping, &axes_by_id)) + .collect(), sources: font .sources() .iter() @@ -160,41 +211,82 @@ impl From<&Axis> for AxisSummary { id: axis.id().to_string(), tag: axis.tag().to_string(), name: axis.name().to_string(), + role: match axis.role() { + AxisRole::External => "external", + AxisRole::Internal => "internal", + } + .to_string(), + kind: match axis.kind() { + AxisKind::Continuous { .. } => "continuous", + AxisKind::Discrete { .. } => "discrete", + } + .to_string(), minimum: axis.minimum(), default: axis.default(), maximum: axis.maximum(), + values: axis.discrete_values().map(<[f64]>::to_vec), + labels: axis.labels().iter().map(AxisLabelSummary::from).collect(), hidden: axis.is_hidden(), } } } +impl From<&AxisLabel> for AxisLabelSummary { + fn from(label: &AxisLabel) -> Self { + Self { + name: label.name.clone(), + value: label.value, + minimum: label.range.as_ref().map(|range| range.minimum), + maximum: label.range.as_ref().map(|range| range.maximum), + linked_value: label.linked_value, + elidable: label.elidable, + } + } +} + +impl AxisMappingSummary { + fn from_mapping(mapping: &AxisMapping, axes_by_id: &HashMap) -> Self { + Self { + id: mapping.id().to_string(), + name: mapping.name().to_string(), + description: mapping.description().map(str::to_string), + kind: if mapping.is_independent() { + "independent" + } else { + "cross-axis" + } + .to_string(), + inputs: axis_references(mapping.inputs(), axes_by_id), + outputs: axis_references(mapping.outputs(), axes_by_id), + points: mapping + .points() + .iter() + .map(|point| AxisMappingPointSummary::from_point(point, axes_by_id)) + .collect(), + } + } +} + +impl AxisMappingPointSummary { + fn from_point(point: &AxisMappingPoint, axes_by_id: &HashMap) -> Self { + Self { + description: point.description.clone(), + input: location_values(&point.input, axes_by_id), + output: location_values(&point.output, axes_by_id), + } + } +} + impl SourceSummary { fn from_source( source: &Source, axes_by_id: &HashMap, default_source_id: &Option, ) -> Self { - let mut location = source - .location() - .iter() - .map(|(axis_id, value)| { - let axis_tag = axes_by_id - .get(axis_id) - .cloned() - .unwrap_or_else(|| axis_id.to_string()); - LocationValue { - axis_id: axis_id.to_string(), - axis_tag, - value: *value, - } - }) - .collect::>(); - location.sort_by(|left, right| left.axis_tag.cmp(&right.axis_tag)); - Self { id: source.id().to_string(), name: source.name().to_string(), - location, + location: location_values(source.location(), axes_by_id), filename: source.filename().map(ToOwned::to_owned), is_default: default_source_id .as_ref() @@ -203,6 +295,28 @@ impl SourceSummary { } } +fn location_values( + location: &Location, + axes_by_id: &HashMap, +) -> Vec { + let mut values = location + .iter() + .map(|(axis_id, value)| { + let axis_tag = axes_by_id + .get(axis_id) + .cloned() + .unwrap_or_else(|| axis_id.to_string()); + LocationValue { + axis_id: axis_id.to_string(), + axis_tag, + value: *value, + } + }) + .collect::>(); + values.sort_by(|left, right| left.axis_tag.cmp(&right.axis_tag)); + values +} + impl GlyphSummary { fn from_glyph(glyph: &Glyph, source_names_by_id: &HashMap) -> Self { let mut layers = glyph @@ -255,6 +369,22 @@ fn axes_by_id(axes: &[Axis]) -> HashMap { .collect() } +fn axis_references( + axis_ids: &[AxisId], + axes_by_id: &HashMap, +) -> Vec { + axis_ids + .iter() + .map(|axis_id| AxisReference { + id: axis_id.to_string(), + tag: axes_by_id + .get(axis_id) + .cloned() + .unwrap_or_else(|| axis_id.to_string()), + }) + .collect() +} + fn source_names_by_id(sources: &[Source]) -> HashMap { sources .iter() diff --git a/crates/shift-font/docs/DOCS.md b/crates/shift-font/docs/DOCS.md index 6ac8e8c9..3595ea82 100644 --- a/crates/shift-font/docs/DOCS.md +++ b/crates/shift-font/docs/DOCS.md @@ -4,7 +4,9 @@ First-class Rust font object model for Shift. ## Object Model -- `Font` owns glyphs, sources, axes, metadata, and font-level data. +- `Font` owns glyphs, sources, axes, axis mappings, metadata, and font-level data. +- `Axis` has stable identity, an external/internal role, a continuous or discrete kind, and optional user-space value labels. +- `AxisMapping` owns an ordered set of mapping points. Independent mappings transform one external axis; the optional cross-axis group maps one design-space location to another. - `Source` is an editable designspace position with a name and location. - `Glyph` is a glyph concept identified by `GlyphId`. - `GlyphLayer` is authored editable data for one glyph at one source. @@ -17,12 +19,13 @@ Stable IDs are identity. Names and Unicode values are editable metadata. - `GlyphId` identifies a glyph. - `SourceId` identifies a source. - `LayerId` identifies a glyph layer: the authored data for one glyph at one source. +- `AxisMappingId` identifies a font-owned mapping independently of its editable name. ## Responsibilities - Own font authoring data structures such as `Font`, `Glyph`, `GlyphLayer`, `Contour`, `Point`, `Source`, and `Axis`. - Keep object-level mutation behavior near the objects it mutates. -- Provide model-native helpers for layer editing, component resolution, variation behavior, and geometry-derived behavior. +- Provide model-native helpers for layer editing, component resolution, variation behavior, axis mapping evaluation, and geometry-derived behavior. - Stay independent of TypeScript, NAPI, and bridge DTOs. ## Boundaries diff --git a/crates/shift-font/src/changes.rs b/crates/shift-font/src/changes.rs index 98fa79b3..f14677da 100644 --- a/crates/shift-font/src/changes.rs +++ b/crates/shift-font/src/changes.rs @@ -1,6 +1,6 @@ use crate::{ - Anchor, AnchorId, Axis, AxisId, Contour, ContourId, Glyph, GlyphId, GlyphLayer, GlyphName, - LayerId, Point, PointId, PointType, Source, SourceId, + Anchor, AnchorId, Axis, AxisId, AxisMapping, Contour, ContourId, Glyph, GlyphId, GlyphLayer, + GlyphName, LayerId, Point, PointId, PointType, Source, SourceId, }; #[derive(Clone, Debug, Default)] @@ -31,7 +31,9 @@ impl From for FontChangeSet { #[derive(Clone, Debug)] pub enum FontChange { AxisCreated(AxisCreated), + AxisUpdated(AxisUpdated), AxisDeleted(AxisDeleted), + AxisMappingsUpdated(AxisMappingsUpdated), SourceCreated(SourceCreated), SourceDeleted(SourceDeleted), GlyphCreated(GlyphCreated), @@ -59,6 +61,16 @@ impl FontChange { Self::AxisDeleted(AxisDeleted { axis_id }) } + pub fn axis_updated(axis: &Axis) -> Self { + Self::AxisUpdated(AxisUpdated { axis: axis.clone() }) + } + + pub fn axis_mappings_updated(mappings: &[AxisMapping]) -> Self { + Self::AxisMappingsUpdated(AxisMappingsUpdated { + mappings: mappings.to_vec(), + }) + } + pub fn source_created(source: &Source) -> Self { Self::SourceCreated(SourceCreated::from(source)) } @@ -164,29 +176,25 @@ impl FontChange { #[derive(Clone, Debug)] pub struct AxisCreated { - pub axis_id: AxisId, - pub tag: String, - pub name: String, - pub minimum: f64, - pub default: f64, - pub maximum: f64, - pub hidden: bool, + pub axis: Axis, } impl From<&Axis> for AxisCreated { fn from(axis: &Axis) -> Self { - Self { - axis_id: axis.id(), - tag: axis.tag().to_string(), - name: axis.name().to_string(), - minimum: axis.minimum(), - default: axis.default(), - maximum: axis.maximum(), - hidden: axis.is_hidden(), - } + Self { axis: axis.clone() } } } +#[derive(Clone, Debug)] +pub struct AxisUpdated { + pub axis: Axis, +} + +#[derive(Clone, Debug)] +pub struct AxisMappingsUpdated { + pub mappings: Vec, +} + #[derive(Clone, Debug)] pub struct SourceCreated { pub source_id: SourceId, diff --git a/crates/shift-font/src/error.rs b/crates/shift-font/src/error.rs index 7ce124c4..3c64ec5d 100644 --- a/crates/shift-font/src/error.rs +++ b/crates/shift-font/src/error.rs @@ -1,4 +1,6 @@ -use crate::{AnchorId, AxisId, ContourId, GlyphId, GlyphName, LayerId, PointId, SourceId}; +use crate::{ + AnchorId, AxisId, AxisMappingId, ContourId, GlyphId, GlyphName, LayerId, PointId, SourceId, +}; #[derive(Debug, thiserror::Error)] pub enum CoreError { @@ -99,6 +101,21 @@ pub enum CoreError { #[error("axis {0} not found")] AxisNotFound(AxisId), + #[error("invalid axis {axis_id}: {message}")] + InvalidAxis { axis_id: AxisId, message: String }, + + #[error("axis mapping {0} already exists")] + DuplicateAxisMappingId(AxisMappingId), + + #[error("axis mapping name {0:?} already exists")] + DuplicateAxisMappingName(String), + + #[error("invalid axis mapping {mapping_id}: {message}")] + InvalidAxisMapping { + mapping_id: AxisMappingId, + message: String, + }, + #[error("invalid source name {0:?}")] InvalidSourceName(String), diff --git a/crates/shift-font/src/intents.rs b/crates/shift-font/src/intents.rs index f4ce6331..1b419b98 100644 --- a/crates/shift-font/src/intents.rs +++ b/crates/shift-font/src/intents.rs @@ -9,8 +9,8 @@ use crate::changes::{AnchorPosition, FontChange, FontChangeSet, PointPosition}; use crate::error::{CoreError, CoreResult}; use crate::ir::{ - Anchor, AnchorId, Axis, AxisId, BooleanOp, Contour, ContourId, Font, Glyph, GlyphId, - GlyphLayer, GlyphName, LayerId, Location, PointId, PointType, Source, SourceId, + Anchor, AnchorId, Axis, AxisId, AxisMapping, BooleanOp, Contour, ContourId, Font, Glyph, + GlyphId, GlyphLayer, GlyphName, LayerId, Location, PointId, PointType, Source, SourceId, }; use crate::layer_edit::BulkNodePositionUpdates; @@ -125,9 +125,15 @@ pub enum FontIntent { CreateAxis { axis: Axis, }, + UpdateAxis { + axis: Axis, + }, DeleteAxis { axis_id: AxisId, }, + SetAxisMappings { + mappings: Vec, + }, DeleteSource { source_id: SourceId, }, @@ -175,7 +181,9 @@ impl FontIntent { Self::CreateGlyph { .. } | Self::UpdateGlyph { .. } | Self::CreateAxis { .. } + | Self::UpdateAxis { .. } | Self::DeleteAxis { .. } + | Self::SetAxisMappings { .. } | Self::DeleteSource { .. } | Self::CreateSource { .. } | Self::CreateGlyphLayer { .. } @@ -297,10 +305,19 @@ impl Font { self.apply_create_axis(axis, changes)?; Ok(Vec::new()) } + FontIntent::UpdateAxis { axis } => { + self.apply_update_axis(axis, changes)?; + Ok(Vec::new()) + } FontIntent::DeleteAxis { axis_id } => { self.apply_delete_axis(axis_id, changes)?; Ok(Vec::new()) } + FontIntent::SetAxisMappings { mappings } => { + self.set_axis_mappings(mappings.clone())?; + changes.push(FontChange::axis_mappings_updated(mappings)); + Ok(Vec::new()) + } FontIntent::DeleteSource { source_id } => { self.apply_delete_source(source_id, changes)?; Ok(Vec::new()) @@ -360,6 +377,7 @@ impl Font { } fn apply_create_axis(&mut self, axis: &Axis, changes: &mut FontChangeSet) -> CoreResult<()> { + axis.validate()?; if self .axes() .iter() @@ -373,13 +391,31 @@ impl Font { Ok(()) } + fn apply_update_axis(&mut self, axis: &Axis, changes: &mut FontChangeSet) -> CoreResult<()> { + if self + .axes() + .iter() + .any(|existing| existing.id() != axis.id() && existing.tag() == axis.tag()) + { + return Err(CoreError::DuplicateAxisTag(axis.tag().to_string())); + } + + self.replace_axis(axis.clone())?; + changes.push(FontChange::axis_updated(axis)); + Ok(()) + } + fn apply_delete_axis( &mut self, axis_id: &AxisId, changes: &mut FontChangeSet, ) -> CoreResult<()> { + let previous_mappings = self.axis_mappings().to_vec(); self.remove_axis(axis_id.clone()) .ok_or_else(|| CoreError::AxisNotFound(axis_id.clone()))?; + if self.axis_mappings() != previous_mappings { + changes.push(FontChange::axis_mappings_updated(self.axis_mappings())); + } changes.push(FontChange::axis_deleted(axis_id.clone())); Ok(()) } @@ -838,7 +874,9 @@ impl Font { FontIntent::CreateGlyph { .. } | FontIntent::UpdateGlyph { .. } | FontIntent::CreateAxis { .. } + | FontIntent::UpdateAxis { .. } | FontIntent::DeleteAxis { .. } + | FontIntent::SetAxisMappings { .. } | FontIntent::DeleteSource { .. } | FontIntent::CreateSource { .. } | FontIntent::CreateGlyphLayer { .. } diff --git a/crates/shift-font/src/ir/axis.rs b/crates/shift-font/src/ir/axis.rs index 9861d8e1..e74bfcfd 100644 --- a/crates/shift-font/src/ir/axis.rs +++ b/crates/shift-font/src/ir/axis.rs @@ -1,16 +1,89 @@ -use crate::entity::AxisId; +use crate::entity::{AxisId, AxisMappingId}; +use crate::error::{CoreError, CoreResult}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum AxisRole { + #[default] + External, + Internal, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "camelCase")] +pub enum AxisKind { + Continuous { + minimum: f64, + default: f64, + maximum: f64, + }, + Discrete { + values: Vec, + default: f64, + }, +} + +impl AxisKind { + pub fn minimum(&self) -> f64 { + match self { + Self::Continuous { minimum, .. } => *minimum, + Self::Discrete { values, default } => { + values.iter().copied().reduce(f64::min).unwrap_or(*default) + } + } + } + + pub fn default(&self) -> f64 { + match self { + Self::Continuous { default, .. } | Self::Discrete { default, .. } => *default, + } + } + + pub fn maximum(&self) -> f64 { + match self { + Self::Continuous { maximum, .. } => *maximum, + Self::Discrete { values, default } => { + values.iter().copied().reduce(f64::max).unwrap_or(*default) + } + } + } + + pub fn values(&self) -> Option<&[f64]> { + match self { + Self::Continuous { .. } => None, + Self::Discrete { values, .. } => Some(values), + } + } +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisLabelRange { + pub minimum: f64, + pub maximum: f64, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisLabel { + pub name: String, + pub value: f64, + pub range: Option, + pub linked_value: Option, + pub elidable: bool, +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Axis { id: AxisId, tag: String, name: String, - minimum: f64, - default: f64, - maximum: f64, + role: AxisRole, + kind: AxisKind, + labels: Vec, hidden: bool, } @@ -26,14 +99,47 @@ impl Axis { minimum: f64, default: f64, maximum: f64, + ) -> Self { + Self::continuous_with_id(id, tag, name, minimum, default, maximum) + } + + pub fn continuous_with_id( + id: AxisId, + tag: String, + name: String, + minimum: f64, + default: f64, + maximum: f64, + ) -> Self { + Self { + id, + tag, + name, + role: AxisRole::External, + kind: AxisKind::Continuous { + minimum, + default, + maximum, + }, + labels: Vec::new(), + hidden: false, + } + } + + pub fn discrete_with_id( + id: AxisId, + tag: String, + name: String, + values: Vec, + default: f64, ) -> Self { Self { id, tag, name, - minimum, - default, - maximum, + role: AxisRole::External, + kind: AxisKind::Discrete { values, default }, + labels: Vec::new(), hidden: false, } } @@ -64,38 +170,136 @@ impl Axis { &self.name } + pub fn role(&self) -> AxisRole { + self.role + } + + pub fn kind(&self) -> &AxisKind { + &self.kind + } + pub fn minimum(&self) -> f64 { - self.minimum + self.kind.minimum() } pub fn default(&self) -> f64 { - self.default + self.kind.default() } pub fn maximum(&self) -> f64 { - self.maximum + self.kind.maximum() + } + + pub fn discrete_values(&self) -> Option<&[f64]> { + self.kind.values() + } + + pub fn labels(&self) -> &[AxisLabel] { + &self.labels } pub fn is_hidden(&self) -> bool { self.hidden } + pub fn set_role(&mut self, role: AxisRole) { + self.role = role; + } + + pub fn set_kind(&mut self, kind: AxisKind) { + self.kind = kind; + } + + pub fn set_labels(&mut self, labels: Vec) { + self.labels = labels; + } + pub fn set_hidden(&mut self, hidden: bool) { self.hidden = hidden; } + pub fn validate(&self) -> CoreResult<()> { + if self.name.trim().is_empty() { + return Err(self.invalid("name must not be blank")); + } + if self.tag.len() != 4 || !self.tag.is_ascii() { + return Err(self.invalid("tag must contain exactly four ASCII characters")); + } + + match &self.kind { + AxisKind::Continuous { + minimum, + default, + maximum, + } => { + if !minimum.is_finite() || !default.is_finite() || !maximum.is_finite() { + return Err(self.invalid("continuous range values must be finite")); + } + if minimum > default || default > maximum { + return Err(self.invalid("expected minimum <= default <= maximum")); + } + } + AxisKind::Discrete { values, default } => { + if values.is_empty() { + return Err(self.invalid("discrete axes require at least one value")); + } + if !default.is_finite() || values.iter().any(|value| !value.is_finite()) { + return Err(self.invalid("discrete values must be finite")); + } + if values.windows(2).any(|pair| pair[0] >= pair[1]) { + return Err(self.invalid("discrete values must be strictly increasing")); + } + if !values.contains(default) { + return Err(self.invalid("default must be one of the discrete values")); + } + } + } + + for label in &self.labels { + if label.name.trim().is_empty() { + return Err(self.invalid("axis label names must not be blank")); + } + if !label.value.is_finite() + || label.linked_value.is_some_and(|value| !value.is_finite()) + { + return Err(self.invalid("axis label values must be finite")); + } + if let Some(range) = &label.range { + if !range.minimum.is_finite() || !range.maximum.is_finite() { + return Err(self.invalid("axis label ranges must be finite")); + } + if range.minimum > label.value || label.value > range.maximum { + return Err(self.invalid("axis label ranges must contain their nominal value")); + } + } + } + + Ok(()) + } + + fn invalid(&self, message: impl Into) -> CoreError { + CoreError::InvalidAxis { + axis_id: self.id(), + message: message.into(), + } + } + pub fn normalize(&self, value: f64) -> f64 { - if value < self.default { - if (self.default - self.minimum).abs() < f64::EPSILON { + let minimum = self.minimum(); + let default = self.default(); + let maximum = self.maximum(); + + if value < default { + if (default - minimum).abs() < f64::EPSILON { 0.0 } else { - (value - self.default) / (self.default - self.minimum) + (value - default) / (default - minimum) } - } else if value > self.default { - if (self.maximum - self.default).abs() < f64::EPSILON { + } else if value > default { + if (maximum - default).abs() < f64::EPSILON { 0.0 } else { - (value - self.default) / (self.maximum - self.default) + (value - default) / (maximum - default) } } else { 0.0 @@ -103,16 +307,181 @@ impl Axis { } pub fn denormalize(&self, value: f64) -> f64 { + let minimum = self.minimum(); + let default = self.default(); + let maximum = self.maximum(); + if value < 0.0 { - self.default + value * (self.default - self.minimum) + default + value * (default - minimum) } else if value > 0.0 { - self.default + value * (self.maximum - self.default) + default + value * (maximum - default) } else { - self.default + default + } + } +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMappingPoint { + pub description: Option, + pub input: Location, + pub output: Location, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMapping { + id: AxisMappingId, + name: String, + description: Option, + inputs: Vec, + outputs: Vec, + points: Vec, +} + +impl AxisMapping { + pub fn new( + name: String, + inputs: Vec, + outputs: Vec, + points: Vec, + ) -> Self { + Self::with_id(AxisMappingId::new(), name, inputs, outputs, points) + } + + pub fn with_id( + id: AxisMappingId, + name: String, + inputs: Vec, + outputs: Vec, + points: Vec, + ) -> Self { + Self { + id, + name, + description: None, + inputs, + outputs, + points, + } + } + + pub fn id(&self) -> AxisMappingId { + self.id.clone() + } + + pub fn name(&self) -> &str { + &self.name + } + + pub fn description(&self) -> Option<&str> { + self.description.as_deref() + } + + pub fn inputs(&self) -> &[AxisId] { + &self.inputs + } + + pub fn outputs(&self) -> &[AxisId] { + &self.outputs + } + + pub fn points(&self) -> &[AxisMappingPoint] { + &self.points + } + + pub fn is_independent(&self) -> bool { + self.inputs.len() == 1 && self.outputs == self.inputs + } + + pub fn set_description(&mut self, description: Option) { + self.description = description; + } + + pub fn validate(&self, axes: &[Axis]) -> CoreResult<()> { + if self.name.trim().is_empty() { + return Err(self.invalid("name must not be blank")); + } + if self.inputs.is_empty() { + return Err(self.invalid("at least one input axis is required")); + } + if self.outputs.is_empty() { + return Err(self.invalid("at least one output axis is required")); + } + if self.points.is_empty() { + return Err(self.invalid("at least one mapping point is required")); + } + if has_duplicates(&self.inputs) || has_duplicates(&self.outputs) { + return Err(self.invalid("input and output axes must be unique")); + } + + for axis_id in self.inputs.iter().chain(&self.outputs) { + if !axes.iter().any(|axis| axis.id() == *axis_id) { + return Err(self.invalid(format!("references unknown axis {axis_id}"))); + } + } + for input_id in &self.inputs { + let axis = axes + .iter() + .find(|axis| axis.id() == *input_id) + .expect("mapping axes were checked above"); + if self.is_independent() && axis.role() != AxisRole::External { + return Err(self.invalid(format!("input axis {input_id} is not external"))); + } + } + + for point in &self.points { + if point.input.iter().next().is_none() || point.output.iter().next().is_none() { + return Err(self.invalid("mapping points require input and output locations")); + } + if self.is_independent() + && (point.input.get(&self.inputs[0]).is_none() + || point.output.get(&self.outputs[0]).is_none()) + { + return Err(self.invalid( + "independent mapping points require explicit input and output values", + )); + } + for (axis_id, value) in point.input.iter() { + if !self.inputs.contains(axis_id) { + return Err( + self.invalid(format!("point input references undeclared axis {axis_id}")) + ); + } + if !value.is_finite() { + return Err(self.invalid("point input values must be finite")); + } + } + for (axis_id, value) in point.output.iter() { + if !self.outputs.contains(axis_id) { + return Err( + self.invalid(format!("point output references undeclared axis {axis_id}")) + ); + } + if !value.is_finite() { + return Err(self.invalid("point output values must be finite")); + } + } + } + + Ok(()) + } + + fn invalid(&self, message: impl Into) -> CoreError { + CoreError::InvalidAxisMapping { + mapping_id: self.id(), + message: message.into(), } } } +fn has_duplicates(ids: &[AxisId]) -> bool { + ids.iter() + .enumerate() + .any(|(index, id)| ids[index + 1..].contains(id)) +} + #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct Location { values: HashMap, @@ -188,6 +557,21 @@ mod tests { assert_eq!(axis.denormalize(1.0), 900.0); } + #[test] + fn discrete_axis_uses_authored_values_for_range() { + let axis = Axis::discrete_with_id( + AxisId::from_raw("italic"), + "ital".to_string(), + "Italic".to_string(), + vec![0.0, 1.0], + 0.0, + ); + + assert_eq!(axis.minimum(), 0.0); + assert_eq!(axis.maximum(), 1.0); + assert_eq!(axis.discrete_values(), Some([0.0, 1.0].as_slice())); + } + #[test] fn location_operations() { let weight = AxisId::from_raw("wght"); diff --git a/crates/shift-font/src/ir/entity.rs b/crates/shift-font/src/ir/entity.rs index fee5c6ae..4182ab66 100644 --- a/crates/shift-font/src/ir/entity.rs +++ b/crates/shift-font/src/ir/entity.rs @@ -128,6 +128,7 @@ typed_id!(LayerId, "layer"); typed_id!(GlyphId, "glyph"); typed_id!(SourceId, "source"); typed_id!(AxisId, "axis"); +typed_id!(AxisMappingId, "axisMapping"); #[cfg(test)] mod tests { diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index 614ce074..6dd5869c 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -1,4 +1,4 @@ -use crate::axis::{Axis, Location}; +use crate::axis::{Axis, AxisMapping, Location}; use crate::binary_data::BinaryData; use crate::entity::{AxisId, GlyphId, LayerId, SourceId}; use crate::error::{CoreError, CoreResult}; @@ -94,6 +94,8 @@ struct FontData { metadata: FontMetadata, metrics: FontMetrics, axes: Vec, + #[serde(default)] + axis_mappings: Vec, sources: Vec, #[serde(default)] default_source_id: Option, @@ -293,6 +295,7 @@ impl Default for Font { metadata: FontMetadata::default(), metrics: FontMetrics::default(), axes: Vec::new(), + axis_mappings: Vec::new(), sources: vec![default_source], default_source_id: Some(default_source_id), glyphs: IndexMap::new(), @@ -322,6 +325,7 @@ impl Font { metadata: FontMetadata::default(), metrics: FontMetrics::default(), axes: Vec::new(), + axis_mappings: Vec::new(), sources: Vec::new(), default_source_id: None, glyphs: IndexMap::new(), @@ -378,10 +382,42 @@ impl Font { self.data_mut().axes.push(axis); } + pub fn replace_axis(&mut self, axis: Axis) -> CoreResult { + axis.validate()?; + let index = self + .axes() + .iter() + .position(|existing| existing.id() == axis.id()) + .ok_or_else(|| CoreError::AxisNotFound(axis.id()))?; + let mut axes = self.axes().to_vec(); + axes[index] = axis.clone(); + validate_axis_mappings(&axes, self.axis_mappings())?; + + let data = self.data_mut(); + Ok(std::mem::replace(&mut data.axes[index], axis)) + } + + pub fn axis_mappings(&self) -> &[AxisMapping] { + &self.data().axis_mappings + } + + pub fn set_axis_mappings(&mut self, mappings: Vec) -> CoreResult<()> { + validate_axis_mappings(self.axes(), &mappings)?; + self.data_mut().axis_mappings = mappings; + Ok(()) + } + + pub fn mapped_location(&self, external: &Location) -> CoreResult { + crate::variation::map_location(external, self.axes(), self.axis_mappings()) + } + pub fn remove_axis(&mut self, axis_id: AxisId) -> Option { let data = self.data_mut(); let index = data.axes.iter().position(|axis| axis.id() == axis_id)?; let axis = data.axes.remove(index); + data.axis_mappings.retain(|mapping| { + !mapping.inputs().contains(&axis_id) && !mapping.outputs().contains(&axis_id) + }); for source in &mut data.sources { source.remove_axis_location(&axis_id); } @@ -703,11 +739,58 @@ impl Font { } } +fn validate_axis_mappings(axes: &[Axis], mappings: &[AxisMapping]) -> CoreResult<()> { + let mut cross_axis_mapping = None; + for mapping in mappings { + mapping.validate(axes)?; + if !mapping.is_independent() { + if cross_axis_mapping.is_some() { + return Err(CoreError::InvalidAxisMapping { + mapping_id: mapping.id(), + message: "only one cross-axis mapping group is supported".to_string(), + }); + } + cross_axis_mapping = Some(mapping.id()); + } + } + + for (index, mapping) in mappings.iter().enumerate() { + if mappings[index + 1..] + .iter() + .any(|other| other.id() == mapping.id()) + { + return Err(CoreError::DuplicateAxisMappingId(mapping.id())); + } + if mappings[index + 1..] + .iter() + .any(|other| other.name() == mapping.name()) + { + return Err(CoreError::DuplicateAxisMappingName( + mapping.name().to_string(), + )); + } + for output in mapping.outputs() { + if mappings[index + 1..].iter().any(|other| { + other.is_independent() == mapping.is_independent() + && other.outputs().contains(output) + }) { + return Err(CoreError::InvalidAxisMapping { + mapping_id: mapping.id(), + message: format!("output axis {output} is controlled by more than one mapping"), + }); + } + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; use crate::{ - test_support::sample_font, Contour, ContourId, GlyphLayer, LayerId, PointId, PointType, + test_support::sample_font, AxisMappingPoint, AxisRole, Contour, ContourId, GlyphLayer, + LayerId, PointId, PointType, }; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -776,6 +859,36 @@ mod tests { assert_eq!(font.default_source().map(Source::name), Some("Regular")); } + #[test] + fn replacing_axis_cannot_invalidate_existing_mapping() { + let mut font = Font::new(); + let axis = Axis::weight(); + let axis_id = axis.id(); + font.add_axis(axis.clone()); + let mut input = Location::new(); + input.set(axis_id.clone(), 900.0); + let mut output = Location::new(); + output.set(axis_id.clone(), 800.0); + font.set_axis_mappings(vec![AxisMapping::new( + "Weight curve".to_string(), + vec![axis_id.clone()], + vec![axis_id.clone()], + vec![AxisMappingPoint { + description: None, + input, + output, + }], + )]) + .unwrap(); + + let mut replacement = axis; + replacement.set_role(AxisRole::Internal); + let result = font.replace_axis(replacement); + + assert!(matches!(result, Err(CoreError::InvalidAxisMapping { .. }))); + assert_eq!(font.axis(axis_id).unwrap().role(), AxisRole::External); + } + #[test] fn font_glyph_operations() { let mut font = Font::new(); diff --git a/crates/shift-font/src/ir/mod.rs b/crates/shift-font/src/ir/mod.rs index f6968ffd..cfcf76b6 100644 --- a/crates/shift-font/src/ir/mod.rs +++ b/crates/shift-font/src/ir/mod.rs @@ -19,14 +19,16 @@ pub mod source; pub mod variation; pub use anchor::Anchor; -pub use axis::{Axis, Location}; +pub use axis::{ + Axis, AxisKind, AxisLabel, AxisLabelRange, AxisMapping, AxisMappingPoint, AxisRole, Location, +}; pub use binary_data::BinaryData; pub use boolean::{boolean, BooleanOp}; pub use component::{Component, DecomposedTransform, Transform}; pub use contour::{Contour, Contours}; pub use entity::{ - AnchorId, AxisId, ComponentId, ContourId, EntityId, GlyphId, GuidelineId, LayerId, PointId, - SourceId, + AnchorId, AxisId, AxisMappingId, ComponentId, ContourId, EntityId, GlyphId, GuidelineId, + LayerId, PointId, SourceId, }; pub use features::FeatureData; pub use font::{Font, FontMetadata}; diff --git a/crates/shift-font/src/ir/variation.rs b/crates/shift-font/src/ir/variation.rs index 1a9b7fc0..f1b03e8f 100644 --- a/crates/shift-font/src/ir/variation.rs +++ b/crates/shift-font/src/ir/variation.rs @@ -1,11 +1,13 @@ +use std::collections::{HashMap, HashSet}; use std::str::FromStr; use fontdrasil::{ coords::{NormalizedCoord, NormalizedLocation}, types::Tag, + variations::{RoundingBehaviour, VariationModel}, }; -use crate::{Axis, Location}; +use crate::{Axis, AxisMapping, AxisRole, CoreError, CoreResult, Location}; pub fn to_fd_location(loc: &Location, axes: &[Axis]) -> NormalizedLocation { let mut result = NormalizedLocation::new(); @@ -22,3 +24,224 @@ pub fn to_fd_location(loc: &Location, axes: &[Axis]) -> NormalizedLocation { result } + +pub fn map_location( + external: &Location, + axes: &[Axis], + mappings: &[AxisMapping], +) -> CoreResult { + let mut mapped = Location::new(); + for axis in axes { + let value = match axis.role() { + AxisRole::External => external.get(&axis.id()).unwrap_or(axis.default()), + AxisRole::Internal => axis.default(), + }; + mapped.set(axis.id(), value); + } + + for mapping in mappings.iter().filter(|mapping| mapping.is_independent()) { + let outputs = evaluate_mapping(mapping, external, axes)?; + for (axis_id, value) in outputs.iter() { + mapped.set(axis_id.clone(), *value); + } + } + + let independently_mapped = mapped.clone(); + for mapping in mappings.iter().filter(|mapping| !mapping.is_independent()) { + let outputs = evaluate_mapping(mapping, &independently_mapped, axes)?; + for (axis_id, value) in outputs.iter() { + mapped.set(axis_id.clone(), *value); + } + } + + Ok(mapped) +} + +fn evaluate_mapping( + mapping: &AxisMapping, + external: &Location, + axes: &[Axis], +) -> CoreResult { + mapping.validate(axes)?; + let input_axes = resolve_axes(mapping.inputs(), axes, mapping)?; + let output_axes = resolve_axes(mapping.outputs(), axes, mapping)?; + let axis_order = input_axes + .iter() + .map(|axis| mapping_tag(axis, mapping)) + .collect::>>()?; + + let mut sample_values: HashMap> = HashMap::new(); + for point in mapping.points() { + let input = normalized_input(&point.input, &input_axes, &axis_order); + let deltas = output_axes + .iter() + .map(|axis| { + let base = point.input.get(&axis.id()).unwrap_or(axis.default()); + let output = point.output.get(&axis.id()).unwrap_or(base); + axis.normalize(output) - axis.normalize(base) + }) + .collect(); + if sample_values.insert(input, deltas).is_some() { + return Err(invalid_mapping( + mapping, + "mapping point inputs must be unique", + )); + } + } + + let default_input: NormalizedLocation = axis_order + .iter() + .map(|tag| (*tag, NormalizedCoord::new(0.0))) + .collect(); + sample_values + .entry(default_input) + .or_insert_with(|| vec![0.0; output_axes.len()]); + + let model = VariationModel::new( + sample_values.keys().cloned().collect::>(), + axis_order, + ); + let deltas = model + .deltas_with_rounding::(&sample_values, RoundingBehaviour::None) + .map_err(|error| invalid_mapping(mapping, error.to_string()))?; + + let target = normalized_input(external, &input_axes, model.axis_order()); + let adjustments = model.interpolate_from_deltas(&target, &deltas); + let mut result = Location::new(); + for (axis, adjustment) in output_axes.iter().zip(adjustments) { + let base = external.get(&axis.id()).unwrap_or(axis.default()); + result.set( + axis.id(), + axis.denormalize(axis.normalize(base) + adjustment), + ); + } + + Ok(result) +} + +fn resolve_axes<'a>( + ids: &[crate::AxisId], + axes: &'a [Axis], + mapping: &AxisMapping, +) -> CoreResult> { + ids.iter() + .map(|axis_id| { + axes.iter() + .find(|axis| axis.id() == *axis_id) + .ok_or_else(|| invalid_mapping(mapping, format!("unknown axis {axis_id}"))) + }) + .collect() +} + +fn mapping_tag(axis: &Axis, mapping: &AxisMapping) -> CoreResult { + Tag::from_str(axis.tag()).map_err(|_| { + invalid_mapping( + mapping, + format!("axis {} has invalid tag {:?}", axis.id(), axis.tag()), + ) + }) +} + +fn normalized_input(location: &Location, axes: &[&Axis], tags: &[Tag]) -> NormalizedLocation { + axes.iter() + .zip(tags) + .map(|(axis, tag)| { + let value = location.get(&axis.id()).unwrap_or(axis.default()); + (*tag, NormalizedCoord::new(axis.normalize(value))) + }) + .collect() +} + +fn invalid_mapping(mapping: &AxisMapping, message: impl Into) -> CoreError { + CoreError::InvalidAxisMapping { + mapping_id: mapping.id(), + message: message.into(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{AxisId, AxisMappingPoint}; + + #[test] + fn maps_independent_axes_before_cross_axis_mappings() { + let weight = Axis::weight(); + let width = Axis::width(); + let mut optical = Axis::continuous_with_id( + AxisId::new(), + "opsz".to_string(), + "Optical size".to_string(), + 8.0, + 12.0, + 72.0, + ); + optical.set_role(AxisRole::Internal); + let axes = vec![weight.clone(), width.clone(), optical.clone()]; + + let independent = AxisMapping::new( + "Weight curve".to_string(), + vec![weight.id()], + vec![weight.id()], + vec![ + point(&[(weight.id(), 100.0)], &[(weight.id(), 100.0)]), + point(&[(weight.id(), 400.0)], &[(weight.id(), 400.0)]), + point(&[(weight.id(), 900.0)], &[(weight.id(), 800.0)]), + ], + ); + let cross = AxisMapping::new( + "Optical compensation".to_string(), + vec![weight.id(), width.id()], + vec![optical.id()], + vec![point( + &[(weight.id(), 800.0), (width.id(), 125.0)], + &[(optical.id(), 72.0)], + )], + ); + + let external = location(&[ + (weight.id(), 900.0), + (width.id(), 125.0), + (optical.id(), 40.0), + ]); + let mapped = map_location(&external, &axes, &[independent, cross]).unwrap(); + + assert!((mapped.get(&weight.id()).unwrap() - 800.0).abs() < 0.001); + assert!((mapped.get(&width.id()).unwrap() - 125.0).abs() < 0.001); + assert!((mapped.get(&optical.id()).unwrap() - 72.0).abs() < 0.001); + } + + #[test] + fn external_locations_cannot_set_internal_axes_directly() { + let mut optical = Axis::continuous_with_id( + AxisId::new(), + "opsz".to_string(), + "Optical size".to_string(), + 8.0, + 12.0, + 72.0, + ); + optical.set_role(AxisRole::Internal); + let external = location(&[(optical.id(), 40.0)]); + + let mapped = map_location(&external, &[optical.clone()], &[]).unwrap(); + + assert_eq!(mapped.get(&optical.id()), Some(12.0)); + } + + fn point(input: &[(AxisId, f64)], output: &[(AxisId, f64)]) -> AxisMappingPoint { + AxisMappingPoint { + description: None, + input: location(input), + output: location(output), + } + } + + fn location(values: &[(AxisId, f64)]) -> Location { + let mut location = Location::new(); + for (axis_id, value) in values { + location.set(axis_id.clone(), *value); + } + location + } +} diff --git a/crates/shift-source/docs/DOCS.md b/crates/shift-source/docs/DOCS.md index c8d8a2b1..77ba8002 100644 --- a/crates/shift-source/docs/DOCS.md +++ b/crates/shift-source/docs/DOCS.md @@ -32,6 +32,7 @@ Family.shift manifest.json font.json axes.json + axis-mappings.json sources.json features.fea # optional verbatim OpenType feature text kerning.json # optional, glyph references use stable glyph ids @@ -48,11 +49,13 @@ Family.shift This crate implements the compact v1 source package contract used by the app and `FontLoader`: -- `axis_*`, `source_*`, `glyph_*`, and layer/component IDs are stable identity. +- `axis_*`, `axisMapping_*`, `source_*`, `glyph_*`, and layer/component IDs are stable identity. - Axis tags and glyph names are labels. They are written for humans and external format interop, but they are not reference keys. -- `axes.json` stores each axis `id` plus its OpenType `tag`, name, range, and - hidden flag. +- `axes.json` stores each axis `id` plus its OpenType `tag`, name, role, + continuous/discrete kind, axis value labels, and hidden flag. +- `axis-mappings.json` stores the ordered font-owned independent mappings and + optional cross-axis mapping group using stable axis IDs. - `sources.json` stores source locations as `axisId -> design-space value`. - Each glyph file is `glyphs/.json`; glyph layers are keyed by `sourceId`. diff --git a/crates/shift-source/src/lib.rs b/crates/shift-source/src/lib.rs index 268c2342..ff183357 100644 --- a/crates/shift-source/src/lib.rs +++ b/crates/shift-source/src/lib.rs @@ -1,8 +1,8 @@ mod package; pub use package::{ - AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, FORMAT_ID, GLYPHS_DIR, - IMAGES_DIR, KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, MODULES_DIR, PackageId, PackageTree, - SCHEMA_VERSION, SOURCES_FILE, ShiftSourcePackage, SourcePackageError, font_to_tree, read_tree, - tree_to_font, write_tree_atomic, + AXES_FILE, AXIS_MAPPINGS_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, + FORMAT_ID, GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, MODULES_DIR, + PackageId, PackageTree, SCHEMA_VERSION, SOURCES_FILE, ShiftSourcePackage, SourcePackageError, + font_to_tree, read_tree, tree_to_font, write_tree_atomic, }; diff --git a/crates/shift-source/src/package.rs b/crates/shift-source/src/package.rs index 83a7c613..cb1b7011 100644 --- a/crates/shift-source/src/package.rs +++ b/crates/shift-source/src/package.rs @@ -9,15 +9,18 @@ use std::{ use serde::{Deserialize, Serialize}; use shift_font::{ - Anchor, Axis, AxisId, Component, ComponentId, Contour, DecomposedTransform, FeatureData, Font, - FontMetadata, FontMetrics, Glyph, GlyphLayer, GlyphName, Guideline, KerningData, KerningPair, - KerningSide, LibData, LibValue, Location, Point, PointType, Source, SourceId, SourceRole, + Anchor, Axis, AxisId, AxisKind, AxisLabel, AxisLabelRange, AxisMapping, AxisMappingId, + AxisMappingPoint, AxisRole, Component, ComponentId, Contour, DecomposedTransform, FeatureData, + Font, FontMetadata, FontMetrics, Glyph, GlyphLayer, GlyphName, Guideline, KerningData, + KerningPair, KerningSide, LibData, LibValue, Location, Point, PointType, Source, SourceId, + SourceRole, }; use zip::{CompressionMethod, ZipArchive, ZipWriter, result::ZipError, write::SimpleFileOptions}; pub const MANIFEST_FILE: &str = "manifest.json"; pub const FONT_FILE: &str = "font.json"; pub const AXES_FILE: &str = "axes.json"; +pub const AXIS_MAPPINGS_FILE: &str = "axis-mappings.json"; pub const SOURCES_FILE: &str = "sources.json"; pub const FEATURES_FILE: &str = "features.fea"; pub const KERNING_FILE: &str = "kerning.json"; @@ -152,16 +155,6 @@ pub enum SourcePackageError { #[error("non-finite number in {field}")] NonFiniteNumber { field: String }, - #[error( - "invalid axis range for {tag}: expected minimum <= default <= maximum, got {minimum} <= {default} <= {maximum}" - )] - InvalidAxisRange { - tag: String, - minimum: f64, - default: f64, - maximum: f64, - }, - #[error("unexpected source package entry: {0}")] UnexpectedEntry(String), @@ -336,10 +329,74 @@ struct AxisDoc { id: String, tag: String, name: String, + role: AxisRoleDoc, + kind: AxisKindDoc, + labels: Vec, + hidden: bool, +} + +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +enum AxisRoleDoc { + #[default] + External, + Internal, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "camelCase")] +enum AxisKindDoc { + Continuous { + minimum: f64, + default: f64, + maximum: f64, + }, + Discrete { + values: Vec, + default: f64, + }, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct AxisLabelDoc { + name: String, + value: f64, + range: Option, + linked_value: Option, + elidable: bool, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct AxisLabelRangeDoc { minimum: f64, - default: f64, maximum: f64, - hidden: bool, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct AxisMappingsDoc { + mappings: Vec, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct AxisMappingDoc { + id: String, + name: String, + description: Option, + inputs: Vec, + outputs: Vec, + points: Vec, +} + +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct AxisMappingPointDoc { + description: Option, + input: BTreeMap, + output: BTreeMap, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -590,10 +647,19 @@ pub fn font_to_tree( .collect::, _>>()?, }; + let axis_mappings_doc = AxisMappingsDoc { + mappings: font + .axis_mappings() + .iter() + .map(AxisMappingDoc::try_from) + .collect::, _>>()?, + }; + let mut tree = vec![ json_entry(MANIFEST_FILE, &manifest)?, json_entry(FONT_FILE, &font_doc)?, json_entry(AXES_FILE, &axes_doc)?, + json_entry(AXIS_MAPPINGS_FILE, &axis_mappings_doc)?, json_entry(SOURCES_FILE, &sources_doc)?, ]; @@ -643,6 +709,7 @@ pub fn tree_to_font(tree: PackageTree) -> Result { let font_doc: FontDoc = take_json(&mut entries, FONT_FILE)?; let axes_doc: AxesDoc = take_json(&mut entries, AXES_FILE)?; + let axis_mappings_doc: AxisMappingsDoc = take_json(&mut entries, AXIS_MAPPINGS_FILE)?; let sources_doc: SourcesDoc = take_json(&mut entries, SOURCES_FILE)?; let features = take_optional_text(&mut entries, FEATURES_FILE)?; let kerning_doc: Option = take_optional_json(&mut entries, KERNING_FILE)?; @@ -680,6 +747,13 @@ pub fn tree_to_font(tree: PackageTree) -> Result { font.add_axis(Axis::try_from(axis_doc)?); } let axis_ids = font.axes().iter().map(Axis::id).collect::>(); + font.set_axis_mappings( + axis_mappings_doc + .mappings + .into_iter() + .map(AxisMapping::try_from) + .collect::, _>>()?, + )?; for source_doc in sources_doc.sources { validate_source_axis_references(&source_doc, &axis_ids)?; @@ -982,19 +1056,6 @@ fn ensure_optional_finite( value.map(|value| ensure_finite(field, value)).transpose() } -fn ensure_axis_range(doc: &AxisDoc) -> Result<(), SourcePackageError> { - if doc.minimum <= doc.default && doc.default <= doc.maximum { - Ok(()) - } else { - Err(SourcePackageError::InvalidAxisRange { - tag: doc.tag.clone(), - minimum: doc.minimum, - default: doc.default, - maximum: doc.maximum, - }) - } -} - fn tmp_path_for(path: &Path) -> PathBuf { let filename = path .file_name() @@ -1755,12 +1816,15 @@ impl TryFrom<&Axis> for AxisDoc { id: axis.id().to_string(), tag: axis.tag().to_string(), name: axis.name().to_string(), - minimum: ensure_finite("axes[].minimum", axis.minimum())?, - default: ensure_finite("axes[].default", axis.default())?, - maximum: ensure_finite("axes[].maximum", axis.maximum())?, + role: axis.role().into(), + kind: AxisKindDoc::try_from(axis.kind())?, + labels: axis + .labels() + .iter() + .map(AxisLabelDoc::try_from) + .collect::, _>>()?, hidden: axis.is_hidden(), }; - ensure_axis_range(&doc)?; Ok(doc) } } @@ -1769,23 +1833,208 @@ impl TryFrom for Axis { type Error = SourcePackageError; fn try_from(doc: AxisDoc) -> Result { - ensure_finite("axes[].minimum", doc.minimum)?; - ensure_finite("axes[].default", doc.default)?; - ensure_finite("axes[].maximum", doc.maximum)?; - ensure_axis_range(&doc)?; - let mut axis = Axis::with_id( - parse_id("axis", &doc.id)?, - doc.tag, - doc.name, - doc.minimum, - doc.default, - doc.maximum, - ); + let axis_id = parse_id("axis", &doc.id)?; + let mut axis = match doc.kind { + AxisKindDoc::Continuous { + minimum, + default, + maximum, + } => Axis::continuous_with_id(axis_id, doc.tag, doc.name, minimum, default, maximum), + AxisKindDoc::Discrete { values, default } => { + Axis::discrete_with_id(axis_id, doc.tag, doc.name, values, default) + } + }; + axis.set_role(doc.role.into()); + axis.set_labels(doc.labels.into_iter().map(AxisLabel::from).collect()); axis.set_hidden(doc.hidden); + axis.validate()?; Ok(axis) } } +impl From for AxisRoleDoc { + fn from(role: AxisRole) -> Self { + match role { + AxisRole::External => Self::External, + AxisRole::Internal => Self::Internal, + } + } +} + +impl From for AxisRole { + fn from(role: AxisRoleDoc) -> Self { + match role { + AxisRoleDoc::External => Self::External, + AxisRoleDoc::Internal => Self::Internal, + } + } +} + +impl TryFrom<&AxisKind> for AxisKindDoc { + type Error = SourcePackageError; + + fn try_from(kind: &AxisKind) -> Result { + Ok(match kind { + AxisKind::Continuous { + minimum, + default, + maximum, + } => Self::Continuous { + minimum: ensure_finite("axes[].kind.minimum", *minimum)?, + default: ensure_finite("axes[].kind.default", *default)?, + maximum: ensure_finite("axes[].kind.maximum", *maximum)?, + }, + AxisKind::Discrete { values, default } => Self::Discrete { + values: values + .iter() + .map(|value| ensure_finite("axes[].kind.values[]", *value)) + .collect::, _>>()?, + default: ensure_finite("axes[].kind.default", *default)?, + }, + }) + } +} + +impl TryFrom<&AxisLabel> for AxisLabelDoc { + type Error = SourcePackageError; + + fn try_from(label: &AxisLabel) -> Result { + Ok(Self { + name: label.name.clone(), + value: ensure_finite("axes[].labels[].value", label.value)?, + range: label + .range + .as_ref() + .map(|range| { + Ok::<_, SourcePackageError>(AxisLabelRangeDoc { + minimum: ensure_finite("axes[].labels[].range.minimum", range.minimum)?, + maximum: ensure_finite("axes[].labels[].range.maximum", range.maximum)?, + }) + }) + .transpose()?, + linked_value: ensure_optional_finite( + "axes[].labels[].linkedValue", + label.linked_value, + )?, + elidable: label.elidable, + }) + } +} + +impl From for AxisLabel { + fn from(label: AxisLabelDoc) -> Self { + Self { + name: label.name, + value: label.value, + range: label.range.map(|range| AxisLabelRange { + minimum: range.minimum, + maximum: range.maximum, + }), + linked_value: label.linked_value, + elidable: label.elidable, + } + } +} + +impl TryFrom<&AxisMapping> for AxisMappingDoc { + type Error = SourcePackageError; + + fn try_from(mapping: &AxisMapping) -> Result { + Ok(Self { + id: mapping.id().to_string(), + name: mapping.name().to_string(), + description: mapping.description().map(str::to_string), + inputs: mapping.inputs().iter().map(ToString::to_string).collect(), + outputs: mapping.outputs().iter().map(ToString::to_string).collect(), + points: mapping + .points() + .iter() + .map(AxisMappingPointDoc::try_from) + .collect::, _>>()?, + }) + } +} + +impl TryFrom for AxisMapping { + type Error = SourcePackageError; + + fn try_from(doc: AxisMappingDoc) -> Result { + let mut mapping = AxisMapping::with_id( + parse_id::("axis mapping", &doc.id)?, + doc.name, + doc.inputs + .iter() + .map(|id| parse_id("axis", id)) + .collect::, _>>()?, + doc.outputs + .iter() + .map(|id| parse_id("axis", id)) + .collect::, _>>()?, + doc.points + .into_iter() + .map(AxisMappingPoint::try_from) + .collect::, _>>()?, + ); + mapping.set_description(doc.description); + Ok(mapping) + } +} + +impl TryFrom<&AxisMappingPoint> for AxisMappingPointDoc { + type Error = SourcePackageError; + + fn try_from(point: &AxisMappingPoint) -> Result { + Ok(Self { + description: point.description.clone(), + input: location_to_doc("axisMappings[].points[].input", &point.input)?, + output: location_to_doc("axisMappings[].points[].output", &point.output)?, + }) + } +} + +impl TryFrom for AxisMappingPoint { + type Error = SourcePackageError; + + fn try_from(point: AxisMappingPointDoc) -> Result { + Ok(Self { + description: point.description, + input: location_from_doc("axisMappings[].points[].input", point.input)?, + output: location_from_doc("axisMappings[].points[].output", point.output)?, + }) + } +} + +fn location_to_doc( + field: &str, + location: &Location, +) -> Result, SourcePackageError> { + location + .iter() + .map(|(axis_id, value)| { + Ok::<_, SourcePackageError>(( + axis_id.to_string(), + ensure_finite(format!("{field}[{axis_id}]"), *value)?, + )) + }) + .collect() +} + +fn location_from_doc( + field: &str, + location: BTreeMap, +) -> Result { + let values = location + .into_iter() + .map(|(axis_id, value)| { + Ok::<_, SourcePackageError>(( + parse_id("axis", &axis_id)?, + ensure_finite(format!("{field}[{axis_id}]"), value)?, + )) + }) + .collect::, _>>()?; + Ok(Location::from_map(values)) +} + impl TryFrom<&Source> for SourceDoc { type Error = SourcePackageError; diff --git a/crates/shift-source/tests/package_test.rs b/crates/shift-source/tests/package_test.rs index 8076384b..61efbdd8 100644 --- a/crates/shift-source/tests/package_test.rs +++ b/crates/shift-source/tests/package_test.rs @@ -1,13 +1,14 @@ use std::fs::{self, File}; use shift_font::{ - Axis, AxisId, Font, KerningPair, LibValue, Location, Source, SourceId, SourceRole, - test_support::sample_font, + Axis, AxisId, AxisLabel, AxisLabelRange, AxisMapping, AxisMappingPoint, AxisRole, Font, + KerningPair, LibValue, Location, Source, SourceId, SourceRole, test_support::sample_font, }; use shift_source::{ - AXES_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, GLYPHS_DIR, IMAGES_DIR, - KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, PackageId, PackageTree, SOURCES_FILE, - ShiftSourcePackage, SourcePackageError, font_to_tree, tree_to_font, write_tree_atomic, + AXES_FILE, AXIS_MAPPINGS_FILE, DATA_DIR, FEATURES_FILE, FONT_FILE, FONTINFO_MODULE_FILE, + GLYPHS_DIR, IMAGES_DIR, KERNING_FILE, LIB_MODULE_FILE, MANIFEST_FILE, PackageId, PackageTree, + SOURCES_FILE, ShiftSourcePackage, SourcePackageError, font_to_tree, tree_to_font, + write_tree_atomic, }; use zip::{CompressionMethod, ZipArchive}; @@ -151,6 +152,54 @@ fn source_tree_round_trip_preserves_whole_font() { assert_eq!(loaded, original); } +#[test] +fn source_tree_round_trip_preserves_axis_authoring_data() { + let mut font = Font::new(); + let mut axis = Axis::weight(); + axis.set_labels(vec![AxisLabel { + name: "Regular".to_string(), + value: 400.0, + range: Some(AxisLabelRange { + minimum: 350.0, + maximum: 450.0, + }), + linked_value: None, + elidable: true, + }]); + let axis_id = axis.id(); + font.add_axis(axis); + let mut optical = Axis::new( + "opsz".to_string(), + "Optical size".to_string(), + 8.0, + 12.0, + 72.0, + ); + optical.set_role(AxisRole::Internal); + font.add_axis(optical); + + let mut input = Location::new(); + input.set(axis_id.clone(), 900.0); + let mut output = Location::new(); + output.set(axis_id.clone(), 800.0); + font.set_axis_mappings(vec![AxisMapping::new( + "Weight curve".to_string(), + vec![axis_id.clone()], + vec![axis_id], + vec![AxisMappingPoint { + description: Some("Bold compression".to_string()), + input, + output, + }], + )]) + .unwrap(); + + let loaded = tree_to_font(font_to_tree(&test_package_id(), &font).unwrap()).unwrap(); + + assert_eq!(loaded.axes(), font.axes()); + assert_eq!(loaded.axis_mappings(), font.axis_mappings()); +} + #[test] fn serializes_same_font_to_byte_identical_tree() { let font = sample_font(); @@ -168,6 +217,7 @@ fn serializes_same_font_to_byte_identical_tree() { MANIFEST_FILE, FONT_FILE, AXES_FILE, + AXIS_MAPPINGS_FILE, SOURCES_FILE, FEATURES_FILE, KERNING_FILE, @@ -255,6 +305,14 @@ fn parses_minimal_handwritten_tree() { br#"{ "axes": [] } +"# + .to_vec(), + ), + ( + AXIS_MAPPINGS_FILE.to_string(), + br#"{ + "mappings": [] +} "# .to_vec(), ), @@ -363,9 +421,14 @@ fn rejects_invalid_axis_ranges_on_load() { "id": "axis_weight", "tag": "wght", "name": "Weight", - "minimum": 900.0, - "default": 400.0, - "maximum": 100.0, + "role": "external", + "kind": { + "type": "continuous", + "minimum": 900.0, + "default": 400.0, + "maximum": 100.0 + }, + "labels": [], "hidden": false } ] @@ -377,7 +440,8 @@ fn rejects_invalid_axis_ranges_on_load() { assert!(matches!( error, - SourcePackageError::InvalidAxisRange { tag, .. } if tag == "wght" + SourcePackageError::Font(shift_font::CoreError::InvalidAxis { axis_id, .. }) + if axis_id == AxisId::from_raw("weight") )); } diff --git a/crates/shift-store/src/change_set.rs b/crates/shift-store/src/change_set.rs index 6263cc2c..3b72ff7a 100644 --- a/crates/shift-store/src/change_set.rs +++ b/crates/shift-store/src/change_set.rs @@ -42,6 +42,7 @@ impl ShiftStore { tx.execute("DELETE FROM source_locations", [])?; tx.execute("DELETE FROM source_lib", [])?; tx.execute("DELETE FROM sources", [])?; + tx.execute("DELETE FROM axis_mappings", [])?; tx.execute("DELETE FROM axes", [])?; upsert_font_info(&tx, font)?; @@ -60,8 +61,9 @@ impl ShiftStore { replace_kerning(&tx, font.kerning())?; for (order_index, axis) in font.axes().iter().enumerate() { - insert_axis_with_order(&tx, &font::AxisCreated::from(axis), order_index as i64)?; + insert_axis(&tx, axis, order_index as i64, false)?; } + replace_axis_mappings(&tx, font.axis_mappings())?; for (order_index, source) in font.sources().iter().enumerate() { upsert_source( @@ -124,7 +126,8 @@ impl ShiftStore { fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), StoreError> { match change { - font::FontChange::AxisCreated(change) => insert_axis_with_order(tx, change, 0), + font::FontChange::AxisCreated(change) => insert_axis(tx, &change.axis, 0, false), + font::FontChange::AxisUpdated(change) => upsert_axis_with_order(tx, &change.axis, 0), font::FontChange::AxisDeleted(change) => { // source_locations cascade from the axis row. let rows_changed = tx.execute( @@ -134,6 +137,9 @@ fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), S require_changed(rows_changed, "axis", change.axis_id.to_string())?; Ok(()) } + font::FontChange::AxisMappingsUpdated(change) => { + replace_axis_mappings(tx, &change.mappings) + } font::FontChange::SourceCreated(change) => { upsert_source( tx, @@ -288,30 +294,74 @@ fn apply_change(tx: &Transaction<'_>, change: &font::FontChange) -> Result<(), S } } -fn insert_axis_with_order( +fn upsert_axis_with_order( + tx: &Transaction<'_>, + axis: &font::Axis, + order_index: i64, +) -> Result<(), StoreError> { + insert_axis(tx, axis, order_index, true) +} + +fn insert_axis( tx: &Transaction<'_>, - axis: &font::AxisCreated, + axis: &font::Axis, order_index: i64, + upsert: bool, ) -> Result<(), StoreError> { + let role = match axis.role() { + font::AxisRole::External => "external", + font::AxisRole::Internal => "internal", + }; + let discrete_values_json = axis + .discrete_values() + .map(serde_json::to_string) + .transpose()?; + let labels_json = serde_json::to_string(axis.labels())?; + let conflict = if upsert { + "ON CONFLICT(id) DO UPDATE SET tag = excluded.tag, name = excluded.name, min_value = excluded.min_value, default_value = excluded.default_value, max_value = excluded.max_value, role = excluded.role, discrete_values_json = excluded.discrete_values_json, labels_json = excluded.labels_json, hidden = excluded.hidden" + } else { + "" + }; + let statement = format!( + "INSERT INTO axes (id, tag, name, min_value, default_value, max_value, role, discrete_values_json, labels_json, hidden, order_index) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11) {conflict}" + ); tx.execute( - " - INSERT INTO axes (id, tag, name, min_value, default_value, max_value, hidden, order_index) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8) - ", + &statement, params![ - axis.axis_id.to_string(), - axis.tag, - axis.name, - axis.minimum, - axis.default, - axis.maximum, - axis.hidden, + axis.id().to_string(), + axis.tag(), + axis.name(), + axis.minimum(), + axis.default(), + axis.maximum(), + role, + discrete_values_json, + labels_json, + axis.is_hidden(), order_index, ], )?; Ok(()) } +fn replace_axis_mappings( + tx: &Transaction<'_>, + mappings: &[font::AxisMapping], +) -> Result<(), StoreError> { + tx.execute("DELETE FROM axis_mappings", [])?; + for (order_index, mapping) in mappings.iter().enumerate() { + tx.execute( + "INSERT INTO axis_mappings (id, mapping_json, order_index) VALUES (?1, ?2, ?3)", + params![ + mapping.id().to_string(), + serde_json::to_string(mapping)?, + order_index as i64, + ], + )?; + } + Ok(()) +} + fn upsert_source_location( tx: &Transaction<'_>, source_id: &font::SourceId, diff --git a/crates/shift-store/src/font_state.rs b/crates/shift-store/src/font_state.rs index ec25bbec..bc4ed5c4 100644 --- a/crates/shift-store/src/font_state.rs +++ b/crates/shift-store/src/font_state.rs @@ -27,6 +27,7 @@ impl ShiftStore { for axis in load_axes(&self.conn)? { font.add_axis(axis); } + font.set_axis_mappings(load_axis_mappings(&self.conn)?)?; for source in load_sources(&self.conn)? { font.add_source(source); @@ -115,22 +116,33 @@ fn load_feature_data(conn: &rusqlite::Connection) -> Result Result, StoreError> { let mut stmt = conn.prepare( " - SELECT id, tag, name, min_value, default_value, max_value, hidden + SELECT id, tag, name, min_value, default_value, max_value, role, + discrete_values_json, labels_json, hidden FROM axes ORDER BY order_index, id ", )?; let rows = stmt.query_map([], |row| { - let mut axis = font::Axis::with_id( - font::AxisId::from_raw(row.get::<_, String>(0)?), - row.get(1)?, - row.get(2)?, - row.get(3)?, - row.get(4)?, - row.get(5)?, - ); - axis.set_hidden(row.get(6)?); + let axis_id = font::AxisId::from_raw(row.get::<_, String>(0)?); + let tag = row.get(1)?; + let name = row.get(2)?; + let default = row.get(4)?; + let values_json = row.get::<_, Option>(7)?; + let mut axis = if let Some(values_json) = values_json { + let values = serde_json::from_str(&values_json).map_err(json_column_error)?; + font::Axis::discrete_with_id(axis_id, tag, name, values, default) + } else { + font::Axis::continuous_with_id(axis_id, tag, name, row.get(3)?, default, row.get(5)?) + }; + axis.set_role(match row.get::<_, String>(6)?.as_str() { + "external" => font::AxisRole::External, + "internal" => font::AxisRole::Internal, + value => return Err(rusqlite::Error::InvalidParameterName(value.to_string())), + }); + let labels_json = row.get::<_, String>(8)?; + axis.set_labels(serde_json::from_str(&labels_json).map_err(json_column_error)?); + axis.set_hidden(row.get(9)?); Ok(axis) })?; @@ -138,6 +150,21 @@ fn load_axes(conn: &rusqlite::Connection) -> Result, StoreError> .map_err(StoreError::from) } +fn load_axis_mappings(conn: &rusqlite::Connection) -> Result, StoreError> { + let mut stmt = + conn.prepare("SELECT mapping_json FROM axis_mappings ORDER BY order_index, id")?; + let rows = stmt.query_map([], |row| { + let json = row.get::<_, String>(0)?; + serde_json::from_str(&json).map_err(json_column_error) + })?; + rows.collect::, _>>() + .map_err(StoreError::from) +} + +fn json_column_error(error: serde_json::Error) -> rusqlite::Error { + rusqlite::Error::FromSqlConversionFailure(0, rusqlite::types::Type::Text, Box::new(error)) +} + fn load_sources(conn: &rusqlite::Connection) -> Result, StoreError> { let mut stmt = conn.prepare( " diff --git a/crates/shift-store/src/schema.rs b/crates/shift-store/src/schema.rs index 735cf780..8fdd655d 100644 --- a/crates/shift-store/src/schema.rs +++ b/crates/shift-store/src/schema.rs @@ -38,6 +38,9 @@ CREATE TABLE IF NOT EXISTS axes ( min_value REAL NOT NULL, default_value REAL NOT NULL, max_value REAL NOT NULL, + role TEXT NOT NULL CHECK (role IN ('external', 'internal')), + discrete_values_json TEXT, + labels_json TEXT NOT NULL DEFAULT '[]', hidden INTEGER NOT NULL DEFAULT 0 CHECK (hidden IN (0, 1)), order_index INTEGER NOT NULL DEFAULT 0 ); @@ -45,6 +48,12 @@ CREATE TABLE IF NOT EXISTS axes ( CREATE UNIQUE INDEX IF NOT EXISTS axes_tag_unique ON axes(tag); +CREATE TABLE IF NOT EXISTS axis_mappings ( + id TEXT PRIMARY KEY, + mapping_json TEXT NOT NULL, + order_index INTEGER NOT NULL +); + CREATE TABLE IF NOT EXISTS sources ( id TEXT PRIMARY KEY, name TEXT, diff --git a/crates/shift-store/src/source.rs b/crates/shift-store/src/source.rs index 62b4f0a8..98d57db8 100644 --- a/crates/shift-store/src/source.rs +++ b/crates/shift-store/src/source.rs @@ -99,9 +99,11 @@ impl ShiftStore { min_value, default_value, max_value, + role, + labels_json, hidden ) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, 'external', '[]', ?7) ", rusqlite::params![ axis.id.as_str(), diff --git a/crates/shift-store/tests/store_test.rs b/crates/shift-store/tests/store_test.rs index 8766655d..d0f207b3 100644 --- a/crates/shift-store/tests/store_test.rs +++ b/crates/shift-store/tests/store_test.rs @@ -1054,6 +1054,47 @@ fn replace_and_load_font_state_preserves_whole_font() { assert_eq!(loaded, original); } +#[test] +fn replace_and_load_font_state_preserves_axis_labels_and_mappings() { + let mut store = ShiftStore::open_memory_for_test().expect("memory store should open"); + let mut font = shift_font::Font::new(); + let mut weight = shift_font::Axis::weight(); + weight.set_labels(vec![shift_font::AxisLabel { + name: "Regular".to_string(), + value: 400.0, + range: Some(shift_font::AxisLabelRange { + minimum: 350.0, + maximum: 450.0, + }), + linked_value: None, + elidable: true, + }]); + let axis_id = weight.id(); + font.add_axis(weight); + + let mut input = shift_font::Location::new(); + input.set(axis_id.clone(), 900.0); + let mut output = shift_font::Location::new(); + output.set(axis_id.clone(), 800.0); + font.set_axis_mappings(vec![shift_font::AxisMapping::new( + "Weight curve".to_string(), + vec![axis_id.clone()], + vec![axis_id], + vec![shift_font::AxisMappingPoint { + description: None, + input, + output, + }], + )]) + .unwrap(); + + store.replace_font_state(&font).expect("replace font state"); + let loaded = store.load_font_state().expect("load font state"); + + assert_eq!(loaded.axes(), font.axes()); + assert_eq!(loaded.axis_mappings(), font.axis_mappings()); +} + #[test] fn refuses_stores_from_newer_schema_versions() { let path = temp_store_path("future"); diff --git a/crates/shift-wire/src/bridges/napi/mod.rs b/crates/shift-wire/src/bridges/napi/mod.rs index 507eec5d..65f4e560 100644 --- a/crates/shift-wire/src/bridges/napi/mod.rs +++ b/crates/shift-wire/src/bridges/napi/mod.rs @@ -5,10 +5,10 @@ use napi_derive::napi; use shift_font::{GlyphId, PointType as IrPointType}; use crate::{ - AnchorData, Axis, AxisTent, ComponentData, ContourData, FontMetadata, FontMetrics, - GlyphChangedEntities, GlyphLayerRecord, GlyphLayerSnapshot, GlyphMaster, GlyphRecord, - GlyphSnapshot, GlyphSnapshotRequest, GlyphState, GlyphStructure, GlyphVariationData, Location, - PointData, PointType, Source, + AnchorData, Axis, AxisLabel, AxisMapping, AxisMappingPoint, AxisTent, ComponentData, + ContourData, FontMetadata, FontMetrics, GlyphChangedEntities, GlyphLayerRecord, + GlyphLayerSnapshot, GlyphMaster, GlyphRecord, GlyphSnapshot, GlyphSnapshotRequest, GlyphState, + GlyphStructure, GlyphVariationData, Location, PointData, PointType, Source, }; #[napi(string_enum = "camelCase")] @@ -117,26 +117,126 @@ pub struct NapiAxis { pub id: String, pub tag: String, pub name: String, - pub minimum: f64, + pub role: NapiAxisRole, + pub axis_type: NapiAxisType, + pub minimum: Option, pub default: f64, - pub maximum: f64, + pub maximum: Option, + pub values: Option>, + pub labels: Vec, pub hidden: bool, } +#[napi(string_enum = "camelCase")] +pub enum NapiAxisRole { + External, + Internal, +} + +#[napi(string_enum = "camelCase")] +pub enum NapiAxisType { + Continuous, + Discrete, +} + +#[napi(object)] +pub struct NapiAxisLabel { + pub name: String, + pub value: f64, + pub minimum: Option, + pub maximum: Option, + pub linked_value: Option, + pub elidable: bool, +} + impl From for NapiAxis { fn from(axis: Axis) -> Self { Self { id: axis.id.to_string(), tag: axis.tag, name: axis.name, + role: match axis.role.as_str() { + "internal" => NapiAxisRole::Internal, + _ => NapiAxisRole::External, + }, + axis_type: match axis.axis_type.as_str() { + "discrete" => NapiAxisType::Discrete, + _ => NapiAxisType::Continuous, + }, minimum: axis.minimum, default: axis.default, maximum: axis.maximum, + values: axis.values, + labels: axis.labels.into_iter().map(Into::into).collect(), hidden: axis.hidden, } } } +impl From for NapiAxisLabel { + fn from(label: AxisLabel) -> Self { + Self { + name: label.name, + value: label.value, + minimum: label.minimum, + maximum: label.maximum, + linked_value: label.linked_value, + elidable: label.elidable, + } + } +} + +#[napi(object)] +pub struct NapiAxisMappingPoint { + pub description: Option, + pub input: NapiLocation, + pub output: NapiLocation, +} + +#[napi(object)] +pub struct NapiAxisMapping { + #[napi(ts_type = "AxisMappingId")] + pub id: String, + pub name: String, + pub description: Option, + #[napi(ts_type = "Array")] + pub inputs: Vec, + #[napi(ts_type = "Array")] + pub outputs: Vec, + pub points: Vec, +} + +impl From for NapiAxisMappingPoint { + fn from(point: AxisMappingPoint) -> Self { + Self { + description: point.description, + input: point.input.into(), + output: point.output.into(), + } + } +} + +impl From for NapiAxisMapping { + fn from(mapping: AxisMapping) -> Self { + Self { + id: mapping.id.to_string(), + name: mapping.name, + description: mapping.description, + inputs: mapping + .inputs + .into_iter() + .map(|id| id.to_string()) + .collect(), + outputs: mapping + .outputs + .into_iter() + .map(|id| id.to_string()) + .collect(), + points: mapping.points.into_iter().map(Into::into).collect(), + } + } +} + #[napi(object)] pub struct NapiSource { #[napi(ts_type = "SourceId")] @@ -568,7 +668,9 @@ pub struct NapiFontIntent { pub create_glyph: Option, pub update_glyph: Option, pub create_axis: Option, + pub update_axis: Option, pub delete_axis: Option, + pub set_axis_mappings: Option, pub create_source: Option, pub delete_source: Option, pub create_glyph_layer: Option, @@ -600,18 +702,21 @@ pub struct NapiUpdateGlyphIntent { pub new_unicodes: Vec, } +#[napi(object)] +pub struct NapiUpdateAxisIntent { + pub axis: NapiAxis, +} + +#[napi(object)] +pub struct NapiSetAxisMappingsIntent { + pub mappings: Vec, +} + /// Font-level axis creation. The axis id is client-minted; the tag is an /// OpenType label and must be unique within the font. #[napi(object)] pub struct NapiCreateAxisIntent { - #[napi(ts_type = "AxisId")] - pub axis_id: String, - pub tag: String, - pub name: String, - pub min: f64, - pub default: f64, - pub max: f64, - pub hidden: bool, + pub axis: NapiAxis, } /// Font-level axis deletion. Removing an axis also reshapes source locations. @@ -683,6 +788,8 @@ pub struct NapiAppliedChange { pub glyphs: Option>, /// Full axes list when font-level axis structure changed; absent otherwise. pub axes: Option>, + /// Full mapping list when font-level axis mappings changed; absent otherwise. + pub axis_mappings: Option>, /// Full sources list when font-level source structure changed (createAxis /// reshapes locations, createSource adds one); absent otherwise. pub sources: Option>, diff --git a/crates/shift-wire/src/lib.rs b/crates/shift-wire/src/lib.rs index ad605c30..42937aa0 100644 --- a/crates/shift-wire/src/lib.rs +++ b/crates/shift-wire/src/lib.rs @@ -7,8 +7,9 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use shift_font::{ - Anchor as IrAnchor, AnchorId, Axis as IrAxis, AxisId, Component as IrComponent, ComponentId, - Contour as IrContour, ContourId, DecomposedTransform as IrTransform, + Anchor as IrAnchor, AnchorId, Axis as IrAxis, AxisId, AxisKind as IrAxisKind, + AxisMapping as IrAxisMapping, AxisMappingId, AxisRole as IrAxisRole, Component as IrComponent, + ComponentId, Contour as IrContour, ContourId, DecomposedTransform as IrTransform, FontMetadata as IrFontMetadata, FontMetrics as IrFontMetrics, Glyph as IrGlyph, GlyphId, GlyphLayer, GlyphName, GuidelineId, LayerId, Location as IrLocation, Point as IrPoint, PointId, PointType as IrPointType, Source as IrSource, SourceId, @@ -110,26 +111,104 @@ pub struct Axis { pub id: AxisId, pub tag: String, pub name: String, - pub minimum: f64, + pub role: String, + pub axis_type: String, + pub minimum: Option, pub default: f64, - pub maximum: f64, + pub maximum: Option, + pub values: Option>, + pub labels: Vec, pub hidden: bool, } +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisLabel { + pub name: String, + pub value: f64, + pub minimum: Option, + pub maximum: Option, + pub linked_value: Option, + pub elidable: bool, +} + impl From<&IrAxis> for Axis { fn from(axis: &IrAxis) -> Self { Self { id: axis.id(), tag: axis.tag().to_string(), name: axis.name().to_string(), - minimum: axis.minimum(), + role: match axis.role() { + IrAxisRole::External => "external", + IrAxisRole::Internal => "internal", + } + .to_string(), + axis_type: match axis.kind() { + IrAxisKind::Continuous { .. } => "continuous", + IrAxisKind::Discrete { .. } => "discrete", + } + .to_string(), + minimum: matches!(axis.kind(), IrAxisKind::Continuous { .. }).then(|| axis.minimum()), default: axis.default(), - maximum: axis.maximum(), + maximum: matches!(axis.kind(), IrAxisKind::Continuous { .. }).then(|| axis.maximum()), + values: axis.discrete_values().map(<[f64]>::to_vec), + labels: axis + .labels() + .iter() + .map(|label| AxisLabel { + name: label.name.clone(), + value: label.value, + minimum: label.range.as_ref().map(|range| range.minimum), + maximum: label.range.as_ref().map(|range| range.maximum), + linked_value: label.linked_value, + elidable: label.elidable, + }) + .collect(), hidden: axis.is_hidden(), } } } +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMappingPoint { + pub description: Option, + pub input: Location, + pub output: Location, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AxisMapping { + pub id: AxisMappingId, + pub name: String, + pub description: Option, + pub inputs: Vec, + pub outputs: Vec, + pub points: Vec, +} + +impl From<&IrAxisMapping> for AxisMapping { + fn from(mapping: &IrAxisMapping) -> Self { + Self { + id: mapping.id(), + name: mapping.name().to_string(), + description: mapping.description().map(str::to_string), + inputs: mapping.inputs().to_vec(), + outputs: mapping.outputs().to_vec(), + points: mapping + .points() + .iter() + .map(|point| AxisMappingPoint { + description: point.description.clone(), + input: (&point.input).into(), + output: (&point.output).into(), + }) + .collect(), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Source { diff --git a/crates/shift-workspace/src/ledger.rs b/crates/shift-workspace/src/ledger.rs index 6a42631e..17b2f934 100644 --- a/crates/shift-workspace/src/ledger.rs +++ b/crates/shift-workspace/src/ledger.rs @@ -7,7 +7,7 @@ //! a renderer reload (it lives with the workspace process), not a utility //! crash; a SQLite ledger table is the later upgrade if that ever matters. -use shift_font::{Axis, Glyph, GlyphId, GlyphLayer, GlyphName, Source, SourceId}; +use shift_font::{Axis, AxisMapping, Glyph, GlyphId, GlyphLayer, GlyphName, Source, SourceId}; /// Generous bound so a marathon session cannot grow memory unboundedly; /// oldest entries fall off first. @@ -31,6 +31,10 @@ pub enum LedgerStep { /// restores them too. pre_locations: Vec<(SourceId, f64)>, }, + AxisMappings { + pre: Vec, + post: Vec, + }, /// Source existence. Sparse glyph-layer existence is represented by /// separate [`LedgerStep::GlyphLayer`] entries. Source { diff --git a/crates/shift-workspace/src/workspace.rs b/crates/shift-workspace/src/workspace.rs index a99de93d..bb1be1a0 100644 --- a/crates/shift-workspace/src/workspace.rs +++ b/crates/shift-workspace/src/workspace.rs @@ -221,6 +221,7 @@ impl FontWorkspace { let mut pre_layers: Vec = Vec::new(); let mut pre_sources: Vec = Vec::new(); let mut pre_axes: Vec = Vec::new(); + let mut pre_axis_mappings: Option> = None; let mut pre_axis_locations: Vec<(AxisId, SourceId, f64)> = Vec::new(); for intent in &set.intents { let Some(layer_id) = intent.layer_id() else { @@ -230,6 +231,7 @@ impl FontWorkspace { &mut pre_layers, &mut pre_sources, &mut pre_axes, + &mut pre_axis_mappings, &mut pre_axis_locations, ); continue; @@ -257,6 +259,7 @@ impl FontWorkspace { &pre_layers, &pre_sources, &pre_axes, + pre_axis_mappings.as_deref(), &pre_axis_locations, &outcome, ); @@ -271,6 +274,7 @@ impl FontWorkspace { pre_layers: &[PreLayer], pre_sources: &[Source], pre_axes: &[Axis], + pre_axis_mappings: Option<&[shift_font::AxisMapping]>, pre_axis_locations: &[(AxisId, SourceId, f64)], outcome: &AppliedIntents, ) -> Vec { @@ -310,10 +314,18 @@ impl FontWorkspace { .font .axes() .iter() - .find(|axis| axis.id() == change.axis_id) + .find(|axis| axis.id() == change.axis.id()) .cloned(), pre_locations: Vec::new(), }), + FontChange::AxisUpdated(change) => steps.push(LedgerStep::Axis { + pre: pre_axes + .iter() + .find(|axis| axis.id() == change.axis.id()) + .cloned(), + post: Some(change.axis.clone()), + pre_locations: Vec::new(), + }), FontChange::AxisDeleted(change) => steps.push(LedgerStep::Axis { pre: pre_axes .iter() @@ -326,6 +338,12 @@ impl FontWorkspace { .map(|(_, source_id, value)| (source_id.clone(), *value)) .collect(), }), + FontChange::AxisMappingsUpdated(change) => { + steps.push(LedgerStep::AxisMappings { + pre: pre_axis_mappings.unwrap_or_default().to_vec(), + post: change.mappings.clone(), + }); + } FontChange::GlyphIdentityChanged(change) => { steps.push(LedgerStep::GlyphIdentity { glyph_id: change.glyph_id.clone(), @@ -476,6 +494,10 @@ impl FontWorkspace { let (from, to) = side.orient(pre, post); replay_axis(font, from, to, &pre_locations, &mut changes)?; } + LedgerStep::AxisMappings { pre, post } => { + let (from, to) = side.orient(pre, post); + replay_axis_mappings(font, from, to, &mut changes)?; + } LedgerStep::Source { pre, post } => { let (from, to) = side.orient(pre, post); replay_source(font, from, to, &mut changes)?; @@ -684,6 +706,7 @@ fn capture_font_level_pre_state( pre_layers: &mut Vec, pre_sources: &mut Vec, pre_axes: &mut Vec, + pre_axis_mappings: &mut Option>, pre_axis_locations: &mut Vec<(AxisId, SourceId, f64)>, ) { match intent { @@ -710,7 +733,20 @@ fn capture_font_level_pre_state( }); } } + FontIntent::UpdateAxis { axis } => { + let axis_id = axis.id(); + if pre_axes.iter().any(|axis| axis.id() == axis_id) { + return; + } + let Some(axis) = font.axes().iter().find(|axis| axis.id() == axis_id) else { + return; + }; + pre_axes.push(axis.clone()); + } FontIntent::DeleteAxis { axis_id } => { + if pre_axis_mappings.is_none() { + *pre_axis_mappings = Some(font.axis_mappings().to_vec()); + } if pre_axes.iter().any(|axis| axis.id() == *axis_id) { return; } @@ -725,6 +761,11 @@ fn capture_font_level_pre_state( } } } + FontIntent::SetAxisMappings { .. } => { + if pre_axis_mappings.is_none() { + *pre_axis_mappings = Some(font.axis_mappings().to_vec()); + } + } _ => {} } } @@ -819,6 +860,14 @@ fn replay_axis( pre_locations: &[(SourceId, f64)], changes: &mut FontChangeSet, ) -> Result<(), WorkspaceError> { + if let (Some(from), Some(to)) = (from.as_ref(), to.as_ref()) + && from.id() == to.id() + { + font.replace_axis(to.clone())?; + changes.push(FontChange::axis_updated(to)); + return Ok(()); + } + if let Some(axis) = from { font.remove_axis(axis.id()) .ok_or_else(|| CoreError::AxisNotFound(axis.id()))?; @@ -850,6 +899,17 @@ fn replay_axis( Ok(()) } +fn replay_axis_mappings( + font: &mut shift_font::Font, + _from: Vec, + to: Vec, + changes: &mut FontChangeSet, +) -> Result<(), WorkspaceError> { + font.set_axis_mappings(to.clone())?; + changes.push(FontChange::axis_mappings_updated(&to)); + Ok(()) +} + fn replay_glyph_identity( font: &mut shift_font::Font, glyph_id: GlyphId, diff --git a/crates/shift-workspace/tests/workspace_test.rs b/crates/shift-workspace/tests/workspace_test.rs index fddcb0af..2b085687 100644 --- a/crates/shift-workspace/tests/workspace_test.rs +++ b/crates/shift-workspace/tests/workspace_test.rs @@ -1186,7 +1186,7 @@ fn delete_axis_undo_redo_restores_full_axis_definition() { let undone = workspace.undo().unwrap().expect("deleteAxis should undo"); assert!(undone.changes.changes.iter().any(|change| matches!( change, - FontChange::AxisCreated(change) if change.axis_id == axis_id + FontChange::AxisCreated(change) if change.axis.id() == axis_id ))); assert_eq!(workspace.font().axes(), std::slice::from_ref(&axis)); diff --git a/packages/types/docs/DOCS.md b/packages/types/docs/DOCS.md index a051ddff..7934a8fa 100644 --- a/packages/types/docs/DOCS.md +++ b/packages/types/docs/DOCS.md @@ -7,7 +7,7 @@ Shared DTO TypeScript types for Shift. This package owns branded IDs and bridge - **Architecture Invariant: CRITICAL:** `src/bridge/generated.ts` is generated from `crates/shift-bridge/index.d.ts` by `scripts/generate-bridge-types.mjs`. Never edit it manually. - **Architecture Invariant: CRITICAL:** `@shift/types` is the canonical TypeScript DTO facade for the native bridge. It strips `Napi*` prefixes and exports type-only DTOs. - **Architecture Invariant:** Editor/domain snapshot types do not live here. -- **Architecture Invariant:** Entity IDs are branded string types. TypeScript mints IDs for synchronous create intents where the renderer must know identity immediately (for example `GlyphId`, `AxisId`, point/contour/anchor IDs); Rust validates and honors those IDs. Use `as*Id()` helpers to cast raw bridge strings into branded types. +- **Architecture Invariant:** Entity IDs are branded string types. TypeScript mints IDs for synchronous create intents where the renderer must know identity immediately (for example `GlyphId`, `AxisId`, `AxisMappingId`, and point/contour/anchor IDs); Rust validates and honors those IDs. Use `as*Id()` helpers to cast raw bridge strings into branded types. - **Architecture Invariant:** This package ships raw `.ts` source. `package.json` points `main` and `types` directly at `src/index.ts`. ## Codemap @@ -31,6 +31,7 @@ Import from `@shift/types`. - `PackageIdentity` / `PackageDraft` -- bridge DTOs used by the desktop utility process to inspect package source identity and working-store ownership. - `GlyphStructure` -- stable glyph structure: contours, anchors, components. - `AppliedChange` -- replace-grade mutation response returned by apply/undo/redo. +- `Axis` / `AxisMapping` -- generated variation authoring DTOs, keyed by branded axis and mapping IDs. - `LayerReplaced` -- one replaced glyph layer in an applied change. - `PointType` -- bridge point type union. diff --git a/packages/types/src/bridge/generated.ts b/packages/types/src/bridge/generated.ts index 693e8e6d..0958c5c8 100644 --- a/packages/types/src/bridge/generated.ts +++ b/packages/types/src/bridge/generated.ts @@ -3,6 +3,7 @@ import type { PointId, AnchorId, AxisId, + AxisMappingId, ComponentId, GuidelineId, GlyphId, @@ -53,6 +54,8 @@ export interface BridgeApi { getGlyphSnapshots(requests: Array): Array isVariable(): boolean getAxes(): Array + getAxisMappings(): Array + mapLocation(location: Location): Location getSources(): Array } @@ -134,6 +137,8 @@ export interface AppliedChange { glyphs?: Array /** Full axes list when font-level axis structure changed; absent otherwise. */ axes?: Array + /** Full mapping list when font-level axis mappings changed; absent otherwise. */ + axisMappings?: Array /** * Full sources list when font-level source structure changed (createAxis * reshapes locations, createSource adds one); absent otherwise. @@ -147,12 +152,42 @@ export interface Axis { id: AxisId tag: string name: string - minimum: number + role: AxisRole + axisType: AxisType + minimum?: number default: number - maximum: number + maximum?: number + values?: Array + labels: Array hidden: boolean } +export interface AxisLabel { + name: string + value: number + minimum?: number + maximum?: number + linkedValue?: number + elidable: boolean +} + +export interface AxisMapping { + id: AxisMappingId + name: string + description?: string + inputs: Array + outputs: Array + points: Array +} + +export interface AxisMappingPoint { + description?: string + input: Location + output: Location +} + +export type AxisRole = "external" | "internal"; + export interface AxisTent { axisTag: string lower: number @@ -160,6 +195,8 @@ export interface AxisTent { upper: number } +export type AxisType = "continuous" | "discrete"; + export interface BooleanOpIntent { layerId: LayerId contourIdA: ContourId @@ -193,13 +230,7 @@ export interface ContourData { * OpenType label and must be unique within the font. */ export interface CreateAxisIntent { - axisId: AxisId - tag: string - name: string - min: number - default: number - max: number - hidden: boolean + axis: Axis } /** @@ -273,7 +304,9 @@ export interface FontIntent { createGlyph?: CreateGlyphIntent updateGlyph?: UpdateGlyphIntent createAxis?: CreateAxisIntent + updateAxis?: UpdateAxisIntent deleteAxis?: DeleteAxisIntent + setAxisMappings?: SetAxisMappingsIntent createSource?: CreateSourceIntent deleteSource?: DeleteSourceIntent createGlyphLayer?: CreateGlyphLayerIntent @@ -440,6 +473,10 @@ export interface ReverseContourIntent { contourId: ContourId } +export interface SetAxisMappingsIntent { + mappings: Array +} + export interface SetContourClosedIntent { layerId: LayerId contourId: ContourId @@ -472,6 +509,10 @@ export interface TranslatePointsIntent { dy: number } +export interface UpdateAxisIntent { + axis: Axis +} + /** * Font-level glyph update. The glyph id targets an existing committed glyph; * names are user-editable labels and are not stable identity. diff --git a/packages/types/src/bridge/index.ts b/packages/types/src/bridge/index.ts index a471a1e5..6e453ded 100644 --- a/packages/types/src/bridge/index.ts +++ b/packages/types/src/bridge/index.ts @@ -18,7 +18,12 @@ export type { SetXAdvanceIntent, TranslatePointsIntent, Axis, + AxisLabel, + AxisMapping, + AxisMappingPoint, + AxisRole, AxisTent, + AxisType, BridgeApi, ComponentData, ContourData, @@ -45,8 +50,10 @@ export type { PointData, PointSeed, PointType, + SetAxisMappingsIntent, Source, Unicode, + UpdateAxisIntent, } from "./generated"; /** diff --git a/packages/types/src/ids.ts b/packages/types/src/ids.ts index 519e15f3..b4daac66 100644 --- a/packages/types/src/ids.ts +++ b/packages/types/src/ids.ts @@ -13,6 +13,7 @@ declare const PointIdBrand: unique symbol; declare const ContourIdBrand: unique symbol; declare const AnchorIdBrand: unique symbol; declare const AxisIdBrand: unique symbol; +declare const AxisMappingIdBrand: unique symbol; declare const ComponentIdBrand: unique symbol; declare const GuidelineIdBrand: unique symbol; declare const GlyphIdBrand: unique symbol; @@ -49,6 +50,11 @@ export type AnchorId = string & { */ export type AxisId = string & { readonly [AxisIdBrand]: typeof AxisIdBrand }; +/** A stable identifier for one font-owned axis mapping. */ +export type AxisMappingId = string & { + readonly [AxisMappingIdBrand]: typeof AxisMappingIdBrand; +}; + /** * A component identifier from Rust. * Branded string type - can't be confused with other IDs or plain strings. @@ -134,6 +140,11 @@ export function asAxisId(id: string): AxisId { return id as AxisId; } +/** Convert a Rust mapping identifier into a typed AxisMappingId. */ +export function asAxisMappingId(id: string): AxisMappingId { + return id as AxisMappingId; +} + /** * Convert a string ID from Rust to a typed ComponentId. * Use this when receiving IDs from Rust snapshots. @@ -210,6 +221,11 @@ export function isAxisId(id: unknown): id is AxisId { return hasIdPrefix(id, "axis"); } +/** Returns whether a value is a runtime-discriminable axis mapping id. */ +export function isAxisMappingId(id: unknown): id is AxisMappingId { + return hasIdPrefix(id, "axisMapping"); +} + /** Returns whether a value is a runtime-discriminable component id. */ export function isComponentId(id: unknown): id is ComponentId { return hasIdPrefix(id, "component"); @@ -254,6 +270,7 @@ type MintedIdByPrefix = { contour: ContourId; anchor: AnchorId; axis: AxisId; + axisMapping: AxisMappingId; component: ComponentId; guideline: GuidelineId; glyph: GlyphId; @@ -312,6 +329,11 @@ export function mintAxisId(): AxisId { return mintPrefixedId("axis"); } +/** Mints a new axis mapping id. See {@link mintAxisId}. */ +export function mintAxisMappingId(): AxisMappingId { + return mintPrefixedId("axisMapping"); +} + /** Mints a new glyph id. See {@link mintPointId}. */ export function mintGlyphId(): GlyphId { return mintPrefixedId("glyph"); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index b1ad8533..50a3dd6c 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -8,6 +8,7 @@ export type { ContourId, AnchorId, AxisId, + AxisMappingId, ComponentId, GuidelineId, GlyphId, @@ -21,6 +22,7 @@ export { asContourId, asAnchorId, asAxisId, + asAxisMappingId, asComponentId, asGuidelineId, asGlyphId, @@ -32,6 +34,7 @@ export { isContourId, isAnchorId, isAxisId, + isAxisMappingId, isComponentId, isGuidelineId, isGlyphId, @@ -42,6 +45,7 @@ export { mintContourId, mintAnchorId, mintAxisId, + mintAxisMappingId, mintPointId, mintGlyphId, mintLayerId, @@ -68,7 +72,12 @@ export type { SetXAdvanceIntent, TranslatePointsIntent, Axis, + AxisLabel, + AxisMapping, + AxisMappingPoint, + AxisRole, AxisTent, + AxisType, BridgeApi, ComponentData, ContourData, @@ -96,6 +105,8 @@ export type { PointData, PointSeed, PointType, + SetAxisMappingsIntent, Source, Unicode, + UpdateAxisIntent, } from "./bridge"; diff --git a/scripts/generate-bridge-types.mjs b/scripts/generate-bridge-types.mjs index 7164178f..a6574c40 100644 --- a/scripts/generate-bridge-types.mjs +++ b/scripts/generate-bridge-types.mjs @@ -22,6 +22,7 @@ const idTypeNames = new Set([ "ContourId", "AnchorId", "AxisId", + "AxisMappingId", "ComponentId", "GuidelineId", "SourceId", From f861c8ff0a4597c4e3a8c8f1af68708c10015187 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Mon, 13 Jul 2026 07:57:40 +0100 Subject: [PATCH 2/2] Address axis mapping review feedback --- .../src/renderer/src/lib/model/Font.ts | 3 +- crates/shift-font/src/ir/font.rs | 165 +++++++++++++++--- packages/types/src/domain.ts | 16 ++ packages/types/src/index.ts | 2 + 4 files changed, 157 insertions(+), 29 deletions(-) create mode 100644 packages/types/src/domain.ts diff --git a/apps/desktop/src/renderer/src/lib/model/Font.ts b/apps/desktop/src/renderer/src/lib/model/Font.ts index b8f7c387..8c9745f4 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.ts @@ -2,6 +2,7 @@ import type { FontMetrics, FontMetadata, Axis, + AxisDefinition, AxisMapping, Source, GlyphId, @@ -1225,7 +1226,7 @@ export class Font { * @param axis - Complete axis definition apart from the id assigned here. * @returns The id submitted to the workspace. */ - createAxis(axis: Omit): AxisId { + createAxis(axis: AxisDefinition): AxisId { const axisId = mintAxisId(); this.editCoordinator.push({ kind: "createAxis", diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index 6dd5869c..4a45e782 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -740,45 +740,45 @@ impl Font { } fn validate_axis_mappings(axes: &[Axis], mappings: &[AxisMapping]) -> CoreResult<()> { + let mut mapping_ids = HashSet::new(); + let mut mapping_names = HashSet::new(); + let mut independent_outputs = HashMap::new(); let mut cross_axis_mapping = None; + for mapping in mappings { mapping.validate(axes)?; - if !mapping.is_independent() { - if cross_axis_mapping.is_some() { - return Err(CoreError::InvalidAxisMapping { - mapping_id: mapping.id(), - message: "only one cross-axis mapping group is supported".to_string(), - }); - } - cross_axis_mapping = Some(mapping.id()); - } - } - for (index, mapping) in mappings.iter().enumerate() { - if mappings[index + 1..] - .iter() - .any(|other| other.id() == mapping.id()) - { + if !mapping_ids.insert(mapping.id()) { return Err(CoreError::DuplicateAxisMappingId(mapping.id())); } - if mappings[index + 1..] - .iter() - .any(|other| other.name() == mapping.name()) - { + if !mapping_names.insert(mapping.name()) { return Err(CoreError::DuplicateAxisMappingName( mapping.name().to_string(), )); } - for output in mapping.outputs() { - if mappings[index + 1..].iter().any(|other| { - other.is_independent() == mapping.is_independent() - && other.outputs().contains(output) - }) { - return Err(CoreError::InvalidAxisMapping { - mapping_id: mapping.id(), - message: format!("output axis {output} is controlled by more than one mapping"), - }); + + if mapping.is_independent() { + for output in mapping.outputs() { + if let Some(owner) = independent_outputs.insert(output.clone(), mapping.id()) { + return Err(CoreError::InvalidAxisMapping { + mapping_id: mapping.id(), + message: format!( + "output axis {output} is already controlled by mapping {owner}" + ), + }); + } } + + continue; + } + + if let Some(owner) = cross_axis_mapping.replace(mapping.id()) { + return Err(CoreError::InvalidAxisMapping { + mapping_id: mapping.id(), + message: format!( + "only one cross-axis mapping group is supported; mapping {owner} already defines it" + ), + }); } } @@ -851,6 +851,43 @@ mod tests { ); } + fn independent_axis_mapping(name: &str, axis: &Axis) -> AxisMapping { + let mut input = Location::new(); + input.set(axis.id(), axis.default()); + let output = input.clone(); + + AxisMapping::new( + name.to_string(), + vec![axis.id()], + vec![axis.id()], + vec![AxisMappingPoint { + description: None, + input, + output, + }], + ) + } + + fn cross_axis_mapping(name: &str, axes: &[Axis]) -> AxisMapping { + let mut input = Location::new(); + for axis in axes { + input.set(axis.id(), axis.default()); + } + let output = input.clone(); + let axis_ids = axes.iter().map(Axis::id).collect::>(); + + AxisMapping::new( + name.to_string(), + axis_ids.clone(), + axis_ids, + vec![AxisMappingPoint { + description: None, + input, + output, + }], + ) + } + #[test] fn font_creation() { let font = Font::new(); @@ -859,6 +896,78 @@ mod tests { assert_eq!(font.default_source().map(Source::name), Some("Regular")); } + #[test] + fn duplicate_axis_mapping_ids_are_rejected() { + let mut font = Font::new(); + let axis = Axis::weight(); + let mapping = independent_axis_mapping("Weight curve", &axis); + font.add_axis(axis); + + let result = font.set_axis_mappings(vec![mapping.clone(), mapping]); + + assert!(matches!(result, Err(CoreError::DuplicateAxisMappingId(_)))); + } + + #[test] + fn duplicate_axis_mapping_names_are_rejected() { + let mut font = Font::new(); + let axis = Axis::weight(); + let first = independent_axis_mapping("Weight curve", &axis); + let second = independent_axis_mapping("Weight curve", &axis); + font.add_axis(axis); + + let result = font.set_axis_mappings(vec![first, second]); + + assert!(matches!( + result, + Err(CoreError::DuplicateAxisMappingName(name)) if name == "Weight curve" + )); + } + + #[test] + fn independent_axis_mappings_cannot_share_an_output() { + let mut font = Font::new(); + let axis = Axis::weight(); + let first = independent_axis_mapping("Weight curve", &axis); + let second = independent_axis_mapping("Weight correction", &axis); + let first_id = first.id(); + let second_id = second.id(); + font.add_axis(axis); + + let result = font.set_axis_mappings(vec![first, second]); + + assert!(matches!( + result, + Err(CoreError::InvalidAxisMapping { + mapping_id, + message, + }) if mapping_id == second_id && message.contains(&first_id.to_string()) + )); + } + + #[test] + fn only_one_cross_axis_mapping_group_is_allowed() { + let mut font = Font::new(); + let axes = [Axis::weight(), Axis::width()]; + let first = cross_axis_mapping("Weight-width correction", &axes); + let second = cross_axis_mapping("Optical correction", &axes); + let first_id = first.id(); + let second_id = second.id(); + for axis in axes { + font.add_axis(axis); + } + + let result = font.set_axis_mappings(vec![first, second]); + + assert!(matches!( + result, + Err(CoreError::InvalidAxisMapping { + mapping_id, + message, + }) if mapping_id == second_id && message.contains(&first_id.to_string()) + )); + } + #[test] fn replacing_axis_cannot_invalidate_existing_mapping() { let mut font = Font::new(); diff --git a/packages/types/src/domain.ts b/packages/types/src/domain.ts new file mode 100644 index 00000000..afb29719 --- /dev/null +++ b/packages/types/src/domain.ts @@ -0,0 +1,16 @@ +import type { Axis } from "./bridge"; + +/** Complete axis definition before the editor assigns its stable identity. */ +export type AxisDefinition = Pick< + Axis, + | "tag" + | "name" + | "role" + | "axisType" + | "minimum" + | "default" + | "maximum" + | "values" + | "labels" + | "hidden" +>; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 50a3dd6c..a49efbf8 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -54,6 +54,8 @@ export { mintSourceId, } from "./ids"; +export type { AxisDefinition } from "./domain"; + export type { AddAnchorsIntent, AddContourIntent,