diff --git a/Cargo.lock b/Cargo.lock index 1c5f7957..65774edf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1715,6 +1715,7 @@ dependencies = [ "shift-backends", "shift-font", "shift-wire", + "shift-workspace", "thiserror 2.0.18", ] diff --git a/apps/desktop/src/main/docs/DOCS.md b/apps/desktop/src/main/docs/DOCS.md index 9949887f..ae880f0a 100644 --- a/apps/desktop/src/main/docs/DOCS.md +++ b/apps/desktop/src/main/docs/DOCS.md @@ -9,7 +9,7 @@ Electron main process: application lifecycle, window management, menus, document - **Architecture Invariant:** IPC channels are type-safe. All `ipcMain.handle` calls use the typed `ipc.handle` wrapper from `shared/ipc/main`, and all `webContents.send` calls use the typed `ipc.send` wrapper. Channel names and payload types are defined in `IpcCommands` (renderer-to-main) and `IpcEvents` (main-to-renderer). - **Architecture Invariant: CRITICAL:** `main.ts` enforces a single-instance lock via `app.requestSingleInstanceLock()`. The second instance forwards its argv to the first instance via the `second-instance` event and then quits. Removing this breaks file-association double-click on all platforms. - **Architecture Invariant: CRITICAL:** The `before-quit` handler in `AppLifecycle` must call `event.preventDefault()` before the async `confirmClose` check. If the guard is removed, the app quits before the save dialog can appear. -- **Architecture Invariant:** `.designspace` is the default writable format, with `.ufo` still accepted for direct UFO saves (`DocumentState.isWritableFormat`). Saving other imported formats triggers Save As. Autosave skips non-writable files silently. +- **Architecture Invariant:** `.shift` is the only direct writable source format (`DocumentState.isWritableFormat`). Imported font formats can be opened, but saving them triggers Save As to a `.shift` package. Export is a separate workflow. ## Codemap @@ -33,7 +33,7 @@ src/main/ - `DebugOverlays` -- per-overlay booleans (`tightBounds`, `hitRadii`, `segmentBounds`, `glyphBbox`) - `IpcCommands` -- renderer-to-main request/response channels (invoke/handle) - `IpcEvents` -- main-to-renderer broadcast channels (send/on) -- `SUPPORTED_FONT_EXTENSIONS` -- the set of file extensions accepted for opening (`.ufo`, `.ttf`, `.otf`, `.glyphs`, `.glyphspackage`, `.designspace`) +- `SUPPORTED_FONT_EXTENSIONS` -- the set of file extensions accepted for opening (`.shift`, `.ufo`, `.ttf`, `.otf`, `.glyphs`, `.glyphspackage`, `.designspace`) ## How it works @@ -55,7 +55,7 @@ Files arrive via three paths: CLI launch args (`handleLaunchArgs`), second-insta ### Save and autosave -`DocumentState.save` checks `isWritableFormat` -- `.designspace` and `.ufo` can be saved in-place. Other imported formats and Save As show the save dialog with Designspace as the default filter. On save, the main process sends `menu:save-font` to the renderer, which does the actual write and calls back `document:saveCompleted`. Autosave runs on a 30-second interval (`AUTOSAVE_INTERVAL_MS`) and only fires if dirty and the file is writable. +`DocumentState.save` checks `isWritableFormat` -- only `.shift` packages can be saved in-place. Imported formats and Save As show the save dialog with Shift Source Package as the default filter. On save, the main process sends `menu:save-font` to the renderer, which writes through the Rust workspace and calls back `document:saveCompleted`. Autosave runs on a 30-second interval (`AUTOSAVE_INTERVAL_MS`) and only fires if dirty and the file is writable. ### Menu @@ -104,7 +104,7 @@ IPC handlers are split across managers. `WindowManager` registers window-control - `npx vitest run apps/desktop/src/main/managers/openFontPath.test.ts` -- openFontPath unit tests - Manual: launch with a font path argument, verify it opens - Manual: edit a document, Cmd+Q, verify save dialog appears -- Manual: open a .ttf, Cmd+S, verify Save As dialog defaults to .designspace +- Manual: open a .ttf, Cmd+S, verify Save As dialog defaults to .shift ## Related diff --git a/apps/desktop/src/main/managers/AppLifecycle.ts b/apps/desktop/src/main/managers/AppLifecycle.ts index cc7786c7..a8805132 100644 --- a/apps/desktop/src/main/managers/AppLifecycle.ts +++ b/apps/desktop/src/main/managers/AppLifecycle.ts @@ -212,7 +212,7 @@ export class AppLifecycle { filters: [ { name: "Fonts", - extensions: ["ttf", "otf", "ufo", "glyphs", "glyphspackage", "designspace"], + extensions: ["shift", "ttf", "otf", "ufo", "glyphs", "glyphspackage", "designspace"], }, ], }); diff --git a/apps/desktop/src/main/managers/DocumentState.ts b/apps/desktop/src/main/managers/DocumentState.ts index c061c04d..c4c9eb43 100644 --- a/apps/desktop/src/main/managers/DocumentState.ts +++ b/apps/desktop/src/main/managers/DocumentState.ts @@ -45,7 +45,7 @@ export class DocumentState { private isWritableFormat(filePath: string | null): boolean { if (!filePath) return false; - return filePath.endsWith(".designspace") || filePath.endsWith(".ufo"); + return filePath.endsWith(".shift"); } async save(saveAs = false): Promise { @@ -56,18 +56,15 @@ export class DocumentState { let savePath = this.filePath; if (!savePath || saveAs || !this.isWritableFormat(savePath)) { - let defaultPath = "Untitled.designspace"; + let defaultPath = "Untitled.shift"; if (this.filePath) { const baseName = path.basename(this.filePath, path.extname(this.filePath)); - defaultPath = `${baseName}.designspace`; + defaultPath = `${baseName}.shift`; } const result = await dialog.showSaveDialog({ defaultPath, - filters: [ - { name: "Designspace Files", extensions: ["designspace"] }, - { name: "UFO Files", extensions: ["ufo"] }, - ], + filters: [{ name: "Shift Source Packages", extensions: ["shift"] }], }); if (result.canceled || !result.filePath) { @@ -75,8 +72,8 @@ export class DocumentState { } savePath = result.filePath; - if (!savePath.endsWith(".designspace") && !savePath.endsWith(".ufo")) { - savePath += ".designspace"; + if (!savePath.endsWith(".shift")) { + savePath += ".shift"; } } diff --git a/apps/desktop/src/main/managers/openFontPath.test.ts b/apps/desktop/src/main/managers/openFontPath.test.ts index 97bb68b6..a271ac93 100644 --- a/apps/desktop/src/main/managers/openFontPath.test.ts +++ b/apps/desktop/src/main/managers/openFontPath.test.ts @@ -5,6 +5,8 @@ import { extractFirstFontPath, isSupportedFontPath, normalizeFontPath } from "./ describe("openFontPath", () => { describe("isSupportedFontPath", () => { it("accepts supported font extensions", () => { + expect(isSupportedFontPath("/tmp/font.shift")).toBe(true); + expect(isSupportedFontPath("/tmp/font.SHIFT")).toBe(true); expect(isSupportedFontPath("/tmp/font.ufo")).toBe(true); expect(isSupportedFontPath("/tmp/font.ttf")).toBe(true); expect(isSupportedFontPath("/tmp/font.otf")).toBe(true); diff --git a/apps/desktop/src/main/managers/openFontPath.ts b/apps/desktop/src/main/managers/openFontPath.ts index ca318c03..c3436c9c 100644 --- a/apps/desktop/src/main/managers/openFontPath.ts +++ b/apps/desktop/src/main/managers/openFontPath.ts @@ -1,6 +1,7 @@ import path from "node:path"; const SUPPORTED_FONT_EXTENSIONS = new Set([ + ".shift", ".ufo", ".ttf", ".otf", diff --git a/apps/desktop/src/renderer/src/app/Document.test.ts b/apps/desktop/src/renderer/src/app/Document.test.ts index 297dbf8f..283d1592 100644 --- a/apps/desktop/src/renderer/src/app/Document.test.ts +++ b/apps/desktop/src/renderer/src/app/Document.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { TestEditor } from "@/testing/TestEditor"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testSourcePath, testStorePath } from "@/testing"; import { Document } from "./Document"; function testDocument() { @@ -9,6 +9,14 @@ function testDocument() { let dirty = true; const document = new Document(editor, { createUntitledId: () => "untitled-1", + createWorkspacePaths: (id) => { + const sourcePath = testSourcePath(id); + return { sourcePath, storePath: `${sourcePath}/working.sqlite` }; + }, + workspacePathsForOpen: (path) => ({ + sourcePath: path, + storePath: testStorePath("document-open"), + }), setFilePath: (nextPath) => { filePath = nextPath; }, diff --git a/apps/desktop/src/renderer/src/app/Document.ts b/apps/desktop/src/renderer/src/app/Document.ts index a1241ce1..b2c8412a 100644 --- a/apps/desktop/src/renderer/src/app/Document.ts +++ b/apps/desktop/src/renderer/src/app/Document.ts @@ -4,10 +4,17 @@ export type DocumentIdentity = | { readonly kind: "untitled"; readonly id: string } | { readonly kind: "file"; readonly path: string }; +export interface WorkspacePaths { + readonly sourcePath: string; + readonly storePath: string; +} + export interface DocumentServices { readonly setFilePath: (filePath: string | null) => void; readonly clearDirty: () => void; readonly createUntitledId?: () => string; + readonly createWorkspacePaths?: (id: string) => WorkspacePaths; + readonly workspacePathsForOpen?: (path: string) => WorkspacePaths; readonly notifySaveCompleted?: (path: string) => Promise | void; } @@ -24,6 +31,8 @@ export class Document { readonly #setFilePath: (filePath: string | null) => void; readonly #clearDirty: () => void; readonly #createUntitledId: () => string; + readonly #createWorkspacePaths: (id: string) => WorkspacePaths; + readonly #workspacePathsForOpen: (path: string) => WorkspacePaths; readonly #notifySaveCompleted: (path: string) => Promise | void; #identity: DocumentIdentity | null = null; @@ -33,6 +42,8 @@ export class Document { this.#setFilePath = services.setFilePath; this.#clearDirty = services.clearDirty; this.#createUntitledId = services.createUntitledId ?? createUntitledId; + this.#createWorkspacePaths = services.createWorkspacePaths ?? createWorkspacePaths; + this.#workspacePathsForOpen = services.workspacePathsForOpen ?? workspacePathsForOpen; this.#notifySaveCompleted = services.notifySaveCompleted ?? (() => undefined); } @@ -46,7 +57,8 @@ export class Document { createFont(): void { const id = this.#createUntitledId(); - this.editor.createFont(); + const paths = this.#createWorkspacePaths(id); + this.editor.createFont(paths.sourcePath, paths.storePath); this.#identity = { kind: "untitled", id }; this.#setFilePath(null); @@ -54,7 +66,8 @@ export class Document { } openFont(path: string): void { - this.editor.loadFont(path); + const paths = this.#workspacePathsForOpen(path); + this.editor.loadFont(paths.sourcePath, paths.storePath); this.#identity = { kind: "file", path }; this.#setFilePath(path); @@ -67,7 +80,14 @@ export class Document { throw new Error("Cannot save an untitled document without a file path"); } - await this.editor.saveFont(savePath); + if ( + this.#identity?.kind === "file" && + (path === undefined || savePath === this.#identity.path) + ) { + await this.editor.saveFont(); + } else { + await this.editor.saveFont(savePath); + } this.#identity = { kind: "file", path: savePath }; this.#setFilePath(savePath); @@ -91,3 +111,22 @@ export class Document { function createUntitledId(): string { return globalThis.crypto?.randomUUID?.() ?? `untitled-${Date.now().toString(36)}`; } + +function createWorkspacePaths(id: string): WorkspacePaths { + const root = `${appWorkspaceRoot()}/${id}`; + const sourcePath = `${root}/Untitled.shift`; + return { sourcePath, storePath: `${sourcePath}/working.sqlite` }; +} + +function workspacePathsForOpen(path: string): WorkspacePaths { + if (path.endsWith(".shift")) { + return { sourcePath: path, storePath: `${path}/working.sqlite` }; + } + + return { sourcePath: path, storePath: `${path}.working.sqlite` }; +} + +function appWorkspaceRoot(): string { + const homePath = typeof window === "undefined" ? null : window.electronAPI?.homePath; + return `${homePath ?? "/tmp"}/.shift/workspaces`; +} diff --git a/apps/desktop/src/renderer/src/lib/commands/primitives/ApplyPositionPatchCommand.test.ts b/apps/desktop/src/renderer/src/lib/commands/primitives/ApplyPositionPatchCommand.test.ts index 3ee191d5..0eaedcf0 100644 --- a/apps/desktop/src/renderer/src/lib/commands/primitives/ApplyPositionPatchCommand.test.ts +++ b/apps/desktop/src/renderer/src/lib/commands/primitives/ApplyPositionPatchCommand.test.ts @@ -4,13 +4,13 @@ import type { PointId } from "@shift/types"; import { ApplyPositionPatchCommand } from "./ApplyPositionPatchCommand"; import { Font } from "@/lib/model/Font"; import type { GlyphSource } from "@/lib/model/Glyph"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import type { CommandContext } from "../core"; function editableSource(): GlyphSource { const bridge = createBridge(); const font = new Font(bridge); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("apply-position-patch")); const handle = { name: "A", unicode: 65 }; const source = font.defaultSource; diff --git a/apps/desktop/src/renderer/src/lib/commands/testUtils.ts b/apps/desktop/src/renderer/src/lib/commands/testUtils.ts index 510b4c88..bd9ca646 100644 --- a/apps/desktop/src/renderer/src/lib/commands/testUtils.ts +++ b/apps/desktop/src/renderer/src/lib/commands/testUtils.ts @@ -4,7 +4,7 @@ import { signal, type Signal } from "@/lib/signals/signal"; import { Font } from "@/lib/model/Font"; import type { GlyphSource } from "@/lib/model/Glyph"; import { Point } from "@shift/glyph-state"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import type { CommandContext } from "./core"; export interface CommandSourceFixture { @@ -16,7 +16,7 @@ export interface CommandSourceFixture { export function commandSourceFixture(): CommandSourceFixture { const bridge = createBridge(); const font = new Font(bridge); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("command-source")); const handle = { name: "A", unicode: 65 }; const source = font.defaultSource; diff --git a/apps/desktop/src/renderer/src/lib/editor/Editor.ts b/apps/desktop/src/renderer/src/lib/editor/Editor.ts index d4e74752..0f86f6f3 100644 --- a/apps/desktop/src/renderer/src/lib/editor/Editor.ts +++ b/apps/desktop/src/renderer/src/lib/editor/Editor.ts @@ -1031,8 +1031,8 @@ export class Editor { * Creates a new loaded font document and resets editor placement to its * default design location. */ - public createFont(): void { - this.font.create(); + public createFont(sourcePath: string, storePath: string): void { + this.font.create(sourcePath, storePath); this.setDesignLocation(this.font.defaultLocation()); this.#events.emit("fontLoaded", { font: this.font }); } @@ -1041,8 +1041,8 @@ export class Editor { * Loads a font from disk and resets editor placement to its default design * location. */ - public loadFont(filePath: string): void { - this.font.load(filePath); + public loadFont(filePath: string, storePath: string): void { + this.font.load(filePath, storePath); this.setDesignLocation(this.font.defaultLocation()); this.#events.emit("fontLoaded", { font: this.font }); } @@ -1052,7 +1052,7 @@ export class Editor { this.setDesignLocation(emptyAxisLocation()); } - public async saveFont(filePath: string): Promise { + public async saveFont(filePath?: string): Promise { return this.font.save(filePath); } diff --git a/apps/desktop/src/renderer/src/lib/editor/SourceEditDraft.test.ts b/apps/desktop/src/renderer/src/lib/editor/SourceEditDraft.test.ts index 3f1d61eb..cdde146e 100644 --- a/apps/desktop/src/renderer/src/lib/editor/SourceEditDraft.test.ts +++ b/apps/desktop/src/renderer/src/lib/editor/SourceEditDraft.test.ts @@ -5,13 +5,13 @@ import { signal } from "@/lib/signals/signal"; import { CommandHistory } from "@/lib/commands/core/CommandHistory"; import { Font } from "@/lib/model/Font"; import type { GlyphSource } from "@/lib/model/Glyph"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import { SourceEditDraft } from "./SourceEditDraft"; function editableSource(): GlyphSource { const bridge = createBridge(); const font = new Font(bridge); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("source-edit-draft")); const handle = { name: "A", unicode: 65 }; const source = font.defaultSource; diff --git a/apps/desktop/src/renderer/src/lib/editor/variation.test.ts b/apps/desktop/src/renderer/src/lib/editor/variation.test.ts index 4d1d6d43..d58c984d 100644 --- a/apps/desktop/src/renderer/src/lib/editor/variation.test.ts +++ b/apps/desktop/src/renderer/src/lib/editor/variation.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { Glyph, GlyphGeometry } from "@/lib/model/Glyph"; -import { TestEditor, MUTATORSANS_DESIGNSPACE } from "@/testing"; +import { TestEditor, MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import { emptyAxisLocation, withAxisValue } from "@/lib/variation/location"; import type { AxisLocation } from "@/types/variation"; @@ -24,7 +24,7 @@ describe("Editor.open — variation-aware glyph reads", () => { // master's stored coordinates, so the canvas didn't // match what the user clicked when the slider was off-default. const editor = new TestEditor(); - editor.loadFont(MUTATORSANS_DESIGNSPACE); + editor.loadFont(MUTATORSANS_DESIGNSPACE, testStorePath("default")); const atDefault = editor.getGlyph({ name: "A", unicode: 65 })!; const defaultGeometry = atDefault.geometryAt(editor.designLocation); @@ -45,7 +45,7 @@ describe("Editor.open — variation-aware glyph reads", () => { // to the grid), then re-open the same glyph. The re-opened glyph should // carry the edit, not revert to the unedited geometry. const editor = new TestEditor(); - editor.loadFont(MUTATORSANS_DESIGNSPACE); + editor.loadFont(MUTATORSANS_DESIGNSPACE, testStorePath("persist")); const opened = editor.getGlyph({ name: "A", unicode: 65 })!; const point = opened.contours[0].points[0]; @@ -68,7 +68,7 @@ describe("Editor.open — variation-aware glyph reads", () => { // glyph closes, the registry's Glyph must reflect the edits the user // just made. Otherwise the grid shows the pre-edit outline. const editor = new TestEditor(); - editor.loadFont(MUTATORSANS_DESIGNSPACE); + editor.loadFont(MUTATORSANS_DESIGNSPACE, testStorePath("registry")); const opened = editor.getGlyph({ name: "A", unicode: 65 })!; const point = opened.contours[0].points[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 8a29521c..c4975040 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,6 @@ import { describe, expect, it } from "vitest"; import { createBridge } from "@shift/bridge"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testSourcePath, testStorePath } from "@/testing"; import { axisLocationFromLocation, axisValue, @@ -14,7 +14,7 @@ import { Font } from "./Font"; function loadFont(): Font { const font = new Font(createBridge()); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("load")); return font; } @@ -62,7 +62,8 @@ describe("Font", () => { it("creates a fresh loaded font with a default source", () => { const font = new Font(createBridge()); - font.create(); + const sourcePath = testSourcePath("create"); + font.create(sourcePath, `${sourcePath}/working.sqlite`); expect(font.loaded).toBe(true); expect(font.sources).toHaveLength(1); diff --git a/apps/desktop/src/renderer/src/lib/model/Font.ts b/apps/desktop/src/renderer/src/lib/model/Font.ts index 8b5454b8..9d4b9bde 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.ts @@ -226,6 +226,14 @@ class GlyphDirectory { type GlyphSourceKey = string & { readonly __glyphSourceKey: unique symbol }; +const DEFAULT_FONT_METRICS: FontMetrics = { + unitsPerEm: 1000, + ascender: 800, + descender: -200, + capHeight: 700, + xHeight: 500, +}; + /** * Reactive domain model for the loaded font. * @@ -254,7 +262,7 @@ export class Font { constructor(bridge: ShiftBridge) { this.#bridge = bridge; - this.#defaultMetrics = bridge.getMetrics(); + this.#defaultMetrics = DEFAULT_FONT_METRICS; this.#$loaded = signal(false); this.#$metrics = signal(this.#defaultMetrics); this.#$sources = signal([]); @@ -710,26 +718,26 @@ export class Font { return this.#bridge.getVariationReports(); } - create(): void { + create(sourcePath: string, storePath: string): void { this.#glyphs.clear(); this.#glyphSources.clear(); - this.#bridge.createFont(); + this.#bridge.createWorkspace(sourcePath, storePath); this.#hydrateFromBridge(); } - load(path: string): void { + load(path: string, storePath: string): void { this.#glyphs.clear(); this.#glyphSources.clear(); - this.#bridge.loadFont(path); + this.#bridge.openWorkspace(path, storePath); this.#hydrateFromBridge(); } - async save(path: string): Promise { - return this.#bridge.saveFont(path); + async save(path?: string): Promise { + return path ? this.#bridge.saveWorkspaceAs(path) : this.#bridge.saveWorkspace(); } async export(path: string): Promise { - await this.#bridge.exportFont({ path, format: "ttf" }); + await this.#bridge.exportWorkspace({ path, format: "ttf" }); } /** @knipclassignore — called when closing a document */ diff --git a/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts b/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts index 95b98e43..a70ec260 100644 --- a/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts @@ -7,7 +7,7 @@ import { withAxisValue, } from "@/lib/variation/location"; import type { AxisLocation } from "@/types/variation"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import { Font } from "./Font"; import type { PointId } from "@shift/types"; import type { Glyph, GlyphSource } from "./Glyph"; @@ -21,7 +21,7 @@ function editGlyph(): { } { const bridge = createBridge(); const font = new Font(bridge); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("glyph-edit")); const handle = { name: "A" }; const source = font.defaultSource; @@ -79,7 +79,7 @@ function sourcePosition(layer: GlyphSource, pointId: PointId): { x: number; y: n function loadMutatorSans(): Font { const font = new Font(createBridge()); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("glyph-load")); return font; } diff --git a/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts b/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts index d0a79673..c0884787 100644 --- a/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts +++ b/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts @@ -6,7 +6,7 @@ * the loaded font, not hardcoded advances. */ import { Font } from "@/lib/model/Font"; -import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { MUTATORSANS_DESIGNSPACE, testStorePath } from "@/testing"; import { TextLayout } from "./TextLayout"; import { Positioner } from "./Positioner"; import type { TextItem, GlyphTextItem, SegmentedRun } from "./types"; @@ -17,7 +17,7 @@ export function loadTestFont(): Font { const bridge = createBridge(); const font = new Font(bridge); - font.load(MUTATORSANS_DESIGNSPACE); + font.load(MUTATORSANS_DESIGNSPACE, testStorePath("layout")); return font; } diff --git a/apps/desktop/src/renderer/src/testing/TestEditor.ts b/apps/desktop/src/renderer/src/testing/TestEditor.ts index edaa8db5..477d212b 100644 --- a/apps/desktop/src/renderer/src/testing/TestEditor.ts +++ b/apps/desktop/src/renderer/src/testing/TestEditor.ts @@ -17,6 +17,7 @@ import { registerBuiltInTools } from "@/lib/tools/tools"; import { createBridge } from "@shift/bridge"; import type { SystemClipboard } from "@/lib/clipboard"; import { MUTATORSANS_DESIGNSPACE } from "./fixtures"; +import { testStorePath } from "./workspacePaths"; const DEFAULT_MODIFIERS = { shiftKey: false, altKey: false, metaKey: false }; @@ -63,7 +64,7 @@ export class TestEditor extends Editor { startSession(handle: GlyphHandle = { name: "A", unicode: 65 }): this { if (!this.font.loaded) { - this.loadFont(MUTATORSANS_DESIGNSPACE); + this.loadFont(MUTATORSANS_DESIGNSPACE, testStorePath("session")); } const source = this.font.defaultSource; diff --git a/apps/desktop/src/renderer/src/testing/index.ts b/apps/desktop/src/renderer/src/testing/index.ts index dc9ecb96..a4284c69 100644 --- a/apps/desktop/src/renderer/src/testing/index.ts +++ b/apps/desktop/src/renderer/src/testing/index.ts @@ -3,3 +3,4 @@ export { TestEditor } from "./TestEditor"; export { makeTestCoordinates } from "./coordinates"; export { expectDefined, expectAt } from "./asserts"; export { FIXTURES_ROOT, MUTATORSANS_DESIGNSPACE } from "./fixtures"; +export { testSourcePath, testStorePath } from "./workspacePaths"; diff --git a/apps/desktop/src/renderer/src/testing/workspacePaths.ts b/apps/desktop/src/renderer/src/testing/workspacePaths.ts new file mode 100644 index 00000000..ea247a54 --- /dev/null +++ b/apps/desktop/src/renderer/src/testing/workspacePaths.ts @@ -0,0 +1,11 @@ +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export function testStorePath(label: string): string { + return join(mkdtempSync(join(tmpdir(), `shift-${label}-`)), "working.sqlite"); +} + +export function testSourcePath(label: string): string { + return join(mkdtempSync(join(tmpdir(), `shift-${label}-`)), "TestFont.shift"); +} diff --git a/crates/shift-bridge/Cargo.toml b/crates/shift-bridge/Cargo.toml index 62c9155b..5415dd18 100644 --- a/crates/shift-bridge/Cargo.toml +++ b/crates/shift-bridge/Cargo.toml @@ -12,6 +12,7 @@ crate-type = ["cdylib"] shift-font = { workspace = true } shift-wire = { workspace = true, features = ["napi"] } shift-backends = { workspace = true } +shift-workspace = { workspace = true } napi = { version = "=3.8.6", default-features = false, features = ["napi6"] } napi-derive = "=3.5.5" diff --git a/crates/shift-bridge/__test__/index.spec.mjs b/crates/shift-bridge/__test__/index.spec.mjs index 5b005718..dd47c64c 100644 --- a/crates/shift-bridge/__test__/index.spec.mjs +++ b/crates/shift-bridge/__test__/index.spec.mjs @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { createRequire } from "module"; import { existsSync, mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; @@ -9,9 +9,16 @@ const { Bridge } = require("../index.js"); describe("Bridge", () => { let bridge; + let tempDir; beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "shift-bridge-")); bridge = new Bridge(); + bridge.createWorkspace(join(tempDir, "TestFont.shift"), join(tempDir, "working.sqlite")); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); }); function defaultSourceId() { @@ -25,7 +32,7 @@ describe("Bridge", () => { }; } - it("starts with default committed font metadata", () => { + it("creates a workspace with default committed font metadata", () => { expect(bridge.getMetadata()).toMatchObject({ familyName: "Untitled Font", styleName: "Regular", @@ -53,46 +60,40 @@ describe("Bridge", () => { ]); }); - it("saves direct glyph layer edits", async () => { - const tempDir = mkdtempSync(join(tmpdir(), "shift-bridge-save-")); - try { - const glyphRef = defaultLayerRef(); - const contourId = bridge.addContour(glyphRef).changed.contourIds[0]; - bridge.addPoint(glyphRef, contourId, 10, 20, "onCurve", false); - - const outputPath = join(tempDir, "output.ufo"); - const savedVersion = await bridge.saveFont(outputPath); - - expect(savedVersion).toBe(2); - expect(bridge.getPersistedVersion()).toBe(2); - expect(bridge.isDirty()).toBe(false); - expect(existsSync(outputPath)).toBe(true); - - const reloaded = new Bridge(); - reloaded.loadFont(outputPath); - expect(reloaded.getGlyphs()).toEqual([ - { name: "A", unicodes: [65], componentBaseGlyphNames: [] }, - ]); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - } + it("saves direct glyph layer edits to a shift source package target", () => { + const glyphRef = defaultLayerRef(); + const contourId = bridge.addContour(glyphRef).changed.contourIds[0]; + bridge.addPoint(glyphRef, contourId, 10, 20, "onCurve", false); + + const outputPath = join(tempDir, "output.shift"); + const savedVersion = bridge.saveWorkspaceAs(outputPath); + + expect(savedVersion).toBe(2); + expect(bridge.getPersistedVersion()).toBe(2); + expect(bridge.isDirty()).toBe(false); + expect(existsSync(outputPath)).toBe(true); + }); + + it("records the persisted version when saving the current workspace", () => { + bridge.addContour(defaultLayerRef()); + + const savedVersion = bridge.saveWorkspace(); + + expect(savedVersion).toBe(1); + expect(bridge.getPersistedVersion()).toBe(1); + expect(bridge.isDirty()).toBe(false); }); - it("records the persisted version when an async save completes", async () => { - const tempDir = mkdtempSync(join(tmpdir(), "shift-bridge-async-save-")); - try { - bridge.addContour(defaultLayerRef()); - - const outputPath = join(tempDir, "async-output.ufo"); - const savedVersion = await bridge.saveFont(outputPath); - - expect(savedVersion).toBe(1); - expect(bridge.getPersistedVersion()).toBe(1); - expect(bridge.isDirty()).toBe(false); - expect(existsSync(outputPath)).toBe(true); - } finally { - rmSync(tempDir, { recursive: true, force: true }); - } + it("exports the live workspace font through an explicit export path", async () => { + const glyphRef = defaultLayerRef(); + const contourId = bridge.addContour(glyphRef).changed.contourIds[0]; + bridge.addPoint(glyphRef, contourId, 10, 20, "onCurve", false); + + const outputPath = join(tempDir, "output.ttf"); + const result = await bridge.exportWorkspace({ path: outputPath, format: "ttf" }); + + expect(result).toMatchObject({ path: outputPath, format: "ttf" }); + expect(existsSync(outputPath)).toBe(true); }); it("adds a point to a contour and returns structure, values, and changed ids", () => { diff --git a/crates/shift-bridge/docs/DOCS.md b/crates/shift-bridge/docs/DOCS.md index c7bcad24..0abc7c11 100644 --- a/crates/shift-bridge/docs/DOCS.md +++ b/crates/shift-bridge/docs/DOCS.md @@ -4,7 +4,7 @@ NAPI bindings that expose the Rust font engine to Node.js and Electron as a `Bri ## Architecture Invariants -**Architecture Invariant:** The bridge does not own hidden edit sessions. Mutation methods receive an explicit `GlyphLayerRef`, resolve the target glyph layer, and mutate the model object in `shift-font`. **WHY:** Renderer selection state stays in TypeScript, while Rust remains the authoritative model and ID allocator. +**Architecture Invariant:** The bridge does not own hidden edit sessions or durable font state directly. It owns an optional `FontWorkspace` and forwards mutation transactions into `shift-workspace`. **WHY:** Renderer selection state stays in TypeScript, transport stays in `shift-bridge`, and workspace/store synchronization has one Rust owner. **Architecture Invariant:** Public bridge DTOs live in `shift-wire`; NAPI-specific wrappers live under `shift-wire::bridges::napi`. **WHY:** Wire shapes remain independent of the native module implementation, while NAPI can still return efficient types such as `Float64Array`. @@ -12,7 +12,7 @@ NAPI bindings that expose the Rust font engine to Node.js and Electron as a `Bri **Architecture Invariant:** Bulk position updates use `Float64Array` values plus typed node descriptors. **WHY:** Drag updates keep the hot path in flat numeric buffers while IDs remain branded strings at the API boundary. -**Architecture Invariant:** Save uses a clone/COW `FontSaveSnapshot` of the current font. **WHY:** Async save can run from a stable view without coupling persistence to renderer focus state. +**Architecture Invariant:** Export uses a clone/COW `FontSaveSnapshot` of the current workspace font. **WHY:** Async export can run from a stable view without coupling output generation to renderer focus state. ## Codemap @@ -20,7 +20,7 @@ NAPI bindings that expose the Rust font engine to Node.js and Electron as a `Bri crates/shift-bridge/ src/ lib.rs -- crate root - bridge.rs -- `Bridge` NAPI class, font reads, mutations, save task + bridge.rs -- `Bridge` NAPI class, workspace lifecycle, font reads, mutations, export task errors.rs -- bridge error type and NAPI mapping input.rs -- boundary parsing/adaptation helpers Cargo.toml -- cdylib crate; depends on shift-font, shift-wire, shift-backends, napi @@ -28,10 +28,10 @@ crates/shift-bridge/ ## Key Types -- `Bridge` -- the exported `#[napi]` class holding the current `Font` and document versions. +- `Bridge` -- the exported `#[napi]` class holding the current `FontWorkspace` and document versions. - `GlyphLayerRef` -- boundary identity for the glyph layer being mutated. -- `FontSaveSnapshot` -- clone/COW save view of the current font. -- `SaveFontTask` -- NAPI `Task` implementation for async font saving. +- `FontSaveSnapshot` -- clone/COW export view of the current workspace font. +- `ExportFontTask` -- NAPI `Task` implementation for async font export. - `BridgeError` -- typed bridge error enum converted once at the NAPI boundary. - `GlyphStructureChange` / `GlyphValueChange` -- canonical wire DTOs returned by mutations. - `NapiGlyphStructureChange` / `NapiGlyphValueChange` -- NAPI adapters for those DTOs. @@ -40,9 +40,10 @@ crates/shift-bridge/ 1. The renderer chooses the active glyph/source and builds a `GlyphLayerRef`. 2. JS calls a mutation such as `closeContour(glyphRef, contourId)`. -3. `Bridge` parses boundary strings, resolves or creates the target glyph layer, and calls the matching `GlyphLayer` method from `shift-font`. +3. `Bridge` parses boundary strings and asks `FontWorkspace` to run the edit against the target glyph layer. 4. The bridge returns a `shift-wire` change DTO and bumps the live version. -5. `saveFont(path)` creates a `FontSaveSnapshot` and saves asynchronously through `shift-backends`. +5. `saveWorkspace()` / `saveWorkspaceAs(path)` update the `.shift` source package target and record the persisted version. +6. `exportWorkspace(request)` creates a `FontSaveSnapshot` and exports asynchronously through `shift-backends`. ## Type Boundary @@ -81,5 +82,6 @@ pnpm generate:bridge-types - `shift-font` -- font/glyph/layer data model and model-level mutation logic. - `shift-wire` -- canonical bridge DTOs and NAPI adapters. -- `shift-backends` -- font loading/saving. +- `shift-workspace` -- open editable font workspace and source/store coordination. +- `shift-backends` -- imported font loading and font export. - `packages/types/src/bridge` -- generated TypeScript bridge facade. diff --git a/crates/shift-bridge/index.d.ts b/crates/shift-bridge/index.d.ts index 10e22ed0..89fee11a 100644 --- a/crates/shift-bridge/index.d.ts +++ b/crates/shift-bridge/index.d.ts @@ -11,10 +11,11 @@ import type { } from "@shift/types"; export declare class Bridge { constructor() - createFont(): void - loadFont(path: string): void - saveFont(path: string): Promise - exportFont(request: NapiFontExportRequest): Promise + createWorkspace(sourcePath: string, storePath: string, options?: NapiNewWorkspace | undefined | null): void + openWorkspace(path: string, storePath: string): void + saveWorkspace(): number + saveWorkspaceAs(path: string): number + exportWorkspace(request: NapiFontExportRequest): Promise getMetadata(): NapiFontMetadata getMetrics(): NapiFontMetrics getGlyphCount(): number @@ -90,6 +91,11 @@ export interface NapiGlyphVariationReport { skippedMasterCount: number diagnostics: Array } + +export interface NapiNewWorkspace { + familyName?: string + unitsPerEm?: number +} export interface NapiAnchorData { id: AnchorId name?: string diff --git a/crates/shift-bridge/src/bridge.rs b/crates/shift-bridge/src/bridge.rs index 705409ee..02e28cba 100644 --- a/crates/shift-bridge/src/bridge.rs +++ b/crates/shift-bridge/src/bridge.rs @@ -4,13 +4,10 @@ use napi::bindgen_prelude::*; use napi::{Error, Status}; use napi_derive::napi; use serde::{Deserialize, Serialize}; -use shift_backends::{ - font_loader::FontLoader, ExportFormat, FontExportRequest, FontExportResult, FontExporter, - FontView, -}; +use shift_backends::{ExportFormat, FontExportRequest, FontExportResult, FontExporter, FontView}; use shift_font::{ - BooleanOp, BulkNodePositionUpdates, ContourId, Font, Glyph, GlyphLayer, GlyphName, LayerId, - PointId, SourceId, + BooleanOp, BulkNodePositionUpdates, ContourId, Font, Glyph, GlyphLayer, LayerId, PointId, + SourceId, }; use shift_wire::{ bridges::napi::{ @@ -22,6 +19,7 @@ use shift_wire::{ Axis, FontMetadata, FontMetrics, GlyphChangedEntities, GlyphRecord, GlyphState, GlyphStructure, GlyphStructureChange, GlyphValueChange, Source, }; +use shift_workspace::{FontWorkspace, GlyphLayerTarget, NewWorkspace}; use std::sync::{ atomic::{AtomicU64, Ordering}, Arc, @@ -57,6 +55,28 @@ pub struct NapiFontExportResult { pub format: String, } +#[napi(object)] +pub struct NapiNewWorkspace { + pub family_name: Option, + pub units_per_em: Option, +} + +fn new_workspace_from_options(options: Option) -> NewWorkspace { + let Some(options) = options else { + return NewWorkspace::default(); + }; + + let mut new_workspace = NewWorkspace::default(); + if let Some(family_name) = options.family_name { + new_workspace.family_name = family_name; + } + if let Some(units_per_em) = options.units_per_em { + new_workspace.units_per_em = units_per_em; + } + + new_workspace +} + impl TryFrom for FontExportRequest { type Error = Error; @@ -142,31 +162,17 @@ fn record_persisted_version(persisted_version: &SharedPersistedVersion, version: #[derive(Clone)] pub struct FontSaveSnapshot { - version: DocumentVersion, font: Font, active_glyph_override: Option>, } impl FontSaveSnapshot { - fn new(version: DocumentVersion, font: Font, active_glyph_override: Option) -> Self { + fn new(font: Font, active_glyph_override: Option) -> Self { Self { - version, font, active_glyph_override: active_glyph_override.map(Arc::new), } } - - fn version(&self) -> DocumentVersion { - self.version - } - - fn to_font(&self) -> Font { - let mut font = self.font.clone(); - if let Some(active_glyph) = self.active_glyph_override.as_ref() { - font.put_glyph(active_glyph.as_ref().clone()); - } - font - } } impl FontView for FontSaveSnapshot { @@ -249,31 +255,6 @@ impl FontView for FontSaveSnapshot { } } -pub struct SaveFontTask { - snapshot: FontSaveSnapshot, - persisted_version: SharedPersistedVersion, - path: String, -} - -impl Task for SaveFontTask { - type Output = DocumentVersion; - type JsValue = u32; - - fn compute(&mut self) -> Result { - let font = self.snapshot.to_font(); - FontLoader::new() - .write_font(&font, &self.path) - .map_err(|e| Error::new(Status::GenericFailure, e.to_string()))?; - - Ok(self.snapshot.version()) - } - - fn resolve(&mut self, _env: Env, output: Self::Output) -> Result { - record_persisted_version(&self.persisted_version, output); - Ok(output.as_u32()) - } -} - pub struct ExportFontTask { snapshot: FontSaveSnapshot, request: FontExportRequest, @@ -296,8 +277,7 @@ impl Task for ExportFontTask { #[napi] pub struct Bridge { - font_loader: FontLoader, - font: Font, + workspace: Option, live_version: DocumentVersion, persisted_version: SharedPersistedVersion, } @@ -307,67 +287,85 @@ impl Bridge { #[napi(constructor)] pub fn new() -> Self { Self { - font_loader: FontLoader::new(), - font: Font::default(), + workspace: None, live_version: DocumentVersion::default(), persisted_version: Arc::new(AtomicU64::new(0)), } } #[napi] - pub fn create_font(&mut self) { - self.font = Font::new(); + pub fn create_workspace( + &mut self, + source_path: String, + store_path: String, + options: Option, + ) -> errors::Result<()> { + self.workspace = Some(FontWorkspace::create( + source_path, + store_path, + new_workspace_from_options(options), + )?); self.live_version = DocumentVersion::default(); self.persisted_version = Arc::new(AtomicU64::new(0)); + Ok(()) } #[napi] - pub fn load_font(&mut self, path: String) -> errors::Result<()> { - self.font = self.font_loader.read_font(&path)?; + pub fn open_workspace(&mut self, path: String, store_path: String) -> errors::Result<()> { + self.workspace = Some(FontWorkspace::open(path, store_path)?); self.live_version = DocumentVersion::default(); self.persisted_version = Arc::new(AtomicU64::new(0)); Ok(()) } - #[napi(ts_return_type = "Promise")] - pub fn save_font(&mut self, path: String) -> AsyncTask { - AsyncTask::new(SaveFontTask { - snapshot: self.save_snapshot(), - persisted_version: self.persisted_version.clone(), - path, - }) + #[napi] + pub fn save_workspace(&mut self) -> errors::Result { + self.workspace_mut()?.save()?; + let version = self.live_version(); + record_persisted_version(&self.persisted_version, version); + Ok(version.as_u32()) + } + + #[napi] + pub fn save_workspace_as(&mut self, path: String) -> errors::Result { + self.workspace_mut()?.save_as(path)?; + let version = self.live_version(); + record_persisted_version(&self.persisted_version, version); + Ok(version.as_u32()) } #[napi(ts_return_type = "Promise")] - pub fn export_font( + pub fn export_workspace( &mut self, request: NapiFontExportRequest, ) -> Result> { Ok(AsyncTask::new(ExportFontTask { - snapshot: self.save_snapshot(), + snapshot: self + .save_snapshot() + .map_err(|e| Error::new(Status::GenericFailure, e.to_string()))?, request: request.try_into()?, })) } #[napi] - pub fn get_metadata(&self) -> NapiFontMetadata { - FontMetadata::from(self.font.metadata()).into() + pub fn get_metadata(&self) -> errors::Result { + Ok(FontMetadata::from(self.font()?.metadata()).into()) } #[napi] - pub fn get_metrics(&self) -> NapiFontMetrics { - FontMetrics::from(self.font.metrics()).into() + pub fn get_metrics(&self) -> errors::Result { + Ok(FontMetrics::from(self.font()?.metrics()).into()) } #[napi] - pub fn get_glyph_count(&self) -> u32 { - self.font.glyph_count() as u32 + pub fn get_glyph_count(&self) -> errors::Result { + Ok(self.font()?.glyph_count() as u32) } #[napi] - pub fn get_glyphs(&self) -> Vec { + pub fn get_glyphs(&self) -> errors::Result> { let mut records: Vec<_> = self - .font + .font()? .glyphs() .values() .map(|glyph| glyph.as_ref()) @@ -375,7 +373,7 @@ impl Bridge { .map(Into::into) .collect(); records.sort_by(|a: &NapiGlyphRecord, b: &NapiGlyphRecord| a.name.cmp(&b.name)); - records + Ok(records) } #[napi] @@ -385,48 +383,9 @@ impl Bridge { #[napi(ts_arg_type = "GlyphName")] name: String, #[napi(ts_arg_type = "Array")] unicodes: Vec, ) -> errors::Result<()> { - let name = name.trim(); - - let existing = self - .font - .glyph(&from_name) - .ok_or_else(|| BridgeError::InvalidInput { - kind: "glyph name", - value: from_name.clone(), - })?; - if from_name == name && existing.unicodes() == unicodes.as_slice() { - return Ok(()); - } - - if name.is_empty() { - return Err(BridgeError::InvalidInput { - kind: "glyph name", - value: name.to_string(), - }); - } - - if from_name != name && self.font.glyph(name).is_some() { - return Err(BridgeError::InvalidInput { - kind: "glyph name", - value: format!("{name} already exists"), - }); - } - - let Some(mut glyph) = self.font.take_glyph(&from_name) else { - return Err(BridgeError::InvalidInput { - kind: "glyph name", - value: from_name, - }); - }; - - let glyph_name = GlyphName::new(name.to_string()).map_err(|_| BridgeError::InvalidInput { - kind: "glyph name", - value: name.to_string(), - })?; - - glyph.set_name(glyph_name); - glyph.set_unicodes(unicodes); - self.font.put_glyph(glyph); + self + .workspace_mut()? + .update_glyph_identity(&from_name, name, unicodes)?; self.mark_font_changed(); Ok(()) @@ -441,7 +400,7 @@ impl Bridge { let source_id = parse::(&source_id)?; let layer_id = self.source_layer_id(source_id)?; - let glyph = match self.glyph_for_read(&glyph_handle.name) { + let glyph = match self.glyph_for_read(&glyph_handle.name)? { Some(glyph) => glyph, None => return Ok(None), }; @@ -451,7 +410,7 @@ impl Bridge { }; let variation_data = self - .variation_build_for_glyph(&glyph) + .variation_build_for_glyph(&glyph)? .and_then(|(_, build)| build.variation_data); Ok(Some(GlyphState::from_layer(layer, variation_data).into())) @@ -462,60 +421,70 @@ impl Bridge { &self, glyph_ref: GlyphHandle, ) -> Option { - let glyph = self.glyph_for_read(&glyph_ref.name)?; - Some(self.variation_report_for_glyph(&glyph_ref.name, &glyph)) + let glyph = self.glyph_for_read(&glyph_ref.name).ok()??; + self + .variation_report_for_glyph(&glyph_ref.name, &glyph) + .ok() } #[napi] - pub fn get_variation_reports(&self) -> Vec { + pub fn get_variation_reports(&self) -> errors::Result> { let mut glyph_names: Vec = self - .font + .font()? .glyphs() .keys() .map(|glyph_name| glyph_name.to_string()) .collect(); glyph_names.sort(); - glyph_names - .into_iter() - .filter_map(|glyph_name| { - self - .glyph_for_read(&glyph_name) - .map(|glyph| self.variation_report_for_glyph(&glyph_name, &glyph)) - }) - .collect() + Ok( + glyph_names + .into_iter() + .filter_map(|glyph_name| { + self + .glyph_for_read(&glyph_name) + .ok() + .flatten() + .and_then(|glyph| self.variation_report_for_glyph(&glyph_name, &glyph).ok()) + }) + .collect(), + ) } #[napi] - pub fn is_variable(&self) -> bool { - self.font.is_variable() + pub fn is_variable(&self) -> errors::Result { + Ok(self.font()?.is_variable()) } #[napi] - pub fn get_axes(&self) -> Vec { - self - .font - .axes() - .iter() - .map(Axis::from) - .map(Into::into) - .collect() + pub fn get_axes(&self) -> errors::Result> { + Ok( + self + .font()? + .axes() + .iter() + .map(Axis::from) + .map(Into::into) + .collect(), + ) } #[napi] - pub fn get_sources(&self) -> Vec { - self - .font - .sources() - .iter() - .map(Source::from) - .map(Into::into) - .collect() + pub fn get_sources(&self) -> errors::Result> { + Ok( + self + .font()? + .sources() + .iter() + .map(Source::from) + .map(Into::into) + .collect(), + ) } fn source_layer_id(&self, source_id: SourceId) -> BridgeResult { if let Some(source) = self - .font + .font()? .sources() .iter() .find(|source| source.id() == source_id) @@ -529,49 +498,43 @@ impl Bridge { }) } - fn save_snapshot(&self) -> FontSaveSnapshot { - FontSaveSnapshot::new(self.live_version(), self.font.clone(), None) + fn save_snapshot(&self) -> BridgeResult { + Ok(FontSaveSnapshot::new(self.font()?.clone(), None)) } - fn glyph_for_read(&self, glyph_name: &str) -> Option { - self.font.glyph(glyph_name).cloned() + fn glyph_for_read(&self, glyph_name: &str) -> BridgeResult> { + Ok(self.font()?.glyph(glyph_name).cloned()) } - fn editable_layer_mut(&mut self, glyph_ref: GlyphLayerRef) -> BridgeResult<&mut GlyphLayer> { + fn glyph_layer_target(&self, glyph_ref: GlyphLayerRef) -> BridgeResult { let layer_id = parse::(&glyph_ref.layer_id)?; - let glyph_name = glyph_ref.glyph_handle.name; - let unicode = glyph_ref.glyph_handle.unicode; - - if self.font.glyph(&glyph_name).is_none() { - let mut glyph = Glyph::new(glyph_name.clone()); - if let Some(unicode) = unicode { - glyph.add_unicode(unicode); - } - glyph.set_layer(layer_id, GlyphLayer::with_width(500.0)); - self.font.insert_glyph(glyph); - } - - let glyph = self - .font - .glyph_mut(&glyph_name) - .ok_or_else(|| BridgeError::InvalidInput { - kind: "glyph name", - value: glyph_name.clone(), - })?; - - if let Some(unicode) = unicode { - glyph.add_unicode(unicode); - } + Ok(GlyphLayerTarget { + glyph_name: glyph_ref.glyph_handle.name, + unicode: glyph_ref.glyph_handle.unicode, + layer_id, + }) + } - Ok(glyph.get_or_create_layer(layer_id)) + fn edit_glyph_layer( + &mut self, + glyph_ref: GlyphLayerRef, + edit: impl FnOnce(&mut GlyphLayer) -> std::result::Result, + ) -> BridgeResult { + let target = self.glyph_layer_target(glyph_ref)?; + Ok(self.workspace_mut()?.edit_glyph_layer(target, edit)?) } - fn variation_build_for_glyph(&self, glyph: &Glyph) -> Option<(usize, GlyphVariationBuild)> { - build_masters(&self.font, glyph).map(|masters| { + fn variation_build_for_glyph( + &self, + glyph: &Glyph, + ) -> BridgeResult> { + let font = self.font()?; + + Ok(build_masters(font, glyph).map(|masters| { let master_count = masters.len(); - let build = build_glyph_variation_data(&masters, self.font.axes()); + let build = build_glyph_variation_data(&masters, font.axes()); (master_count, build) - }) + })) } fn variation_diagnostics_for_build( @@ -624,9 +587,9 @@ impl Bridge { &self, glyph_name: &str, glyph: &Glyph, - ) -> NapiGlyphVariationReport { - let Some((master_count, build)) = self.variation_build_for_glyph(glyph) else { - return NapiGlyphVariationReport { + ) -> BridgeResult { + let Some((master_count, build)) = self.variation_build_for_glyph(glyph)? else { + return Ok(NapiGlyphVariationReport { glyph_name: glyph_name.to_string(), status: "static".to_string(), variation_data_available: false, @@ -634,7 +597,7 @@ impl Bridge { compatible_master_count: 0, skipped_master_count: 0, diagnostics: Vec::new(), - }; + }); }; let diagnostics = Self::variation_diagnostics_for_build(glyph_name, &build); @@ -654,7 +617,7 @@ impl Bridge { (false, _) => "unavailable", }; - NapiGlyphVariationReport { + Ok(NapiGlyphVariationReport { glyph_name: glyph_name.to_string(), status: status.to_string(), variation_data_available, @@ -662,7 +625,31 @@ impl Bridge { compatible_master_count: compatible_master_count.min(u32::MAX as usize) as u32, skipped_master_count: skipped_master_count.min(u32::MAX as usize) as u32, diagnostics, - } + }) + } + + fn workspace(&self) -> BridgeResult<&FontWorkspace> { + self + .workspace + .as_ref() + .ok_or_else(|| BridgeError::InvalidInput { + kind: "workspace", + value: "no workspace is open".to_string(), + }) + } + + fn workspace_mut(&mut self) -> BridgeResult<&mut FontWorkspace> { + self + .workspace + .as_mut() + .ok_or_else(|| BridgeError::InvalidInput { + kind: "workspace", + value: "no workspace is open".to_string(), + }) + } + + fn font(&self) -> BridgeResult<&Font> { + Ok(self.workspace()?.font()) } fn mark_font_changed(&mut self) { @@ -697,11 +684,10 @@ impl Bridge { glyph_ref: GlyphLayerRef, width: f64, ) -> errors::Result { - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.set_x_advance(width); - GlyphValueChange::from_layer(layer, Default::default()) - }; + Ok(GlyphValueChange::from_layer(layer, Default::default())) + })?; self.mark_font_changed(); Ok(change.into()) @@ -714,11 +700,10 @@ impl Bridge { dx: f64, dy: f64, ) -> errors::Result { - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.translate_layer(dx, dy); - GlyphValueChange::from_layer(layer, Default::default()) - }; + Ok(GlyphValueChange::from_layer(layer, Default::default())) + })?; self.mark_font_changed(); Ok(change.into()) @@ -737,15 +722,14 @@ impl Bridge { let contour_id = parse::(&contour_id)?; let point_type = point_type.into(); - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { let point_id = layer.add_point_to_contour(contour_id, x, y, point_type, smooth)?; let changed = GlyphChangedEntities { point_ids: vec![point_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -764,15 +748,14 @@ impl Bridge { let before_point_id = parse::(&before_point_id)?; let point_type = point_type.into(); - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { let point_id = layer.insert_point_before(before_point_id, x, y, point_type, smooth)?; let changed = GlyphChangedEntities { point_ids: vec![point_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -780,15 +763,14 @@ impl Bridge { #[napi] pub fn add_contour(&mut self, glyph_ref: GlyphLayerRef) -> Result { - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { let contour_id = layer.add_empty_contour(); let changed = GlyphChangedEntities { contour_ids: vec![contour_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -801,15 +783,14 @@ impl Bridge { #[napi(ts_arg_type = "ContourId")] contour_id: String, ) -> errors::Result { let contour_id = parse::(&contour_id)?; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.open_contour(contour_id)?; let changed = GlyphChangedEntities { contour_ids: vec![contour_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -822,15 +803,14 @@ impl Bridge { #[napi(ts_arg_type = "ContourId")] contour_id: String, ) -> errors::Result { let contour_id = parse::(&contour_id)?; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.close_contour(contour_id)?; let changed = GlyphChangedEntities { contour_ids: vec![contour_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -843,15 +823,14 @@ impl Bridge { #[napi(ts_arg_type = "ContourId")] contour_id: String, ) -> errors::Result { let contour_id = parse::(&contour_id)?; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.reverse_contour(contour_id)?; let changed = GlyphChangedEntities { contour_ids: vec![contour_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -881,15 +860,14 @@ impl Bridge { } }; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { let created_ids = layer.apply_boolean_op(cid_a, cid_b, op)?; let changed = GlyphChangedEntities { contour_ids: created_ids, ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -904,12 +882,11 @@ impl Bridge { let point_ids: BridgeResult> = point_ids.iter().map(|id| parse::(id)).collect(); let point_ids = point_ids?; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.remove_points(&point_ids)?; let changed = GlyphChangedEntities::points(point_ids); - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -922,15 +899,14 @@ impl Bridge { #[napi(ts_arg_type = "PointId")] point_id: String, ) -> errors::Result { let parsed_id = parse::(&point_id)?; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { layer.toggle_smooth(parsed_id)?; let changed = GlyphChangedEntities { point_ids: vec![parsed_id], ..Default::default() }; - GlyphStructureChange::from_layer(layer, changed) - }; + Ok(GlyphStructureChange::from_layer(layer, changed)) + })?; self.mark_font_changed(); Ok(change.into()) @@ -947,8 +923,7 @@ impl Bridge { anchor_ids: Option, anchor_coords: Option, ) -> errors::Result<()> { - { - let layer = self.editable_layer_mut(glyph_ref)?; + self.edit_glyph_layer(glyph_ref, |layer| { layer.apply_bulk_node_positions(BulkNodePositionUpdates { point_ids: point_ids.as_ref().map(|ids| { let ids: &[u64] = ids; @@ -967,7 +942,8 @@ impl Bridge { coords }), })?; - } + Ok(()) + })?; self.mark_font_changed(); Ok(()) @@ -983,11 +959,10 @@ impl Bridge { let structure = GlyphStructure::from(structure); let values: &[f64] = &values; - let change = { - let layer = self.editable_layer_mut(glyph_ref)?; + let change = self.edit_glyph_layer(glyph_ref, |layer| { apply_state_to_layer(layer, &structure, values)?; - GlyphStructureChange::from_layer(layer, Default::default()) - }; + Ok(GlyphStructureChange::from_layer(layer, Default::default())) + })?; self.mark_font_changed(); Ok(change.into()) @@ -997,8 +972,7 @@ impl Bridge { #[cfg(test)] mod tests { use super::*; - use shift_font::{Contour, PointType}; - use std::time::{Duration, Instant}; + use std::sync::atomic::{AtomicUsize, Ordering}; fn glyph_handle(name: &str, unicode: Option) -> GlyphHandle { GlyphHandle { @@ -1007,98 +981,62 @@ mod tests { } } - #[derive(Clone, Copy)] - struct PerfFontMark { - label: &'static str, - glyphs: usize, - contours_per_glyph: usize, - points_per_contour: usize, - } - - impl PerfFontMark { - fn total_points(self) -> usize { - self.glyphs * self.contours_per_glyph * self.points_per_contour - } - } - - fn point_heavy_layer(mark: PerfFontMark, glyph_index: usize) -> GlyphLayer { - let mut layer = GlyphLayer::with_width(500.0 + glyph_index as f64); - - for contour_index in 0..mark.contours_per_glyph { - let mut contour = Contour::new(); - for point_index in 0..mark.points_per_contour { - contour.add_point( - point_index as f64, - (glyph_index + contour_index + point_index) as f64, - PointType::OnCurve, - false, - ); - } - layer.add_contour(contour); - } - - layer - } + static TEST_ID: AtomicUsize = AtomicUsize::new(0); - fn point_heavy_glyph( - name: impl Into, - unicode: u32, - layer_id: LayerId, - mark: PerfFontMark, - ) -> Glyph { - let mut glyph = Glyph::with_unicode(name.into(), unicode); - glyph.set_layer(layer_id, point_heavy_layer(mark, unicode as usize)); - glyph + fn test_paths(label: &str) -> (String, String) { + let id = TEST_ID.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("shift-bridge-{label}-{id}")); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + ( + dir.join("TestFont.shift").to_string_lossy().into_owned(), + dir.join("working.sqlite").to_string_lossy().into_owned(), + ) } - fn point_heavy_font(mark: PerfFontMark) -> Font { - let mut font = Font::new(); - let default_layer_id = font.default_layer_id(); - - for glyph_index in 0..mark.glyphs { - font.insert_glyph(point_heavy_glyph( - format!("g{glyph_index:05}"), - glyph_index as u32, - default_layer_id, - mark, - )); - } - - font + fn bridge_with_workspace() -> Bridge { + let mut bridge = Bridge::new(); + let (source_path, store_path) = test_paths("workspace"); + bridge + .create_workspace(source_path, store_path, None) + .unwrap(); + bridge } fn default_source_id(bridge: &Bridge) -> String { - bridge.get_sources()[0].id.clone() + bridge.get_sources().unwrap()[0].id.clone() } fn default_layer_ref(bridge: &Bridge, name: &str, unicode: Option) -> GlyphLayerRef { GlyphLayerRef { glyph_handle: glyph_handle(name, unicode), - layer_id: bridge.get_sources()[0].layer_id.clone(), + layer_id: bridge.get_sources().unwrap()[0].layer_id.clone(), } } - fn print_perf_mark(operation: &str, mark: PerfFontMark, elapsed: Duration) { - eprintln!( - "perf_mark {operation} [{}]: {} glyphs / {} points in {:?}", - mark.label, - mark.glyphs, - mark.total_points(), - elapsed - ); + #[test] + fn new_bridge_requires_workspace_before_font_reads() { + let bridge = Bridge::new(); + + let error = match bridge.get_metadata() { + Ok(_) => panic!("metadata read should require an open workspace"), + Err(error) => error, + }; + + assert!(error.to_string().contains("no workspace is open")); } #[test] - fn new_bridge_exposes_empty_committed_font_state() { - let bridge = Bridge::new(); + fn create_workspace_exposes_empty_font_state() { + let bridge = bridge_with_workspace(); - let metadata = bridge.get_metadata(); - let metrics = bridge.get_metrics(); + let metadata = bridge.get_metadata().unwrap(); + let metrics = bridge.get_metrics().unwrap(); - assert_eq!(bridge.get_glyph_count(), 0); - assert!(bridge.get_glyphs().is_empty()); - assert_eq!(bridge.get_sources().len(), 1); - assert_eq!(bridge.get_sources()[0].name, "Regular"); + assert_eq!(bridge.get_glyph_count().unwrap(), 0); + assert!(bridge.get_glyphs().unwrap().is_empty()); + assert_eq!(bridge.get_sources().unwrap().len(), 1); + assert_eq!(bridge.get_sources().unwrap()[0].name, "Regular"); assert_eq!(metadata.family_name.as_deref(), Some("Untitled Font")); assert_eq!(metadata.style_name.as_deref(), Some("Regular")); assert_eq!(metrics.units_per_em, 1000.0); @@ -1107,23 +1045,26 @@ mod tests { } #[test] - fn create_font_resets_to_fresh_font_state() { - let mut bridge = Bridge::new(); + fn create_workspace_resets_to_fresh_font_state() { + let mut bridge = bridge_with_workspace(); bridge .set_x_advance(default_layer_ref(&bridge, "A", Some(65)), 500.0) .unwrap(); - bridge.create_font(); + let (source_path, store_path) = test_paths("reset"); + bridge + .create_workspace(source_path, store_path, None) + .unwrap(); - assert_eq!(bridge.get_glyph_count(), 0); - assert!(bridge.get_axes().is_empty()); - assert_eq!(bridge.get_sources().len(), 1); - assert_eq!(bridge.get_sources()[0].name, "Regular"); + assert_eq!(bridge.get_glyph_count().unwrap(), 0); + assert!(bridge.get_axes().unwrap().is_empty()); + assert_eq!(bridge.get_sources().unwrap().len(), 1); + assert_eq!(bridge.get_sources().unwrap()[0].name, "Regular"); } #[test] fn add_contour_returns_structure_change() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); let change = bridge.add_contour(glyph_ref).unwrap(); @@ -1139,7 +1080,7 @@ mod tests { #[test] fn save_snapshot_includes_direct_glyph_layer_edit() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); let contour_id = bridge .add_contour(glyph_ref.clone()) @@ -1161,7 +1102,7 @@ mod tests { .point_ids[0] .clone(); - let snapshot = bridge.save_snapshot(); + let snapshot = bridge.save_snapshot().unwrap(); let glyph = snapshot .glyph("A") .expect("snapshot should include edited A"); @@ -1169,7 +1110,7 @@ mod tests { .layer(snapshot.default_layer_id()) .expect("edited glyph should include default layer"); - assert_eq!(bridge.get_glyphs().len(), 1); + assert_eq!(bridge.get_glyphs().unwrap().len(), 1); assert_eq!(glyph.unicodes(), &[65]); assert_eq!(layer.contours().len(), 1); assert_eq!( @@ -1181,48 +1122,24 @@ mod tests { } #[test] - fn save_task_routes_designspace_paths_through_font_loader() { + fn open_workspace_imports_designspace_fonts() { let mut bridge = Bridge::new(); - let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); - bridge.add_contour(glyph_ref).unwrap(); - - let dir = std::env::temp_dir().join("shift_bridge_designspace_save_task"); - let designspace_path = dir.join("Smoke.designspace"); - let ufo_path = dir.join("Smoke.ufo"); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).unwrap(); - - let mut task = SaveFontTask { - snapshot: bridge.save_snapshot(), - persisted_version: bridge.persisted_version.clone(), - path: designspace_path.to_string_lossy().into_owned(), - }; - - task.compute().unwrap(); + let (_, store_path) = test_paths("import"); + let designspace_path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .join("fixtures/fonts/mutatorsans-variable/MutatorSans.designspace"); - assert!( - designspace_path.is_file(), - "designspace save should create an XML file, not a UFO directory" - ); - assert!( - ufo_path.is_dir(), - "designspace save should create a companion UFO source" - ); - - let mut reloaded = Bridge::new(); - reloaded - .load_font(designspace_path.to_string_lossy().into_owned()) + bridge + .open_workspace(designspace_path.to_string_lossy().into_owned(), store_path) .unwrap(); - let glyphs = reloaded.get_glyphs(); - assert_eq!(glyphs.len(), 1); - assert_eq!(glyphs[0].name, "A"); - let _ = std::fs::remove_dir_all(&dir); + assert!(bridge.get_glyph_count().unwrap() > 0); + assert!(bridge.is_variable().unwrap()); } #[test] fn persisted_older_snapshot_keeps_document_dirty_after_new_edit() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); let contour_id = bridge .add_contour(glyph_ref.clone()) @@ -1230,7 +1147,7 @@ mod tests { .changed .contour_ids[0] .clone(); - let snapshot = bridge.save_snapshot(); + let snapshot_version = bridge.live_version(); bridge .add_point( @@ -1242,24 +1159,25 @@ mod tests { false, ) .unwrap(); - record_persisted_version(&bridge.persisted_version, snapshot.version()); + record_persisted_version(&bridge.persisted_version, snapshot_version); - assert_eq!(snapshot.version().as_u32(), 1); + assert_eq!(snapshot_version.as_u32(), 1); assert_eq!(bridge.live_version().as_u32(), 2); assert_eq!(bridge.get_persisted_version(), 1); assert!(bridge.is_dirty()); } #[test] - fn load_resets_persisted_version_handle_for_old_async_saves() { - let mut bridge = Bridge::new(); + fn opening_workspace_resets_persisted_version_handle_for_old_saves() { + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); bridge.add_contour(glyph_ref).unwrap(); let old_persisted_version = bridge.persisted_version.clone(); - bridge.font = Font::default(); - bridge.live_version = DocumentVersion::default(); - bridge.persisted_version = Arc::new(AtomicU64::new(0)); + let (source_path, store_path) = test_paths("reopen"); + bridge + .create_workspace(source_path, store_path, None) + .unwrap(); record_persisted_version(&old_persisted_version, DocumentVersion(1)); assert_eq!(bridge.get_persisted_version(), 0); @@ -1268,7 +1186,7 @@ mod tests { #[test] fn add_point_returns_structure_and_changed_point() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); let contour_id = bridge .add_contour(glyph_ref.clone()) @@ -1298,7 +1216,7 @@ mod tests { #[test] fn get_glyph_state_reads_direct_glyph_layer_edit() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let glyph_ref = default_layer_ref(&bridge, "A", Some(65)); let contour_id = bridge .add_contour(glyph_ref.clone()) @@ -1322,7 +1240,7 @@ mod tests { .unwrap() .expect("edited glyph should be readable"); - assert_eq!(bridge.get_glyphs().len(), 1); + assert_eq!(bridge.get_glyphs().unwrap().len(), 1); assert_eq!(state.structure.contours.len(), 1); assert_eq!(state.structure.contours[0].points.len(), 1); assert_eq!(&state.values[..], &[500.0, 10.0, 20.0]); @@ -1330,7 +1248,7 @@ mod tests { #[test] fn get_glyph_state_returns_none_for_missing_glyph() { - let bridge = Bridge::new(); + let bridge = bridge_with_workspace(); assert!(bridge .get_glyph_state(glyph_handle("missing", None), default_source_id(&bridge)) @@ -1340,7 +1258,7 @@ mod tests { #[test] fn edit_methods_require_valid_layer_ref() { - let mut bridge = Bridge::new(); + let mut bridge = bridge_with_workspace(); let result = bridge.add_contour(GlyphLayerRef { glyph_handle: glyph_handle("A", Some(65)), @@ -1349,32 +1267,4 @@ mod tests { assert!(result.err().unwrap().reason.contains("invalid layer ID")); } - - #[test] - fn perf_mark_save_snapshot_setup_with_committed_font() { - let committed_mark = PerfFontMark { - label: "cjk-scale committed", - glyphs: 10_000, - contours_per_glyph: 2, - points_per_contour: 8, - }; - let mut bridge = Bridge::new(); - bridge.font = point_heavy_font(committed_mark); - - let start = Instant::now(); - let snapshots: Vec<_> = (0..128).map(|_| bridge.save_snapshot()).collect(); - let elapsed = start.elapsed(); - - for snapshot in &snapshots { - assert_eq!(snapshot.version().as_u32(), bridge.live_version().as_u32()); - assert_eq!(snapshot.glyphs().len(), committed_mark.glyphs); - } - assert_eq!(bridge.get_glyph_count(), committed_mark.glyphs as u32); - - print_perf_mark("save_snapshot committed x128", committed_mark, elapsed); - assert!( - elapsed < Duration::from_secs(1), - "committed save snapshot setup should stay comfortably sub-second; got {elapsed:?}" - ); - } } diff --git a/crates/shift-bridge/src/errors.rs b/crates/shift-bridge/src/errors.rs index cf7833d8..d048e3c6 100644 --- a/crates/shift-bridge/src/errors.rs +++ b/crates/shift-bridge/src/errors.rs @@ -1,6 +1,7 @@ use napi::{Error, JsError, Status}; use shift_backends::BackendError; use shift_font::error::CoreError; +use shift_workspace::WorkspaceError; #[derive(Debug, thiserror::Error)] pub enum BridgeError { @@ -12,6 +13,9 @@ pub enum BridgeError { #[error(transparent)] Backend(#[from] BackendError), + + #[error(transparent)] + Workspace(#[from] WorkspaceError), } pub fn to_napi_error(error: BridgeError) -> Error { @@ -22,7 +26,9 @@ pub fn to_napi_error(error: BridgeError) -> Error { | BridgeError::Backend(BackendError::InvalidExtensionUtf8 { .. }) | BridgeError::Backend(BackendError::UnsupportedFormat { .. }) | BridgeError::Backend(BackendError::UnsupportedWriteFormat { .. }) => Status::InvalidArg, - BridgeError::Core(_) | BridgeError::Backend(_) => Status::GenericFailure, + BridgeError::Core(_) | BridgeError::Backend(_) | BridgeError::Workspace(_) => { + Status::GenericFailure + } }; Error::new(status, error.to_string()) diff --git a/crates/shift-workspace/src/lib.rs b/crates/shift-workspace/src/lib.rs index 91ff39cf..53e8d1d5 100644 --- a/crates/shift-workspace/src/lib.rs +++ b/crates/shift-workspace/src/lib.rs @@ -2,4 +2,4 @@ mod new_workspace; mod workspace; pub use new_workspace::NewWorkspace; -pub use workspace::{FontWorkspace, WorkspaceError, WorkspaceSource}; +pub use workspace::{FontWorkspace, GlyphLayerTarget, WorkspaceError, WorkspaceSource}; diff --git a/crates/shift-workspace/src/workspace.rs b/crates/shift-workspace/src/workspace.rs index 9d2692ce..a80b8282 100644 --- a/crates/shift-workspace/src/workspace.rs +++ b/crates/shift-workspace/src/workspace.rs @@ -1,6 +1,7 @@ use std::path::{Path, PathBuf}; use shift_backends::{FontExportRequest, FontExportResult, FontExporter, font_loader::FontLoader}; +use shift_font::{Glyph, GlyphLayer, LayerId, error::CoreError}; use shift_source::ShiftSourcePackage; use shift_store::ShiftStore; @@ -8,6 +9,12 @@ use crate::NewWorkspace; #[derive(Debug, thiserror::Error)] pub enum WorkspaceError { + #[error("invalid {kind}: {value}")] + InvalidInput { kind: &'static str, value: String }, + + #[error(transparent)] + Font(#[from] CoreError), + #[error("source package error")] Source(#[from] shift_source::SourcePackageError), @@ -33,6 +40,13 @@ pub enum WorkspaceSource { Imported { original_path: PathBuf }, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GlyphLayerTarget { + pub glyph_name: String, + pub unicode: Option, + pub layer_id: LayerId, +} + pub struct FontWorkspace { font: shift_font::Font, source: WorkspaceSource, @@ -99,6 +113,99 @@ impl FontWorkspace { .map_err(WorkspaceError::from) } + pub fn update_glyph_identity( + &mut self, + from_name: &str, + name: String, + unicodes: Vec, + ) -> Result<(), WorkspaceError> { + let name = name.trim(); + let mut next_font = self.font.clone(); + + let existing = next_font + .glyph(from_name) + .ok_or_else(|| WorkspaceError::InvalidInput { + kind: "glyph name", + value: from_name.to_string(), + })?; + if from_name == name && existing.unicodes() == unicodes.as_slice() { + return Ok(()); + } + + if name.is_empty() { + return Err(WorkspaceError::InvalidInput { + kind: "glyph name", + value: name.to_string(), + }); + } + + if from_name != name && next_font.glyph(name).is_some() { + return Err(WorkspaceError::InvalidInput { + kind: "glyph name", + value: format!("{name} already exists"), + }); + } + + let Some(mut glyph) = next_font.take_glyph(from_name) else { + return Err(WorkspaceError::InvalidInput { + kind: "glyph name", + value: from_name.to_string(), + }); + }; + + let glyph_name = shift_font::GlyphName::new(name.to_string()).map_err(|_| { + WorkspaceError::InvalidInput { + kind: "glyph name", + value: name.to_string(), + } + })?; + + glyph.set_name(glyph_name); + glyph.set_unicodes(unicodes); + next_font.put_glyph(glyph); + + self.commit_font(next_font); + Ok(()) + } + + pub fn edit_glyph_layer( + &mut self, + target: GlyphLayerTarget, + edit: impl FnOnce(&mut GlyphLayer) -> Result, + ) -> Result { + let mut next_font = self.font.clone(); + + if next_font.glyph(&target.glyph_name).is_none() { + let mut glyph = Glyph::new(target.glyph_name.clone()); + if let Some(unicode) = target.unicode { + glyph.add_unicode(unicode); + } + glyph.set_layer(target.layer_id, GlyphLayer::with_width(500.0)); + next_font.insert_glyph(glyph); + } + + let glyph = next_font.glyph_mut(&target.glyph_name).ok_or_else(|| { + WorkspaceError::InvalidInput { + kind: "glyph name", + value: target.glyph_name.clone(), + } + })?; + + if let Some(unicode) = target.unicode { + glyph.add_unicode(unicode); + } + + let result = edit(glyph.get_or_create_layer(target.layer_id))?; + self.commit_font(next_font); + + Ok(result) + } + + fn commit_font(&mut self, next_font: shift_font::Font) { + // TODO: apply a shift-font domain change set to shift-store before swapping. + self.font = next_font; + } + fn open_package( source_path: impl AsRef, store_path: impl AsRef, diff --git a/packages/types/src/bridge/generated.ts b/packages/types/src/bridge/generated.ts index ed006d70..3f5b9b82 100644 --- a/packages/types/src/bridge/generated.ts +++ b/packages/types/src/bridge/generated.ts @@ -16,10 +16,11 @@ export type GlyphName = string; export type Unicode = number; export interface BridgeApi { - createFont(): void - loadFont(path: string): void - saveFont(path: string): Promise - exportFont(request: FontExportRequest): Promise + createWorkspace(sourcePath: string, storePath: string, options?: NewWorkspace | undefined | null): void + openWorkspace(path: string, storePath: string): void + saveWorkspace(): number + saveWorkspaceAs(path: string): number + exportWorkspace(request: FontExportRequest): Promise getMetadata(): FontMetadata getMetrics(): FontMetrics getGlyphCount(): number @@ -95,6 +96,11 @@ export interface GlyphVariationReport { skippedMasterCount: number diagnostics: Array } + +export interface NewWorkspace { + familyName?: string + unitsPerEm?: number +} export interface AnchorData { id: AnchorId name?: string