From 30a9ae23ea60894cb1cbc8181914d8775f7e1b94 Mon Sep 17 00:00:00 2001 From: Kostya Farber Date: Sun, 28 Jun 2026 18:37:54 +0100 Subject: [PATCH] Split glyph API by identity layer and instance --- AGENTS.md | 4 +- apps/desktop/src/renderer/src/app/Screens.tsx | 2 +- .../src/components/debug/DebugPanel.tsx | 7 +- .../renderer/src/components/editor/Canvas.tsx | 2 +- .../editor/sidebar-right/AnchorSection.tsx | 13 +- .../editor/sidebar-right/GlyphSection.tsx | 11 +- .../src/components/home/GlyphGrid.tsx | 23 +- .../src/components/home/GlyphPreview.tsx | 41 +- .../src/renderer/src/context/DebugContext.tsx | 2 +- .../renderer/src/hooks/useDesignLocation.ts | 2 +- .../src/hooks/useGlyphSidebearings.ts | 9 +- .../renderer/src/hooks/useGlyphXAdvance.ts | 7 +- .../renderer/src/hooks/useLayerSourceId.ts | 2 +- .../src/lib/commands/core/CommandRunner.ts | 8 +- .../renderer/src/lib/editor/Editor.test.ts | 25 +- .../src/renderer/src/lib/editor/Editor.ts | 143 +++--- .../renderer/src/lib/editor/EditorState.ts | 113 ++--- .../src/renderer/src/lib/editor/docs/DOCS.md | 6 +- .../src/lib/editor/rendering/RenderFrame.ts | 44 +- .../renderer/src/lib/editor/rendering/Text.ts | 6 +- .../rendering/overlays/DebugOverlays.ts | 49 +- .../src/lib/keyboard/KeyboardRouter.test.ts | 4 +- .../src/renderer/src/lib/model/Font.test.ts | 106 ++-- .../src/renderer/src/lib/model/Font.ts | 455 +++++++++++++----- .../src/renderer/src/lib/model/Glyph.test.ts | 78 ++- .../src/renderer/src/lib/model/Glyph.ts | 250 ++++------ .../renderer/src/lib/model/GlyphOutline.ts | 60 ++- .../renderer/src/lib/model/variation.test.ts | 63 ++- .../src/renderer/src/lib/signals/docs/DOCS.md | 26 +- .../src/lib/text/layout/Positioner.test.ts | 3 +- .../src/lib/text/layout/Positioner.ts | 18 +- .../src/lib/text/layout/TextLayout.test.ts | 2 +- .../renderer/src/lib/text/layout/testUtils.ts | 2 +- .../renderer/src/lib/tools/pen/PenStroke.ts | 7 +- .../src/lib/tools/select/BoundingBox.test.ts | 8 +- .../src/lib/tools/select/BoundingBox.ts | 9 +- .../renderer/src/lib/tools/select/Select.ts | 5 +- .../lib/tools/select/behaviors/BendCurve.ts | 4 +- .../src/lib/tools/select/behaviors/Marquee.ts | 2 +- .../src/lib/tools/select/behaviors/Resize.ts | 2 +- .../src/lib/tools/select/behaviors/Rotate.ts | 4 +- .../select/behaviors/SegmentDoubleClick.ts | 4 +- .../lib/tools/select/behaviors/SelectHover.ts | 2 +- .../lib/tools/select/behaviors/Selection.ts | 2 +- .../tools/select/behaviors/ToggleSmooth.ts | 4 +- .../lib/tools/select/behaviors/Translate.ts | 6 +- .../tools/select/behaviors/UpgradeSegment.ts | 4 +- .../src/renderer/src/lib/tools/text/Text.ts | 15 +- .../src/renderer/src/testing/TestEditor.ts | 24 +- docs/architecture/index.md | 2 +- scripts/context-drift-check.py | 139 +++++- 51 files changed, 1110 insertions(+), 719 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ae506840..f4b1853b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,8 @@ - **Domain types are plain nouns.** `Glyph`, `Contour`, `Point`, `Anchor` — not `Glyph`, `GlyphInfo`, `GlyphState`, `GlyphRenderData`. If you need a modifier, it should describe the _kind_ of thing (`EditableGlyph`, `RenderContour`), not append generic suffixes. - **Avoid `-Data`, `-Info`, `-State` suffixes** on types unless it genuinely represents transient mutable state (e.g. `TextRunRenderState` for a signal value consumed by a render pass). If the type represents a domain concept, name it after the concept. +- **Signals are named `*Cell`; values use the plain noun.** A `Signal` is `glyphCell`, and the unwrapped value is `glyph`. Do not introduce new dollar-prefixed signal names; those are legacy style. +- **Use `track(cell)` for invalidation-only subscriptions.** Inside `computed`/`effect`, use `track(fooCell)` when the code needs to rerun on `fooCell` changes but does not use the value. Use `const foo = fooCell.value` when the value is actually read. ## Roadmap @@ -161,7 +163,7 @@ Rules enforced by `scripts/oxlint/shift-plugin.mjs` are omitted here — the lin **NEVER call `bridge.setNodePositions()` inside the draft hot path.** That sends N individual NAPI struct marshals to Rust per frame. For glyphs with thousands of points this causes ~450ms frames + GC pressure. The draft exists specifically to avoid this. -**Render effects track `glyph.contours` and `glyph.anchors`**, not `$glyph`. The `$glyph` signal on NativeBridge is for glyph identity (loaded/unloaded). Glyph data changes propagate through the Glyph model's internal signals. `#patchPositions` fires `#contours` with a new array reference so glyph-level effects see the change. +**Render effects track `glyph.contours` and `glyph.anchors`**, not a bridge glyph identity cell. Bridge glyph identity signals are for loaded/unloaded identity changes. Glyph data changes propagate through the Glyph model's internal signals. `#patchPositions` fires the contour cell with a new array reference so glyph-level effects see the change. ## Documentation Routing diff --git a/apps/desktop/src/renderer/src/app/Screens.tsx b/apps/desktop/src/renderer/src/app/Screens.tsx index 249cc9d2..ff793cc6 100644 --- a/apps/desktop/src/renderer/src/app/Screens.tsx +++ b/apps/desktop/src/renderer/src/app/Screens.tsx @@ -42,7 +42,7 @@ const WorkspaceScreens = () => { const workspace = useWorkspace(); const font = useFont(); const editor = useEditor(); - const documentLoaded = useSignalState(font.$loaded); + const documentLoaded = useSignalState(font.loadedCell); const [connectionError, setConnectionError] = useState(null); useEffect(() => { diff --git a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx index a5d50ea5..856b4773 100644 --- a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx +++ b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx @@ -17,20 +17,21 @@ function formatBytes(bytes: number): string { export function DebugPanel() { const editor = useEditor(); - const instance = useSignalState(editor.glyphInstanceCell); + const instance = useSignalState(editor.previewGlyphInstanceCell); + const layer = useSignalState(editor.editingGlyphLayerCell); useSignalTrigger(instance?.xAdvanceCell); const glyphStats = useMemo(() => { if (!instance) return { pointCount: "0", snapshotSize: "—" }; - const snapshot = instance.layer?.state ?? null; + const snapshot = layer?.state ?? null; const bytes = snapshot ? new Blob([JSON.stringify(snapshot)]).size : null; return { pointCount: `${instance.geometry.allPoints.length}`, snapshotSize: bytes === null ? "—" : formatBytes(bytes), }; - }, [instance]); + }, [instance, layer]); useEffect(() => { editor.startFpsMonitor(); diff --git a/apps/desktop/src/renderer/src/components/editor/Canvas.tsx b/apps/desktop/src/renderer/src/components/editor/Canvas.tsx index f24afc9b..f52dc2f2 100644 --- a/apps/desktop/src/renderer/src/components/editor/Canvas.tsx +++ b/apps/desktop/src/renderer/src/components/editor/Canvas.tsx @@ -28,7 +28,7 @@ export const Canvas: FC = () => { } const glyphId = asGlyphId(glyphIdParam); - if (!editor.font.recordForId(glyphId)) { + if (!editor.font.glyph(glyphId)) { editor.scene.clear(); return undefined; } diff --git a/apps/desktop/src/renderer/src/components/editor/sidebar-right/AnchorSection.tsx b/apps/desktop/src/renderer/src/components/editor/sidebar-right/AnchorSection.tsx index d69b633a..e99bf5f5 100644 --- a/apps/desktop/src/renderer/src/components/editor/sidebar-right/AnchorSection.tsx +++ b/apps/desktop/src/renderer/src/components/editor/sidebar-right/AnchorSection.tsx @@ -12,7 +12,7 @@ export const AnchorSection = () => { const [singleAnchorId, setSingleAnchorId] = useState(null); const [anchorName, setAnchorName] = useState(null); - const [hasLayer, setHasLayer] = useState(false); + const [editable, setEditable] = useState(false); const [anchorX, setAnchorX] = useState(0); const [anchorY, setAnchorY] = useState(0); @@ -21,10 +21,11 @@ export const AnchorSection = () => { const yRef = useRef(null); useSignalEffect(() => { - const instance = editor.glyphInstanceCell.value; + const instance = editor.previewGlyphInstanceCell.value; + const layer = editor.editingGlyphLayerCell.value; const ids = [...editor.selection.stateCell.value.anchorIds]; - setHasLayer(instance?.hasLayer ?? false); + setEditable(layer !== null); if (!instance || ids.length !== 1) { setSingleAnchorId(null); @@ -54,7 +55,7 @@ export const AnchorSection = () => { const handlePositionChange = (axis: "x" | "y", value: number) => { if (!singleAnchorId) return; - const layer = editor.glyphInstanceCell.peek()?.layer; + const layer = editor.editingGlyphLayer; if (!layer) return; const nextX = axis === "x" ? value : anchorX; @@ -69,13 +70,13 @@ export const AnchorSection = () => { handlePositionChange("x", value)} /> handlePositionChange("y", value)} /> diff --git a/apps/desktop/src/renderer/src/components/editor/sidebar-right/GlyphSection.tsx b/apps/desktop/src/renderer/src/components/editor/sidebar-right/GlyphSection.tsx index 7da5388c..47b5689b 100644 --- a/apps/desktop/src/renderer/src/components/editor/sidebar-right/GlyphSection.tsx +++ b/apps/desktop/src/renderer/src/components/editor/sidebar-right/GlyphSection.tsx @@ -10,17 +10,18 @@ import { useGlyphXAdvance } from "@/hooks/useGlyphXAdvance"; export const GlyphSection = () => { const editor = useEditor(); - const glyph = useSignalState(editor.glyph); + const glyph = useSignalState(editor.previewGlyphRecordCell); const sidebearings = useGlyphSidebearings(); const xAdvance = useGlyphXAdvance(); const glyphInfo = getGlyphInfo(); - const unicode = formatCodepointAsUPlus(glyph?.unicode ?? 0); + const unicodeValue = glyph?.unicodes[0] ?? 0; + const unicode = formatCodepointAsUPlus(unicodeValue); const lsb = sidebearings.sidebearings.lsb === null ? null : Math.round(sidebearings.sidebearings.lsb); const rsb = sidebearings.sidebearings.rsb === null ? null : Math.round(sidebearings.sidebearings.rsb); - const sidebearingsEnabled = sidebearings.hasLayer && lsb !== null && rsb !== null; + const sidebearingsEnabled = sidebearings.editable && lsb !== null && rsb !== null; return ( @@ -52,11 +53,11 @@ export const GlyphSection = () => { editor.setXAdvance(width)} /> -
{glyphInfo.getGlyphName(glyph?.unicode ?? 0)}
+
{glyphInfo.getGlyphName(unicodeValue)}
 ); diff --git a/apps/desktop/src/renderer/src/components/home/GlyphGrid.tsx b/apps/desktop/src/renderer/src/components/home/GlyphGrid.tsx index a642cf6b..c328864b 100644 --- a/apps/desktop/src/renderer/src/components/home/GlyphGrid.tsx +++ b/apps/desktop/src/renderer/src/components/home/GlyphGrid.tsx @@ -50,7 +50,6 @@ import { getGlyphInfo } from "@/workspace/glyphInfo"; import { type GlyphCatalogItem, useGlyphCatalog } from "@/context/GlyphCatalogContext"; import { Button, Input } from "@shift/ui"; import type { GlyphId, GlyphName } from "@shift/types"; -import type { Glyph } from "@/lib/model/Glyph"; const ROW_HEIGHT = CELL_HEIGHT + 40 + 8; const NOMINAL_CELL_WIDTH = 100; @@ -87,7 +86,7 @@ export const GlyphGrid = memo(function GlyphGrid() { const editor = useEditor(); const font = editor.font; const metrics = font.metrics; - const designLocation = editor.$designLocation; + const designLocation = editor.designLocationCell; const { filteredGlyphs: glyphs } = useGlyphCatalog(); const scrollContainerRef = useRef(null); @@ -97,7 +96,10 @@ export const GlyphGrid = memo(function GlyphGrid() { // is the single source for that. We avoid a sync width read on mount so layout doesn't jitter when // the container isn't laid out yet or when navigating back. We only set state when the computed // layout (columns, cellWidth) actually changes. - const [layout, setLayout] = useState(() => ({ columns: 1, cellWidth: NOMINAL_CELL_WIDTH })); + const [layout, setLayout] = useState(() => ({ + columns: 1, + cellWidth: NOMINAL_CELL_WIDTH, + })); useEffect(() => { const container = scrollContainerRef.current; @@ -130,7 +132,7 @@ export const GlyphGrid = memo(function GlyphGrid() { () => visibleGlyphIdsForRows(glyphs, columns, virtualRows), [glyphs, columns, visibleRowsKey], ); - const [loadedGlyphs, setLoadedGlyphs] = useState>(() => new Map()); + const [loadedGlyphIds, setLoadedGlyphIds] = useState>(() => new Set()); useEffect(() => { let cancelled = false; @@ -138,10 +140,10 @@ export const GlyphGrid = memo(function GlyphGrid() { async function load(): Promise { try { const loaded = await font.loadGlyphs(visibleGlyphIds); - if (!cancelled) setLoadedGlyphs(loaded); + if (!cancelled) setLoadedGlyphIds(loaded); } catch (error) { console.error("failed to load visible glyphs", error); - if (!cancelled) setLoadedGlyphs(new Map()); + if (!cancelled) setLoadedGlyphIds(new Set()); } } @@ -155,8 +157,8 @@ export const GlyphGrid = memo(function GlyphGrid() { const handleCellClick = useCallback( async (glyph: GlyphCatalogItem) => { try { - const loadedGlyph = await font.loadGlyph(glyph.id); - if (!loadedGlyph) return; + const loaded = await font.loadGlyph(glyph.id); + if (!loaded) return; navigate(`/editor/${encodeURIComponent(glyph.id)}`); } catch (error) { @@ -199,7 +201,7 @@ export const GlyphGrid = memo(function GlyphGrid() { className="flex gap-2 px-4" > {rowGlyphs.map((glyph) => { - const previewGlyph = loadedGlyphs.get(glyph.id) ?? null; + const previewGlyphId = loadedGlyphIds.has(glyph.id) ? glyph.id : null; return (
handleCellClick(glyph)} > ; @@ -51,21 +52,23 @@ interface GlyphPreviewProps { } export function GlyphPreview({ - glyph, + font, + glyphId, unicode, metrics, designLocation, height = CELL_HEIGHT, }: GlyphPreviewProps) { - if (!glyph) { + if (!glyphId) { return ; } return ( @@ -73,35 +76,33 @@ export function GlyphPreview({ } function GlyphCell({ + font, metrics, height, - glyph, + glyphId, unicode, designLocation, }: { + font: Font; metrics: FontMetrics; height: number; - glyph: Glyph; + glyphId: GlyphId; unicode: number | null; designLocation: Signal; }) { - const outline = glyph.instance(designLocation).render.outline; + const instance = font.instance(glyphId, designLocation); + const outline = instance?.render.outline ?? null; - const svgPath = useSignalState(outline.$svgPath); - const advance = useSignalState(glyph.$xAdvance); + useSignalTrigger(outline?.svgPathCell); + useSignalTrigger(instance?.xAdvanceCell); + const svgPath = outline?.svgPath ?? ""; + const advance = instance?.xAdvance ?? null; const cellWidth = computeCellWidth(metrics, advance, height); const containerStyle = { width: cellWidth, height }; if (!svgPath) { - return ( - - ); + return ; } const viewBox = glyphPreviewViewBox(metrics, advance); diff --git a/apps/desktop/src/renderer/src/context/DebugContext.tsx b/apps/desktop/src/renderer/src/context/DebugContext.tsx index 486a9754..cedc86a3 100644 --- a/apps/desktop/src/renderer/src/context/DebugContext.tsx +++ b/apps/desktop/src/renderer/src/context/DebugContext.tsx @@ -30,7 +30,7 @@ export function DebugProvider({ children }: DebugProviderProps) { const editor = useEditor(); const dumpSnapshot = useCallback(() => { - const glyph = editor.glyph.peek(); + const glyph = editor.previewGlyphRecordCell.peek(); if (!glyph) { return; } diff --git a/apps/desktop/src/renderer/src/hooks/useDesignLocation.ts b/apps/desktop/src/renderer/src/hooks/useDesignLocation.ts index 51f57a6e..61a37530 100644 --- a/apps/desktop/src/renderer/src/hooks/useDesignLocation.ts +++ b/apps/desktop/src/renderer/src/hooks/useDesignLocation.ts @@ -5,7 +5,7 @@ import type { AxisLocation } from "@/types/variation"; export const useDesignLocation = (): [AxisLocation, (next: AxisLocation) => void] => { const editor = useEditor(); - const location = useSignalState(editor.$designLocation); + const location = useSignalState(editor.designLocationCell); const setLocation = useCallback((next: AxisLocation) => editor.setDesignLocation(next), [editor]); diff --git a/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts b/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts index 17fce7cf..f5c8e1f8 100644 --- a/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts +++ b/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts @@ -6,25 +6,26 @@ const EMPTY_SIDEBEARINGS: GlyphSidebearings = { lsb: null, rsb: null }; export interface GlyphSidebearingsState { readonly sidebearings: GlyphSidebearings; - readonly hasLayer: boolean; + readonly editable: boolean; } /** * Current glyph sidebearings (LSB/RSB), live-updating. * * Subscribes to the displayed glyph instance. Interpolated instances still - * expose resolved values, but report `hasLayer: false` so inputs can display + * expose resolved values, but report `editable: false` so inputs can display * them without mutating a missing authored glyph layer. * * @returns Current values and whether the displayed instance can be edited. */ export function useGlyphSidebearings(): GlyphSidebearingsState { const editor = useEditor(); - const instance = useSignalState(editor.glyphInstanceCell); + const instance = useSignalState(editor.previewGlyphInstanceCell); + const layer = useSignalState(editor.editingGlyphLayerCell); useSignalTrigger(instance?.sidebearingsCell, { schedule: "frame" }); return { sidebearings: instance?.sidebearings ?? EMPTY_SIDEBEARINGS, - hasLayer: instance?.hasLayer ?? false, + editable: layer !== null, }; } diff --git a/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts b/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts index 837b8869..6fbe6c65 100644 --- a/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts +++ b/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts @@ -3,7 +3,7 @@ import { useSignalState, useSignalTrigger } from "@/lib/signals"; export interface GlyphXAdvanceState { readonly xAdvance: number; - readonly hasLayer: boolean; + readonly editable: boolean; } /** @@ -11,12 +11,13 @@ export interface GlyphXAdvanceState { */ export function useGlyphXAdvance(): GlyphXAdvanceState { const editor = useEditor(); - const instance = useSignalState(editor.glyphInstanceCell); + const instance = useSignalState(editor.previewGlyphInstanceCell); + const layer = useSignalState(editor.editingGlyphLayerCell); useSignalTrigger(instance?.xAdvanceCell, { schedule: "frame" }); return { xAdvance: instance?.xAdvance ?? 0, - hasLayer: instance?.hasLayer ?? false, + editable: layer !== null, }; } diff --git a/apps/desktop/src/renderer/src/hooks/useLayerSourceId.ts b/apps/desktop/src/renderer/src/hooks/useLayerSourceId.ts index 41bf23f6..156d6e34 100644 --- a/apps/desktop/src/renderer/src/hooks/useLayerSourceId.ts +++ b/apps/desktop/src/renderer/src/hooks/useLayerSourceId.ts @@ -3,5 +3,5 @@ import { useSignalState } from "@/lib/signals"; export const useLayerSourceId = (): string | null => { const editor = useEditor(); - return useSignalState(editor.$layerSourceId); + return useSignalState(editor.layerSourceIdCell); }; diff --git a/apps/desktop/src/renderer/src/lib/commands/core/CommandRunner.ts b/apps/desktop/src/renderer/src/lib/commands/core/CommandRunner.ts index 2eb553db..0c482210 100644 --- a/apps/desktop/src/renderer/src/lib/commands/core/CommandRunner.ts +++ b/apps/desktop/src/renderer/src/lib/commands/core/CommandRunner.ts @@ -11,10 +11,10 @@ import type { GlyphLayer } from "@/lib/model/Glyph"; import type { Command, CommandContext } from "./Command"; export class CommandRunner { - readonly #$layer: Signal; + readonly #layerCell: Signal; - constructor($layer: Signal) { - this.#$layer = $layer; + constructor(layerCell: Signal) { + this.#layerCell = layerCell; } run(command: Command): TResult { @@ -22,7 +22,7 @@ export class CommandRunner { } #context(): CommandContext { - const layer = this.#$layer.peek(); + const layer = this.#layerCell.peek(); if (!layer) { throw new Error("Cannot execute command without an active glyph layer"); } diff --git a/apps/desktop/src/renderer/src/lib/editor/Editor.test.ts b/apps/desktop/src/renderer/src/lib/editor/Editor.test.ts index bac8690b..615f7b6e 100644 --- a/apps/desktop/src/renderer/src/lib/editor/Editor.test.ts +++ b/apps/desktop/src/renderer/src/lib/editor/Editor.test.ts @@ -28,11 +28,14 @@ describe("Editor", () => { expect(itemId && editor.layerForItem(itemId)).not.toBeNull(); }); - it("derives active glyph from the geometry-shown scene item", () => { + it("derives the preview glyph record from the geometry-shown scene item", () => { const record = editor.font.recordForName("S")!; editor.scene.clear(); - const itemId = editor.scene.addGlyph({ glyphId: record.id, origin: { x: 0, y: 0 } }); + const itemId = editor.scene.addGlyph({ + glyphId: record.id, + origin: { x: 0, y: 0 }, + }); editor.scene.setGeometryItems([itemId]); expect(editor.scene.value.items).toHaveLength(1); @@ -42,23 +45,26 @@ describe("Editor", () => { placement: { origin: { x: 0, y: 0 } }, }); expect(editor.scene.isGeometryShown(itemId)).toBe(true); - expect(editor.glyph.peek()?.id).toBe(record.id); + expect(editor.previewGlyphRecordCell.peek()?.id).toBe(record.id); expect(editor.layerForItem(itemId)).not.toBeNull(); }); - it("clearing the scene clears the derived active glyph", () => { + it("clearing the scene clears the derived preview glyph record", () => { const record = editor.font.recordForName("S")!; editor.scene.clear(); - const itemId = editor.scene.addGlyph({ glyphId: record.id, origin: { x: 0, y: 0 } }); + const itemId = editor.scene.addGlyph({ + glyphId: record.id, + origin: { x: 0, y: 0 }, + }); editor.scene.setGeometryItems([itemId]); - expect(editor.glyph.peek()?.id).toBe(record.id); + expect(editor.previewGlyphRecordCell.peek()?.id).toBe(record.id); editor.scene.clear(); expect(editor.scene.value.items).toEqual([]); - expect(editor.glyph.peek()).toBeNull(); + expect(editor.previewGlyphRecordCell.peek()).toBeNull(); }); it("can place the same glyph id twice with distinct item ids", async () => { @@ -98,7 +104,10 @@ describe("Editor", () => { expect(editor.scene.isGeometryShown(right)).toBe(false); editor.scene.moveItemBy(right, { x: 30, y: 5 }); - expect(editor.scene.item(right)?.placement.origin).toEqual({ x: 730, y: 5 }); + expect(editor.scene.item(right)?.placement.origin).toEqual({ + x: 730, + y: 5, + }); }); }); diff --git a/apps/desktop/src/renderer/src/lib/editor/Editor.ts b/apps/desktop/src/renderer/src/lib/editor/Editor.ts index 63108a8a..7785106d 100644 --- a/apps/desktop/src/renderer/src/lib/editor/Editor.ts +++ b/apps/desktop/src/renderer/src/lib/editor/Editor.ts @@ -10,7 +10,7 @@ import type { } from "@shift/types"; import type { AxisLocation } from "@/types/variation"; import type { Coordinates } from "@/types/coordinates"; -import type { Glyph, GlyphInstance, GlyphLayer } from "@/lib/model/Glyph"; +import type { GlyphInstance, GlyphLayer } from "@/lib/model/Glyph"; import { axisLocationFromLocation, cloneAxisLocation, @@ -89,11 +89,10 @@ import { Scene } from "./Scene"; import { EditorGlyphState, EditorGesture, - GlyphDisplay, + GlyphPresentation, EditorInput, EditorViewState, TextEditingState, - type GlyphDisplayState, type GlyphPlacement, } from "./EditorState"; @@ -129,7 +128,7 @@ export class Editor { /** * User-facing editor display toggles. * - * These are session preferences for how the active glyph is presented. They + * These are session preferences for how preview geometry is presented. They * are not glyph data and should eventually live behind an `EditorViewState` * object so rendering code can consume them as one named concept. */ @@ -212,7 +211,7 @@ export class Editor { */ #text: TextEditingState; readonly gesture: EditorGesture; - #glyphDisplay: GlyphDisplay; + #glyphPresentation: GlyphPresentation; readonly input: EditorInput; #toolState: { app: Map; @@ -229,7 +228,9 @@ export class Editor { this.font = options.font; this.scene = new Scene(); - this.#designLocation = signal(emptyAxisLocation(), { name: "editor.designLocation" }); + this.#designLocation = signal(emptyAxisLocation(), { + name: "editor.designLocation", + }); this.#glyph = new EditorGlyphState(this.font, this.scene, this.#designLocation); this.#view = new EditorViewState(); @@ -271,13 +272,13 @@ export class Editor { this.#textRuns = new TextRuns(this.font, new Positioner(), this.#designLocation); this.#text = new TextEditingState(this.#textRuns); - this.#glyphDisplay = new GlyphDisplay(this.#text, this.#textRuns); + this.#glyphPresentation = new GlyphPresentation(this.#text, this.#textRuns); this.#renderer = new Renderer(this); this.#cameraMetricsEffect = effect( () => { - this.font.$loaded.value; + this.font.loadedCell.value; this.updateMetricsFromFont(); }, { name: "editor.cameraMetrics" }, @@ -421,11 +422,11 @@ export class Editor { } public get proofMode(): boolean { - return this.#glyphDisplay.proofMode; + return this.#glyphPresentation.proofMode; } public get proofModeCell(): Signal { - return this.#glyphDisplay.proofModeCell; + return this.#glyphPresentation.proofModeCell; } /** @@ -434,25 +435,25 @@ export class Editor { * @param enabled - `true` to show proof artwork and suppress edit handles. */ public setProofMode(enabled: boolean): void { - this.#glyphDisplay.setProofMode(enabled); + this.#glyphPresentation.setProofMode(enabled); } - /** Enables proof rendering for the active glyph. */ + /** Enables proof rendering for preview geometry. */ public enableProofMode(): void { this.setProofMode(true); } - /** Disables proof rendering for the active glyph. */ + /** Disables proof rendering for preview geometry. */ public disableProofMode(): void { this.setProofMode(false); } public get handlesVisible(): boolean { - return this.#glyphDisplay.handlesVisible; + return this.#glyphPresentation.handlesVisible; } public get handlesVisibleCell(): Signal { - return this.#glyphDisplay.handlesVisibleCell; + return this.#glyphPresentation.handlesVisibleCell; } /** @@ -461,7 +462,7 @@ export class Editor { * @param visible - `true` to draw handles for focused glyph instances. */ public setHandlesVisible(visible: boolean): void { - this.#glyphDisplay.setHandlesVisible(visible); + this.#glyphPresentation.setHandlesVisible(visible); } /** Shows point handles and structured geometry controls. */ @@ -474,20 +475,6 @@ export class Editor { this.setHandlesVisible(false); } - /** - * Display state for the active glyph in the editor canvas. - * - * @returns A snapshot combining glyph display toggles and text-focus visibility. - */ - public get glyphDisplay(): GlyphDisplayState { - return this.#glyphDisplay.cell.peek(); - } - - /** Reactive display state for render layers and canvas items. */ - public get glyphDisplayCell(): Signal { - return this.#glyphDisplay.cell; - } - public setBackgroundSurface(surface: Canvas2DSurface): void { this.#renderer.setBackgroundSurface(surface); } @@ -565,13 +552,13 @@ export class Editor { return this.#text.glyphPlacement.peek(); } - /** Clears the current glyph focus and active contour selection. */ + /** Clears scene glyph focus and the active contour selection. */ public close(): void { this.scene.clear(); - this.#glyph.active.activeContourId.set(null); + this.#glyph.sceneGlyph.activeContourId.set(null); } - public get $designLocation(): Signal { + public get designLocationCell(): Signal { return this.#designLocation; } @@ -585,7 +572,7 @@ export class Editor { * * `null` means the current design location does not exactly match a source. */ - public get $layerSourceId(): Signal { + public get layerSourceIdCell(): Signal { return this.#glyph.layerEditing.sourceId; } @@ -613,8 +600,13 @@ export class Editor { return this.#glyph.layerEditing.glyphLayer.peek(); } - /** Glyph instance resolved at the current design location. */ - public get glyphInstance(): GlyphInstance | null { + /** Reactive authored glyph layer currently mutated by edit commands. */ + public get editingGlyphLayerCell(): Signal { + return this.#glyph.layerEditing.glyphLayer; + } + + /** Glyph instance shown by the editor preview at the current design location. */ + public get previewGlyphInstance(): GlyphInstance | null { return this.#glyph.preview.instance.peek(); } @@ -626,15 +618,16 @@ export class Editor { } /** - * Select every point in the active authored glyph layer. + * Select every point in the current authored glyph layer. * * This intentionally uses the authored glyph layer rather than interpolated * design-location geometry: selection mutates an authored layer, so it must * refer to point IDs that commands can mutate. */ public selectAll(): void { - const instance = this.glyphInstance; - if (!instance?.layer) return; + if (!this.editingGlyphLayer) return; + const instance = this.previewGlyphInstance; + if (!instance) return; this.selection.select( instance.geometry.allPoints.map((point) => ({ @@ -689,7 +682,11 @@ export class Editor { /** @knipclassignore Indirectly consumed through Renderer. */ public focusedGlyphVisible(): boolean { - return this.#glyphDisplay.cell.peek().focusedGlyphVisible; + return this.#glyphPresentation.focusedGlyphVisibleCell.peek(); + } + + public get focusedGlyphVisibleCell(): Signal { + return this.#glyphPresentation.focusedGlyphVisibleCell; } public getToolState(scope: ToolStateScope, toolId: string, key: string): unknown { @@ -709,34 +706,33 @@ export class Editor { if (!scopedState.delete(stateKey)) return; } - public get glyph(): Signal { - return this.#glyph.active.glyph; + public get previewGlyphRecordCell(): Signal { + return this.#glyph.sceneGlyph.record; } - public get glyphInstanceCell(): Signal { + public get previewGlyphInstanceCell(): Signal { return this.#glyph.preview.instance; } - public glyphForItem(itemId: ItemId): Glyph | null { + public glyphForItem(itemId: ItemId): GlyphRecord | null { const item = this.scene.glyphItem(itemId); if (!item) return null; - return this.font.glyphForId(item.glyphId); + return this.font.glyph(item.glyphId); } public instanceForItem(itemId: ItemId): GlyphInstance | null { const item = this.scene.glyphItem(itemId); - const glyph = item ? this.glyphForItem(itemId) : null; - if (!item || !glyph) return null; - return glyph.instance(this.#designLocation); + if (!item) return null; + + return this.font.instance(item.glyphId, this.#designLocation); } public layerForItem(itemId: ItemId): GlyphLayer | null { const item = this.scene.glyphItem(itemId); if (!item) return null; - const source = this.font.sourceAt(this.designLocation); - if (!source) return null; - return this.font.glyphLayerForId(item.glyphId, source.id); + + return this.font.editableLayerAt(item.glyphId, this.designLocation); } /** Stateless command executor; undo authority is the workspace ledger. */ @@ -801,17 +797,18 @@ export class Editor { } public get xAdvance(): number { - return this.glyphInstance?.xAdvance ?? 0; + return this.previewGlyphInstance?.xAdvance ?? 0; } /** - * Sets the active glyph layer's horizontal advance through command history. + * Sets the editable glyph layer's horizontal advance through command history. * * @param width - New advance width in UPM units. */ public setXAdvance(width: number): void { - const instance = this.glyphInstance; - if (!instance?.layer) return; + if (!this.editingGlyphLayer) return; + const instance = this.previewGlyphInstance; + if (!instance) return; if (instance.xAdvance === width) return; @@ -819,13 +816,14 @@ export class Editor { } /** - * Sets the active glyph layer's left sidebearing by translating its outline. + * Sets the editable glyph layer's left sidebearing by translating its outline. * * @param value - Desired left sidebearing in UPM units. */ public setLeftSidebearing(value: number): void { - const instance = this.glyphInstance; - if (!instance?.layer) return; + if (!this.editingGlyphLayer) return; + const instance = this.previewGlyphInstance; + if (!instance) return; const bbox = instance.render.outline.bounds; if (!bbox) return; @@ -837,13 +835,14 @@ export class Editor { } /** - * Sets the active glyph layer's right sidebearing by changing its advance. + * Sets the editable glyph layer's right sidebearing by changing its advance. * * @param value - Desired right sidebearing in UPM units. */ public setRightSidebearing(value: number): void { - const instance = this.glyphInstance; - if (!instance?.layer) return; + if (!this.editingGlyphLayer) return; + const instance = this.previewGlyphInstance; + if (!instance) return; const bbox = instance.render.outline.bounds; if (!bbox) return; @@ -998,9 +997,9 @@ export class Editor { this.#renderer.fpsMonitor.stop(); } - /** Deletes currently selected points from the active authored glyph layer. */ + /** Deletes currently selected points from the editable authored glyph layer. */ public deleteSelectedPoints(): void { - const edit = this.glyphInstance?.layer; + const edit = this.editingGlyphLayer; if (!edit) return; const selectedIds = [...this.selection.pointIds]; @@ -1014,7 +1013,7 @@ export class Editor { const content = this.#selectedClipboardContent(); if (!content || content.contours.length === 0) return false; - const glyph = this.glyph.peek(); + const glyph = this.#glyph.sceneGlyph.record.peek(); if (!glyph) return false; return this.#clipboard.write(content, { sourceGlyph: glyph.name }); @@ -1024,7 +1023,7 @@ export class Editor { const content = this.#selectedClipboardContent(); if (!content || content.contours.length === 0) return false; - const glyph = this.glyph.peek(); + const glyph = this.#glyph.sceneGlyph.record.peek(); if (!glyph) return false; const written = await this.#clipboard.write(content, { @@ -1155,33 +1154,33 @@ export class Editor { } public get activeContourIdCell(): Signal { - return this.#glyph.active.activeContourId; + return this.#glyph.sceneGlyph.activeContourId; } public getActiveContourId(): ContourId | null { - const id = this.#glyph.active.activeContourId.peek(); + const id = this.#glyph.sceneGlyph.activeContourId.peek(); if (!id) return null; return id; } public setActiveContour(contourId: ContourId | null): void { - this.#glyph.active.activeContourId.set(contourId); + this.#glyph.sceneGlyph.activeContourId.set(contourId); } public clearActiveContour(): void { - this.#glyph.active.activeContourId.set(null); + this.#glyph.sceneGlyph.activeContourId.set(null); } public getActiveContour(): Contour | null { const activeContourId = this.getActiveContourId(); if (!activeContourId) return null; - return this.glyphInstance?.geometry.contour(activeContourId) ?? null; + return this.previewGlyphInstance?.geometry.contour(activeContourId) ?? null; } public continueContour(contourId: ContourId, fromStart: boolean, pointId: PointId): void { - this.#glyph.active.activeContourId.set(contourId); + this.#glyph.sceneGlyph.activeContourId.set(contourId); if (fromStart) { this.#commands.run(new ReverseContourCommand(contourId)); } @@ -1233,7 +1232,7 @@ export class Editor { } /** @knipclassignore */ - public get $drawOffset(): Signal { + public get drawOffsetCell(): Signal { return this.#text.drawOffset; } diff --git a/apps/desktop/src/renderer/src/lib/editor/EditorState.ts b/apps/desktop/src/renderer/src/lib/editor/EditorState.ts index 4dbdf12a..3c78e4a1 100644 --- a/apps/desktop/src/renderer/src/lib/editor/EditorState.ts +++ b/apps/desktop/src/renderer/src/lib/editor/EditorState.ts @@ -1,11 +1,11 @@ import type { Point2D } from "@shift/geo"; -import type { ContourId, LayerId, Source, SourceId } from "@shift/types"; +import type { ContourId, GlyphId, GlyphRecord, Source, SourceId } from "@shift/types"; import type { Coordinates } from "@/types/coordinates"; import type { AxisLocation } from "@/types/variation"; import type { DebugOverlays } from "@/types/uiState"; import type { Modifiers } from "../tools/core/GestureDetector"; import type { Font } from "../model/Font"; -import type { Glyph, GlyphInstance, GlyphLayer } from "../model/Glyph"; +import type { GlyphInstance, GlyphLayer } from "../model/Glyph"; import type { Scene } from "./Scene"; import type { FocusedGlyph } from "../text/TextRun"; import type { GlyphAnchor } from "../text/layout"; @@ -31,7 +31,7 @@ const DEFAULT_DEBUG_OVERLAYS: DebugOverlays = { glyphBbox: false, }; -export interface GlyphDisplayState { +export interface GlyphPresentationState { readonly proofMode: boolean; readonly handlesVisible: boolean; readonly focusedGlyphVisible: boolean; @@ -82,39 +82,31 @@ export class EditorGesture { } } -export class GlyphDisplay { +export class GlyphPresentation { readonly proofModeCell: WritableSignal; readonly handlesVisibleCell: WritableSignal; - readonly cell: ComputedSignal; + readonly focusedGlyphVisibleCell: ComputedSignal; constructor(text: TextEditingState, textRuns: TextRuns) { this.proofModeCell = signal(false, { - name: "editor.glyph.display.proofMode", + name: "editor.glyph.presentation.proofMode", }); this.handlesVisibleCell = signal(true, { - name: "editor.glyph.display.handlesVisible", + name: "editor.glyph.presentation.handlesVisible", }); - this.cell = computed( + this.focusedGlyphVisibleCell = computed( () => { const run = textRuns.activeCell.value; const focusedGlyph = text.focusedGlyph.value; const hasTextActivity = run.buffer.itemsCell.value.length > 0 || run.cursorVisibleCell.value; - return { - proofMode: this.proofModeCell.value, - handlesVisible: this.handlesVisibleCell.value, - focusedGlyphVisible: !hasTextActivity || focusedGlyph?.anchor.runId === run.id, - }; + return !hasTextActivity || focusedGlyph?.anchor.runId === run.id; }, - { name: "editor.glyph.display" }, + { name: "editor.glyph.presentation.focusedGlyphVisible" }, ); } - get value(): GlyphDisplayState { - return this.cell.peek(); - } - get proofMode(): boolean { return this.proofModeCell.peek(); } @@ -182,40 +174,37 @@ export class EditorViewState { } } -/** Active glyph state derived from placed scene items. */ -export class ActiveGlyphState { - /** Glyph model for the first geometry-shown glyph item, or first placed glyph. */ - readonly glyph: Signal; +/** Glyph preview state derived from placed scene items. */ +export class SceneGlyphPreviewState { + /** Glyph id for the first geometry-shown glyph item, or first placed glyph. */ + readonly glyphId: Signal; + + /** Identity record for the first geometry-shown glyph item, or first placed glyph. */ + readonly record: Signal; /** Contour receiving appended pen points, or `null` when no contour is active. */ readonly activeContourId: WritableSignal; constructor(font: Font, scene: Scene) { - this.glyph = computed( + this.glyphId = computed( () => { const value = scene.cell.value; - font.glyphRecordsCell.value; - font.glyphSnapshotStatusCell.value; const geometryItem = value.geometryItems .map((itemId) => value.items.find((item) => item.id === itemId) ?? null) .find((item) => item?.kind === "glyph"); const fallbackItem = value.items.find((item) => item.kind === "glyph") ?? null; const item = geometryItem ?? fallbackItem; - return item?.kind === "glyph" ? font.glyphForId(item.glyphId) : null; + return item?.kind === "glyph" ? item.glyphId : null; }, - { name: "editor.glyph.active" }, + { name: "editor.glyph.previewRecordId" }, ); + this.record = font.glyphCell(this.glyphId); this.activeContourId = signal(null, { name: "editor.glyph.activeContourId", }); } } -export interface GlyphLayerResolution { - readonly sourceId: SourceId; - readonly layerId: LayerId | null; -} - /** * Resolves the authored glyph layer for the current designspace source. * @@ -229,50 +218,16 @@ export class GlyphLayerEditingState { /** Exact font source selected by the current design location. */ readonly selectedSource: Signal; - /** Selected source with the current glyph's authored layer when one exists. */ - readonly layer: Signal; - /** Authored glyph layer data for the selected source. */ readonly glyphLayer: Signal; - constructor(font: Font, glyph: Signal, location: Signal) { - this.selectedSource = computed( - () => { - return font.sourceAt(location.value); - }, - { name: "editor.glyph.layerEditing.source" }, - ); + constructor(font: Font, glyphId: Signal, location: Signal) { + this.selectedSource = font.sourceAtCell(location); this.sourceId = computed(() => this.selectedSource.value?.id ?? null, { name: "editor.glyph.layerEditing.sourceId", }); - this.layer = computed( - () => { - const source = this.selectedSource.value; - if (!source) return null; - - const activeGlyph = glyph.value; - if (!activeGlyph) return { sourceId: source.id, layerId: null }; - - const layerId = font.layerRecordForId(activeGlyph.id, source.id)?.id ?? null; - return { sourceId: source.id, layerId }; - }, - { name: "editor.glyph.layerEditing.layer" }, - ); - - this.glyphLayer = computed( - () => { - const activeGlyph = glyph.value; - if (!activeGlyph) return null; - - const source = this.selectedSource.value; - if (!source) return null; - if (this.layer.value?.layerId === null) return null; - - return font.glyphLayerForId(activeGlyph.id, source.id); - }, - { name: "editor.glyph.layerEditing.glyphLayer" }, - ); + this.glyphLayer = font.layerCell(glyphId, this.sourceId); } } @@ -287,16 +242,8 @@ export class PreviewGlyphState { /** Glyph resolved at the current design location. */ readonly instance: Signal; - constructor(glyph: Signal, location: Signal) { - this.instance = computed( - () => { - const activeGlyph = glyph.value; - if (!activeGlyph) return null; - - return activeGlyph.instance(location); - }, - { name: "editor.glyph.preview.instance" }, - ); + constructor(font: Font, glyphId: Signal, location: Signal) { + this.instance = font.instanceCell(glyphId, location); } } /** @@ -306,14 +253,14 @@ export class PreviewGlyphState { * displayed glyph model at the current design location. */ export class EditorGlyphState { - readonly active: ActiveGlyphState; + readonly sceneGlyph: SceneGlyphPreviewState; readonly layerEditing: GlyphLayerEditingState; readonly preview: PreviewGlyphState; constructor(font: Font, scene: Scene, location: Signal) { - this.active = new ActiveGlyphState(font, scene); - this.layerEditing = new GlyphLayerEditingState(font, this.active.glyph, location); - this.preview = new PreviewGlyphState(this.active.glyph, location); + this.sceneGlyph = new SceneGlyphPreviewState(font, scene); + this.layerEditing = new GlyphLayerEditingState(font, this.sceneGlyph.glyphId, location); + this.preview = new PreviewGlyphState(font, this.sceneGlyph.glyphId, location); } } diff --git a/apps/desktop/src/renderer/src/lib/editor/docs/DOCS.md b/apps/desktop/src/renderer/src/lib/editor/docs/DOCS.md index 3b88fee8..a7b6cd0f 100644 --- a/apps/desktop/src/renderer/src/lib/editor/docs/DOCS.md +++ b/apps/desktop/src/renderer/src/lib/editor/docs/DOCS.md @@ -10,7 +10,7 @@ Central orchestrator for the canvas-based glyph editing surface, wiring viewport **Architecture Invariant:** `drawOffset` is derived render state. Text tools focus glyphs by `GlyphAnchor { runId, itemId }`; `Editor` resolves that anchor through `TextRuns` and `TextLayout.editOriginForItem()`. Tools must not set text-run edit placement coordinates directly. -**Architecture Invariant: CRITICAL:** `Camera` owns the affine matrices (`$upmToScreenMatrix`, `$screenToUpmMatrix`) as lazily computed signals. Anything that reads viewport-derived values inside a `computed` or `effect` will auto-track. Calling `setRect()`, changing zoom/pan, or changing UPM invalidates both matrices and triggers downstream redraws automatically. Never cache matrix results outside a signal. +**Architecture Invariant: CRITICAL:** `Camera` owns the affine matrices as lazily computed cells. Anything that reads viewport-derived values inside a `computed` or `effect` will auto-track. Calling `setRect()`, changing zoom/pan, or changing UPM invalidates both matrices and triggers downstream redraws automatically. Never cache matrix results outside a signal. **Architecture Invariant: CRITICAL:** Rendering is driven by named reactive effects owned by `Renderer`. Each effect reads explicit dependency signals before drawing. If editor state should trigger a redraw, add a named read to the correct render dependency boundary -- do not request redraw imperatively from tools or UI handlers. @@ -20,7 +20,7 @@ Central orchestrator for the canvas-based glyph editing surface, wiring viewport **Architecture Invariant:** Lifecycle events (`EventEmitter`) are for one-shot imperative actions (`fontLoaded`, `fontSaved`, `destroying`). Continuous state changes use signals. Do not mix the two patterns. -**Architecture Invariant:** `Selection` uses discriminated `Selectable` unions (`{ kind: "point" | "anchor" | "segment", id }`). Mutations go through `select()`, `add()`, `remove()`, `toggle()`. Derived contour and bounds queries are computed from the single `$state` signal and the current glyph geometry. +**Architecture Invariant:** `Selection` uses discriminated `Selectable` unions (`{ kind: "point" | "anchor" | "segment", id }`). Mutations go through `select()`, `add()`, `remove()`, `toggle()`. Derived contour and bounds queries are computed from `stateCell` and the current glyph geometry. **Architecture Invariant:** Glyph-domain hit testing belongs to glyph geometry and editor glyph lookup helpers. Tool-specific controls, such as select bounding-box handles, are owned and hit-tested by the tool that renders them. @@ -58,7 +58,7 @@ editor/ - **`Renderer`** -- Manages four stacked canvas layers (background, scene, markers/WebGL, overlay), their `FrameHandler` instances, and the canvas item layers that draw each pass. - **`Canvas`** -- Thin wrapper around `CanvasRenderingContext2D` with `pxToUpm()` conversion and themed drawing primitives. Carries `CameraTransform` and `Theme`. - **`CameraTransform`** -- Value object: `{ zoom, panX, panY, centre, upmScale, logicalHeight, layoutHeight, padding, descender }`. Snapshot of viewport state passed to rendering code. -- **`Selection`** -- Unified selection state for points, anchors, and segments. Computed `DerivedSelection` tracks selected contours and bounds. Exposes one raw `$state` signal plus unwrapped ID getters. +- **`Selection`** -- Unified selection state for points, anchors, and segments. Computed `DerivedSelection` tracks selected contours and bounds. Exposes `stateCell` plus unwrapped ID getters. - **`Selectable`** -- Discriminated union: `{ kind: "point" | "anchor" | "segment", id }`. - **`Coordinates`** -- Triple of `{ screen, scene, glyphLocal }` for a single position. Built via `Editor.fromScreen()` etc. - **`GlyphLayerEditDraft`** -- Transactional interface for continuous layer manipulation: `previewPositionPatch()` / `previewTranslate()` / `previewRotate()` / `previewScale()` during drag, `commit(label)` or `discard()` at end. diff --git a/apps/desktop/src/renderer/src/lib/editor/rendering/RenderFrame.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/RenderFrame.ts index 82cf18a3..b604fbc5 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/RenderFrame.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/RenderFrame.ts @@ -1,11 +1,11 @@ import type { Point2D } from "@shift/geo"; import type { DebugOverlays as DebugOverlayState } from "@/types/uiState"; -import type { Glyph, GlyphInstance } from "@/lib/model/Glyph"; +import type { GlyphInstance } from "@/lib/model/Glyph"; import type { FocusedGlyph, TextRun } from "@/lib/text/TextRun"; import type { SelectionState } from "@/lib/editor/Selection"; import type { HoverState } from "@/lib/editor/Hover"; import type { Editor } from "@/lib/editor/Editor"; -import type { GlyphDisplayState } from "@/lib/editor/EditorState"; +import type { GlyphPresentationState } from "@/lib/editor/EditorState"; import type { SceneGlyph } from "@/lib/editor/Scene"; import type { AxisLocation } from "@/types/variation"; import type { GlyphRecord, Unicode } from "@shift/types"; @@ -27,7 +27,6 @@ interface BackgroundGlyphFrame { interface GuideAdvanceInput { readonly record: GlyphRecord; - readonly model: Glyph | null; readonly xAdvance: number; } @@ -37,7 +36,7 @@ export interface BackgroundLayerProps { interface SceneGlyphFrame { readonly item: SceneGlyph; - readonly model: Glyph | null; + readonly record: GlyphRecord | null; readonly instance: GlyphInstance | null; readonly geometryShown: boolean; } @@ -53,7 +52,7 @@ interface SceneViewProps { export interface SceneLayerProps { readonly glyphs: readonly SceneGlyphFrame[]; - readonly display: GlyphDisplayState; + readonly display: GlyphPresentationState; readonly interaction: SceneInteractionProps; readonly view: SceneViewProps; } @@ -109,16 +108,14 @@ export class BackgroundLayer extends CanvasItem { const geometryShown = scene.geometryItems.includes(item.id); if (!geometryShown) continue; - const record = this.#editor.font.recordForId(item.glyphId); + const record = this.#editor.font.glyph(item.glyphId); if (!record) continue; - const glyph = this.#editor.glyphForItem(item.id); const instance = this.#editor.instanceForItem(item.id); glyphs.push({ item, advance: guideAdvance({ record, - model: glyph, xAdvance: instance?.xAdvanceCell.value ?? this.#editor.font.defaultXAdvance, }), geometryShown, @@ -143,17 +140,10 @@ export class BackgroundLayer extends CanvasItem { } function guideAdvance(input: GuideAdvanceInput): number { - return displayAdvance( - input.xAdvance, - input.record.name, - primaryUnicode(input.model, input.record), - ); + return displayAdvance(input.xAdvance, input.record.name, primaryUnicode(input.record)); } -function primaryUnicode(glyph: Glyph | null, record: GlyphRecord): Unicode | null { - const unicode = glyph?.unicode; - if (unicode !== null && unicode !== undefined && Number.isFinite(unicode)) return unicode; - +function primaryUnicode(record: GlyphRecord): Unicode | null { return record.unicodes[0] ?? null; } @@ -191,7 +181,7 @@ export class TextLayer extends CanvasItem { return { run, - designLocation: this.#editor.$designLocation, + designLocation: this.#editor.designLocationCell, drawOffset: { x: 0, y: 0 }, focusedGlyph: this.#editor.focusedGlyphCell.value, }; @@ -274,7 +264,7 @@ export class SceneLayer extends CanvasItem { glyphs.push({ item, - model: this.#editor.glyphForItem(item.id), + record: this.#editor.glyphForItem(item.id), instance, geometryShown: scene.geometryItems.includes(item.id), }); @@ -282,7 +272,11 @@ export class SceneLayer extends CanvasItem { return { glyphs, - display: this.#editor.glyphDisplayCell.value, + display: { + proofMode: this.#editor.proofModeCell.value, + handlesVisible: this.#editor.handlesVisibleCell.value, + focusedGlyphVisible: this.#editor.focusedGlyphVisibleCell.value, + }, interaction: { selection: this.#editor.selection.stateCell.value, hover: this.#editor.hover.targetCell.value, @@ -315,9 +309,9 @@ export class SceneLayer extends CanvasItem { } #drawGlyphOutline(canvas: Canvas, props: SceneLayerProps, glyph: SceneGlyphFrame): void { - const { model, instance, geometryShown } = glyph; + const { record, instance, geometryShown } = glyph; const { display } = props; - if (!model || !instance) return; + if (!record || !instance) return; if (geometryShown && !display.focusedGlyphVisible) return; canvas.withTranslation(glyph.item.placement.origin, () => { @@ -339,16 +333,16 @@ export class SceneLayer extends CanvasItem { } #drawDebugOverlays(canvas: Canvas, props: SceneLayerProps, glyph: SceneGlyphFrame): void { - const { model, geometryShown } = glyph; + const { instance, geometryShown } = glyph; const { display } = props; - if (!geometryShown || !model || display.proofMode || !display.focusedGlyphVisible) return; + if (!geometryShown || !instance || display.proofMode || !display.focusedGlyphVisible) return; const { hover } = props.interaction; const hoveredSegmentId = hover?.type === "segment" ? hover.segmentId : null; canvas.withTranslation(glyph.item.placement.origin, () => { this.#debugOverlays.draw( canvas, - model, + instance.geometry, props.view.debugOverlays, hoveredSegmentId, canvas.pxToUpm(SCREEN_HIT_RADIUS), diff --git a/apps/desktop/src/renderer/src/lib/editor/rendering/Text.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/Text.ts index 07f88219..945d7e4e 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/Text.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/Text.ts @@ -68,10 +68,10 @@ export class Text { continue; } - const glyph = g.glyphId ? font.glyphForId(g.glyphId) : null; - if (!glyph) continue; + const instance = g.glyphId ? font.instance(g.glyphId, designLocation) : null; + if (!instance) continue; - const outline = glyph.instance(designLocation).render.outline; + const outline = instance.render.outline; canvas.save(); canvas.translate(runBase + g.origin.x + g.xOffset, line.y + g.origin.y + g.yOffset); diff --git a/apps/desktop/src/renderer/src/lib/editor/rendering/overlays/DebugOverlays.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/overlays/DebugOverlays.ts index dedf66c9..804844af 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/overlays/DebugOverlays.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/overlays/DebugOverlays.ts @@ -1,11 +1,11 @@ import type { Canvas } from "../Canvas"; -import type { Glyph } from "@/lib/model/Glyph"; +import type { GlyphInstanceGeometry } from "@/lib/model/Glyph"; import type { SegmentId } from "@/types/indicator"; export class DebugOverlays { draw( canvas: Canvas, - glyph: Glyph, + geometry: GlyphInstanceGeometry, overlays: { segmentBounds: boolean; tightBounds: boolean; @@ -18,50 +18,59 @@ export class DebugOverlays { const { debug } = canvas.theme; if (overlays.segmentBounds) { - this.#drawSegmentBounds(canvas, glyph, debug.segmentBounds); + this.#drawSegmentBounds(canvas, geometry, debug.segmentBounds); } if (overlays.tightBounds) { - this.#drawTightBounds(canvas, glyph, hoveredSegmentId, debug.tightBounds); + this.#drawTightBounds(canvas, geometry, hoveredSegmentId, debug.tightBounds); } if (overlays.hitRadii) { - this.#drawHitRadii(canvas, glyph, hitRadiusUpm, debug.hitRadii); + this.#drawHitRadii(canvas, geometry, hitRadiusUpm, debug.hitRadii); } if (overlays.glyphBbox) { - this.#drawGlyphBbox(canvas, glyph, debug.glyphBbox); + this.#drawGlyphBbox(canvas, geometry, debug.glyphBbox); } } - #drawSegmentBounds(canvas: Canvas, glyph: Glyph, color: string): void { - for (const { segment } of glyph.segments()) { - const b = segment.bounds; - canvas.strokeRect(b.min.x, b.min.y, b.max.x - b.min.x, b.max.y - b.min.y, color, 1); + #drawSegmentBounds(canvas: Canvas, geometry: GlyphInstanceGeometry, color: string): void { + for (const contour of geometry.contours) { + for (const segment of contour.segments()) { + const b = segment.bounds; + canvas.strokeRect(b.min.x, b.min.y, b.max.x - b.min.x, b.max.y - b.min.y, color, 1); + } } } #drawTightBounds( canvas: Canvas, - glyph: Glyph, + geometry: GlyphInstanceGeometry, hoveredSegmentId: SegmentId | null, color: string, ): void { if (hoveredSegmentId === null) return; - for (const { segment } of glyph.segments()) { - if (segment.id !== hoveredSegmentId) continue; - const b = segment.bounds; - canvas.strokeRect(b.min.x, b.min.y, b.max.x - b.min.x, b.max.y - b.min.y, color, 1); - return; + for (const contour of geometry.contours) { + for (const segment of contour.segments()) { + if (segment.id !== hoveredSegmentId) continue; + const b = segment.bounds; + canvas.strokeRect(b.min.x, b.min.y, b.max.x - b.min.x, b.max.y - b.min.y, color, 1); + return; + } } } - #drawHitRadii(canvas: Canvas, glyph: Glyph, hitRadiusUpm: number, color: string): void { + #drawHitRadii( + canvas: Canvas, + geometry: GlyphInstanceGeometry, + hitRadiusUpm: number, + color: string, + ): void { const r = hitRadiusUpm * canvas.camera.upmScale * canvas.camera.zoom; - for (const point of glyph.allPoints) { + for (const point of geometry.allPoints) { canvas.strokeCircle({ x: point.x, y: point.y }, r, color, 1); } } - #drawGlyphBbox(canvas: Canvas, glyph: Glyph, color: string): void { - const b = glyph.bounds; + #drawGlyphBbox(canvas: Canvas, geometry: GlyphInstanceGeometry, color: string): void { + const b = geometry.bounds; if (!b) return; canvas.strokeRect(b.min.x, b.min.y, b.max.x - b.min.x, b.max.y - b.min.y, color, 1); } diff --git a/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts b/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts index 235af3bc..4ade983c 100644 --- a/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts +++ b/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts @@ -41,8 +41,8 @@ describe("KeyboardRouter", () => { beforeEach(async () => { editor = new TestEditor(); await editor.startSession(); - // setActiveTool short-circuits if the target matches the current $activeTool, - // and $activeTool defaults to "select" without actually activating a tool + // setActiveTool short-circuits if the target matches the activeToolCell value, + // and activeToolCell defaults to "select" without actually activating a tool // instance. Force activation directly so primaryTool is populated. editor.toolManager.activate("select"); canvasActive = true; 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 cbc373b7..6959c174 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.test.ts @@ -64,15 +64,15 @@ describe("Font projects the workspace snapshot", () => { expect(font.sources.map((source) => source.name)).toEqual(["Regular"]); }); - it("$loaded flips reactively when the snapshot changes", () => { + it("loadedCell flips reactively when the snapshot changes", () => { const store = new FontStore(); const font = new Font(store); - expect(font.$loaded.value).toBe(false); + expect(font.loadedCell.value).toBe(false); store.replaceWorkspace(SNAPSHOT); - expect(font.$loaded.value).toBe(true); + expect(font.loadedCell.value).toBe(true); }); it("resets to the unloaded projection when the workspace goes null", () => { @@ -111,7 +111,11 @@ describe("font-level intents make the font variable", () => { await stack.editCoordinator.apply([ { kind: "createGlyph", - createGlyph: { glyphId, name: "A" as GlyphName, unicodes: [65 as Unicode] }, + createGlyph: { + glyphId, + name: "A" as GlyphName, + unicodes: [65 as Unicode], + }, }, ]); expect(stack.font.isVariable()).toBe(false); @@ -141,7 +145,9 @@ describe("font-level intents make the font variable", () => { createSource: { sourceId: boldSourceId, name: "Bold", - location: { values: { [weightAxisId]: 700 } as Record }, + location: { + values: { [weightAxisId]: 700 } as Record, + }, }, }, ]); @@ -149,7 +155,9 @@ describe("font-level intents make the font variable", () => { expect(bold?.id).toBe(boldSourceId); expect(applied.sources?.find((source) => source.name === "Bold")?.id).toBe(boldSourceId); expect(applied.layers).toEqual([]); - expect(stack.font.layerRecordForId(glyphId, boldSourceId)).toBeNull(); + expect(stack.font.glyph(glyphId)?.layers.find((layer) => layer.sourceId === boldSourceId)).toBe( + undefined, + ); }); it("createGlyphLayer projects sparse glyph-layer membership", async () => { @@ -159,7 +167,11 @@ describe("font-level intents make the font variable", () => { await stack.editCoordinator.apply([ { kind: "createGlyph", - createGlyph: { glyphId, name: "A" as GlyphName, unicodes: [65 as Unicode] }, + createGlyph: { + glyphId, + name: "A" as GlyphName, + unicodes: [65 as Unicode], + }, }, ]); @@ -173,7 +185,7 @@ describe("font-level intents make the font variable", () => { ]); expect(applied.glyphs?.[0]?.layers).toEqual([{ id: layerId, sourceId }]); - expect(stack.font.layerRecordForId(glyphId, sourceId)).toEqual({ id: layerId, sourceId }); + expect(stack.font.glyph(glyphId)?.layers).toEqual([{ id: layerId, sourceId }]); }); it("createGlyph authors a default layer for fresh glyphs", async () => { @@ -188,22 +200,26 @@ describe("font-level intents make the font variable", () => { await stack.editCoordinator.settled(); - const committed = stack.font.recordForId(record.id); + const committed = stack.font.glyph(record.id); expect(committed?.layers).toEqual(record.layers); - expect(stack.font.layerRecordForId(record.id, source.id)).toEqual(record.layers[0]); + expect( + stack.font.glyph(record.id)?.layers.find((layer) => layer.sourceId === source.id), + ).toEqual(record.layers[0]); - const glyph = await stack.font.loadGlyph(record.id, { sourceIds: [source.id] }); - expect(glyph?.xAdvance).toBe(stack.font.defaultXAdvance); + const loaded = await stack.font.loadGlyph(record.id, { + sourceIds: [source.id], + }); + expect(loaded).toBe(true); + expect(stack.font.layer(record.id, source.id)?.xAdvance).toBe(stack.font.defaultXAdvance); }); - it("preserves glyph object identity while record names change", async () => { + it("resolves updated glyph identity after record names change", async () => { const stack = createWorkspaceStack(); await stack.createWorkspace(); const record = stack.font.createGlyph("A" as GlyphName); await stack.editCoordinator.settled(); - const glyph = await stack.font.loadGlyph(record.id); - if (!glyph) throw new Error("Expected loaded glyph"); + expect(await stack.font.loadGlyph(record.id)).toBe(true); await stack.editCoordinator.apply([ { @@ -216,9 +232,9 @@ describe("font-level intents make the font variable", () => { }, ]); - expect(stack.font.glyphForId(record.id)).toBe(glyph); - expect(glyph.name).toBe("A.alt"); - expect(glyph.unicode).toBe(0xe001); + expect(stack.font.instance(record.id, stack.font.defaultLocation())).not.toBeNull(); + expect(stack.font.glyph(record.id)?.name).toBe("A.alt"); + expect(stack.font.glyph(record.id)?.unicodes).toEqual([0xe001]); }); it("exact sources without glyph layers have no live layer and do not render default geometry", async () => { @@ -229,7 +245,11 @@ describe("font-level intents make the font variable", () => { await stack.editCoordinator.apply([ { kind: "createGlyph", - createGlyph: { glyphId, name: "A" as GlyphName, unicodes: [65 as Unicode] }, + createGlyph: { + glyphId, + name: "A" as GlyphName, + unicodes: [65 as Unicode], + }, }, { kind: "createGlyphLayer", @@ -239,7 +259,10 @@ describe("font-level intents make the font variable", () => { sourceId: stack.font.defaultSource.id, }, }, - { kind: "setXAdvance", setXAdvance: { layerId: defaultLayerId, width: 640 } }, + { + kind: "setXAdvance", + setXAdvance: { layerId: defaultLayerId, width: 640 }, + }, ]); const axisId = mintAxisId(); @@ -269,18 +292,19 @@ describe("font-level intents make the font variable", () => { }, ]); - const glyph = await stack.font.loadGlyph(glyphId, { + const loaded = await stack.font.loadGlyph(glyphId, { sourceIds: [stack.font.defaultSource.id], }); - if (!glyph) throw new Error("Expected default glyph layer to open"); - expect(glyph.xAdvance).toBe(640); + expect(loaded).toBe(true); + expect(stack.font.layer(glyphId, stack.font.defaultSource.id)?.xAdvance).toBe(640); const bold = stack.font.source(sourceId); if (!bold) throw new Error("Expected created source"); - const instance = glyph.instanceAt(axisLocationFromLocation(bold.location)); + const boldLocation = axisLocationFromLocation(bold.location); + const instance = stack.font.instance(glyphId, boldLocation); + if (!instance) throw new Error("Expected glyph instance"); - expect(instance.layer).toBeNull(); - expect(instance.hasLayer).toBe(false); + expect(stack.font.editableLayerAt(glyphId, boldLocation)).toBeNull(); expect(instance.xAdvance).toBe(0); expect(instance.geometry.allPoints).toEqual([]); }); @@ -294,11 +318,19 @@ describe("font-level intents make the font variable", () => { await stack.editCoordinator.apply([ { kind: "createGlyph", - createGlyph: { glyphId, name: "A" as GlyphName, unicodes: [65 as Unicode] }, + createGlyph: { + glyphId, + name: "A" as GlyphName, + unicodes: [65 as Unicode], + }, }, { kind: "createGlyphLayer", - createGlyphLayer: { layerId: defaultLayerId, glyphId, sourceId: defaultSourceId }, + createGlyphLayer: { + layerId: defaultLayerId, + glyphId, + sourceId: defaultSourceId, + }, }, ]); @@ -330,7 +362,11 @@ describe("font-level intents make the font variable", () => { }, { kind: "createGlyphLayer", - createGlyphLayer: { layerId: boldLayerId, glyphId, sourceId: boldSourceId }, + createGlyphLayer: { + layerId: boldLayerId, + glyphId, + sourceId: boldSourceId, + }, }, ]); @@ -338,11 +374,15 @@ describe("font-level intents make the font variable", () => { const boldSource = stack.font.source(boldSourceId); if (!regularSource || !boldSource) throw new Error("Expected both sources"); - const regularLoad = stack.font.loadGlyph(glyphId, { sourceIds: [defaultSourceId] }); - const boldLoad = stack.font.loadGlyph(glyphId, { sourceIds: [boldSourceId] }); + const regularLoad = stack.font.loadGlyph(glyphId, { + sourceIds: [defaultSourceId], + }); + const boldLoad = stack.font.loadGlyph(glyphId, { + sourceIds: [boldSourceId], + }); await Promise.all([regularLoad, boldLoad]); - expect(stack.font.glyphLayerForId(glyphId, regularSource.id)?.id).toBe(defaultLayerId); - expect(stack.font.glyphLayerForId(glyphId, boldSource.id)?.id).toBe(boldLayerId); + expect(stack.font.layer(glyphId, regularSource.id)?.id).toBe(defaultLayerId); + expect(stack.font.layer(glyphId, boldSource.id)?.id).toBe(boldLayerId); }); }); diff --git a/apps/desktop/src/renderer/src/lib/model/Font.ts b/apps/desktop/src/renderer/src/lib/model/Font.ts index e55681c9..7cbb8050 100644 --- a/apps/desktop/src/renderer/src/lib/model/Font.ts +++ b/apps/desktop/src/renderer/src/lib/model/Font.ts @@ -13,12 +13,19 @@ import type { AxisId, LayerId, Location, + GlyphStructure, } from "@shift/types"; import { mintAxisId, mintGlyphId, mintLayerId, mintSourceId } from "@shift/types"; -import { computed, type Signal } from "@/lib/signals/signal"; +import { computed, signal, track, type Signal } from "@/lib/signals/signal"; import type { WorkspaceEditCoordinator } from "@/lib/workspace/WorkspaceEditCoordinator"; import type { WorkspaceGlyphSnapshotRequest } from "@shared/workspace/protocol"; -import { Glyph, type GlyphLayer } from "./Glyph"; +import { + Glyph, + GlyphGeometry, + type GlyphInstance, + type GlyphInstanceInput, + type GlyphLayer, +} from "./Glyph"; import { GlyphOutline } from "./GlyphOutline"; import type { FontStore, GlyphSnapshotStatus } from "./FontStore"; import type { GlyphLayerState } from "./GlyphLayerState"; @@ -33,6 +40,7 @@ import { import type { AxisLocation } from "@/types/variation"; import { defaultResources, GlyphInfo } from "@shift/glyph-info"; import { fallbackGlyphNameForUnicode } from "../utils/unicode"; +import { interpolate, normalize } from "@/lib/interpolation/interpolate"; export type GlyphLoadOptions = { readonly sourceIds?: readonly SourceId[]; @@ -45,6 +53,12 @@ type SnapshotRequest = { type InFlightKey = string & { readonly __inFlightKey: unique symbol }; +const EMPTY_GLYPH_STRUCTURE: GlyphStructure = { + contours: [], + anchors: [], + components: [], +}; + /** * Immutable lookup index for committed glyph records. * @@ -288,25 +302,23 @@ const DEFAULT_FONT_METRICS: FontMetrics = { * Reactive facade for the loaded font. * * `Font` exposes font-level metadata, source lookup, and domain editing verbs - * over `FontStore`. Getters such as `metrics`, `unicodes`, and `sources` are - * signal-backed: reading them inside a computed or effect subscribes to later - * font loads/resets. + * over `FontStore`. Plain getters such as `metrics`, `unicodes`, and `sources` + * return snapshots; reactive callers use the matching `*Cell` APIs. * * A glyph handle is only an identity. It may name a glyph that is not committed * in the font yet. Use {@link glyph} for existing glyph data, and use the * editor layer API when the caller intends to create or edit authored glyph data. */ export class Font { - // TODO: these all need to renamed to have the Cell naming convention - readonly #$loaded: Signal; - readonly #$metrics: Signal; - readonly #$metadata: Signal; - readonly #$sources: Signal; - readonly #$axes: Signal; - readonly #$unicodes: Signal; - readonly #$glyphRecords: Signal; - - readonly #directory: Signal; + readonly #loadedCell: Signal; + readonly #metricsCell: Signal; + readonly #metadataCell: Signal; + readonly #sourcesCell: Signal; + readonly #axesCell: Signal; + readonly #unicodesCell: Signal; + readonly #glyphRecordsCell: Signal; + + readonly #directoryCell: Signal; readonly #store: FontStore; readonly #editCoordinator: WorkspaceEditCoordinator | null; readonly #glyphLoadsInFlight = new Map>(); @@ -322,74 +334,75 @@ export class Font { this.#store = store; this.#editCoordinator = editCoordinator ?? null; const workspaceCell = store.workspaceCell; - this.#$loaded = computed(() => workspaceCell.value !== null); - this.#$metrics = computed(() => workspaceCell.value?.metrics ?? DEFAULT_FONT_METRICS); - this.#$metadata = computed(() => workspaceCell.value?.metadata ?? {}); - this.#$sources = computed(() => workspaceCell.value?.sources ?? []); - this.#$axes = computed(() => workspaceCell.value?.axes ?? []); - this.#directory = computed(() => GlyphDirectory.fromRecords(workspaceCell.value?.glyphs ?? [])); - this.#$unicodes = computed(() => [...this.#directory.value.unicodes]); - this.#$glyphRecords = computed(() => this.#directory.value.records); + this.#loadedCell = computed(() => workspaceCell.value !== null); + this.#metricsCell = computed(() => workspaceCell.value?.metrics ?? DEFAULT_FONT_METRICS); + this.#metadataCell = computed(() => workspaceCell.value?.metadata ?? {}); + this.#sourcesCell = computed(() => workspaceCell.value?.sources ?? []); + this.#axesCell = computed(() => workspaceCell.value?.axes ?? []); + this.#directoryCell = computed(() => + GlyphDirectory.fromRecords(workspaceCell.value?.glyphs ?? []), + ); + this.#unicodesCell = computed(() => [...this.#directoryCell.value.unicodes]); + this.#glyphRecordsCell = computed(() => this.#directoryCell.value.records); } /** @knipclassignore */ get loaded(): boolean { - return this.#$loaded.peek(); + return this.#loadedCell.peek(); } get defaultXAdvance(): number { - return this.#$metrics.peek().unitsPerEm / 2; + return this.#metricsCell.peek().unitsPerEm / 2; } /** @knipclassignore */ get metrics(): FontMetrics { - return this.#$metrics.peek(); + return this.#metricsCell.peek(); } /** @knipclassignore */ get unicodes(): readonly Unicode[] { - return this.#directory.peek().unicodes; + return this.#directoryCell.peek().unicodes; } - /** Raw signals for React hooks that need Signal. */ - /** @knipclassignore */ - get $loaded() { - return this.#$loaded; + /** Reactive loaded state for React hooks and effects. */ + get loadedCell(): Signal { + return this.#loadedCell; } - /** @knipclassignore */ - get $metrics() { - return this.#$metrics; + /** Reactive committed font metrics. */ + get metricsCell(): Signal { + return this.#metricsCell; } - /** @knipclassignore */ - get $unicodes(): Signal { - return this.#$unicodes; + /** Reactive committed Unicode assignments. */ + get unicodesCell(): Signal { + return this.#unicodesCell; } /** Reactive committed variation axes for sidebar controls. */ get axesCell(): Signal { - return this.#$axes; + return this.#axesCell; } /** Reactive committed variation sources for sidebar controls. */ get sourcesCell(): Signal { - return this.#$sources; + return this.#sourcesCell; } /** Reactive committed glyph directory records for UI lists and grids. */ get glyphRecordsCell(): Signal { - return this.#$glyphRecords; + return this.#glyphRecordsCell; } - /** Reactive glyph snapshot freshness for model consumers that derive loaded glyph views. */ + /** Reactive glyph snapshot bookkeeping for model consumers that derive local glyph views. */ get glyphSnapshotStatusCell(): Signal> { return this.#store.snapshotStatusCell; } /** @knipclassignore */ get metadata(): FontMetadata { - return this.#$metadata.peek(); + return this.#metadataCell.peek(); } /** @@ -398,7 +411,7 @@ export class Font { * @returns A read-only record list rebuilt after load, create, rename, or reset. */ glyphRecords(): readonly GlyphRecord[] { - return this.#directory.peek().records; + return this.#directoryCell.peek().records; } /** @@ -412,7 +425,7 @@ export class Font { * @returns A production glyph name suitable for opening or creating a glyph. */ nameForUnicode(unicode: Unicode): GlyphName { - return this.#directory.peek().nameForUnicode(unicode); + return this.#directoryCell.peek().nameForUnicode(unicode); } /** @@ -423,7 +436,7 @@ export class Font { * @knipclassignore */ hasGlyph(glyphId: GlyphId): boolean { - return this.#store.hasGlyph(glyphId); + return this.#directoryCell.peek().hasGlyph(glyphId); } /** @@ -434,12 +447,7 @@ export class Font { * @knipclassignore */ recordForName(name: GlyphName): GlyphRecord | null { - return this.#directory.peek().recordForName(name); - } - - /** Returns the committed glyph record for a stable glyph id. */ - recordForId(glyphId: GlyphId): GlyphRecord | null { - return this.#store.recordForId(glyphId); + return this.#directoryCell.peek().recordForName(name); } /** @@ -450,7 +458,7 @@ export class Font { * @knipclassignore */ unicodesForName(name: GlyphName): readonly Unicode[] { - return this.#directory.peek().unicodesForName(name); + return this.#directoryCell.peek().unicodesForName(name); } /** @@ -461,7 +469,7 @@ export class Font { * @knipclassignore */ primaryUnicodeForName(name: GlyphName): Unicode | null { - return this.#directory.peek().primaryUnicodeForName(name); + return this.#directoryCell.peek().primaryUnicodeForName(name); } /** @@ -471,7 +479,7 @@ export class Font { * @returns Base glyph names from the committed record; empty when absent. */ componentBaseNamesForName(name: GlyphName): readonly GlyphName[] { - return this.#directory.peek().componentBaseNamesForName(name); + return this.#directoryCell.peek().componentBaseNamesForName(name); } /** @@ -481,7 +489,7 @@ export class Font { * @returns Sorted dependent glyph names; empty when no committed glyph references it. */ dependentNamesForName(name: GlyphName): readonly GlyphName[] { - return this.#directory.peek().dependentNamesForName(name); + return this.#directoryCell.peek().dependentNamesForName(name); } /** @@ -497,7 +505,7 @@ export class Font { * @returns A glyph identity handle. The handle may refer to a missing glyph. */ glyphHandleForName(name: GlyphName): GlyphHandle { - return this.#directory.peek().glyphHandleForName(name); + return this.#directoryCell.peek().glyphHandleForName(name); } /** @@ -573,17 +581,6 @@ export class Font { return layerId; } - /** - * Returns the authored layer record for a glyph/source pair. - * - * @param glyphId - Committed glyph identity to inspect. - * @param sourceId - Source whose authored glyph data is requested. - * @returns The sparse layer record, or `null` when that source has no layer for the glyph. - */ - layerRecordForId(glyphId: GlyphId, sourceId: SourceId): GlyphLayerRecord | null { - return this.#store.layerRecordForId(glyphId, sourceId); - } - /** * Finds the next unused glyph name for an auto-incrementing base name. * @@ -620,17 +617,142 @@ export class Font { return { name, unicode }; } + /** + * Returns canonical identity and metadata for a committed glyph. + * + * @param glyphId - Stable glyph identity to inspect. + * @returns The committed glyph record, or `null` when the font does not contain the glyph. + */ + glyph(glyphId: GlyphId): GlyphRecord | null { + return this.#directoryCell.peek().recordForId(glyphId); + } + + /** + * Returns a reactive glyph identity record for a glyph id cell. + * + * @param glyphId - Cell containing the glyph to resolve, or `null` when no glyph is selected. + * @returns A cell whose value is the committed glyph record, or `null` when unavailable. + */ + glyphCell(glyphId: Signal): Signal { + return computed( + () => { + const id = glyphId.value; + if (!id) return null; + + return this.#directoryCell.value.recordForId(id); + }, + { name: "font.glyph" }, + ); + } + + /** + * Returns the authored mutable layer for an exact glyph/source pair. + * + * @param glyphId - Stable glyph identity to resolve. + * @param sourceId - Authored source identity to resolve. + * @returns The loaded authored layer, or `null` when the glyph/source pair is unavailable. + */ + layer(glyphId: GlyphId, sourceId: SourceId): GlyphLayer | null { + return this.#glyphLayer(glyphId, sourceId); + } + + /** + * Returns a reactive authored layer for glyph and source id cells. + * + * @param glyphId - Cell containing the glyph to resolve, or `null` when no glyph is selected. + * @param sourceId - Cell containing the exact source to resolve, or `null` when interpolated. + * @returns A cell whose value is the loaded authored layer, or `null` when unavailable. + */ + layerCell( + glyphId: Signal, + sourceId: Signal, + ): Signal { + return computed( + () => { + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#sourcesCell); + + const id = glyphId.value; + const source = sourceId.value; + if (!id || !source) return null; + + return this.layer(id, source); + }, + { name: "font.layer" }, + ); + } + + /** + * Returns the resolved read/render/hit-test instance for a glyph at a design location. + * + * @param glyphId - Stable glyph identity to resolve. + * @param location - Reactive designspace location for the instance. + * @returns The resolved instance, or `null` when the glyph is not available locally. + */ + instance(glyphId: GlyphId, location: AxisLocation | Signal): GlyphInstance | null { + const glyph = this.#glyphModel(glyphId); + if (!glyph) return null; + + const locationSignal = axisLocationSignal(location); + const existing = glyph.cachedInstanceForFont(locationSignal); + if (existing) return existing; + + return glyph.createInstanceForFont(this.#glyphInstanceInput(glyph, locationSignal)); + } + + /** + * Returns a reactive glyph instance for a glyph id cell and design location cell. + * + * @param glyphId - Cell containing the glyph to resolve, or `null` when no glyph is selected. + * @param location - Cell containing the designspace location to render and hit-test. + * @returns A cell whose value is the resolved glyph instance, or `null` when unavailable. + */ + instanceCell( + glyphId: Signal, + location: Signal, + ): Signal { + return computed( + () => { + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#axesCell); + track(this.#sourcesCell); + + const id = glyphId.value; + if (!id) return null; + + return this.instance(id, location); + }, + { name: "font.instance" }, + ); + } + + /** + * Returns the authored layer that exactly matches a designspace location. + * + * @param glyphId - Stable glyph identity to resolve. + * @param location - Current designspace location. + * @returns The editable layer at the exact source location, or `null` for interpolated views. + */ + editableLayerAt(glyphId: GlyphId, location: AxisLocation): GlyphLayer | null { + const source = this.sourceAt(location); + if (!source) return null; + + return this.layer(glyphId, source.id); + } + /** * Returns the local glyph model for a stable glyph id. * * @param glyphId - document glyph identity to resolve. * @returns the id-keyed glyph model, or null when the glyph is missing or not loaded. */ - glyphForId(glyphId: GlyphId): Glyph | null { + #glyphModel(glyphId: GlyphId): Glyph | null { if (!this.loaded) return null; - const directory = this.#directory.peek(); - const record = this.#store.recordForId(glyphId); + const directory = this.#directoryCell.peek(); + const record = directory.recordForId(glyphId); if (!record) return null; return this.#store.glyphModel(glyphId, () => { @@ -652,15 +774,15 @@ export class Font { * @param sourceId - exact source whose authored layer should be loaded. * @returns the id-keyed glyph layer model, or null when record or geometry is unavailable. */ - glyphLayerForId(glyphId: GlyphId, sourceId: SourceId): GlyphLayer | null { + #glyphLayer(glyphId: GlyphId, sourceId: SourceId): GlyphLayer | null { const source = this.source(sourceId); if (!source) return null; - const layer = this.#store.layerRecordForId(glyphId, source.id); + const layer = this.#directoryCell.peek().layerForGlyphAtSource(glyphId, source.id); if (!layer) return null; return this.#store.glyphLayerModel(glyphId, source.id, () => { - const glyph = this.glyphForId(glyphId); + const glyph = this.#glyphModel(glyphId); if (!glyph) return null; const state = glyph.isPrimarySource(source) ? undefined : this.layerState(layer.id); @@ -668,6 +790,64 @@ export class Font { }); } + #glyphGeometry(glyphId: GlyphId, location: AxisLocation): GlyphGeometry { + const glyph = this.#glyphModel(glyphId); + if (!glyph) return emptyGlyphGeometry(); + + const sourceLocation = axisLocationFromLocation(glyph.primarySourceForFont.location); + if (axisLocationsEqual(location, sourceLocation, [...this.getAxes()])) { + return glyph.primaryGeometryForFont; + } + + const exactSource = this.sourceAt(location); + if (exactSource) { + return this.layer(glyphId, exactSource.id)?.geometry ?? emptyGlyphGeometry(); + } + + const variationData = this.variationData(glyphId); + if (!variationData) { + return glyph.primaryGeometryForFont; + } + + const values = interpolate(variationData, normalize(location, [...this.getAxes()])); + if (values.length === 0) { + return glyph.primaryGeometryForFont; + } + + return new GlyphGeometry(glyph.interpolationStructureForFont, values); + } + + #glyphInstanceInput(glyph: Glyph, location: Signal): GlyphInstanceInput { + return { + location, + layer: computed( + () => { + const currentLocation = location.value; + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#axesCell); + track(this.#sourcesCell); + + return this.editableLayerAt(glyph.id, currentLocation); + }, + { name: "font.instance.layer" }, + ), + geometry: computed( + () => { + const currentLocation = location.value; + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#axesCell); + track(this.#sourcesCell); + + return this.#glyphGeometry(glyph.id, currentLocation); + }, + { name: "font.instance.geometry" }, + ), + outline: this.outline(glyph, location), + }; + } + /** * Reports whether every authored source for each glyph has local geometry. * @@ -675,7 +855,7 @@ export class Font { */ areGlyphsLoaded(glyphIds: readonly GlyphId[]): boolean { for (const glyphId of glyphIds) { - if (!this.recordForId(glyphId)) return false; + if (!this.glyph(glyphId)) return false; for (const sourceId of this.#store.sourceIdsForGlyph(glyphId)) { if (this.#store.needsGlyphSource(glyphId, sourceId)) return false; } @@ -684,40 +864,41 @@ export class Font { } /** - * Loads one glyph's geometry and returns its live model. + * Loads one glyph's geometry. * * @param glyphId - Stable glyph identity whose local geometry should be available. * @param options - Optional source scope; omitted means every authored layer for the glyph. - * @returns null when the glyph is not present in the current font. + * @returns `true` when the glyph exists and is locally available after the load. * @see {@link loadGlyphs} */ - async loadGlyph(glyphId: GlyphId, options: GlyphLoadOptions = {}): Promise { - return (await this.loadGlyphs([glyphId], options)).get(glyphId) ?? null; + async loadGlyph(glyphId: GlyphId, options: GlyphLoadOptions = {}): Promise { + return (await this.loadGlyphs([glyphId], options)).has(glyphId); } /** - * Loads missing or stale glyph geometry and returns the requested live models. + * Requests local glyph geometry and returns the requested glyph ids that are available. * * @remarks - * Component bases discovered while loading are hydrated as a side effect, but - * the returned map is keyed only by the glyph IDs requested by the caller. + * Component bases discovered while reading snapshots are requested as a side + * effect, but the returned set is keyed only by the glyph IDs requested by the + * caller. * * @param glyphIds - Stable glyph identities whose local geometry should be available. * @param options - Optional source scope; omitted means every authored layer for each glyph. - * @returns A fresh map containing the requested glyphs that exist in the current font. + * @returns A fresh set containing requested glyph ids that exist in the current font. */ async loadGlyphs( glyphIds: readonly GlyphId[], options: GlyphLoadOptions = {}, - ): Promise> { + ): Promise> { await this.#loadGlyphSnapshots(glyphIds, options); - const glyphs = new Map(); + const loaded = new Set(); for (const glyphId of uniqueGlyphIds(glyphIds)) { - const glyph = this.glyphForId(glyphId); - if (glyph) glyphs.set(glyphId, glyph); + const glyph = this.#glyphModel(glyphId); + if (glyph) loaded.add(glyphId); } - return glyphs; + return loaded; } async #loadGlyphSnapshots( @@ -840,13 +1021,37 @@ export class Font { /** * Create a reactive composed outline for a glyph. * - * Font owns glyph lookup, so component expansion is rooted here instead of - * passing resolver callbacks through individual glyphs. + * Font supplies glyph, layer, and geometry lookup for component expansion. * * @returns A new outline object that follows `location`. */ outline(glyph: Glyph, location: Signal): GlyphOutline { - return new GlyphOutline(glyph, location, this); + return new GlyphOutline( + glyph, + location, + (glyphId) => { + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + + return this.#glyphModel(glyphId); + }, + (glyphId, resolvedLocation) => { + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#axesCell); + track(this.#sourcesCell); + + return this.editableLayerAt(glyphId, resolvedLocation); + }, + (glyphId, resolvedLocation) => { + track(this.#directoryCell); + track(this.#store.snapshotStatusCell); + track(this.#axesCell); + track(this.#sourcesCell); + + return this.#glyphGeometry(glyphId, resolvedLocation); + }, + ); } /** @@ -888,13 +1093,7 @@ export class Font { source(sourceId: SourceId): Source | null { const sources = this.sources; - for (const source of sources) { - if (source.id === sourceId) { - return source; - } - } - - return null; + return sourceById(sources, sourceId); } /** @@ -907,17 +1106,20 @@ export class Font { * @returns The exact matching source, or `null` when the location is interpolated. */ sourceAt(location: AxisLocation): Source | null { - const axes = this.getAxes(); - const sources = this.sources; - - for (const source of sources) { - const sourceLocation = axisLocationFromLocation(source.location); - if (axisLocationsEqual(sourceLocation, location, axes)) { - return source; - } - } + return sourceAtLocation(this.sources, this.getAxes(), location); + } - return null; + /** + * Returns a reactive exact source for a design location cell. + * + * @param location - Cell containing the designspace location to match exactly. + * @returns A cell whose value is the exact source, or `null` when interpolated. + */ + sourceAtCell(location: Signal): Signal { + return computed( + () => sourceAtLocation(this.#sourcesCell.value, this.#axesCell.value, location.value), + { name: "font.sourceAt" }, + ); } /** @@ -1019,12 +1221,12 @@ export class Font { /** @knipclassignore — used by VariationPanel component */ getAxes(): Axis[] { - return this.#$axes.peek(); + return this.#axesCell.peek(); } /** @knipclassignore — used by VariationPanel component */ get sources(): Source[] { - return this.#$sources.peek(); + return this.#sourcesCell.peek(); } defaultLocation(): AxisLocation { @@ -1032,6 +1234,27 @@ export class Font { } } +function sourceById(sources: readonly Source[], sourceId: SourceId): Source | null { + for (const source of sources) { + if (source.id === sourceId) return source; + } + + return null; +} + +function sourceAtLocation( + sources: readonly Source[], + axes: readonly Axis[], + location: AxisLocation, +): Source | null { + for (const source of sources) { + const sourceLocation = axisLocationFromLocation(source.location); + if (axisLocationsEqual(sourceLocation, location, axes)) return source; + } + + return null; +} + function glyphLayerKey(glyphId: GlyphId, sourceId: SourceId): string { return `${glyphId}:${sourceId}`; } @@ -1040,6 +1263,20 @@ function inFlightKey(glyphId: GlyphId, sourceId: SourceId): InFlightKey { return `${glyphId}:${sourceId}` as InFlightKey; } +function axisLocationSignal(location: AxisLocation | Signal): Signal { + if (isSignal(location)) return location; + + return signal(location); +} + +function emptyGlyphGeometry(): GlyphGeometry { + return new GlyphGeometry(EMPTY_GLYPH_STRUCTURE, new Float64Array([0])); +} + +function isSignal(value: T | Signal): value is Signal { + return typeof value === "object" && value !== null && "peek" in value && "value" in value; +} + function uniqueGlyphIds(glyphIds: readonly GlyphId[]): GlyphId[] { return [...new Set(glyphIds)]; } 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 1b4d4938..4452f377 100644 --- a/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/Glyph.test.ts @@ -1,10 +1,10 @@ import { beforeEach, describe, expect, it } from "vitest"; -import type { GlyphName, PointId } from "@shift/types"; +import type { GlyphRecord, PointId } from "@shift/types"; import type { Point } from "@shift/glyph-state"; import { effect } from "@/lib/signals/signal"; import { axisLocationFromLocation } from "@/lib/variation/location"; import { TestEditor } from "@/testing/TestEditor"; -import type { Glyph, GlyphLayer } from "./Glyph"; +import type { GlyphLayer } from "./Glyph"; /** * Restored from the WS6 behavioral inventory (git show ef037c6e^), rebuilt on @@ -17,9 +17,24 @@ import type { Glyph, GlyphLayer } from "./Glyph"; async function addTriangle(editor: TestEditor, layer: GlyphLayer): Promise { const contourId = layer.addContour(); - layer.addPoint(contourId, { x: 0, y: 0, pointType: "onCurve", smooth: false }); - layer.addPoint(contourId, { x: 100, y: 0, pointType: "onCurve", smooth: false }); - layer.addPoint(contourId, { x: 50, y: 100, pointType: "onCurve", smooth: false }); + layer.addPoint(contourId, { + x: 0, + y: 0, + pointType: "onCurve", + smooth: false, + }); + layer.addPoint(contourId, { + x: 100, + y: 0, + pointType: "onCurve", + smooth: false, + }); + layer.addPoint(contourId, { + x: 50, + y: 100, + pointType: "onCurve", + smooth: false, + }); layer.closeContour(contourId); await editor.settle(); @@ -44,21 +59,22 @@ function sourcePosition(layer: GlyphLayer, pointId: PointId): { x: number; y: nu describe("Glyph", () => { let editor: TestEditor; - let glyph: Glyph; + let record: GlyphRecord; let layer: GlyphLayer; beforeEach(async () => { editor = new TestEditor(); await editor.startSession(); layer = editor.editingGlyphLayer!; - const record = editor.font.recordForName("A" as GlyphName)!; - glyph = editor.font.glyphForId(record.id)!; + const previewRecord = editor.previewGlyphRecordCell.peek(); + if (!previewRecord) throw new Error("Expected preview glyph record"); + record = previewRecord; }); - it("hydrates identity and state from the workspace", () => { - expect(glyph.name).toBe("A"); - expect(glyph.unicode).toBe(65); - expect(glyph.contours.length).toBe(0); + it("loads identity and state from the workspace", () => { + expect(record.name).toBe("A"); + expect(record.unicodes[0]).toBe(65); + expect(layer.contours.length).toBe(0); }); it("applies structural edits echoed by the workspace", async () => { @@ -74,12 +90,14 @@ describe("Glyph", () => { it("updates positions synchronously and keeps them after the echo folds", async () => { const [first] = await addTriangle(editor, layer); + const instance = editor.previewGlyphInstance; + if (!instance) throw new Error("Expected glyph instance"); layer.applyPositionPatch([{ kind: "point", id: first!.id, x: 25, y: 75 }]); - expect(glyph.point(first!.id)).toMatchObject({ x: 25, y: 75 }); + expect(instance.geometry.point(first!.id)).toMatchObject({ x: 25, y: 75 }); await editor.settle(); - expect(glyph.point(first!.id)).toMatchObject({ x: 25, y: 75 }); + expect(instance.geometry.point(first!.id)).toMatchObject({ x: 25, y: 75 }); }); it("feeds consumers that track source coordinate changes before reading geometry", async () => { @@ -88,7 +106,7 @@ describe("Glyph", () => { const subscription = effect(() => { layer.coordinateBuffersChangedCell.value; - pointX = glyph.point(first!.id)?.x ?? pointX; + pointX = layer.point(first!.id)?.x ?? pointX; }); layer.applyPositionPatch([{ kind: "point", id: first!.id, x: 33, y: 44 }]); @@ -174,15 +192,16 @@ describe("anchors edit through the workspace", () => { describe("glyph layers keep public geometry coherent across position edits", () => { let editor: TestEditor; - let glyph: Glyph; + let record: GlyphRecord; let layer: GlyphLayer; beforeEach(async () => { editor = new TestEditor(); await editor.startSession(); layer = editor.editingGlyphLayer!; - const record = editor.font.recordForName("A" as GlyphName)!; - glyph = editor.font.glyphForId(record.id)!; + const previewRecord = editor.previewGlyphRecordCell.peek(); + if (!previewRecord) throw new Error("Expected preview glyph record"); + record = previewRecord; }); it("previews point patches through every public layer geometry view", async () => { @@ -197,7 +216,7 @@ describe("glyph layers keep public geometry coherent across position edits", () x: 25, y: 75, }); - expect(layer.bounds).toEqual(glyph.bounds); + expect(layer.bounds).toEqual(editor.previewGlyphInstance?.geometry.bounds); }); it("applies committed point patches to the source and owning glyph geometry", async () => { @@ -208,7 +227,7 @@ describe("glyph layers keep public geometry coherent across position edits", () expect(sourcePosition(layer, second!.id)).toEqual({ x: 25, y: 75 }); expect(pointPosition(layer, second!.id)).toEqual({ x: 25, y: 75 }); - expect(glyph.point(second!.id)).toMatchObject({ x: 25, y: 75 }); + expect(editor.previewGlyphInstance?.geometry.point(second!.id)).toMatchObject({ x: 25, y: 75 }); }); it("commits a preview without stale geometry or double-applying local positions", async () => { @@ -225,7 +244,12 @@ describe("glyph layers keep public geometry coherent across position edits", () if (!committed) throw new Error("Expected committed position"); layer.previewPositionPatch([ - { kind: "point", id: second!.id, x: committed.x + 10, y: committed.y + 5 }, + { + kind: "point", + id: second!.id, + x: committed.x + 10, + y: committed.y + 5, + }, ]); expect(pointPosition(layer, second!.id)).toEqual({ x: 35, y: 80 }); @@ -233,7 +257,11 @@ describe("glyph layers keep public geometry coherent across position edits", () it("keeps source-backed instance geometry contours fresh after position edits", async () => { const [, second] = await addTriangle(editor, layer); - const instance = glyph.instanceAt(axisLocationFromLocation(layer.source.location)); + const instance = editor.font.instance( + record.id, + axisLocationFromLocation(layer.source.location), + ); + if (!instance) throw new Error("Expected glyph instance"); layer.applyPositionPatch([{ kind: "point", id: second!.id, x: 25, y: 75 }]); await editor.settle(); @@ -250,7 +278,11 @@ describe("glyph layers keep public geometry coherent across position edits", () it("invalidates source-backed instance contours that were read before a position edit", async () => { const [, second] = await addTriangle(editor, layer); - const instance = glyph.instanceAt(axisLocationFromLocation(layer.source.location)); + const instance = editor.font.instance( + record.id, + axisLocationFromLocation(layer.source.location), + ); + if (!instance) throw new Error("Expected glyph instance"); expect( instance.geometry.contours.at(-1)?.points.find((point) => point.id === second!.id), diff --git a/apps/desktop/src/renderer/src/lib/model/Glyph.ts b/apps/desktop/src/renderer/src/lib/model/Glyph.ts index 379cc9df..a415308c 100644 --- a/apps/desktop/src/renderer/src/lib/model/Glyph.ts +++ b/apps/desktop/src/renderer/src/lib/model/Glyph.ts @@ -15,9 +15,7 @@ import type { } from "@shift/types"; import { mintAnchorId, mintContourId, mintPointId } from "@shift/types"; import type { GlyphHandle } from "@shift/bridge"; -import { computed, keyedCache, signal, type ComputedSignal, type Signal } from "@/lib/signals"; -import { interpolate, normalize } from "@/lib/interpolation/interpolate"; -import { axisLocationFromLocation, axisLocationsEqual } from "@/lib/variation/location"; +import { computed, keyedCache, type ComputedSignal, type Signal } from "@/lib/signals"; import type { AxisLocation } from "@/types/variation"; import { Transform } from "@/lib/transform/Transform"; import { Alignment } from "@/lib/transform/Alignment"; @@ -69,21 +67,25 @@ export { type GlyphLayerPositionTarget, }; -const EMPTY_GLYPH_STRUCTURE: GlyphStructure = { - contours: [], - anchors: [], - components: [], -}; - -function emptyGlyphGeometry(): GlyphGeometry { - return new GlyphGeometry(EMPTY_GLYPH_STRUCTURE, new Float64Array([0])); -} - interface GlyphEditState { readonly state: GlyphLayerState; readonly geometry: Signal; } +/** + * Resolved reactive inputs used to construct one read-only glyph instance. + * + * @remarks + * `Font` owns these signals so source matching, interpolation, and outline + * component lookup stay outside `GlyphInstance`. + */ +export interface GlyphInstanceInput { + readonly location: Signal; + readonly layer: Signal; + readonly geometry: Signal; + readonly outline: GlyphOutline; +} + /** * Geometry lookup surface for a glyph instance. * @@ -98,6 +100,7 @@ export interface GlyphInstanceGeometry { readonly xAdvanceCell: Signal; readonly sidebearings: GlyphSidebearings; readonly sidebearingsCell: Signal; + readonly bounds: BoundsType | null; readonly contours: readonly Contour[]; readonly allPoints: readonly Point[]; contour(contourId: ContourId): Contour | null; @@ -204,7 +207,10 @@ class GlyphEditSession { // No contourId: Rust derives the contour from the anchor point — the // renderer never bookkeeps pending point→contour maps. - this.#intents.addPoints({ before: beforePointId, points: [this.#seed(pointId, edit)] }); + this.#intents.addPoints({ + before: beforePointId, + points: [this.#seed(pointId, edit)], + }); return pointId; } @@ -240,7 +246,14 @@ class GlyphEditSession { const anchorId = mintAnchorId(); this.#intents.addAnchors({ - anchors: [{ id: anchorId, x: position.x, y: position.y, ...(name === null ? {} : { name }) }], + anchors: [ + { + id: anchorId, + x: position.x, + y: position.y, + ...(name === null ? {} : { name }), + }, + ], }); return anchorId; @@ -742,48 +755,31 @@ export class GlyphLayer { * Represents one glyph resolved at one designspace location. * * @remarks - * `geometry` is the lookup and hit-testing surface, `render` is the drawing - * surface, and `edit` is present only when this location maps to an authored - * source that can be mutated. Interpolated locations still read like normal - * glyph instances: they render and hit-test through the same API, while - * mutation is absent until a source exists at that location. + * `geometry` is the lookup and hit-testing surface, and `render` is the drawing + * surface. Editability is resolved separately through `Font.layer` or + * `Font.editableLayerAt`; instances are read/render views only. */ export class GlyphInstance { readonly #location: Signal; - readonly #layer: ComputedSignal; readonly geometry: GlyphInstanceGeometry; readonly render: GlyphRenderModel; /** * Creates a glyph instance tied to a live designspace location. * - * @param glyph - Glyph identity whose sources and variation data are resolved. - * @param location - Live designspace location for geometry, rendering, and editability. + * @param input - Resolved reactive inputs assembled by the owning font. */ - constructor(glyph: Glyph, location: Signal) { - this.#location = location; + constructor(input: GlyphInstanceInput) { + this.#location = input.location; - this.#layer = computed(() => glyph.layerAt(location.value), { - name: "glyphInstance.layer", - }); - - this.geometry = new InstanceGeometry(glyph, location); - this.render = new InstanceRender(glyph, location).model; + this.geometry = new InstanceGeometry(input.layer, input.geometry); + this.render = new InstanceRender(input.layer, input.geometry, input.outline).model; } get location(): AxisLocation { return this.#location.peek(); } - /** - * Returns the authored glyph layer at this instance location. - * - * @returns The matching glyph layer, or `null` when this instance is interpolated. - */ - get layer(): GlyphLayer | null { - return this.#layer.peek(); - } - get xAdvance(): number { return this.geometry.xAdvance; } @@ -799,15 +795,6 @@ export class GlyphInstance { get sidebearingsCell(): Signal { return this.geometry.sidebearingsCell; } - - /** - * Returns whether this instance sits on an authored glyph layer location. - * - * @returns `true` when {@link layer} is present. - */ - get hasLayer(): boolean { - return this.layer !== null; - } } class InstanceRender { @@ -825,27 +812,27 @@ class InstanceRender { readonly model: GlyphRenderModel; - constructor(glyph: Glyph, location: Signal) { - const outline = glyph.outline(location); - + constructor( + layer: Signal, + geometry: Signal, + outline: GlyphOutline, + ) { const contours = computed(() => { - const currentLocation = location.value; - const source = glyph.layerAt(currentLocation); + const source = layer.value; if (source) { return this.#sourceContours(source.structureCell.value, source.coordinateBuffersCell.value); } - return GlyphRenderModel.geometryContours(glyph.geometryAt(currentLocation)); + return GlyphRenderModel.geometryContours(geometry.value); }); const anchors = computed(() => { - const currentLocation = location.value; - const source = glyph.layerAt(currentLocation); + const source = layer.value; if (source) { return this.#sourceAnchors(source.structureCell.value, source.coordinateBuffersCell.value); } - return GlyphRenderModel.geometryAnchors(glyph.geometryAt(currentLocation)); + return GlyphRenderModel.geometryAnchors(geometry.value); }); this.model = new GlyphRenderModel(contours, anchors, outline); @@ -892,15 +879,13 @@ class InstanceGeometry implements GlyphInstanceGeometry { readonly #xAdvance: ComputedSignal; readonly #sidebearings: ComputedSignal; - constructor(glyph: Glyph, location: Signal) { + constructor(layer: Signal, geometry: Signal) { this.#resolved = computed( () => { - const currentLocation = location.value; - - const source = glyph.layerAt(currentLocation); + const source = layer.value; if (source) return new SourceGeometryCache(source); - return new SnapshotGeometryCache(glyph.geometryAt(currentLocation)); + return new SnapshotGeometryCache(geometry.value); }, { name: "glyphInstance.geometry" }, ); @@ -936,6 +921,10 @@ class InstanceGeometry implements GlyphInstanceGeometry { return this.#resolved.peek().contours; } + get bounds(): BoundsType | null { + return this.#resolved.peek().bounds; + } + contour(contourId: ContourId): Contour | null { return this.#resolved.peek().contour(contourId); } @@ -1004,6 +993,10 @@ class SnapshotGeometryCache implements GlyphInstanceGeometry { return this.#geometry.contours; } + get bounds(): BoundsType | null { + return this.#geometry.bounds; + } + contour(contourId: ContourId): Contour | null { return this.#geometry.contour(contourId); } @@ -1105,6 +1098,10 @@ class SourceGeometryCache implements GlyphInstanceGeometry { return this.#contours.peek(); } + get bounds(): BoundsType | null { + return this.#source.bounds; + } + contour(contourId: ContourId): Contour | null { return ( this.#sourceContours @@ -1310,13 +1307,11 @@ class ContourCache { } /** - * Reactive model for one glyph identity. + * Reactive model for one local glyph source. * - * `Glyph` exposes rendered/interpolated glyph data and can create exact - * layer models through `Font.glyphLayer`. The primary - * geometry is the source used when the glyph was constructed; use - * {@link geometryAt} or {@link outlineAt} when rendering at another designspace - * location. + * @remarks + * Public callers should resolve glyph identity, authored layers, and rendered + * instances through `Font.glyph`, `Font.layer`, and `Font.instance`. */ export class Glyph { readonly #font: Font; @@ -1331,7 +1326,6 @@ export class Glyph { readonly #xAdvance: ComputedSignal; readonly #edit: GlyphEditSession; readonly #instances = new WeakMap, GlyphInstance>(); - readonly #outlines = new WeakMap, GlyphOutline>(); constructor( font: Font, @@ -1361,7 +1355,7 @@ export class Glyph { } get handle(): GlyphHandle { - const record = this.#font.recordForId(this.#glyphId); + const record = this.#font.glyph(this.#glyphId); if (!record) return this.#fallbackHandle; const unicode = record.unicodes[0]; @@ -1406,114 +1400,46 @@ export class Glyph { return this.#geometry.peek().allPoints; } - /** @knipclassignore — reactive contour API for UI consumers. */ - get $contours(): Signal { - return computed(() => this.contours); - } - - get $xAdvance(): Signal { - return this.#xAdvance; - } - - /** - * Create a read-only location view for this glyph. - * - * @returns A wrapper whose geometry and outline are resolved at `location`. - */ - instanceAt(location: AxisLocation): GlyphInstance { - return this.instance(signal(location)); - } - /** * Returns the cached instance for a live designspace location. * * @param location - Signal whose value controls source resolution and interpolation. - * @returns The stable instance object for this glyph/location signal pair. + * @returns The cached instance object for this glyph/location signal pair. + * @internal */ - instance(location: Signal): GlyphInstance { - const existing = this.#instances.get(location); - if (existing) return existing; - - const instance = new GlyphInstance(this, location); - this.#instances.set(location, instance); - return instance; + cachedInstanceForFont(location: Signal): GlyphInstance | null { + return this.#instances.get(location) ?? null; } /** - * Resolve glyph geometry at a designspace location. + * Stores a font-created instance for this glyph/location signal pair. * - * If the location matches an exact source, that layer geometry is used. If - * the glyph has variation data, interpolated geometry is returned. Otherwise - * the primary layer geometry is returned. - * - * @returns A geometry snapshot for rendering or hit testing at `location`. + * @param input - Resolved reactive inputs assembled by the owning font. + * @returns The created instance object. + * @internal */ - geometryAt(location: AxisLocation): GlyphGeometry { - const sourceLocation = axisLocationFromLocation(this.#source.location); - if (axisLocationsEqual(location, sourceLocation, [...this.#font.getAxes()])) { - return this.#geometry.peek(); - } - - const exactSource = this.#font.sourceAt(location); - if (exactSource) { - return ( - this.#font.glyphLayerForId(this.#glyphId, exactSource.id)?.geometry ?? emptyGlyphGeometry() - ); - } - - const variationData = this.#font.variationData(this.#glyphId); - if (!variationData) { - return this.#geometry.peek(); - } - - const values = interpolate(variationData, normalize(location, [...this.#font.getAxes()])); - - if (values.length === 0) { - return this.#geometry.peek(); - } + createInstanceForFont(input: GlyphInstanceInput): GlyphInstance { + const existing = this.#instances.get(input.location); + if (existing) return existing; - return new GlyphGeometry(this.#layerState.structure, values); + const instance = new GlyphInstance(input); + this.#instances.set(input.location, instance); + return instance; } - /** - * Resolve an authored glyph layer that exactly matches a designspace location. - * - * Interpolated locations return `null`; callers should use - * {@link geometryAt} for those snapshots. - */ - layerAt(location: AxisLocation): GlyphLayer | null { - const exactSource = this.#font.sourceAt(location); - if (!exactSource) return null; - - return this.#font.glyphLayerForId(this.#glyphId, exactSource.id); + /** @internal Primary authored source backing the cached glyph model. */ + get primarySourceForFont(): Source { + return this.#source; } - /** - * Create a reactive outline model for this glyph. - * - * The outline follows the provided variation-location signal and resolves - * component glyphs through the owning font. Calls with the same location - * signal return the same outline object, so repeated text glyph instances - * share reactive outline parts. - */ - outline(location: Signal): GlyphOutline { - const existing = this.#outlines.get(location); - if (existing) return existing; - - const outline = this.#font.outline(this, location); - this.#outlines.set(location, outline); - return outline; + /** @internal Primary source geometry backing fallback and interpolation. */ + get primaryGeometryForFont(): GlyphGeometry { + return this.#geometry.peek(); } - /** - * Create an outline model at a fixed designspace location. - * - * @returns A new outline object backed by an internal constant location - * signal. Store the result when reading `parts`, `svgPath`, or `bounds` - * repeatedly. - */ - outlineAt(location: AxisLocation): GlyphOutline { - return this.outline(signal(location)); + /** @internal Structure used by variation interpolation values. */ + get interpolationStructureForFont(): GlyphStructure { + return this.#layerState.structure; } isPrimarySource(source: Source): boolean { diff --git a/apps/desktop/src/renderer/src/lib/model/GlyphOutline.ts b/apps/desktop/src/renderer/src/lib/model/GlyphOutline.ts index 82bddd35..466d94ab 100644 --- a/apps/desktop/src/renderer/src/lib/model/GlyphOutline.ts +++ b/apps/desktop/src/renderer/src/lib/model/GlyphOutline.ts @@ -3,13 +3,13 @@ import type { Signal } from "@/lib/signals/signal"; import { computed, signal, track, type ComputedSignal } from "@/lib/signals/signal"; import type { AxisLocation } from "@/types/variation"; import { Contour, Segment } from "@shift/glyph-state"; -import type { Glyph } from "./Glyph"; +import type { Glyph, GlyphGeometry, GlyphLayer } from "./Glyph"; import type { ContourData, GlyphId } from "@shift/types"; import type { LayerContourCoordinates } from "./GlyphLayerState"; -interface GlyphResolver { - glyphForId(glyphId: GlyphId): Glyph | null; -} +type GlyphForComponent = (glyphId: GlyphId) => Glyph | null; +type AuthoredLayerForLocation = (glyphId: GlyphId, location: AxisLocation) => GlyphLayer | null; +type ResolvedGeometryForLocation = (glyphId: GlyphId, location: AxisLocation) => GlyphGeometry; type OutlineCommand = | { readonly kind: "move"; readonly to: Point2D } @@ -229,7 +229,9 @@ class GeometryOutlinePart implements OutlinePart { export class GlyphOutline { readonly #glyph: Glyph; readonly #variationLocation: Signal; - readonly #resolver: GlyphResolver; + readonly #glyphForComponent: GlyphForComponent; + readonly #authoredLayerForLocation: AuthoredLayerForLocation; + readonly #resolvedGeometryForLocation: ResolvedGeometryForLocation; readonly #data: ComputedSignal; readonly #bounds: ComputedSignal; readonly #drawPath: ComputedSignal; @@ -240,15 +242,30 @@ export class GlyphOutline { * * @param glyph - Glyph whose contours and components are expanded. * @param variationLocation - Design location that selects layer-backed or geometry-backed outline parts. - * @param resolver - Glyph lookup used to expand component references. + * @param glyphForComponent - Glyph lookup used to expand component references. + * @param authoredLayerForLocation - Exact authored layer lookup for layer-backed outline parts. + * @param resolvedGeometryForLocation - Resolved geometry lookup for interpolated outline parts. */ - constructor(glyph: Glyph, variationLocation: Signal, resolver: GlyphResolver) { + constructor( + glyph: Glyph, + variationLocation: Signal, + glyphForComponent: GlyphForComponent, + authoredLayerForLocation: AuthoredLayerForLocation, + resolvedGeometryForLocation: ResolvedGeometryForLocation, + ) { this.#glyph = glyph; this.#variationLocation = variationLocation; - this.#resolver = resolver; + this.#glyphForComponent = glyphForComponent; + this.#authoredLayerForLocation = authoredLayerForLocation; + this.#resolvedGeometryForLocation = resolvedGeometryForLocation; this.#data = computed(() => - new GlyphOutlineBuilder(this.#variationLocation.value, this.#resolver).build(this.#glyph), + new GlyphOutlineBuilder( + this.#variationLocation.value, + this.#glyphForComponent, + this.#authoredLayerForLocation, + this.#resolvedGeometryForLocation, + ).build(this.#glyph), ); this.#bounds = computed(() => { @@ -308,7 +325,7 @@ export class GlyphOutline { * * @returns A signal that invalidates when the current SVG path data changes. */ - get $svgPath(): Signal { + get svgPathCell(): Signal { return this.#svgPath; } @@ -353,12 +370,21 @@ export class GlyphOutline { */ class GlyphOutlineBuilder { readonly #location: AxisLocation; - readonly #resolver: GlyphResolver; + readonly #glyphForComponent: GlyphForComponent; + readonly #authoredLayerForLocation: AuthoredLayerForLocation; + readonly #resolvedGeometryForLocation: ResolvedGeometryForLocation; readonly #stack = new Set(); - constructor(location: AxisLocation, resolver: GlyphResolver) { + constructor( + location: AxisLocation, + glyphForComponent: GlyphForComponent, + authoredLayerForLocation: AuthoredLayerForLocation, + resolvedGeometryForLocation: ResolvedGeometryForLocation, + ) { this.#location = location; - this.#resolver = resolver; + this.#glyphForComponent = glyphForComponent; + this.#authoredLayerForLocation = authoredLayerForLocation; + this.#resolvedGeometryForLocation = resolvedGeometryForLocation; } build(glyph: Glyph): OutlineData { @@ -370,7 +396,7 @@ class GlyphOutlineBuilder { this.#stack.add(glyph.id); const parts: OutlinePart[] = []; - const source = glyph.layerAt(this.#location); + const source = this.#authoredLayerForLocation(glyph.id, this.#location); if (source) { track(source.structureCell); @@ -390,7 +416,7 @@ class GlyphOutlineBuilder { const componentValues = coordinates.components[index]; if (!componentValues) continue; - const componentGlyph = this.#resolver.glyphForId(component.baseGlyphId); + const componentGlyph = this.#glyphForComponent(component.baseGlyphId); if (!componentGlyph) continue; track(componentValues.matrix); @@ -401,13 +427,13 @@ class GlyphOutlineBuilder { parts.push(...child.parts); } } else { - const geometry = glyph.geometryAt(this.#location); + const geometry = this.#resolvedGeometryForLocation(glyph.id, this.#location); for (const contour of geometry.contours) { parts.push(new GeometryOutlinePart(contour, matrix)); } for (const component of geometry.components) { - const componentGlyph = this.#resolver.glyphForId(component.baseGlyphId); + const componentGlyph = this.#glyphForComponent(component.baseGlyphId); if (!componentGlyph) continue; const child = this.#collect(componentGlyph, Mat.Compose(matrix, component.matrix)); 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 ff6c0af5..421f4eaf 100644 --- a/apps/desktop/src/renderer/src/lib/model/variation.test.ts +++ b/apps/desktop/src/renderer/src/lib/model/variation.test.ts @@ -47,7 +47,10 @@ async function drawSquare(stack: WorkspaceStack, layerId: LayerId, width: number })), }, }, - { kind: "setContourClosed", setContourClosed: { layerId, contourId, closed: true } }, + { + kind: "setContourClosed", + setContourClosed: { layerId, contourId, closed: true }, + }, ]); } @@ -66,7 +69,11 @@ async function variableFont(): Promise<{ const created = await stack.editCoordinator.apply([ { kind: "createGlyph", - createGlyph: { glyphId, name: "A" as GlyphName, unicodes: [65 as Unicode] }, + createGlyph: { + glyphId, + name: "A" as GlyphName, + unicodes: [65 as Unicode], + }, }, { kind: "createGlyphLayer", @@ -116,7 +123,11 @@ async function variableFont(): Promise<{ await stack.editCoordinator.apply([ { kind: "createGlyphLayer", - createGlyphLayer: { layerId: boldLayerId, glyphId, sourceId: boldSourceId }, + createGlyphLayer: { + layerId: boldLayerId, + glyphId, + sourceId: boldSourceId, + }, }, ]); @@ -125,7 +136,10 @@ async function variableFont(): Promise<{ await drawSquare(stack, regularLayerId, 100); await drawSquare(stack, boldLayerId, 200); await stack.editCoordinator.apply([ - { kind: "setXAdvance", setXAdvance: { layerId: regularLayerId, width: 300 } }, + { + kind: "setXAdvance", + setXAdvance: { layerId: regularLayerId, width: 300 }, + }, ]); await stack.editCoordinator.apply([ { kind: "setXAdvance", setXAdvance: { layerId: boldLayerId, width: 500 } }, @@ -135,18 +149,17 @@ async function variableFont(): Promise<{ } async function loadGlyph(stack: WorkspaceStack, glyphId: GlyphId) { - const glyph = await stack.font.loadGlyph(glyphId, { + const loaded = await stack.font.loadGlyph(glyphId, { sourceIds: [stack.font.defaultSource.id], }); - if (!glyph) throw new Error("Expected default glyph layer to load"); - return glyph; + if (!loaded) throw new Error("Expected default glyph layer to load"); } async function loadGlyphLayer(stack: WorkspaceStack, glyphId: GlyphId, source: Source) { await stack.font.loadGlyph(glyphId, { sourceIds: [stack.font.defaultSource.id, source.id], }); - const layer = stack.font.glyphLayerForId(glyphId, source.id); + const layer = stack.font.layer(glyphId, source.id); if (!layer) throw new Error("Expected glyph layer to load"); return layer; } @@ -182,14 +195,15 @@ describe("variable editing across sources", () => { }); it("interpolates geometry and metrics between masters", async () => { - const glyph = await loadGlyph(stack, glyphId); + await loadGlyph(stack, glyphId); const axis = stack.font.getAxes()[0]!; // wght 550 is halfway between the masters at 400 and 700. const mid = withAxisValue(defaultAxisLocation(stack.font.getAxes()), axis, 550); - const instance = glyph.instanceAt(mid); + const instance = stack.font.instance(glyphId, mid); + if (!instance) throw new Error("Expected glyph instance"); - expect(instance.hasLayer).toBe(false); + expect(stack.font.editableLayerAt(glyphId, mid)).toBeNull(); expect(instance.xAdvance).toBeCloseTo(300 + (500 - 300) * 0.5); const xs = instance.geometry.allPoints.map((point) => point.x); @@ -197,15 +211,34 @@ describe("variable editing across sources", () => { }); it("resolves live layer geometry at exact master locations", async () => { - const glyph = await loadGlyph(stack, glyphId); + await loadGlyph(stack, glyphId); await loadGlyphLayer(stack, glyphId, bold); const axis = stack.font.getAxes()[0]!; const atBold = withAxisValue(defaultAxisLocation(stack.font.getAxes()), axis, 700); - const instance = glyph.instanceAt(atBold); + const instance = stack.font.instance(glyphId, atBold); + if (!instance) throw new Error("Expected glyph instance"); + + expect(stack.font.editableLayerAt(glyphId, atBold)).not.toBeNull(); + expect(instance.xAdvance).toBe(500); + }); + + it("refreshes an existing instance when its exact source layer loads", async () => { + await loadGlyph(stack, glyphId); + + const axis = stack.font.getAxes()[0]!; + const atBold = withAxisValue(defaultAxisLocation(stack.font.getAxes()), axis, 700); + const instance = stack.font.instance(glyphId, atBold); + if (!instance) throw new Error("Expected glyph instance"); + + expect(stack.font.editableLayerAt(glyphId, atBold)).toBeNull(); + expect(instance.xAdvance).toBe(0); + expect(instance.render.outline.bounds).toBeNull(); + + await loadGlyphLayer(stack, glyphId, bold); - expect(instance.hasLayer).toBe(true); - expect(instance.hasLayer).toBe(true); + expect(stack.font.editableLayerAt(glyphId, atBold)).not.toBeNull(); expect(instance.xAdvance).toBe(500); + expect(instance.render.outline.bounds?.max.x).toBe(200); }); }); diff --git a/apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md b/apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md index de9815f3..7cf0150d 100644 --- a/apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md +++ b/apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md @@ -10,14 +10,14 @@ Fine-grained reactivity system providing automatic dependency tracking and effic - **Architecture Invariant:** During `batch`, only effects are deferred. Computed values remain available with fresh data inside the batch body. - **Architecture Invariant: CRITICAL:** The module-level `currentComputation` variable is the sole mechanism for dependency tracking. Any code that saves/restores it incorrectly will silently break the entire reactive graph. `untracked` and the internal `#recompute`/`execute` methods carefully save and restore this variable. - **Architecture Invariant: CRITICAL:** Re-entrant notification is guarded by the `isNotifying` flag. Signals written during notification are queued in `pendingNotifications` and flushed after the current notification pass. Without this, subscribers could see inconsistent state. -- **Architecture Invariant:** Classes expose `WritableSignal` fields with a `$` prefix (e.g., `$zoom`). Public getters return the read-only `Signal` type. Use `private` (not `#`) for `$`-prefixed fields to avoid `#$` awkwardness. -- **Architecture Invariant: Convention:** `$foo` public accessors are for **raw state** — writable signals or cheap computeds — safe to subscribe to via `useSignalState`/`useSignalTrigger`. **Derived values** (bounds, paths, sidebearings) are exposed only as plain getters (`.foo`) and pulled on demand. For React live display of a derived value, write a purpose-specific hook (e.g. `useSelectionBounds`) that subscribes to the raw inputs and pulls the getter at render time. Subscribing directly to an expensive ComputedSignal forces it to re-run on every input fire — that's the footgun to avoid. +- **Architecture Invariant:** Signal-bearing fields and accessors use the `*Cell` suffix. The plain noun is the unwrapped snapshot value: `zoomCell` is `Signal`, `zoom` is `number`. +- **Architecture Invariant: Convention:** `fooCell` accessors are for raw state or cheap computeds that are safe to subscribe to via `useSignalState`/`useSignalTrigger`. Expensive derived values (bounds, paths, sidebearings) are exposed as plain getters and pulled on demand. For React live display of a derived value, write a purpose-specific hook (e.g. `useSelectionBounds`) that subscribes to the raw inputs and pulls the getter at render time. - **Architecture Invariant:** `ComputedSignal.dispose()` clears both its `dependencies` and its `#subscribers`. Anything that was reaching the source signal _through_ this computed loses that path. If the consumer needs to keep firing across the lifetime of the source, it must hold a **direct** subscription to the source — not rely on a chain that passes through a disposable intermediate (e.g. an LRU-cached object's computed). ## Codemap ``` -reactive/ +signals/ signal.ts — signal, computed, effect, batch, untracked, isTracking useSignal.ts — useSignalState, useSignalTrigger (React bridges) index.ts — public re-exports @@ -58,14 +58,15 @@ Purpose-specific hooks for derived values live under `hooks/`: ### Add a new reactive property to a manager class -1. Add a `private $fieldName: WritableSignal` field. -2. Initialize it in the constructor: `this.$fieldName = signal(initialValue)`. -3. Add a public getter returning `Signal`: `get fieldName(): Signal { return this.$fieldName; }`. -4. Write mutators that call `this.$fieldName.set(newValue)` or `.update(fn)`. +1. Add a private `fieldNameCell: WritableSignal` field. +2. Initialize it in the constructor: `this.fieldNameCell = signal(initialValue)`. +3. Add a public getter for the snapshot value: `get fieldName(): T { return this.fieldNameCell.peek(); }`. +4. Add a public getter for the signal when callers need reactivity: `get fieldNameCell(): Signal { return this.fieldNameCell; }`. +5. Write mutators that call `this.fieldNameCell.set(newValue)` or `.update(fn)`. ### Subscribe to a signal in a React component -1. Import `useSignalState` from `@/lib/reactive`. +1. Import `useSignalState` from `@/lib/signals`. 2. Call `const value = useSignalState(someSignal)` in the component body. 3. The component re-renders when the signal changes. @@ -88,6 +89,7 @@ Pass `{ equals: () => false }` as the second argument to `signal()`. This is use - **Effects run synchronously.** Setting a signal inside an effect body can trigger other effects immediately (unless inside a `batch`). Careless writes inside effects can cause cascading re-executions. - **Computed propagates eagerly on dirty, evaluates lazily.** When a computed's dependency changes, it marks itself dirty and immediately notifies its own subscribers (which may be other computeds or effects). But it does not recompute its value until `.value` is accessed. - **`peek()` inside a computed breaks reactivity.** If a computed reads a signal via `.peek()`, it will not re-derive when that signal changes. This is intentional but easy to forget. +- **Use `track(cell)` for invalidation-only dependencies.** Inside a computed/effect, prefer `track(fooCell)` when the code needs to subscribe to `fooCell` but does not need the current value. Prefer `const foo = fooCell.value` when the value is actually used. - **Circular computed chains.** There is no cycle detection. A computed that reads itself (directly or indirectly) will hit the `#computing` re-entrancy guard and return the stale value. - **`useSignalState` must not be called conditionally.** It is a React hook and follows the rules of hooks. - **Disposing a computed silently breaks chains that flowed through it.** If `A -> B -> C` (A is a source signal, B is a computed, C subscribes to B), and B is disposed, A no longer notifies C -- but C does not know it has been orphaned. Pattern: when C's lifetime can outlive B's, give C a direct edge to A in addition to the indirect one. The canonical case is `GlyphOutline`: it reads the variation-location signal directly so composite outlines stay reactive through base glyph lookups. @@ -96,7 +98,7 @@ Pass `{ equals: () => false }` as the second argument to `signal()`. This is use ```bash # Run reactive module tests -cd apps/desktop && npx vitest run src/renderer/src/lib/reactive/signal.test.ts +cd apps/desktop && npx vitest run src/renderer/src/lib/signals/signal.test.ts # Run full test suite cd apps/desktop && npm test @@ -105,10 +107,10 @@ cd apps/desktop && npm test ## Related - `Editor` -- primary consumer; holds `WritableSignal` fields for tool state, cursor, preview mode -- `Camera` -- uses `$zoom`, `$panX`, `$panY` as `WritableSignal` fields -- `HoverManager` -- uses `$hoveredPointId`, `$hoveredSegmentId`, etc. +- `Camera` -- uses `zoomCell`, pan cells, and affine transform cells +- `Hover` -- uses `targetCell` for hovered editor state - `Selection` -- uses `WritableSignal` fields for selected point/anchor/segment state -- `NativeBridge` -- `$glyph` signal with `equals: () => false` for identity changes +- `NativeBridge` -- uses a glyph identity cell with `equals: () => false` for identity changes - `useSignalState` -- React bridge hook (in this module) - `useSignalEffect` -- lifecycle-aware effect hook (in `@/hooks/useSignalEffect`) - `CommandHistory` -- imports from reactive for undo/redo state signals diff --git a/apps/desktop/src/renderer/src/lib/text/layout/Positioner.test.ts b/apps/desktop/src/renderer/src/lib/text/layout/Positioner.test.ts index d204fad1..c8a60236 100644 --- a/apps/desktop/src/renderer/src/lib/text/layout/Positioner.test.ts +++ b/apps/desktop/src/renderer/src/lib/text/layout/Positioner.test.ts @@ -42,8 +42,9 @@ describe("Positioner", () => { const positioned = positioner.position(run, font, signal(font.defaultLocation())); const record = font.recordForName("A"); + const location = signal(font.defaultLocation()); const expectedBounds = record - ? font.glyphForId(record.id)?.outline(signal(font.defaultLocation())).bounds + ? font.instance(record.id, location)?.render.outline.bounds : null; expect(positioned.glyphs[0].glyphId).toBe(record?.id); diff --git a/apps/desktop/src/renderer/src/lib/text/layout/Positioner.ts b/apps/desktop/src/renderer/src/lib/text/layout/Positioner.ts index c4be713a..ede95fe1 100644 --- a/apps/desktop/src/renderer/src/lib/text/layout/Positioner.ts +++ b/apps/desktop/src/renderer/src/lib/text/layout/Positioner.ts @@ -22,13 +22,13 @@ export class Positioner { for (const [idx, g] of run.glyphs.entries()) { const record = font.recordForName(g.glyphName); - const glyph = record ? font.glyphForId(record.id) : null; + const instance = record ? font.instance(record.id, designLocation) : null; let glyphName = g.glyphName; let bounds: Bounds | null = null; - if (glyph) { - glyphName = glyph.name; - bounds = glyph.instance(designLocation).render.outline.bounds; + if (record && instance) { + glyphName = record.name; + bounds = instance.render.outline.bounds; } const xAdvance = resolveAdvance(g, font, source); @@ -57,7 +57,7 @@ export class Positioner { /** Resolve a glyph item to its display advance (handles invisibles, fallbacks). */ export function resolveAdvance(item: GlyphTextItem, font: Font, source: Source | null): number { const record = recordForTextItem(item, font); - const raw = source && record ? (font.glyphLayerForId(record.id, source.id)?.xAdvance ?? 0) : 0; + const raw = source && record ? (font.layer(record.id, source.id)?.xAdvance ?? 0) : 0; return displayAdvance(raw, item.glyphName, item.codepoint); } @@ -70,8 +70,8 @@ export function resolveGlyphOffset( if (!source) return { x: 0, y: 0 }; const record = recordForTextItem(item, font); - const glyph = record ? font.glyphLayerForId(record.id, source.id) : null; - if (!glyph) return { x: 0, y: 0 }; + const layer = record ? font.layer(record.id, source.id) : null; + if (!layer) return { x: 0, y: 0 }; const metrics = font.metrics; const targetX = 300; @@ -88,7 +88,7 @@ export function resolveGlyphOffset( } }; - const attachingAnchor = glyph.anchors.find((anchor) => { + const attachingAnchor = layer.anchors.find((anchor) => { const name = anchor.name ?? ""; return name.startsWith("_") && name.length > 1; }); @@ -101,7 +101,7 @@ export function resolveGlyphOffset( }; } - const bounds = glyph.bounds; + const bounds = layer.bounds; if (!bounds) return { x: 0, y: 0 }; const centerX = (bounds.min.x + bounds.max.x) / 2; diff --git a/apps/desktop/src/renderer/src/lib/text/layout/TextLayout.test.ts b/apps/desktop/src/renderer/src/lib/text/layout/TextLayout.test.ts index 2c4c536e..7451930b 100644 --- a/apps/desktop/src/renderer/src/lib/text/layout/TextLayout.test.ts +++ b/apps/desktop/src/renderer/src/lib/text/layout/TextLayout.test.ts @@ -104,5 +104,5 @@ describe("TextLayout", () => { function xAdvance(name: string, font: Font): number { const record = font.recordForName(name); - return record ? (font.glyphLayerForId(record.id, font.defaultSource.id)?.xAdvance ?? 0) : 0; + return record ? (font.layer(record.id, font.defaultSource.id)?.xAdvance ?? 0) : 0; } 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 06909414..25eeb01a 100644 --- a/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts +++ b/apps/desktop/src/renderer/src/lib/text/layout/testUtils.ts @@ -51,7 +51,7 @@ export async function layoutTestFont(): Promise { } const record = stack.font.recordForName("A" as GlyphName); - const a = record ? stack.font.glyphLayerForId(record.id, stack.font.defaultSource.id) : null; + const a = record ? stack.font.layer(record.id, stack.font.defaultSource.id) : null; if (!a) throw new Error("Expected authored glyph layer for A"); const contourId = a.addContour(); diff --git a/apps/desktop/src/renderer/src/lib/tools/pen/PenStroke.ts b/apps/desktop/src/renderer/src/lib/tools/pen/PenStroke.ts index 8b4e4181..776a5985 100644 --- a/apps/desktop/src/renderer/src/lib/tools/pen/PenStroke.ts +++ b/apps/desktop/src/renderer/src/lib/tools/pen/PenStroke.ts @@ -19,10 +19,11 @@ export class PenStroke { } static active(editor: Editor): PenStroke | null { - const instance = editor.glyphInstance; - if (!instance?.layer) return null; + const instance = editor.previewGlyphInstance; + const edit = editor.editingGlyphLayer; + if (!instance || !edit) return null; - return new PenStroke(editor, instance.geometry, instance.layer); + return new PenStroke(editor, instance.geometry, edit); } get activeContour(): Contour | null { diff --git a/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.test.ts b/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.test.ts index 74449cd8..7bfd7ed1 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.test.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.test.ts @@ -36,11 +36,9 @@ function createBoundingBox(rect: Rect2D) { camera: { trackViewportTransform: () => {}, }, - glyphDisplayCell: signal({ - proofMode: false, - handlesVisible: true, - focusedGlyphVisible: true, - }), + proofModeCell: signal(false), + handlesVisibleCell: signal(true), + focusedGlyphVisibleCell: signal(true), fromGlyphLocal: project, screenToUpmDistance: (pixels: number) => pixels, } as unknown as Editor; diff --git a/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.ts b/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.ts index 987aaf95..369c505a 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/BoundingBox.ts @@ -112,8 +112,13 @@ export class SelectBoundingBox extends CanvasItem { const state = this.#select.stateCell.value; if (state.type === "brushing") return null; - const display = this.#editor.glyphDisplayCell.value; - if (!display.handlesVisible || display.proofMode || !display.focusedGlyphVisible) return null; + if ( + !this.#editor.handlesVisibleCell.value || + this.#editor.proofModeCell.value || + !this.#editor.focusedGlyphVisibleCell.value + ) { + return null; + } const selection = this.#editor.selection.stateCell.value; const selectedCount = diff --git a/apps/desktop/src/renderer/src/lib/tools/select/Select.ts b/apps/desktop/src/renderer/src/lib/tools/select/Select.ts index d6c2f94a..af52ffe0 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/Select.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/Select.ts @@ -92,10 +92,9 @@ export class Select extends BaseTool { } override drawScene(canvas: Canvas): void { - const display = this.editor.glyphDisplay; - if (display.proofMode || !display.focusedGlyphVisible) return; + if (this.editor.proofMode || !this.editor.focusedGlyphVisible()) return; - const instance = this.editor.glyphInstance; + const instance = this.editor.previewGlyphInstance; if (!instance) return; this.#segments.draw(canvas, instance.geometry, this.editor.selection, this.editor.hover); diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/BendCurve.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/BendCurve.ts index 979738b5..0088c3eb 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/BendCurve.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/BendCurve.ts @@ -15,8 +15,8 @@ export class BendCurve implements SelectBehavior { ): boolean { if (state.type !== "ready" || !event.metaKey) return false; - const instance = ctx.editor.glyphInstance; - if (!instance?.layer) return false; + const instance = ctx.editor.previewGlyphInstance; + if (!instance || !ctx.editor.editingGlyphLayer) return false; const geometry = instance.geometry; const hit = geometry.hitSegment(event.coords.glyphLocal, ctx.editor.hitRadius); diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Marquee.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Marquee.ts index 74352289..4a3272e8 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Marquee.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Marquee.ts @@ -61,7 +61,7 @@ export class Marquee implements SelectBehavior { } private getPointsInRect(rect: Rect2D, ctx: ToolContext): Set { - const instance = ctx.editor.glyphInstance; + const instance = ctx.editor.previewGlyphInstance; if (!instance) return new Set(); const hitPoints = instance.geometry.allPoints.filter((p) => Rect.containsPoint(rect, p)); diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Resize.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Resize.ts index e2288b1b..02ac8f83 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Resize.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Resize.ts @@ -18,7 +18,7 @@ export class Resize implements SelectBehavior { if (!ctx.editor.selection.hasSelection()) return false; const editor = ctx.editor; - if (!editor.glyphInstance?.layer) return false; + if (!editor.previewGlyphInstance || !editor.editingGlyphLayer) return false; const bbHit = ctx.tool.boundingBox.hit(event.coords); if (!bbHit) return false; diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Rotate.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Rotate.ts index 6dee950b..4f785bab 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Rotate.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Rotate.ts @@ -103,8 +103,8 @@ export class Rotate implements SelectBehavior { editor: Editor, tool: Select, ): SelectState | null { - const instance = editor.glyphInstance; - if (!instance?.layer) return null; + const instance = editor.previewGlyphInstance; + if (!instance || !editor.editingGlyphLayer) return null; const bbHit = tool.boundingBox.hit(event.coords); if (!bbHit) return null; diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SegmentDoubleClick.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SegmentDoubleClick.ts index d0be1534..98dee434 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SegmentDoubleClick.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SegmentDoubleClick.ts @@ -10,8 +10,8 @@ export class SegmentDoubleClick implements SelectBehavior { ): boolean { if (state.type !== "ready") return false; - const instance = ctx.editor.glyphInstance; - if (!instance?.layer) return false; + const instance = ctx.editor.previewGlyphInstance; + if (!instance || !ctx.editor.editingGlyphLayer) return false; const geometry = instance.geometry; const segmentHit = geometry.hitSegment(event.coords.glyphLocal, ctx.editor.hitRadius); diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SelectHover.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SelectHover.ts index 95f99129..b6c7c1d3 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SelectHover.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/SelectHover.ts @@ -21,7 +21,7 @@ export class SelectHover implements SelectBehavior { return false; } - const instance = ctx.editor.glyphInstance; + const instance = ctx.editor.previewGlyphInstance; if (!instance) { ctx.editor.hover.clear(); return false; diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Selection.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Selection.ts index 5b062193..9e8a2f08 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Selection.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Selection.ts @@ -8,7 +8,7 @@ export class Selection implements SelectBehavior { if (state.type !== "ready" && ctx.editor.selection.hasSelection()) return false; const editor = ctx.editor; - const instance = editor.glyphInstance; + const instance = editor.previewGlyphInstance; if (!instance) return false; const geometry = instance.geometry; diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ToggleSmooth.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ToggleSmooth.ts index 0ef1240f..7ba78765 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ToggleSmooth.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ToggleSmooth.ts @@ -11,8 +11,8 @@ export class ToggleSmooth implements SelectBehavior { event: ToolEventOf<"doubleClick">, ): boolean { if (state.type !== "ready" && ctx.editor.selection.hasSelection()) return false; - const instance = ctx.editor.glyphInstance; - if (!instance?.layer) return false; + const instance = ctx.editor.previewGlyphInstance; + if (!instance || !ctx.editor.editingGlyphLayer) return false; const geometry = instance.geometry; const hit = geometry.hitPoint(event.coords.glyphLocal, ctx.editor.hitRadius); diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Translate.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Translate.ts index 9e938ff9..d9a1678c 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Translate.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/Translate.ts @@ -104,8 +104,8 @@ class TranslateTarget { } static fromDragStart(editor: Editor, event: ToolEventOf<"dragStart">): TranslateTarget | null { - const instance = editor.glyphInstance; - if (!instance?.layer) return null; + const instance = editor.previewGlyphInstance; + if (!instance || !editor.editingGlyphLayer) return null; const geometry = instance.geometry; const pos = event.coords.glyphLocal; @@ -272,7 +272,7 @@ class TranslateDrag { constructor(editor: Editor, target: TranslateTarget, pointerStart: Point2D) { this.#target = target; - const instance = editor.glyphInstance; + const instance = editor.previewGlyphInstance; this.#draft = editor.beginGlyphLayerEditDraft({ points: target.pointIds, diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/UpgradeSegment.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/UpgradeSegment.ts index 96f8907c..24952303 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/UpgradeSegment.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/UpgradeSegment.ts @@ -6,8 +6,8 @@ export class UpgradeSegment implements SelectBehavior { onClick(state: SelectState, ctx: ToolContext, event: ToolEventOf<"click">): boolean { if (state.type !== "ready" || !event.altKey) return false; - const instance = ctx.editor.glyphInstance; - if (!instance?.layer) return false; + const instance = ctx.editor.previewGlyphInstance; + if (!instance || !ctx.editor.editingGlyphLayer) return false; const geometry = instance.geometry; const hit = geometry.hitSegment(event.coords.glyphLocal, ctx.editor.hitRadius); diff --git a/apps/desktop/src/renderer/src/lib/tools/text/Text.ts b/apps/desktop/src/renderer/src/lib/tools/text/Text.ts index 0d2bfa9b..afc6266c 100644 --- a/apps/desktop/src/renderer/src/lib/tools/text/Text.ts +++ b/apps/desktop/src/renderer/src/lib/tools/text/Text.ts @@ -18,23 +18,20 @@ export class TextTool extends BaseTool { } override activate(): void { - const owner = this.editor.glyph.peek()?.handle ?? null; + const owner = this.editor.previewGlyphRecordCell.peek(); if (!owner) { this.state = { type: "typing" }; - this.editor.glyphDisplay; return; } const ownerName = owner.name; - const record = this.editor.font.recordForName(ownerName); - if (record) { - this.editor.font.loadGlyph(record.id).catch((error) => { - console.error("failed to load text owner glyph", error); - }); - } + const ownerUnicode = owner.unicodes[0] ?? null; + this.editor.font.loadGlyph(owner.id).catch((error) => { + console.error("failed to load text owner glyph", error); + }); const run = this.editor.textRuns.switchTo(ownerName); - run.seed(glyphTextItem(ownerName, owner.unicode ?? null), this.editor.drawOffset.x); + run.seed(glyphTextItem(ownerName, ownerUnicode), this.editor.drawOffset.x); run.interaction.suspend(); run.setCursorVisible(true); diff --git a/apps/desktop/src/renderer/src/testing/TestEditor.ts b/apps/desktop/src/renderer/src/testing/TestEditor.ts index 7465d075..51f79907 100644 --- a/apps/desktop/src/renderer/src/testing/TestEditor.ts +++ b/apps/desktop/src/renderer/src/testing/TestEditor.ts @@ -12,10 +12,16 @@ */ import { Editor } from "@/lib/editor/Editor"; -import type { Glyph } from "@/lib/model/Glyph"; import type { ToolName } from "@/lib/tools/core"; import { registerBuiltInTools } from "@/lib/tools/tools"; -import { mintGlyphId, mintLayerId, type GlyphId, type GlyphName, type Unicode } from "@shift/types"; +import { + mintGlyphId, + mintLayerId, + type GlyphId, + type GlyphName, + type GlyphRecord, + type Unicode, +} from "@shift/types"; import type { SystemClipboard } from "@/lib/clipboard"; import { createWorkspaceStack, type WorkspaceStack } from "./workspaceStack"; @@ -56,9 +62,7 @@ export class TestEditor extends Editor { async startSession(name = "A", unicode: number | null = 65): Promise { await this.#stack.createWorkspace(); - const glyph = await this.#createAndOpenGlyph(name, unicode); - const record = this.font.recordForName(glyph.handle.name); - if (!record) throw new Error("created glyph did not appear in the font directory"); + const record = await this.#createAndOpenGlyph(name, unicode); this.#placeGlyph(record.id); return this; } @@ -68,7 +72,7 @@ export class TestEditor extends Editor { await this.#createAndOpenGlyph(name, unicode); } - async #createAndOpenGlyph(name: string, unicode: number | null): Promise { + async #createAndOpenGlyph(name: string, unicode: number | null): Promise { const glyphId = mintGlyphId(); const sourceId = this.font.defaultSource.id; const applied = await this.#stack.editCoordinator.apply([ @@ -93,9 +97,11 @@ export class TestEditor extends Editor { const record = applied.glyphs?.find((glyph) => glyph.name === name); if (!record) throw new Error("createGlyph did not echo the new record"); - const glyph = await this.font.loadGlyph(record.id, { sourceIds: [sourceId] }); - if (!glyph) throw new Error("glyphForId returned null for a loaded glyph"); - return glyph; + const loaded = await this.font.loadGlyph(record.id, { + sourceIds: [sourceId], + }); + if (!loaded) throw new Error("created glyph did not load"); + return record; } #placeGlyph(glyphId: GlyphId): void { diff --git a/docs/architecture/index.md b/docs/architecture/index.md index 0d4d6f39..efba0d05 100644 --- a/docs/architecture/index.md +++ b/docs/architecture/index.md @@ -39,7 +39,7 @@ Central routing table for Shift's distributed documentation. Before creating new | `apps/desktop/src/renderer/src/lib/graphics/**` | [`apps/desktop/src/renderer/src/lib/graphics/docs/DOCS.md`](../../apps/desktop/src/renderer/src/lib/graphics/docs/DOCS.md) | Rendering abstraction with Canvas 2D backend and path caching | | `apps/desktop/src/renderer/src/lib/transform/**` | [`apps/desktop/src/renderer/src/lib/transform/docs/DOCS.md`](../../apps/desktop/src/renderer/src/lib/transform/docs/DOCS.md) | Geometry transforms: rotate, scale, reflect selected points | | `apps/desktop/src/renderer/src/lib/commands/**` | [`apps/desktop/src/renderer/src/lib/commands/docs/DOCS.md`](../../apps/desktop/src/renderer/src/lib/commands/docs/DOCS.md) | Command pattern with undo/redo for all editing operations | -| `apps/desktop/src/renderer/src/lib/reactive/**` | [`apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md`](../../apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md) | Fine-grained reactivity: dependency tracking and efficient updates | +| `apps/desktop/src/renderer/src/lib/signals/**` | [`apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md`](../../apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md) | Fine-grained reactivity: dependency tracking and efficient updates | ### Packages diff --git a/scripts/context-drift-check.py b/scripts/context-drift-check.py index 7cf7dcff..9f6a7602 100755 --- a/scripts/context-drift-check.py +++ b/scripts/context-drift-check.py @@ -16,12 +16,10 @@ import re import subprocess import sys +from dataclasses import dataclass from pathlib import Path -from markdown_it import MarkdownIt - REPO_ROOT = Path(__file__).resolve().parent.parent -MD_PARSER = MarkdownIt("commonmark") # --------------------------------------------------------------------------- # Configuration @@ -43,7 +41,7 @@ "apps/desktop/src/renderer/src/lib/graphics/docs/DOCS.md", "apps/desktop/src/renderer/src/lib/transform/docs/DOCS.md", "apps/desktop/src/renderer/src/lib/commands/docs/DOCS.md", - "apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md", + "apps/desktop/src/renderer/src/lib/signals/docs/DOCS.md", "packages/types/docs/DOCS.md", "packages/geo/docs/DOCS.md", "packages/glyph-state/docs/DOCS.md", @@ -96,7 +94,7 @@ def __init__(self, category: str, severity: str, doc: str, message: str): def __str__(self) -> str: prefix = "WARN" if self.severity == "warning" else self.category.upper().replace("_", " ") - return f"{prefix}: {self.message}" + return f"{prefix}: {self.doc}: {self.message}" def to_dict(self) -> dict: return { @@ -107,13 +105,122 @@ def to_dict(self) -> dict: } +@dataclass +class MarkdownToken: + """Small token shape for the markdown constructs this checker needs.""" + + type: str + tag: str = "" + info: str = "" + content: str = "" + children: list["MarkdownToken"] | None = None + attrs: dict[str, str] | None = None + + # --------------------------------------------------------------------------- # AST helpers # --------------------------------------------------------------------------- def parse_doc(path: Path) -> list: - """Parse a markdown file and return its token list.""" - return MD_PARSER.parse(path.read_text()) + """Parse a markdown file and return the token list used by this checker.""" + tokens: list[MarkdownToken] = [] + in_fence = False + fence_marker = "" + fence_info = "" + fence_content: list[str] = [] + + for line in path.read_text().splitlines(): + stripped = line.lstrip() + fence_match = re.match(r"^(```+|~~~+)\s*(.*)$", stripped) + + if in_fence: + if fence_match and fence_match.group(1).startswith(fence_marker[0]): + tokens.append(MarkdownToken( + type="fence", + info=fence_info, + content="\n".join(fence_content), + )) + in_fence = False + fence_marker = "" + fence_info = "" + fence_content = [] + else: + fence_content.append(line) + continue + + if fence_match: + in_fence = True + fence_marker = fence_match.group(1) + fence_info = fence_match.group(2).strip() + fence_content = [] + continue + + heading_match = re.match(r"^(#{1,6})\s+(.+?)\s*#*\s*$", stripped) + if heading_match: + level = len(heading_match.group(1)) + content = heading_match.group(2).strip() + tokens.append(MarkdownToken(type="heading_open", tag=f"h{level}")) + tokens.append(MarkdownToken( + type="inline", + content=content, + children=_parse_inline(content), + )) + continue + + if stripped: + tokens.append(MarkdownToken( + type="inline", + content=stripped, + children=_parse_inline(stripped), + )) + + if in_fence: + tokens.append(MarkdownToken( + type="fence", + info=fence_info, + content="\n".join(fence_content), + )) + + return tokens + + +def _parse_inline(text: str) -> list[MarkdownToken]: + """Parse links and inline code spans from a single markdown line.""" + children: list[MarkdownToken] = [] + i = 0 + + while i < len(text): + if text[i] == "`": + end = text.find("`", i + 1) + if end == -1: + children.append(MarkdownToken(type="text", content=text[i:])) + break + children.append(MarkdownToken(type="code_inline", content=text[i + 1:end])) + i = end + 1 + continue + + if text[i] == "[": + label_end = text.find("]", i + 1) + if label_end != -1 and label_end + 1 < len(text) and text[label_end + 1] == "(": + href_end = text.find(")", label_end + 2) + if href_end != -1: + label = text[i + 1:label_end] + href = text[label_end + 2:href_end] + children.append(MarkdownToken(type="link_open", attrs={"href": href})) + children.append(MarkdownToken(type="text", content=label)) + children.append(MarkdownToken(type="link_close")) + i = href_end + 1 + continue + + next_code = text.find("`", i) + next_link = text.find("[", i) + stops = [pos for pos in (next_code, next_link) if pos != -1] + end = min(stops) if stops else len(text) + children.append(MarkdownToken(type="text", content=text[i:end])) + i = end + + return children + def extract_headings(tokens: list) -> list[tuple[int, str]]: @@ -403,7 +510,16 @@ def _last_commit_date(path: str) -> str | None: def _symbol_exists(symbol: str) -> bool: """Check if a symbol exists anywhere in the codebase source files.""" result = subprocess.run( - ["git", "grep", "-lq", symbol, "--", "*.ts", "*.rs", "*.tsx"], + [ + "git", + "grep", + "-lq", + symbol, + "--", + ":(glob)**/*.ts", + ":(glob)**/*.tsx", + ":(glob)**/*.rs", + ], capture_output=True, text=True, cwd=REPO_ROOT, ) return result.returncode == 0 @@ -500,10 +616,12 @@ def main(): # Report if json_output: + errors = [i for i in all_issues if i.severity == "error"] + warnings = [i for i in all_issues if i.severity == "warning"] output = { "total": len(all_issues), - "errors": len([i for i in all_issues if i.severity == "error"]), - "warnings": len([i for i in all_issues if i.severity == "warning"]), + "errors": len(errors), + "warnings": len(warnings), "by_category": {}, "issues": [i.to_dict() for i in all_issues], } @@ -511,6 +629,7 @@ def main(): output["by_category"].setdefault(issue.category, 0) output["by_category"][issue.category] += 1 print(json_mod.dumps(output, indent=2)) + sys.exit(1 if errors or warnings else 0) else: print() errors = [i for i in all_issues if i.severity == "error"]