diff --git a/apps/desktop/.oxlintrc.json b/apps/desktop/.oxlintrc.json index df3ee649..b7d00dce 100644 --- a/apps/desktop/.oxlintrc.json +++ b/apps/desktop/.oxlintrc.json @@ -34,6 +34,9 @@ } ], "shift/no-raw-contour-access": "error", + "shift/prefer-instance-method-on-glyph": "error", + "shift/no-raw-segment-parse": "error", + "shift/no-get-signal-value-method": "error", "shift/no-unbranded-ids": "error", "shift/no-snapshot-in-domain": "error", "shift/no-raw-point-type-check": "error", diff --git a/apps/desktop/src/renderer/src/components/EditorView.tsx b/apps/desktop/src/renderer/src/components/EditorView.tsx index 332a2a1d..6532e90a 100644 --- a/apps/desktop/src/renderer/src/components/EditorView.tsx +++ b/apps/desktop/src/renderer/src/components/EditorView.tsx @@ -20,11 +20,11 @@ export const EditorView: FC = ({ glyphId }) => { const debug = useDebugSafe(); const containerRef = useRef(null); - const [cursorStyle, setCursorStyle] = useState(() => editor.getCursor()); + const [cursorStyle, setCursorStyle] = useState(() => editor.cursor); useEffect(() => { const fx = effect(() => { - setCursorStyle(editor.getCursor()); + setCursorStyle(editor.cursor); }); return () => fx.dispose(); }, [editor]); diff --git a/apps/desktop/src/renderer/src/components/GlyphSidebar.tsx b/apps/desktop/src/renderer/src/components/GlyphSidebar.tsx index 6dc5faf2..9d097390 100644 --- a/apps/desktop/src/renderer/src/components/GlyphSidebar.tsx +++ b/apps/desktop/src/renderer/src/components/GlyphSidebar.tsx @@ -13,7 +13,7 @@ import { BooleanOps } from "./BooleanOps"; export const GlyphSidebar = () => { const editor = getEditor(); const { familyName } = editor.font.metadata; - const zoom = useSignalState(editor.zoom); + const zoom = useSignalState(editor.$zoom); const zoomPercent = Math.round(zoom * 100); const [hasPointSelection, setHasPointSelection] = useState(false); diff --git a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx index 2f114740..6a5a8b7a 100644 --- a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx +++ b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx @@ -3,7 +3,6 @@ import { useSignalText } from "@/hooks/useSignalText"; import { getEditor } from "@/store/store"; import { Separator } from "@shift/ui"; import { effect } from "@/lib/reactive"; -import { Glyphs } from "@shift/font"; function formatCoords(x: number, y: number): string { return `(${Math.round(x)}, ${Math.round(y)})`; @@ -33,7 +32,7 @@ export function DebugPanel() { const glyph = editor.glyph.value; if (!glyph) return "0"; - return `${Glyphs.getAllPoints(glyph).length}`; + return `${glyph.allPoints.length}`; }); const glyphMemoryRef = useSignalText(() => { diff --git a/apps/desktop/src/renderer/src/components/sidebar-right/GlyphSection.tsx b/apps/desktop/src/renderer/src/components/sidebar-right/GlyphSection.tsx index da52fdf8..7a812870 100644 --- a/apps/desktop/src/renderer/src/components/sidebar-right/GlyphSection.tsx +++ b/apps/desktop/src/renderer/src/components/sidebar-right/GlyphSection.tsx @@ -1,26 +1,25 @@ import { formatCodepointAsUPlus } from "@/lib/utils/unicode"; import { SidebarSection } from "./SidebarSection"; import { EditableSidebarInput } from "./EditableSidebarInput"; -import Glyph from "@/assets/sidebar-right/placeholder-glyph.svg"; +import PlaceholderGlyph from "@/assets/sidebar-right/placeholder-glyph.svg"; import { getEditor } from "@/store/store"; import { useSignalState } from "@/lib/reactive"; import { getGlyphInfo } from "@/store/glyphInfo"; -import { deriveGlyphSidebearings, roundSidebearing } from "@/lib/editor/sidebearings"; +import { useGlyphSidebearings } from "@/hooks/useGlyphSidebearings"; +import { useGlyphXAdvance } from "@/hooks/useGlyphXAdvance"; export const GlyphSection = () => { const editor = getEditor(); const glyph = useSignalState(editor.glyph); + const sidebearings = useGlyphSidebearings(); + const xAdvance = useGlyphXAdvance(); const glyphInfo = getGlyphInfo(); if (!glyph) return null; const unicode = formatCodepointAsUPlus(glyph.unicode); - const sidebearings = deriveGlyphSidebearings(glyph); - const xAdvance = glyph.xAdvance; - - const lsb = roundSidebearing(sidebearings.lsb); - const rsb = roundSidebearing(sidebearings.rsb); - + const lsb = sidebearings.lsb === null ? null : Math.round(sidebearings.lsb); + const rsb = sidebearings.rsb === null ? null : Math.round(sidebearings.rsb); const sidebearingsEnabled = lsb !== null && rsb !== null; return ( @@ -38,7 +37,7 @@ export const GlyphSection = () => { onValueChange={(next) => editor.setLeftSidebearing(next)} />
- +
{ className="text-center" value={xAdvance} onValueChange={(width) => editor.setXAdvance(width)} - disabled={!glyph} /> -
{glyphInfo.getGlyphName(glyph?.unicode ?? 0)}
+
{glyphInfo.getGlyphName(glyph.unicode)}
); diff --git a/apps/desktop/src/renderer/src/components/sidebar-right/ScaleSection.tsx b/apps/desktop/src/renderer/src/components/sidebar-right/ScaleSection.tsx index 66a6309c..70bf73d3 100644 --- a/apps/desktop/src/renderer/src/components/sidebar-right/ScaleSection.tsx +++ b/apps/desktop/src/renderer/src/components/sidebar-right/ScaleSection.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef } from "react"; +import { useCallback, useEffect, useRef } from "react"; import { SidebarSection } from "./SidebarSection"; import { TransformGrid } from "./TransformGrid"; import { EditableSidebarInput, type EditableSidebarInputHandle } from "./EditableSidebarInput"; @@ -7,17 +7,12 @@ import { getEditor } from "@/store/store"; import { anchorToPoint } from "@/lib/transform/anchor"; import { Bounds } from "@shift/geo"; import ScaleIcon from "@/assets/sidebar-right/scale.svg"; -import { useSignalState } from "@/lib/reactive"; +import { useSelectionBounds } from "@/hooks/useSelectionBounds"; export const ScaleSection = () => { const editor = getEditor(); const { anchor, setAnchor } = useTransformOrigin(); - const glyph = useSignalState(editor.glyph); - const selectedPointIds = useSignalState(editor.selection.$pointIds); - const selectionBounds = useMemo( - () => editor.getSelectionBounds(), - [editor, glyph, selectedPointIds], - ); + const selectionBounds = useSelectionBounds(); const widthRef = useRef(null); const heightRef = useRef(null); diff --git a/apps/desktop/src/renderer/src/components/sidebar-right/TransformSection.tsx b/apps/desktop/src/renderer/src/components/sidebar-right/TransformSection.tsx index 9d0375c3..01a2ea9e 100644 --- a/apps/desktop/src/renderer/src/components/sidebar-right/TransformSection.tsx +++ b/apps/desktop/src/renderer/src/components/sidebar-right/TransformSection.tsx @@ -6,6 +6,7 @@ import { useTransformOrigin } from "@/context/TransformOriginContext"; import { getEditor } from "@/store/store"; import { anchorToPoint } from "@/lib/transform/anchor"; import { useSignalState } from "@/lib/reactive"; +import { useSelectionBounds } from "@/hooks/useSelectionBounds"; import RotateIcon from "@/assets/sidebar-right/rotate.svg"; import RotateCwIcon from "@/assets/sidebar-right/rotate-cw.svg"; @@ -92,11 +93,7 @@ export const TransformSection = () => { const editor = getEditor(); const { anchor } = useTransformOrigin(); const selectedPointIds = useSignalState(editor.selection.$pointIds); - const glyph = useSignalState(editor.glyph); - const selectionBounds = useMemo( - () => editor.getSelectionBounds(), - [editor, glyph, selectedPointIds], - ); + const selectionBounds = useSelectionBounds(); const [rotation, setRotation] = useState(0); const xRef = useRef(null); diff --git a/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts b/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts new file mode 100644 index 00000000..4b6c8175 --- /dev/null +++ b/apps/desktop/src/renderer/src/hooks/useGlyphSidebearings.ts @@ -0,0 +1,22 @@ +import type { GlyphSidebearings } from "@/lib/model/Glyph"; +import { getEditor } from "@/store/store"; +import { useSignalState, useSignalTrigger } from "@/lib/reactive"; + +const EMPTY_SIDEBEARINGS: GlyphSidebearings = { lsb: null, rsb: null }; + +/** + * Current glyph sidebearings (LSB/RSB), live-updating. + * + * Subscribes to glyph identity, contour patches, and xAdvance; pulls the + * lazy `glyph.sidebearings` getter at render time. Keeps the sidebearings + * computation out of the drag hot path. + * + * @returns `{ lsb, rsb }` — both `null` when the glyph is unavailable. + */ +export function useGlyphSidebearings(): GlyphSidebearings { + const editor = getEditor(); + const glyph = useSignalState(editor.glyph); + useSignalTrigger(glyph?.$contours); + useSignalTrigger(glyph?.$xAdvance); + return glyph?.sidebearings ?? EMPTY_SIDEBEARINGS; +} diff --git a/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts b/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts new file mode 100644 index 00000000..a9d57332 --- /dev/null +++ b/apps/desktop/src/renderer/src/hooks/useGlyphXAdvance.ts @@ -0,0 +1,12 @@ +import { getEditor } from "@/store/store"; +import { useSignalState, useSignalTrigger } from "@/lib/reactive"; + +/** + * Current glyph xAdvance, live-updating. Returns `0` when no glyph is loaded. + */ +export function useGlyphXAdvance(): number { + const editor = getEditor(); + const glyph = useSignalState(editor.glyph); + useSignalTrigger(glyph?.$xAdvance); + return glyph?.xAdvance ?? 0; +} diff --git a/apps/desktop/src/renderer/src/hooks/useSelectionBounds.ts b/apps/desktop/src/renderer/src/hooks/useSelectionBounds.ts new file mode 100644 index 00000000..85ea88ff --- /dev/null +++ b/apps/desktop/src/renderer/src/hooks/useSelectionBounds.ts @@ -0,0 +1,24 @@ +import type { Bounds } from "@shift/geo"; +import { getEditor } from "@/store/store"; +import { useSignalState, useSignalTrigger } from "@/lib/reactive"; + +/** + * Current selection bounds (axis-aligned, point-based), live-updating. + * + * Subscribes to the raw inputs that affect bounds (glyph identity, glyph + * contour patches, and selected point ids), then pulls the lazy + * `selection.bounds` getter at render time. This keeps the bounds + * computation out of the reactive hot path during drag — the compute only + * runs when React actually renders, which happens at most once per + * animation frame. + * + * @returns The current selection bounds, or `null` when the glyph is + * unavailable or nothing is selected. + */ +export function useSelectionBounds(): Bounds | null { + const editor = getEditor(); + const glyph = useSignalState(editor.glyph); + useSignalTrigger(glyph?.$contours); + useSignalTrigger(editor.selection.$pointIds); + return editor.selection.bounds; +} diff --git a/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.test.ts b/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.test.ts index 6c77a3fa..ceb0b550 100644 --- a/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.test.ts +++ b/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.test.ts @@ -3,7 +3,8 @@ import { CloseContourCommand, NudgePointsCommand, SplitSegmentCommand } from "./ import { createBridge, getAllPoints, getPointCount } from "@/testing"; import type { NativeBridge } from "@/bridge"; import type { CommandContext } from "../core"; -import type { LineSegment, QuadSegment, CubicSegment } from "@/types/segments"; +import { Segment } from "@/lib/model/Segment"; +import type { QuadSegment, CubicSegment } from "@/types/segments"; import type { PointId } from "@shift/types"; let bridge: NativeBridge; @@ -93,18 +94,18 @@ describe("NudgePointsCommand", () => { }); describe("SplitSegmentCommand", () => { - function makeLineSegment(p1Id: PointId, p2Id: PointId): LineSegment { + function makeLineSegment(p1Id: PointId, p2Id: PointId): Segment { const points = getAllPoints(bridge.getEditingSnapshot()); const p1 = points.find((p) => p.id === p1Id)!; const p2 = points.find((p) => p.id === p2Id)!; - return { + return new Segment({ type: "line", points: { anchor1: { id: p1.id, x: p1.x, y: p1.y, pointType: "onCurve", smooth: false }, anchor2: { id: p2.id, x: p2.x, y: p2.y, pointType: "onCurve", smooth: false }, }, - }; + }); } describe("line segment", () => { @@ -173,7 +174,7 @@ describe("SplitSegmentCommand", () => { anchor2: { id: p2, x: 100, y: 0, pointType: "onCurve", smooth: false }, }, }; - const cmd = new SplitSegmentCommand(segment, 0.5); + const cmd = new SplitSegmentCommand(new Segment(segment), 0.5); cmd.execute(ctx()); @@ -201,7 +202,7 @@ describe("SplitSegmentCommand", () => { anchor2: { id: p2, x: 100, y: 0, pointType: "onCurve", smooth: false }, }, }; - const cmd = new SplitSegmentCommand(segment, 0.5); + const cmd = new SplitSegmentCommand(new Segment(segment), 0.5); cmd.execute(ctx()); cmd.undo(ctx()); @@ -233,7 +234,7 @@ describe("SplitSegmentCommand", () => { anchor2: { id: p2, x: 100, y: 0, pointType: "onCurve", smooth: false }, }, }; - const cmd = new SplitSegmentCommand(segment, 0.5); + const cmd = new SplitSegmentCommand(new Segment(segment), 0.5); const result = cmd.execute(ctx()); @@ -264,7 +265,7 @@ describe("SplitSegmentCommand", () => { anchor2: { id: p2, x: 100, y: 0, pointType: "onCurve", smooth: false }, }, }; - const cmd = new SplitSegmentCommand(segment, 0.5); + const cmd = new SplitSegmentCommand(new Segment(segment), 0.5); cmd.execute(ctx()); cmd.undo(ctx()); diff --git a/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.ts b/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.ts index 901c4bc7..2f40940e 100644 --- a/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.ts +++ b/apps/desktop/src/renderer/src/lib/commands/primitives/BezierCommands.ts @@ -1,8 +1,8 @@ import type { PointId, ContourId, Point2D } from "@shift/types"; import { BaseCommand, type CommandContext } from "../core/Command"; -import { Curve, type CubicCurve, type QuadraticCurve } from "@shift/geo"; -import type { Segment, QuadSegment, CubicSegment, LineSegment } from "@/types/segments"; -import { Segments as SegmentOps } from "@/lib/geo/Segments"; +import { type CubicCurve, type QuadraticCurve } from "@shift/geo"; +import type { LineSegment } from "@/types/segments"; +import type { Segment } from "@/lib/model/Segment"; /** * Closes the active contour, connecting the last point back to the first. @@ -160,10 +160,9 @@ export class SplitSegmentCommand extends BaseCommand { } #splitLine(ctx: CommandContext): PointId { - const curve = SegmentOps.toCurve(this.#segment); - const splitPoint = Curve.pointAt(curve, this.#t); + const splitPoint = this.#segment.pointAt(this.#t); - const anchor2Id = this.#segment.points.anchor2.id; + const anchor2Id = this.#segment.anchor2.id; this.#splitPointId = ctx.glyph.insertPointBefore(anchor2Id, { x: splitPoint.x, @@ -177,20 +176,19 @@ export class SplitSegmentCommand extends BaseCommand { } #splitQuadratic(ctx: CommandContext): PointId { - const segment = this.#segment as QuadSegment; - const curve = SegmentOps.toCurve(segment) as QuadraticCurve; - const [curveA, curveB] = Curve.splitAt(curve, this.#t) as [QuadraticCurve, QuadraticCurve]; + const data = this.#segment.asQuad()!; + const [curveA, curveB] = this.#segment.splitAt(this.#t) as [QuadraticCurve, QuadraticCurve]; const cA = curveA.c; const mid = curveA.p1; const cB = curveB.c; - const controlId = segment.points.control.id; - const anchor2Id = segment.points.anchor2.id; + const controlId = data.points.control.id; + const anchor2Id = data.points.anchor2.id; this.#originalPositions.set(controlId, { - x: segment.points.control.x, - y: segment.points.control.y, + x: data.points.control.x, + y: data.points.control.y, }); this.#splitPointId = ctx.glyph.insertPointBefore(anchor2Id, { @@ -215,9 +213,8 @@ export class SplitSegmentCommand extends BaseCommand { } #splitCubic(ctx: CommandContext): PointId { - const segment = this.#segment as CubicSegment; - const curve = SegmentOps.toCurve(segment) as CubicCurve; - const [curveA, curveB] = Curve.splitAt(curve, this.#t) as [CubicCurve, CubicCurve]; + const data = this.#segment.asCubic()!; + const [curveA, curveB] = this.#segment.splitAt(this.#t) as [CubicCurve, CubicCurve]; const c0A = curveA.c0; const c1A = curveA.c1; @@ -225,16 +222,16 @@ export class SplitSegmentCommand extends BaseCommand { const c0B = curveB.c0; const c1B = curveB.c1; - const control1Id = segment.points.control1.id; - const control2Id = segment.points.control2.id; + const control1Id = data.points.control1.id; + const control2Id = data.points.control2.id; this.#originalPositions.set(control1Id, { - x: segment.points.control1.x, - y: segment.points.control1.y, + x: data.points.control1.x, + y: data.points.control1.y, }); this.#originalPositions.set(control2Id, { - x: segment.points.control2.x, - y: segment.points.control2.y, + x: data.points.control2.x, + y: data.points.control2.y, }); const c1AId = ctx.glyph.insertPointBefore(control2Id, { diff --git a/apps/desktop/src/renderer/src/lib/editor/Editor.ts b/apps/desktop/src/renderer/src/lib/editor/Editor.ts index 9ab91c97..aca709e5 100644 --- a/apps/desktop/src/renderer/src/lib/editor/Editor.ts +++ b/apps/desktop/src/renderer/src/lib/editor/Editor.ts @@ -10,14 +10,14 @@ import { ToolManager } from "../tools/core/ToolManager"; import { SnapshotCommand } from "../commands/primitives/SnapshotCommand"; import { SetNodePositionsCommand } from "../commands/primitives/SetNodePositionsCommand"; import type { NodePositionUpdateList } from "@/types/positionUpdate"; -import { Segment, type SegmentHitResult } from "../geo/Segment"; +import { Segment, type SegmentHitResult } from "@/lib/model/Segment"; import { Bounds, Vec2 } from "@shift/geo"; -import { Contours, Glyphs } from "@shift/font"; +import { Contours } from "@shift/font"; import type { BoundingBoxHitResult } from "@/types/boundingBox"; import type { Coordinates } from "@/types/coordinates"; import { ViewportManager } from "./managers"; -import { isLikelyNonSpacingGlyphRef } from "@/lib/utils/unicode"; +import { displayAdvance, isNonSpacingGlyph } from "@/lib/utils/unicode"; import { NativeBridge } from "@/bridge"; import { CommandHistory, @@ -37,7 +37,6 @@ import { MoveSelectionToCommand, AlignPointsCommand, DistributePointsCommand, - getSegmentAwareBounds, type ReflectAxis, type AlignmentType, type DistributeType, @@ -105,11 +104,10 @@ const defaultAppSettings: AppSettings = { import type { CompositeGlyph } from "@shift/types"; import type { ToolDescriptor, ToolShortcutEntry } from "@/types/tools"; import type { ToolStateScope } from "@/types/editor"; -import { deriveGlyphSidebearings, roundSidebearing } from "./sidebearings"; import { EventEmitter } from "./lifecycle"; import { StateRegistry, type ShiftState, type ShiftStateOptions } from "@/lib/state/ShiftState"; -import type { Segment as GlyphSegment, LineSegment } from "@/types/segments"; +import type { LineSegment } from "@/types/segments"; import type { GlyphDraft } from "@/types/draft"; /** @@ -134,9 +132,9 @@ import type { GlyphDraft } from "@/types/draft"; * @knipclassignore */ export class Editor { - private $previewMode: WritableSignal; - private $handlesVisible: WritableSignal; - private $gpuHandlesEnabled: WritableSignal; + #previewMode: WritableSignal; + #handlesVisible: WritableSignal; + #gpuHandlesEnabled: WritableSignal; readonly selection: Selection; readonly font: Font; @@ -166,7 +164,7 @@ export class Editor { #commandHistory: CommandHistory; #bridge: NativeBridge; #$glyph: ComputedSignal; - #$segmentIndex: ComputedSignal>; + #$segmentIndex: ComputedSignal>; #staticEffect: Effect; #overlayEffect: Effect; @@ -183,7 +181,7 @@ export class Editor { #marqueePreviewPointIds: WritableSignal | null>; #drawOffset: WritableSignal; - private $cursor: WritableSignal; + #cursor: WritableSignal; #currentModifiers: WritableSignal; #isHoveringNode: ComputedSignal; #settings: ShiftState; @@ -207,18 +205,18 @@ export class Editor { this.#$segmentIndex = computed(() => { const glyph = this.#$glyph.value; if (!glyph) return new Map(); - const segmentsById = new Map(); - for (const { segment } of Segment.iterateGlyph(glyph.contours)) { - segmentsById.set(Segment.id(segment), segment); + const segmentsById = new Map(); + for (const { segment } of glyph.segments()) { + segmentsById.set(segment.id, segment); } return segmentsById; }); this.#commandHistory = new CommandHistory(this.#$glyph); - this.$previewMode = signal(false); - this.$cursor = signal("default"); - this.$handlesVisible = signal(true); - this.$gpuHandlesEnabled = signal(true); + this.#previewMode = signal(false); + this.#cursor = signal("default"); + this.#handlesVisible = signal(true); + this.#gpuHandlesEnabled = signal(true); this.#stateRegistry = new StateRegistry(); this.#currentModifiers = signal({ shiftKey: false, @@ -323,14 +321,14 @@ export class Editor { this.selection.anchorIds; this.selection.segmentIds; this.selection.mode; - this.$previewMode.value; - this.$handlesVisible.value; + this.#previewMode.value; + this.#handlesVisible.value; this.#hover.hoveredPointId.value; this.#hover.hoveredAnchorId.value; this.#hover.hoveredSegmentId.value; this.#hover.hoveredBoundingBoxHandle.value; this.#debugOverlays.value; - this.$gpuHandlesEnabled.value; + this.#gpuHandlesEnabled.value; this.#textRunController.state.value; this.#renderer.requestSceneRedraw(); this.#renderer.requestBackgroundRedraw(); @@ -348,8 +346,8 @@ export class Editor { this.#hover.hoveredAnchorId.value; this.#hover.hoveredSegmentId.value; this.#hover.hoveredBoundingBoxHandle.value; - this.$previewMode.value; - this.$handlesVisible.value; + this.#previewMode.value; + this.#handlesVisible.value; this.#snapIndicator.value; this.$activeToolState.value; this.#renderer.requestOverlayRedraw(); @@ -410,6 +408,7 @@ export class Editor { return this.$activeTool; } + // oxlint-disable-next-line shift/no-get-signal-value-method -- retained for upcoming tool refactor public getActiveTool(): ToolName { return this.$activeTool.value; } @@ -422,6 +421,7 @@ export class Editor { return this.$activeToolState; } + // oxlint-disable-next-line shift/no-get-signal-value-method -- retained for upcoming tool refactor public getActiveToolState(): ActiveToolState { return this.$activeToolState.value; } @@ -455,10 +455,11 @@ export class Editor { public renderToolBackground(canvas: Canvas): void { const glyph = this.glyph.peek(); - const previewMode = this.isPreviewMode(); + const previewMode = this.previewMode; if (glyph && this.shouldRenderGlyph() && !previewMode) { - const advance = this.getVisualGlyphAdvance(glyph); + const unicode = Number.isFinite(glyph.unicode) ? glyph.unicode : null; + const advance = displayAdvance(glyph.xAdvance, glyph.name, unicode); this.#guides.draw(canvas, this.font.getMetrics(), advance); } @@ -472,8 +473,8 @@ export class Editor { public renderToolScene(canvas: Canvas): void { const glyph = this.glyph.peek(); - const previewMode = this.isPreviewMode(); - const handlesVisible = this.isHandlesVisible(); + const previewMode = this.previewMode; + const handlesVisible = this.handlesVisible; if (glyph && this.shouldRenderGlyph()) { glyph.drawOutline(canvas); @@ -481,16 +482,16 @@ export class Editor { } if (!previewMode && glyph && this.shouldRenderGlyph()) { - const hoveredSegmentId = this.getHoveredSegmentId(); + const hoveredSegmentId = this.hoveredSegmentId; const hoveredSegment = hoveredSegmentId ? this.getSegmentById(hoveredSegmentId) : null; - const selectedSegments: GlyphSegment[] = []; + const selectedSegments: Segment[] = []; for (const segId of this.selection.segmentIds) { const seg = this.getSegmentById(segId); if (seg) selectedSegments.push(seg); } this.#segments.draw(canvas, hoveredSegment ?? null, selectedSegments); - const debugOverlays = this.getDebugOverlays(); + const debugOverlays = this.debugOverlays; this.#debugOverlaysIndicator.draw( canvas, glyph, @@ -504,7 +505,7 @@ export class Editor { if (!previewMode && handlesVisible && glyph && this.shouldRenderGlyph()) { const viewport = this.getViewportTransform(); - const drawOffset = this.getDrawOffset(); + const drawOffset = this.drawOffset; const sceneBounds = getVisibleSceneBounds(viewport, 64); this.#controlLines.draw(canvas, glyph, (from, to) => { @@ -525,7 +526,7 @@ export class Editor { { getHandleState: (id) => this.getHandleState(id) }, viewport, drawOffset, - this.isGpuHandlesEnabled(), + this.gpuHandlesEnabled, ); if (!renderedOnGpu) { this.#handles.drawCpu(canvas, glyph, { @@ -542,14 +543,14 @@ export class Editor { public renderOverlay(canvas: Canvas): void { // Screen-space pass: bounding box handles (skip during drag — handles aren't interactive) if ( - !this.isPreviewMode() && + !this.previewMode && !this.#isDragging() && - this.isHandlesVisible() && + this.handlesVisible && this.shouldRenderGlyph() ) { const rect = this.getSelectionBoundingRect(); if (rect) { - const offset = this.getDrawOffset(); + const offset = this.drawOffset; const topLeft = this.projectSceneToScreen({ x: rect.x + offset.x, y: rect.y + rect.height + offset.y, @@ -576,7 +577,7 @@ export class Editor { // UPM-space pass: snap lines + tool overlay this.#renderer.beginUpmSpace(canvas); - const indicator = this.getSnapIndicator(); + const indicator = this.snapIndicator; if (indicator) { this.#snapLines.draw(canvas, indicator); } @@ -611,7 +612,7 @@ export class Editor { } public hitTestBoundingBoxAt(coords: Coordinates): BoundingBoxHitResult { - if (!isBoundingBoxVisibleAtZoom(this.getZoom())) return null; + if (!isBoundingBoxVisibleAtZoom(this.zoom)) return null; const rect = this.getSelectionBoundingRect(); if (!rect) return null; @@ -630,7 +631,7 @@ export class Editor { ); } - public getHoveredBoundingBoxHandle(): BoundingBoxHitResult { + public get hoveredBoundingBoxHandle(): BoundingBoxHitResult { return this.#hover.getHoveredBoundingBoxHandle(); } @@ -638,14 +639,22 @@ export class Editor { this.#hover.clearHover(); } - public getIsHoveringNode(): boolean { + public get isHoveringNode(): boolean { return this.#isHoveringNode.value; } - public getCurrentModifiers(): Modifiers { + public get $isHoveringNode(): Signal { + return this.#isHoveringNode; + } + + public get currentModifiers(): Modifiers { return this.#currentModifiers.value; } + public get $currentModifiers(): Signal { + return this.#currentModifiers; + } + public setCurrentModifiers(modifiers: Modifiers): void { this.#currentModifiers.set(modifiers); } @@ -673,9 +682,13 @@ export class Editor { this.#snapIndicator.set(indicator); } - /** @knipclassignore Indirectly consumed through Viewport. */ - public getSnapIndicator(): SnapIndicator | null { - return this.#snapIndicator.peek(); + public get snapIndicator(): SnapIndicator | null { + return this.#snapIndicator.value; + } + + /** @knipclassignore */ + public get $snapIndicator(): Signal { + return this.#snapIndicator; } public createDraft(): GlyphDraft { @@ -721,20 +734,22 @@ export class Editor { return this.#toolStateVersion; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public getDebugOverlays(): DebugOverlays { + public get debugOverlays(): DebugOverlays { return this.#debugOverlays.value; } + /** @knipclassignore */ + public get $debugOverlays(): Signal { + return this.#debugOverlays; + } + public setDebugOverlays(overlays: DebugOverlays): void { this.#debugOverlays.set(overlays); } - public getHoveredSegmentId(): SegmentId | null { - const hoveredSegment = this.#hover.hoveredSegmentId.peek(); - if (hoveredSegment == null) return null; - - return hoveredSegment.segmentId; + public get hoveredSegmentId(): SegmentId | null { + const hoveredSegment = this.#hover.hoveredSegmentId.value; + return hoveredSegment?.segmentId ?? null; } public isPointInMarqueePreview(pointId: PointId): boolean { @@ -744,17 +759,16 @@ export class Editor { return marqueePreviewPointIds.has(pointId); } - public get previewMode(): Signal { - return this.$previewMode; + public get previewMode(): boolean { + return this.#previewMode.value; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public isPreviewMode(): boolean { - return this.$previewMode.peek(); + public get $previewMode(): Signal { + return this.#previewMode; } public setPreviewMode(enabled: boolean): void { - this.$previewMode.set(enabled); + this.#previewMode.set(enabled); } public setMarqueePreviewRect(rect: Rect2D | null): void { @@ -769,30 +783,28 @@ export class Editor { this.requestSceneRedraw(); } - public get handlesVisible(): Signal { - return this.$handlesVisible; + public get handlesVisible(): boolean { + return this.#handlesVisible.value; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public isHandlesVisible(): boolean { - return this.$handlesVisible.peek(); + public get $handlesVisible(): Signal { + return this.#handlesVisible; } public setHandlesVisible(visible: boolean): void { - this.$handlesVisible.set(visible); + this.#handlesVisible.set(visible); } - public get gpuHandlesEnabled(): Signal { - return this.$gpuHandlesEnabled; + public get gpuHandlesEnabled(): boolean { + return this.#gpuHandlesEnabled.value; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public isGpuHandlesEnabled(): boolean { - return this.$gpuHandlesEnabled.peek(); + public get $gpuHandlesEnabled(): Signal { + return this.#gpuHandlesEnabled; } public setGpuHandlesEnabled(enabled: boolean): void { - this.$gpuHandlesEnabled.set(enabled); + this.#gpuHandlesEnabled.set(enabled); } public setBackgroundContext(ctx: CanvasRenderingContext2D) { @@ -988,16 +1000,6 @@ export class Editor { return this.glyph.value?.xAdvance ?? 0; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public getVisualGlyphAdvance(glyph: Glyph): number { - if (glyph.xAdvance > 0) return glyph.xAdvance; - const unicode = Number.isFinite(glyph.unicode) ? glyph.unicode : null; - if (!isLikelyNonSpacingGlyphRef(glyph.name, unicode)) { - return glyph.xAdvance; - } - return 600; - } - public setXAdvance(width: number): void { const glyph = this.#$glyph.value; if (!glyph) return; @@ -1010,19 +1012,18 @@ export class Editor { const glyph = this.#$glyph.value; if (!glyph) return; - const sidebearings = deriveGlyphSidebearings(glyph); - if (sidebearings.lsb === null) return; + // Command path — use bezier-accurate bbox so the delta reflects the true + // current LSB including curve extension, not the cheap point-based + // approximation used for sidebar display. + const bbox = glyph.bbox; + if (!bbox) return; - const current = roundSidebearing(sidebearings.lsb)!; - const target = roundSidebearing(value)!; - const delta = target - current; + const delta = Math.round(value) - Math.round(bbox.min.x); if (delta === 0) return; const beforeXAdvance = glyph.xAdvance; - const afterXAdvance = beforeXAdvance + delta; - this.#commandHistory.execute( - new SetLeftSidebearingCommand(beforeXAdvance, afterXAdvance, delta), + new SetLeftSidebearingCommand(beforeXAdvance, beforeXAdvance + delta, delta), ); } @@ -1030,18 +1031,17 @@ export class Editor { const glyph = this.#$glyph.value; if (!glyph) return; - const sidebearings = deriveGlyphSidebearings(glyph); - if (sidebearings.rsb === null) return; + const bbox = glyph.bbox; + if (!bbox) return; - const current = roundSidebearing(sidebearings.rsb)!; - const target = roundSidebearing(value)!; - const delta = target - current; + const currentRsb = glyph.xAdvance - bbox.max.x; + const delta = Math.round(value) - Math.round(currentRsb); if (delta === 0) return; const beforeXAdvance = glyph.xAdvance; - const afterXAdvance = beforeXAdvance + delta; - - this.#commandHistory.execute(new SetRightSidebearingCommand(beforeXAdvance, afterXAdvance)); + this.#commandHistory.execute( + new SetRightSidebearingCommand(beforeXAdvance, beforeXAdvance + delta), + ); } public updateMetricsFromFont(): void { @@ -1193,21 +1193,25 @@ export class Editor { } public setCursor(cursor: CursorType): void { - this.$cursor.set(cursorToCSS(cursor)); + this.#cursor.set(cursorToCSS(cursor)); } - public getCursor(): string { - return this.$cursor.value; + public get cursor(): string { + return this.#cursor.value; } - public get zoom(): Signal { - return this.#viewport.zoom; + public get $cursor(): Signal { + return this.#cursor; } - public getZoom(): number { + public get zoom(): number { return this.#viewport.zoomLevel; } + public get $zoom(): Signal { + return this.#viewport.zoom; + } + public get fps(): Signal { return this.#renderer.fpsMonitor.fps; } @@ -1243,17 +1247,9 @@ export class Editor { return this.#clipboard.paste(); } - public getSelectionBounds(): Bounds | null { - const glyph = this.#$glyph.value; - const pointIds = this.selection.$pointIds.peek(); - if (!glyph || pointIds.size === 0) return null; - return getSegmentAwareBounds(glyph, Array.from(pointIds)); - } - - public getSelectionCenter(): Point2D | null { - const bounds = this.getSelectionBounds(); - if (!bounds) return null; - return Bounds.center(bounds); + #selectionCenter(): Point2D | null { + const bounds = this.selection.bounds; + return bounds ? Bounds.center(bounds) : null; } /** @param angle - Rotation in radians. */ @@ -1261,7 +1257,7 @@ export class Editor { const pointIds = [...this.selection.pointIds]; if (pointIds.length === 0) return; - const center = origin ?? this.getSelectionCenter(); + const center = origin ?? this.#selectionCenter(); if (!center) return; const cmd = new RotatePointsCommand([...pointIds], angle, center); @@ -1272,7 +1268,7 @@ export class Editor { const pointIds = [...this.selection.pointIds]; if (pointIds.length === 0) return; - const o = origin ?? this.getSelectionCenter(); + const o = origin ?? this.#selectionCenter(); if (!o) return; const cmd = new ScalePointsCommand([...pointIds], sx, sy, o); @@ -1283,7 +1279,7 @@ export class Editor { const pointIds = [...this.selection.pointIds]; if (pointIds.length === 0) return; - const center = origin ?? this.getSelectionCenter(); + const center = origin ?? this.#selectionCenter(); if (!center) return; const cmd = new ReflectPointsCommand([...pointIds], axis, center); @@ -1360,7 +1356,7 @@ export class Editor { }); } - public splitSegment(segment: GlyphSegment, t: number): PointId { + public splitSegment(segment: Segment, t: number): PointId { return this.#commandHistory.execute(new SplitSegmentCommand(segment, t)); } @@ -1379,8 +1375,8 @@ export class Editor { this.#commandHistory.execute(new NudgePointsCommand([...pointIds], dx, dy)); } - public upgradeLineToCubic(segment: LineSegment): void { - this.#commandHistory.execute(new UpgradeLineToCubicCommand(segment)); + public upgradeLineToCubic(segment: Segment): void { + this.#commandHistory.execute(new UpgradeLineToCubicCommand(segment.raw as LineSegment)); } public applyBooleanOp( @@ -1398,7 +1394,7 @@ export class Editor { const glyph = this.#$glyph.value; if (!glyph) return null; - return Glyphs.getPointAt(glyph, coords.glyphLocal, this.hitRadius); + return glyph.getPointAt(coords.glyphLocal, this.hitRadius); } private getAnchorAt( @@ -1421,8 +1417,8 @@ export class Editor { if (!glyph) return null; let bestHit: SegmentHitResult | null = null; - for (const { segment } of Segment.iterateGlyph(glyph.contours)) { - const hit = Segment.hitTest(segment, coords.glyphLocal, this.hitRadius); + for (const { segment } of glyph.segments()) { + const hit = segment.hitTest(coords.glyphLocal, this.hitRadius); if (hit && (!bestHit || hit.distance < bestHit.distance)) { bestHit = hit; } @@ -1530,7 +1526,7 @@ export class Editor { return null; } - const points = Glyphs.findPoints(glyph, selectedPointIds); + const points = glyph.points([...selectedPointIds]); if (points.length <= 1) return null; const bounds = Bounds.fromPoints(points); @@ -1579,7 +1575,7 @@ export class Editor { const glyph = this.#$glyph.value; if (!glyph) return []; - return Glyphs.getAllPoints(glyph); + return glyph.allPoints; } public duplicateSelection(): PointId[] { @@ -1635,11 +1631,15 @@ export class Editor { return null; } - /** @knipclassignore Indirectly consumed through Viewport. */ - public getDrawOffset(): Point2D { + public get drawOffset(): Point2D { return this.#drawOffset.value; } + /** @knipclassignore */ + public get $drawOffset(): Signal { + return this.#drawOffset; + } + public setDrawOffsetForGlyph( offset: Point2D, glyphName: string | null, @@ -1692,7 +1692,7 @@ export class Editor { glyphName: string | null, unicode: number | null, ): Point2D { - if (!glyphName || !isLikelyNonSpacingGlyphRef(glyphName, unicode)) { + if (!glyphName || !isNonSpacingGlyph(glyphName, unicode)) { return offset; } diff --git a/apps/desktop/src/renderer/src/lib/editor/managers/SnapManager.ts b/apps/desktop/src/renderer/src/lib/editor/managers/SnapManager.ts index ebbdfd79..6fe9ab27 100644 --- a/apps/desktop/src/renderer/src/lib/editor/managers/SnapManager.ts +++ b/apps/desktop/src/renderer/src/lib/editor/managers/SnapManager.ts @@ -1,5 +1,5 @@ import { Vec2 } from "@shift/geo"; -import { Contours, Glyphs } from "@shift/font"; +import { Contours } from "@shift/font"; import { Validate } from "@shift/validation"; import type { Point2D, PointId, FontMetrics } from "@shift/types"; import type { Glyph } from "@/lib/model/Glyph"; @@ -125,7 +125,7 @@ export class SnapManager { const excluded = new Set(query.excludedPointIds ?? []); const glyph = this.#$glyph.peek(); if (glyph) { - for (const { point } of Glyphs.points(glyph)) { + for (const point of glyph.allPoints) { if (excluded.has(point.id)) continue; result.push({ kind: "pointTarget", @@ -145,7 +145,7 @@ export class SnapManager { #getAnchorPosition(snapshot: Glyph | null, pointId: PointId, fallback: Point2D): Point2D { if (!snapshot) return fallback; - const result = Glyphs.findPoint(snapshot, pointId); + const result = snapshot.point(pointId); if (result) return { x: result.point.x, y: result.point.y }; return fallback; } @@ -153,7 +153,7 @@ export class SnapManager { #resolveSnapReference(snapshot: Glyph | null, pointId: PointId, fallback: Point2D): Point2D { if (!snapshot) return fallback; - const found = Glyphs.findPoint(snapshot, pointId); + const found = snapshot.point(pointId); if (found) { const { point, contour, index: idx } = found; if (Validate.isOnCurve(point)) { diff --git a/apps/desktop/src/renderer/src/lib/editor/rendering/Viewport.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/Viewport.ts index 40bbea1e..2260bf87 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/Viewport.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/Viewport.ts @@ -170,7 +170,7 @@ export class Viewport { ); const baselineY = vt.logicalHeight - vt.padding - vt.descender * vt.upmScale; ctx.transform(vt.upmScale, 0, 0, -vt.upmScale, vt.padding, baselineY); - const { x, y } = this.#editor.getDrawOffset(); + const { x, y } = this.#editor.drawOffset; ctx.translate(x, y); } diff --git a/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/DebugOverlays.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/DebugOverlays.ts index ef63f207..88240311 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/DebugOverlays.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/DebugOverlays.ts @@ -1,9 +1,6 @@ import type { Canvas } from "../Canvas"; import type { Glyph } from "@/lib/model/Glyph"; import type { SegmentId } from "@/types/indicator"; -import { Bounds as BoundsUtil, type Bounds } from "@shift/geo"; -import { Segments } from "@/lib/geo/Segments"; -import { Glyphs } from "@shift/font"; export class DebugOverlays { draw( @@ -35,11 +32,9 @@ export class DebugOverlays { } #drawSegmentBounds(canvas: Canvas, glyph: Glyph, color: string): void { - for (const { segment } of Segments.iterateGlyph(glyph.contours)) { - const bounds = Segments.bounds(segment); - const w = bounds.max.x - bounds.min.x; - const h = bounds.max.y - bounds.min.y; - canvas.strokeRect(bounds.min.x, bounds.min.y, w, h, color, 1); + 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); } } @@ -50,36 +45,24 @@ export class DebugOverlays { color: string, ): void { if (hoveredSegmentId === null) return; - for (const { segment } of Segments.iterateGlyph(glyph.contours)) { - if (Segments.id(segment) !== hoveredSegmentId) continue; - const bounds = Segments.bounds(segment); - const w = bounds.max.x - bounds.min.x; - const h = bounds.max.y - bounds.min.y; - canvas.strokeRect(bounds.min.x, bounds.min.y, w, h, color, 1); + 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; } } #drawHitRadii(canvas: Canvas, glyph: Glyph, hitRadiusUpm: number, color: string): void { - for (const { point } of Glyphs.points(glyph)) { - canvas.strokeCircle( - { x: point.x, y: point.y }, - hitRadiusUpm * canvas.viewport.upmScale * canvas.viewport.zoom, - color, - 1, - ); + const r = hitRadiusUpm * canvas.viewport.upmScale * canvas.viewport.zoom; + for (const point of glyph.allPoints) { + canvas.strokeCircle({ x: point.x, y: point.y }, r, color, 1); } } #drawGlyphBbox(canvas: Canvas, glyph: Glyph, color: string): void { - let bbox: Bounds | null = null; - for (const { segment } of Segments.iterateGlyph(glyph.contours)) { - const sb = Segments.bounds(segment); - bbox = bbox ? BoundsUtil.union(bbox, sb) : sb; - } - if (!bbox) return; - const w = bbox.max.x - bbox.min.x; - const h = bbox.max.y - bbox.min.y; - canvas.strokeRect(bbox.min.x, bbox.min.y, w, h, color, 1); + const b = glyph.bbox; + 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/editor/rendering/indicators/Segments.ts b/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/Segments.ts index 9e6c7dc5..f35883ce 100644 --- a/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/Segments.ts +++ b/apps/desktop/src/renderer/src/lib/editor/rendering/indicators/Segments.ts @@ -1,9 +1,8 @@ import type { Canvas } from "../Canvas"; -import type { Segment as SegmentType } from "@/types/segments"; -import { Segment } from "@/lib/geo/Segment"; +import type { Segment } from "@/lib/model/Segment"; export class Segments { - draw(canvas: Canvas, hovered: SegmentType | null, selected: readonly SegmentType[]): void { + draw(canvas: Canvas, hovered: Segment | null, selected: readonly Segment[]): void { if (!hovered && selected.length === 0) return; const theme = canvas.theme.segment; @@ -22,7 +21,7 @@ export class Segments { canvas.ctx.restore(); } - if (hovered && !selected.some((s) => Segment.id(s) === Segment.id(hovered))) { + if (hovered && !selected.some((s) => s.id === hovered.id)) { const lw = canvas.pxToUpm(theme.hoverWidthPx); canvas.ctx.save(); canvas.ctx.strokeStyle = theme.hoverColor; @@ -36,8 +35,8 @@ export class Segments { } } -function appendSegmentCurve(ctx: CanvasRenderingContext2D, segment: SegmentType): void { - const curve = Segment.toCurve(segment); +function appendSegmentCurve(ctx: CanvasRenderingContext2D, segment: Segment): void { + const curve = segment.toCurve(); ctx.moveTo(curve.p0.x, curve.p0.y); switch (curve.type) { diff --git a/apps/desktop/src/renderer/src/lib/editor/sidebearings.test.ts b/apps/desktop/src/renderer/src/lib/editor/sidebearings.test.ts deleted file mode 100644 index 8af052f1..00000000 --- a/apps/desktop/src/renderer/src/lib/editor/sidebearings.test.ts +++ /dev/null @@ -1,136 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { asContourId } from "@shift/types"; -import type { Point, RenderContour } from "@shift/types"; -import type { Glyph } from "@shift/types"; -import { deriveGlyphSidebearings, deriveGlyphXBounds } from "./sidebearings"; - -function makePoint( - id: string, - x: number, - y: number, - pointType: Point["pointType"] = "onCurve", -): Point { - return { - id: id as Point["id"], - x, - y, - pointType, - smooth: false, - }; -} - -function makeGlyph(input: { - xAdvance: number; - contours?: Glyph["contours"]; - compositeContours?: readonly RenderContour[]; -}): Glyph { - return { - unicode: 65, - name: "A", - xAdvance: input.xAdvance, - contours: input.contours ?? [], - anchors: [], - compositeContours: input.compositeContours ?? [], - activeContourId: null, - }; -} - -describe("deriveGlyphXBounds", () => { - it("returns null for empty glyph geometry", () => { - const glyph = makeGlyph({ xAdvance: 500 }); - expect(deriveGlyphXBounds(glyph)).toBeNull(); - }); - - it("includes contour segments in bounds", () => { - const glyph = makeGlyph({ - xAdvance: 600, - contours: [ - { - id: asContourId("c1"), - closed: true, - points: [ - makePoint("p1", 10, 0), - makePoint("p2", 110, 0), - makePoint("p3", 110, 100), - makePoint("p4", 10, 100), - ], - }, - ], - }); - - const bounds = deriveGlyphXBounds(glyph); - expect(bounds).toEqual({ minX: 10, maxX: 110 }); - }); - - it("uses tight curve bounds (can exceed anchor x)", () => { - const glyph = makeGlyph({ - xAdvance: 600, - contours: [ - { - id: asContourId("c1"), - closed: false, - points: [ - makePoint("p1", 0, 0, "onCurve"), - makePoint("p2", 300, 100, "offCurve"), - makePoint("p3", 200, 0, "onCurve"), - ], - }, - ], - }); - - const bounds = deriveGlyphXBounds(glyph); - expect(bounds).not.toBeNull(); - expect(bounds!.maxX).toBeCloseTo(225, 4); - }); - - it("includes composite contours in bounds", () => { - const glyph = makeGlyph({ - xAdvance: 700, - contours: [], - compositeContours: [ - { - closed: true, - points: [ - { x: 50, y: 0, pointType: "onCurve", smooth: false }, - { x: 150, y: 0, pointType: "onCurve", smooth: false }, - { x: 150, y: 100, pointType: "onCurve", smooth: false }, - { x: 50, y: 100, pointType: "onCurve", smooth: false }, - ], - }, - ], - }); - - const bounds = deriveGlyphXBounds(glyph); - expect(bounds).toEqual({ minX: 50, maxX: 150 }); - }); -}); - -describe("deriveGlyphSidebearings", () => { - it("returns null sidebearings when glyph has no drawable bounds", () => { - const glyph = makeGlyph({ xAdvance: 500 }); - expect(deriveGlyphSidebearings(glyph)).toEqual({ lsb: null, rsb: null }); - }); - - it("computes lsb and rsb from bounds and advance", () => { - const glyph = makeGlyph({ - xAdvance: 600, - contours: [ - { - id: asContourId("c1"), - closed: true, - points: [ - makePoint("p1", 20, 0), - makePoint("p2", 120, 0), - makePoint("p3", 120, 50), - makePoint("p4", 20, 50), - ], - }, - ], - }); - - expect(deriveGlyphSidebearings(glyph)).toEqual({ - lsb: 20, - rsb: 480, - }); - }); -}); diff --git a/apps/desktop/src/renderer/src/lib/editor/sidebearings.ts b/apps/desktop/src/renderer/src/lib/editor/sidebearings.ts deleted file mode 100644 index 246a6bbd..00000000 --- a/apps/desktop/src/renderer/src/lib/editor/sidebearings.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { deriveGlyphTightBounds } from "@shift/font"; -import type { Glyph } from "@shift/types"; - -export interface GlyphSidebearings { - readonly lsb: number | null; - readonly rsb: number | null; -} - -export function deriveGlyphXBounds(glyph: Glyph): { minX: number; maxX: number } | null { - const bounds = deriveGlyphTightBounds(glyph); - if (!bounds) return null; - return { minX: bounds.min.x, maxX: bounds.max.x }; -} - -export function deriveGlyphSidebearings(glyph: Glyph | null): GlyphSidebearings { - if (!glyph) { - return { lsb: null, rsb: null }; - } - - const bounds = deriveGlyphXBounds(glyph); - if (!bounds) { - return { lsb: null, rsb: null }; - } - - return { - lsb: bounds.minX, - rsb: glyph.xAdvance - bounds.maxX, - }; -} - -export function roundSidebearing(value: number | null): number | null { - if (value === null) return null; - return Math.round(value); -} diff --git a/apps/desktop/src/renderer/src/lib/geo/Segment.test.ts b/apps/desktop/src/renderer/src/lib/geo/Segment.test.ts deleted file mode 100644 index 769bf09b..00000000 --- a/apps/desktop/src/renderer/src/lib/geo/Segment.test.ts +++ /dev/null @@ -1,686 +0,0 @@ -import { describe, it, expect } from "vitest"; - -import { Segment } from "./Segment"; -import type { - LineSegment, - QuadSegment, - CubicSegment, - Segment as SegmentType, -} from "@/types/segments"; -import { asContourId, asPointId } from "@shift/types"; -import { expectAt } from "@/testing"; - -const makeSegmentPoint = ( - id: string, - x: number, - y: number, - pointType: "onCurve" | "offCurve" = "onCurve", -) => ({ - id: asPointId(id), - x, - y, - pointType, - smooth: false, -}); - -function expectLineSegment(segment: SegmentType): LineSegment { - expect(segment.type).toBe("line"); - return segment as LineSegment; -} - -function expectQuadSegment(segment: SegmentType): QuadSegment { - expect(segment.type).toBe("quad"); - return segment as QuadSegment; -} - -function expectCubicSegment(segment: SegmentType): CubicSegment { - expect(segment.type).toBe("cubic"); - return segment as CubicSegment; -} - -describe("Segment", () => { - describe("id", () => { - it("should create id for line segment", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 100), - }, - }; - expect(Segment.id(segment)).toBe("p1:p2"); - }); - - it("should create id for quad segment", () => { - const segment: QuadSegment = { - type: "quad", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control: makeSegmentPoint("c1", 50, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - expect(Segment.id(segment)).toBe("p1:p2"); - }); - - it("should create id for cubic segment", () => { - const segment: CubicSegment = { - type: "cubic", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control1: makeSegmentPoint("c1", 25, 100, "offCurve"), - control2: makeSegmentPoint("c2", 75, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - expect(Segment.id(segment)).toBe("p1:p2"); - }); - }); - - describe("toCurve", () => { - it("should convert line segment to line curve", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 100), - }, - }; - const curve = Segment.toCurve(segment); - expect(curve.type).toBe("line"); - expect(curve.p0).toEqual({ - x: 0, - y: 0, - id: "p1", - pointType: "onCurve", - smooth: false, - }); - expect(curve.p1).toEqual({ - x: 100, - y: 100, - id: "p2", - pointType: "onCurve", - smooth: false, - }); - }); - - it("should convert quad segment to quadratic curve", () => { - const segment: QuadSegment = { - type: "quad", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control: makeSegmentPoint("c1", 50, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - const curve = Segment.toCurve(segment); - expect(curve.type).toBe("quadratic"); - }); - - it("should convert cubic segment to cubic curve", () => { - const segment: CubicSegment = { - type: "cubic", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control1: makeSegmentPoint("c1", 25, 100, "offCurve"), - control2: makeSegmentPoint("c2", 75, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - const curve = Segment.toCurve(segment); - expect(curve.type).toBe("cubic"); - }); - }); - - describe("bounds", () => { - it("should compute bounds for line segment", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 10, 20), - anchor2: makeSegmentPoint("p2", 100, 80), - }, - }; - const bounds = Segment.bounds(segment); - expect(bounds.min.x).toBe(10); - expect(bounds.min.y).toBe(20); - expect(bounds.max.x).toBe(100); - expect(bounds.max.y).toBe(80); - }); - - it("should compute bounds for cubic segment", () => { - const segment: CubicSegment = { - type: "cubic", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control1: makeSegmentPoint("c1", 0, 100, "offCurve"), - control2: makeSegmentPoint("c2", 100, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - const bounds = Segment.bounds(segment); - expect(bounds.min.x).toBeLessThanOrEqual(0); - expect(bounds.min.y).toBeLessThanOrEqual(0); - expect(bounds.max.x).toBeGreaterThanOrEqual(100); - expect(bounds.max.y).toBeGreaterThan(0); - }); - }); - - describe("hitTest", () => { - it("should hit test a line segment", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - - const hitOnLine = Segment.hitTest(segment, { x: 50, y: 0 }, 5); - expect(hitOnLine).not.toBeNull(); - expect(hitOnLine!.distance).toBeLessThan(5); - expect(hitOnLine!.segmentId).toBe("p1:p2"); - - const hitNearLine = Segment.hitTest(segment, { x: 50, y: 3 }, 5); - expect(hitNearLine).not.toBeNull(); - - const missLine = Segment.hitTest(segment, { x: 50, y: 10 }, 5); - expect(missLine).toBeNull(); - }); - - it("should return closest point on hit", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - - const hit = Segment.hitTest(segment, { x: 50, y: 2 }, 5); - expect(hit).not.toBeNull(); - expect(hit!.point.x).toBeCloseTo(50, 1); - expect(hit!.point.y).toBeCloseTo(0, 1); - expect(hit!.t).toBeCloseTo(0.5, 1); - }); - - it("should hit test a cubic segment", () => { - const segment: CubicSegment = { - type: "cubic", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control1: makeSegmentPoint("c1", 25, 50, "offCurve"), - control2: makeSegmentPoint("c2", 75, 50, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - - const hitNearStart = Segment.hitTest(segment, { x: 0, y: 0 }, 5); - expect(hitNearStart).not.toBeNull(); - - const missSegment = Segment.hitTest(segment, { x: 50, y: 100 }, 5); - expect(missSegment).toBeNull(); - }); - - it("should early reject based on bounds", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - - const farAway = Segment.hitTest(segment, { x: 500, y: 500 }, 5); - expect(farAway).toBeNull(); - }); - }); - - describe("hitTestMultiple", () => { - it("should return the closest hit from multiple segments", () => { - const segments: SegmentType[] = [ - { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }, - { - type: "line", - points: { - anchor1: makeSegmentPoint("p3", 0, 50), - anchor2: makeSegmentPoint("p4", 100, 50), - }, - }, - ]; - - const hit = Segment.hitTestMultiple(segments, { x: 50, y: 1 }, 10); - expect(hit).not.toBeNull(); - expect(hit!.segmentId).toBe("p1:p2"); - }); - - it("should return null if no segments hit", () => { - const segments: SegmentType[] = [ - { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }, - ]; - - const hit = Segment.hitTestMultiple(segments, { x: 50, y: 100 }, 5); - expect(hit).toBeNull(); - }); - - it("should return null for empty segments array", () => { - const hit = Segment.hitTestMultiple([], { x: 50, y: 0 }, 5); - expect(hit).toBeNull(); - }); - }); - - describe("parse", () => { - it("should return empty array for fewer than 2 points", () => { - expect(Segment.parse([], false)).toEqual([]); - expect(Segment.parse([makeSegmentPoint("p1", 0, 0)], false)).toEqual([]); - }); - - it("should parse two onCurve points as a line", () => { - const points = [makeSegmentPoint("p1", 0, 0), makeSegmentPoint("p2", 100, 100)]; - const segments = Segment.parse(points, false); - expect(segments).toHaveLength(1); - expect(expectAt(segments, 0).type).toBe("line"); - }); - - it("should parse quadratic curve (onCurve, offCurve, onCurve)", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 50, 100, "offCurve"), - makeSegmentPoint("p2", 100, 0), - ]; - const segments = Segment.parse(points, false); - expect(segments).toHaveLength(1); - expect(expectAt(segments, 0).type).toBe("quad"); - }); - - it("should parse cubic curve (onCurve, offCurve, offCurve, onCurve)", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 25, 100, "offCurve"), - makeSegmentPoint("c2", 75, 100, "offCurve"), - makeSegmentPoint("p2", 100, 0), - ]; - const segments = Segment.parse(points, false); - expect(segments).toHaveLength(1); - expect(expectAt(segments, 0).type).toBe("cubic"); - }); - - it("should parse closed contour with trailing offCurve points as cubic wrap-around", () => { - const points = [ - makeSegmentPoint("p1", 684, 895), - makeSegmentPoint("c1", 813, 895, "offCurve"), - makeSegmentPoint("c2", 893, 784, "offCurve"), - makeSegmentPoint("p2", 893, 565), - makeSegmentPoint("c3", 893, 346, "offCurve"), - makeSegmentPoint("c4", 813, 227, "offCurve"), - makeSegmentPoint("p3", 688, 227), - makeSegmentPoint("c5", 518, 227, "offCurve"), - makeSegmentPoint("c6", 465, 346, "offCurve"), - makeSegmentPoint("p4", 465, 563), - makeSegmentPoint("p5", 465, 596), - makeSegmentPoint("c7", 469, 797, "offCurve"), - makeSegmentPoint("c8", 524, 895, "offCurve"), - ]; - - const segments = Segment.parse(points, true); - - expect(segments.length).toBe(5); - - const first = expectCubicSegment(expectAt(segments, 0)); - expect(first.points.anchor1.id).toBe("p1"); - expect(first.points.control1.id).toBe("c1"); - expect(first.points.control2.id).toBe("c2"); - expect(first.points.anchor2.id).toBe("p2"); - - const second = expectCubicSegment(expectAt(segments, 1)); - expect(second.points.anchor1.id).toBe("p2"); - expect(second.points.anchor2.id).toBe("p3"); - - const third = expectCubicSegment(expectAt(segments, 2)); - expect(third.points.anchor1.id).toBe("p3"); - expect(third.points.anchor2.id).toBe("p4"); - - const fourth = expectLineSegment(expectAt(segments, 3)); - expect(fourth.points.anchor1.id).toBe("p4"); - expect(fourth.points.anchor2.id).toBe("p5"); - - const fifth = expectCubicSegment(expectAt(segments, 4)); - expect(fifth.points.anchor1.id).toBe("p5"); - expect(fifth.points.control1.id).toBe("c7"); - expect(fifth.points.control2.id).toBe("c8"); - expect(fifth.points.anchor2.id).toBe("p1"); - }); - - it("should handle closed contour with quad wrap-around", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("c1", 50, 50, "offCurve"), - ]; - - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(2); - expectLineSegment(expectAt(segments, 0)); - const wrap = expectQuadSegment(expectAt(segments, 1)); - expect(wrap.points.anchor1.id).toBe("p2"); - expect(wrap.points.control.id).toBe("c1"); - expect(wrap.points.anchor2.id).toBe("p1"); - }); - - it("should handle closed contour with line wrap-around", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 50, 50), - ]; - - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(3); - expectLineSegment(expectAt(segments, 0)); - expectLineSegment(expectAt(segments, 1)); - const wrap = expectLineSegment(expectAt(segments, 2)); - expect(wrap.points.anchor1.id).toBe("p3"); - expect(wrap.points.anchor2.id).toBe("p1"); - }); - - it("should parse mixed line and cubic segments", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("c1", 150, 50, "offCurve"), - makeSegmentPoint("c2", 150, 100, "offCurve"), - makeSegmentPoint("p3", 100, 150), - ]; - const segments = Segment.parse(points, false); - - expect(segments).toHaveLength(2); - expect(expectAt(segments, 0).type).toBe("line"); - expect(expectAt(segments, 1).type).toBe("cubic"); - }); - - it("should parse multiple consecutive line segments", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 100, 100), - makeSegmentPoint("p4", 0, 100), - ]; - const segments = Segment.parse(points, false); - - expect(segments).toHaveLength(3); - expect(segments.every((s) => s.type === "line")).toBe(true); - }); - - it("should parse open contour ending with offCurve points (incomplete curve)", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("c1", 150, 50, "offCurve"), - makeSegmentPoint("c2", 150, 100, "offCurve"), - ]; - const segments = Segment.parse(points, false); - - expect(segments).toHaveLength(1); - expect(expectAt(segments, 0).type).toBe("line"); - }); - - it("should parse alternating line and quad segments", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 50, 50, "offCurve"), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 200, 0), - makeSegmentPoint("c2", 250, 50, "offCurve"), - makeSegmentPoint("p4", 300, 0), - ]; - const segments = Segment.parse(points, false); - - expect(segments).toHaveLength(3); - expect(expectAt(segments, 0).type).toBe("quad"); - expect(expectAt(segments, 1).type).toBe("line"); - expect(expectAt(segments, 2).type).toBe("quad"); - }); - - it("should handle closed triangle (three onCurve points)", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 50, 87), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(3); - expect(segments.every((s) => s.type === "line")).toBe(true); - const first = expectLineSegment(expectAt(segments, 0)); - const second = expectLineSegment(expectAt(segments, 1)); - const third = expectLineSegment(expectAt(segments, 2)); - expect(first.points.anchor1.id).toBe("p1"); - expect(first.points.anchor2.id).toBe("p2"); - expect(second.points.anchor1.id).toBe("p2"); - expect(second.points.anchor2.id).toBe("p3"); - expect(third.points.anchor1.id).toBe("p3"); - expect(third.points.anchor2.id).toBe("p1"); - }); - - it("should handle closed rectangle (four onCurve points)", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 100, 50), - makeSegmentPoint("p4", 0, 50), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(4); - expect(segments.every((s) => s.type === "line")).toBe(true); - }); - - it("should handle closed oval (all cubics)", () => { - const points = [ - makeSegmentPoint("p1", 50, 0), - makeSegmentPoint("c1", 77, 0, "offCurve"), - makeSegmentPoint("c2", 100, 22, "offCurve"), - makeSegmentPoint("p2", 100, 50), - makeSegmentPoint("c3", 100, 78, "offCurve"), - makeSegmentPoint("c4", 77, 100, "offCurve"), - makeSegmentPoint("p3", 50, 100), - makeSegmentPoint("c5", 22, 100, "offCurve"), - makeSegmentPoint("c6", 0, 78, "offCurve"), - makeSegmentPoint("p4", 0, 50), - makeSegmentPoint("c7", 0, 22, "offCurve"), - makeSegmentPoint("c8", 22, 0, "offCurve"), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(4); - expect(segments.every((s) => s.type === "cubic")).toBe(true); - const wrap = expectCubicSegment(expectAt(segments, 3)); - expect(wrap.points.anchor1.id).toBe("p4"); - expect(wrap.points.control1.id).toBe("c7"); - expect(wrap.points.control2.id).toBe("c8"); - expect(wrap.points.anchor2.id).toBe("p1"); - }); - - it("should handle closed contour starting with cubic then line", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 25, 50, "offCurve"), - makeSegmentPoint("c2", 75, 50, "offCurve"), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 50, -20), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(3); - expect(expectAt(segments, 0).type).toBe("cubic"); - expect(expectAt(segments, 1).type).toBe("line"); - expect(expectAt(segments, 2).type).toBe("line"); - }); - - it("should parse two points as single line for open contour", () => { - const points = [makeSegmentPoint("p1", 0, 0), makeSegmentPoint("p2", 100, 100)]; - const segments = Segment.parse(points, false); - - expect(segments).toHaveLength(1); - expect(expectAt(segments, 0).type).toBe("line"); - }); - - it("should parse two points as single line segment for closed contour", () => { - const points = [makeSegmentPoint("p1", 0, 0), makeSegmentPoint("p2", 100, 100)]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(2); - expectLineSegment(expectAt(segments, 0)); - const wrap = expectLineSegment(expectAt(segments, 1)); - expect(wrap.points.anchor1.id).toBe("p2"); - expect(wrap.points.anchor2.id).toBe("p1"); - }); - - it("should handle single quad in closed contour", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 50, 100, "offCurve"), - makeSegmentPoint("p2", 100, 0), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(2); - expect(expectAt(segments, 0).type).toBe("quad"); - expect(expectAt(segments, 1).type).toBe("line"); - }); - - it("should handle single cubic in closed contour", () => { - const points = [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("c1", 25, 100, "offCurve"), - makeSegmentPoint("c2", 75, 100, "offCurve"), - makeSegmentPoint("p2", 100, 0), - ]; - const segments = Segment.parse(points, true); - - expect(segments).toHaveLength(2); - expect(expectAt(segments, 0).type).toBe("cubic"); - expect(expectAt(segments, 1).type).toBe("line"); - }); - }); - - describe("iterateGlyph", () => { - it("yields all segments across multiple contours", () => { - const contours = [ - { - id: asContourId("c1"), - points: [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 100, 100), - ], - closed: false, - }, - { - id: asContourId("c2"), - points: [makeSegmentPoint("p4", 200, 0), makeSegmentPoint("p5", 300, 0)], - closed: false, - }, - ]; - const result = [...Segment.iterateGlyph(contours)]; - - expect(result).toHaveLength(3); - expect(expectAt(result, 0).contourId).toBe("c1"); - expect(expectAt(result, 0).segment.type).toBe("line"); - expect(expectAt(result, 1).contourId).toBe("c1"); - expect(expectAt(result, 1).segment.type).toBe("line"); - expect(expectAt(result, 2).contourId).toBe("c2"); - expect(expectAt(result, 2).segment.type).toBe("line"); - }); - - it("yields nothing for empty contours array", () => { - const result = [...Segment.iterateGlyph([])]; - expect(result).toHaveLength(0); - }); - - it("yields nothing for contours with fewer than 2 points", () => { - const contours = [ - { id: asContourId("c1"), points: [makeSegmentPoint("p1", 0, 0)], closed: false }, - ]; - const result = [...Segment.iterateGlyph(contours)]; - expect(result).toHaveLength(0); - }); - - it("handles closed contour wrap-around segments", () => { - const contours = [ - { - id: asContourId("c1"), - points: [ - makeSegmentPoint("p1", 0, 0), - makeSegmentPoint("p2", 100, 0), - makeSegmentPoint("p3", 50, 87), - ], - closed: true, - }, - ]; - const result = [...Segment.iterateGlyph(contours)]; - - expect(result).toHaveLength(3); - expect(result.every((r) => r.contourId === "c1")).toBe(true); - expect(result.every((r) => r.segment.type === "line")).toBe(true); - }); - }); - - describe("type guards", () => { - it("should identify line segments", () => { - const segment: LineSegment = { - type: "line", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - expect(Segment.isLine(segment)).toBe(true); - expect(Segment.isQuad(segment)).toBe(false); - expect(Segment.isCubic(segment)).toBe(false); - }); - - it("should identify quad segments", () => { - const segment: QuadSegment = { - type: "quad", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control: makeSegmentPoint("c1", 50, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - expect(Segment.isLine(segment)).toBe(false); - expect(Segment.isQuad(segment)).toBe(true); - expect(Segment.isCubic(segment)).toBe(false); - }); - - it("should identify cubic segments", () => { - const segment: CubicSegment = { - type: "cubic", - points: { - anchor1: makeSegmentPoint("p1", 0, 0), - control1: makeSegmentPoint("c1", 25, 100, "offCurve"), - control2: makeSegmentPoint("c2", 75, 100, "offCurve"), - anchor2: makeSegmentPoint("p2", 100, 0), - }, - }; - expect(Segment.isLine(segment)).toBe(false); - expect(Segment.isQuad(segment)).toBe(false); - expect(Segment.isCubic(segment)).toBe(true); - }); - }); -}); diff --git a/apps/desktop/src/renderer/src/lib/geo/Segment.ts b/apps/desktop/src/renderer/src/lib/geo/Segment.ts deleted file mode 100644 index b988b824..00000000 --- a/apps/desktop/src/renderer/src/lib/geo/Segment.ts +++ /dev/null @@ -1,273 +0,0 @@ -/** - * Segment - Operations for font editing segments with point IDs. - * - * This module bridges the gap between: - * - Pure geometry curves (Curve module - no IDs) - * - Font segments (types/segments.ts - have IDs) - * - * It provides operations on segments like hit testing, bounds, and - * conversion to pure geometry curves for math operations. - * - * @example - * ```ts - * import { Segment as SegmentOps } from '@/lib/geo/Segment'; - * import type { Segment } from '@/types/segments'; - * - * // Hit test a segment - * const hit = SegmentOps.hitTest(segment, mousePos, radius); - * if (hit) { - * console.log('Hit segment:', hit.segmentId, 'at t:', hit.t); - * } - * - * // Get segment ID - * const id = SegmentOps.id(segment); - * - * // Convert to curve for math operations - * const curve = SegmentOps.toCurve(segment); - * const point = Curve.pointAt(curve, 0.5); - * ``` - */ - -import { Curve, Vec2, type Bounds, type CurveType, type Point2D } from "@shift/geo"; -import { Validate } from "@shift/validation"; -import type { - Segment as SegmentType, - LineSegment, - QuadSegment, - CubicSegment, - SegmentPoint, -} from "@/types/segments"; -import type { SegmentId } from "@/types/indicator"; -import { asSegmentId } from "@/types/indicator"; -import type { PointId, Point, Contour, ContourId } from "@shift/types"; - -export interface SegmentHitResult { - segment: SegmentType; - segmentId: SegmentId; - t: number; - point: Point2D; - distance: number; -} - -export const Segment = { - id(segment: SegmentType): SegmentId { - switch (segment.type) { - case "line": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - case "quad": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - case "cubic": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - } - }, - - toCurve(segment: SegmentType): CurveType { - switch (segment.type) { - case "line": - return Curve.line(segment.points.anchor1, segment.points.anchor2); - case "quad": - return Curve.quadratic( - segment.points.anchor1, - segment.points.control, - segment.points.anchor2, - ); - case "cubic": - return Curve.cubic( - segment.points.anchor1, - segment.points.control1, - segment.points.control2, - segment.points.anchor2, - ); - } - }, - - bounds(segment: SegmentType): Bounds { - const curve = Segment.toCurve(segment); - return Curve.bounds(curve); - }, - - hitTest(segment: SegmentType, pos: Point2D, radius: number): SegmentHitResult | null { - const bounds = Segment.bounds(segment); - const radiusVec = { x: radius, y: radius }; - const expandedMin = Vec2.sub(bounds.min, radiusVec); - const expandedMax = Vec2.add(bounds.max, radiusVec); - - if ( - pos.x < expandedMin.x || - pos.x > expandedMax.x || - pos.y < expandedMin.y || - pos.y > expandedMax.y - ) { - return null; - } - - const curve = Segment.toCurve(segment); - const closest = Curve.closestPoint(curve, pos); - - if (closest.distance < radius) { - return { - segment, - segmentId: Segment.id(segment), - t: closest.t, - point: closest.point, - distance: closest.distance, - }; - } - - return null; - }, - - hitTestMultiple(segments: SegmentType[], pos: Point2D, radius: number): SegmentHitResult | null { - let bestHit: SegmentHitResult | null = null; - - for (const segment of segments) { - const hit = Segment.hitTest(segment, pos, radius); - if (hit && (bestHit === null || hit.distance < bestHit.distance)) { - bestHit = hit; - } - } - - return bestHit; - }, - - isLine(segment: SegmentType): segment is LineSegment { - return segment.type === "line"; - }, - - isQuad(segment: SegmentType): segment is QuadSegment { - return segment.type === "quad"; - }, - - isCubic(segment: SegmentType): segment is CubicSegment { - return segment.type === "cubic"; - }, - - getPointIds(segment: SegmentType): PointId[] { - switch (segment.type) { - case "line": - return [segment.points.anchor1.id, segment.points.anchor2.id]; - case "quad": - return [segment.points.anchor1.id, segment.points.control.id, segment.points.anchor2.id]; - case "cubic": - return [ - segment.points.anchor1.id, - segment.points.control1.id, - segment.points.control2.id, - segment.points.anchor2.id, - ]; - } - }, - - getPoints(segment: SegmentType): SegmentPoint[] { - switch (segment.type) { - case "line": - return [segment.points.anchor1, segment.points.anchor2]; - case "quad": - return [segment.points.anchor1, segment.points.control, segment.points.anchor2]; - case "cubic": - return [ - segment.points.anchor1, - segment.points.control1, - segment.points.control2, - segment.points.anchor2, - ]; - } - }, - - parse(points: readonly Point[], closed: boolean): SegmentType[] { - if (points.length < 2) { - return []; - } - - const segments: SegmentType[] = []; - let index = 0; - - const getPoint = (i: number): Point | undefined => { - if (i < points.length) { - return points[i]; - } - if (closed) { - return points[i - points.length]; - } - return undefined; - }; - - const limit = closed ? points.length : points.length - 1; - - while (index < limit) { - const p1 = getPoint(index); - const p2 = getPoint(index + 1); - - if (!p1 || !p2) { - break; - } - - if (Validate.isOnCurve(p1) && Validate.isOnCurve(p2)) { - segments.push({ - type: "line", - points: { anchor1: p1, anchor2: p2 }, - }); - index += 1; - continue; - } - - if (Validate.isOnCurve(p1) && Validate.isOffCurve(p2)) { - const p3 = getPoint(index + 2); - - if (!p3) { - break; - } - - if (Validate.isOnCurve(p3)) { - segments.push({ - type: "quad", - points: { anchor1: p1, control: p2, anchor2: p3 }, - }); - index += 2; - continue; - } - - if (Validate.isOffCurve(p3)) { - const p4 = getPoint(index + 3); - if (!p4) { - break; - } - - segments.push({ - type: "cubic", - points: { anchor1: p1, control1: p2, control2: p3, anchor2: p4 }, - }); - index += 3; - continue; - } - } - - index += 1; - } - - return segments; - }, - - parseGlyph(contours: readonly Contour[]): Map { - const result = new Map(); - - for (const contour of contours) { - const segments = Segment.parse(contour.points, contour.closed); - result.set(contour.id, segments); - } - - return result; - }, - - *iterateGlyph(contours: readonly Contour[]): Generator<{ - segment: SegmentType; - contourId: ContourId; - }> { - for (const contour of contours) { - const segments = Segment.parse(contour.points, contour.closed); - for (const segment of segments) { - yield { segment, contourId: contour.id }; - } - } - }, -} as const; diff --git a/apps/desktop/src/renderer/src/lib/geo/Segments.ts b/apps/desktop/src/renderer/src/lib/geo/Segments.ts deleted file mode 100644 index 8a335a96..00000000 --- a/apps/desktop/src/renderer/src/lib/geo/Segments.ts +++ /dev/null @@ -1,273 +0,0 @@ -/** - * Segment - Operations for font editing segments with point IDs. - * - * This module bridges the gap between: - * - Pure geometry curves (Curve module - no IDs) - * - Font segments (types/segments.ts - have IDs) - * - * It provides operations on segments like hit testing, bounds, and - * conversion to pure geometry curves for math operations. - * - * @example - * ```ts - * import { Segment as SegmentOps } from '@/lib/geo/Segment'; - * import type { Segment } from '@/types/segments'; - * - * // Hit test a segment - * const hit = SegmentOps.hitTest(segment, mousePos, radius); - * if (hit) { - * console.log('Hit segment:', hit.segmentId, 'at t:', hit.t); - * } - * - * // Get segment ID - * const id = SegmentOps.id(segment); - * - * // Convert to curve for math operations - * const curve = SegmentOps.toCurve(segment); - * const point = Curve.pointAt(curve, 0.5); - * ``` - */ - -import { Curve, Vec2, type Bounds, type CurveType, type Point2D } from "@shift/geo"; -import { Validate } from "@shift/validation"; -import type { - Segment as SegmentType, - LineSegment, - QuadSegment, - CubicSegment, - SegmentPoint, -} from "@/types/segments"; -import type { SegmentId } from "@/types/indicator"; -import { asSegmentId } from "@/types/indicator"; -import type { PointId, Point, Contour, ContourId } from "@shift/types"; - -export interface SegmentHitResult { - segment: SegmentType; - segmentId: SegmentId; - t: number; - point: Point2D; - distance: number; -} - -export const Segments = { - id(segment: SegmentType): SegmentId { - switch (segment.type) { - case "line": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - case "quad": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - case "cubic": - return asSegmentId(`${segment.points.anchor1.id}:${segment.points.anchor2.id}`); - } - }, - - toCurve(segment: SegmentType): CurveType { - switch (segment.type) { - case "line": - return Curve.line(segment.points.anchor1, segment.points.anchor2); - case "quad": - return Curve.quadratic( - segment.points.anchor1, - segment.points.control, - segment.points.anchor2, - ); - case "cubic": - return Curve.cubic( - segment.points.anchor1, - segment.points.control1, - segment.points.control2, - segment.points.anchor2, - ); - } - }, - - bounds(segment: SegmentType): Bounds { - const curve = Segments.toCurve(segment); - return Curve.bounds(curve); - }, - - hitTest(segment: SegmentType, pos: Point2D, radius: number): SegmentHitResult | null { - const bounds = Segments.bounds(segment); - const radiusVec = { x: radius, y: radius }; - const expandedMin = Vec2.sub(bounds.min, radiusVec); - const expandedMax = Vec2.add(bounds.max, radiusVec); - - if ( - pos.x < expandedMin.x || - pos.x > expandedMax.x || - pos.y < expandedMin.y || - pos.y > expandedMax.y - ) { - return null; - } - - const curve = Segments.toCurve(segment); - const closest = Curve.closestPoint(curve, pos); - - if (closest.distance < radius) { - return { - segment, - segmentId: Segments.id(segment), - t: closest.t, - point: closest.point, - distance: closest.distance, - }; - } - - return null; - }, - - hitTestMultiple(segments: SegmentType[], pos: Point2D, radius: number): SegmentHitResult | null { - let bestHit: SegmentHitResult | null = null; - - for (const segment of segments) { - const hit = Segments.hitTest(segment, pos, radius); - if (hit && (bestHit === null || hit.distance < bestHit.distance)) { - bestHit = hit; - } - } - - return bestHit; - }, - - isLine(segment: SegmentType): segment is LineSegment { - return segment.type === "line"; - }, - - isQuad(segment: SegmentType): segment is QuadSegment { - return segment.type === "quad"; - }, - - isCubic(segment: SegmentType): segment is CubicSegment { - return segment.type === "cubic"; - }, - - getPointIds(segment: SegmentType): PointId[] { - switch (segment.type) { - case "line": - return [segment.points.anchor1.id, segment.points.anchor2.id]; - case "quad": - return [segment.points.anchor1.id, segment.points.control.id, segment.points.anchor2.id]; - case "cubic": - return [ - segment.points.anchor1.id, - segment.points.control1.id, - segment.points.control2.id, - segment.points.anchor2.id, - ]; - } - }, - - getPoints(segment: SegmentType): SegmentPoint[] { - switch (segment.type) { - case "line": - return [segment.points.anchor1, segment.points.anchor2]; - case "quad": - return [segment.points.anchor1, segment.points.control, segment.points.anchor2]; - case "cubic": - return [ - segment.points.anchor1, - segment.points.control1, - segment.points.control2, - segment.points.anchor2, - ]; - } - }, - - parse(points: readonly Point[], closed: boolean): SegmentType[] { - if (points.length < 2) { - return []; - } - - const segments: SegmentType[] = []; - let index = 0; - - const getPoint = (i: number): Point | undefined => { - if (i < points.length) { - return points[i]; - } - if (closed) { - return points[i - points.length]; - } - return undefined; - }; - - const limit = closed ? points.length : points.length - 1; - - while (index < limit) { - const p1 = getPoint(index); - const p2 = getPoint(index + 1); - - if (!p1 || !p2) { - break; - } - - if (Validate.isOnCurve(p1) && Validate.isOnCurve(p2)) { - segments.push({ - type: "line", - points: { anchor1: p1, anchor2: p2 }, - }); - index += 1; - continue; - } - - if (Validate.isOnCurve(p1) && Validate.isOffCurve(p2)) { - const p3 = getPoint(index + 2); - - if (!p3) { - break; - } - - if (Validate.isOnCurve(p3)) { - segments.push({ - type: "quad", - points: { anchor1: p1, control: p2, anchor2: p3 }, - }); - index += 2; - continue; - } - - if (Validate.isOffCurve(p3)) { - const p4 = getPoint(index + 3); - if (!p4) { - break; - } - - segments.push({ - type: "cubic", - points: { anchor1: p1, control1: p2, control2: p3, anchor2: p4 }, - }); - index += 3; - continue; - } - } - - index += 1; - } - - return segments; - }, - - parseGlyph(contours: readonly Contour[]): Map { - const result = new Map(); - - for (const contour of contours) { - const segments = Segments.parse(contour.points, contour.closed); - result.set(contour.id, segments); - } - - return result; - }, - - *iterateGlyph(contours: readonly Contour[]): Generator<{ - segment: SegmentType; - contourId: ContourId; - }> { - for (const contour of contours) { - const segments = Segments.parse(contour.points, contour.closed); - for (const segment of segments) { - yield { segment, contourId: contour.id }; - } - } - }, -} as const; 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 8205ec7c..4437c380 100644 --- a/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts +++ b/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.test.ts @@ -72,7 +72,7 @@ describe("KeyboardRouter", () => { ]), requestTemporaryTool: vi.fn((_tool, options) => options?.onActivate?.()), returnFromTemporaryTool: vi.fn(), - isPreviewMode: vi.fn(() => false), + previewMode: false, setPreviewMode: vi.fn(), openGlyphFinder: vi.fn(), }; diff --git a/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.ts b/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.ts index f7e36f1e..b203d2f8 100644 --- a/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.ts +++ b/apps/desktop/src/renderer/src/lib/keyboard/KeyboardRouter.ts @@ -107,7 +107,7 @@ export class KeyboardRouter { return true; } - const wasPreview = ctx.editor.isPreviewMode(); + const wasPreview = ctx.editor.previewMode; this.#spacePreviewBeforeTemporary = wasPreview; ctx.editor.requestTemporaryTool("hand", { onActivate: () => { diff --git a/apps/desktop/src/renderer/src/lib/keyboard/types.ts b/apps/desktop/src/renderer/src/lib/keyboard/types.ts index 3af21d78..97254194 100644 --- a/apps/desktop/src/renderer/src/lib/keyboard/types.ts +++ b/apps/desktop/src/renderer/src/lib/keyboard/types.ts @@ -19,7 +19,7 @@ export interface KeyboardEditorActions { options?: { onActivate?: () => void; onReturn?: () => void }, ): void; returnFromTemporaryTool(): void; - isPreviewMode(): boolean; + readonly previewMode: boolean; setPreviewMode(enabled: boolean): void; openGlyphFinder(): void; } diff --git a/apps/desktop/src/renderer/src/lib/model/Glyph.ts b/apps/desktop/src/renderer/src/lib/model/Glyph.ts index e2c4b0f1..30a3ae4e 100644 --- a/apps/desktop/src/renderer/src/lib/model/Glyph.ts +++ b/apps/desktop/src/renderer/src/lib/model/Glyph.ts @@ -29,6 +29,7 @@ import { batch, type WritableSignal, type ComputedSignal, + type Signal, } from "@/lib/reactive/signal"; import { Contours, @@ -40,6 +41,12 @@ import { } from "@shift/font"; import { Bounds, Curve, Vec2, type Bounds as BoundsType } from "@shift/geo"; import type { NodePositionUpdate, NodePositionUpdateList } from "@/types/positionUpdate"; +import { Segment } from "@/lib/model/Segment"; + +export interface GlyphSidebearings { + readonly lsb: number | null; + readonly rsb: number | null; +} export type GlyphChange = GlyphSnapshot | NodePositionUpdateList; @@ -108,6 +115,29 @@ export class Contour { yield* Contours.withNeighbors(this); } + *segments(): Generator { + yield* Segment.parse(this.#points.value, this.#closed.value); + } + + /** + * Tight bounds for the subset of this contour's points in `ids`. + * Fully-selected segments contribute their bezier envelope; partially-selected + * segments contribute the raw points of their selected endpoints. + */ + selectionBounds(ids: ReadonlySet): BoundsType | null { + const parts: (BoundsType | null)[] = []; + + for (const segment of this.segments()) { + if (segment.pointIds.every((id) => ids.has(id))) { + parts.push(segment.bounds); + } + } + + parts.push(Bounds.fromPoints(this.#points.value.filter((p) => ids.has(p.id)))); + + return Bounds.unionAll(parts); + } + canClose(position: Point2D, hitRadius: number): boolean { return Contours.canClose(this, position, hitRadius); } @@ -136,6 +166,7 @@ export class Glyph { readonly #activeContourId: WritableSignal; readonly #path: ComputedSignal; readonly #bbox: ComputedSignal; + readonly #sidebearings: ComputedSignal; constructor(bridge: NativeBridge) { this.#bridge = bridge; @@ -178,12 +209,41 @@ export class Glyph { if (all.length === 0) return null; return Bounds.unionAll(all); }); + + // Point-based x-range — cheap, matches integer-rounded sidebar display. + // Avoids warming the bezier #bbox chain which transitively computes + // every contour's bezier bounds. Consumers use `useGlyphSidebearings` + // React hook to subscribe; not exposed as a `$sidebearings` signal to + // prevent accidental subscription-driven recomputation. + this.#sidebearings = computed(() => { + let minX = Infinity; + let maxX = -Infinity; + for (const contour of this.#contours.value) { + for (const p of contour.points) { + if (p.x < minX) minX = p.x; + if (p.x > maxX) maxX = p.x; + } + } + if (minX === Infinity) return { lsb: null, rsb: null }; + return { lsb: minX, rsb: this.#xAdvance.value - maxX }; + }); } get contours(): readonly Contour[] { return this.#contours.value; } + /** + * @knipclassignore — used by purpose-specific React hooks in `@/hooks/` + * Signal that fires once per structural/position change — use this to + * subscribe to "something about the glyph changed" without forcing the + * bounds / path / bbox computeds to eagerly recompute. Consumers pull + * derived values on demand after the signal fires. + */ + get $contours(): Signal { + return this.#contours; + } + get xAdvance(): number { return this.#xAdvance.value; } @@ -211,6 +271,20 @@ export class Glyph { return this.#bbox.value; } + /** + * @knipclassignore — used by Editor command path and `useGlyphSidebearings` + * Sidebearings (point-based x-range) — pull at read time; for React live + * display use `useGlyphSidebearings()`. + */ + get sidebearings(): GlyphSidebearings { + return this.#sidebearings.value; + } + + /** @knipclassignore — subscribed by `useGlyphXAdvance` hook */ + get $xAdvance(): Signal { + return this.#xAdvance; + } + /** @knipclassignore Fill the glyph's complete path using the theme's glyph fill color. */ draw(canvas: Canvas): void { canvas.fillPath(this.path, canvas.theme.glyph.fill); @@ -241,6 +315,15 @@ export class Glyph { return Glyphs.getAllPoints(this); } + /** @knipclassignore */ + *segments(): Generator<{ segment: Segment; contourId: ContourId }> { + for (const contour of this.#contours.value) { + for (const segment of contour.segments()) { + yield { segment, contourId: contour.id }; + } + } + } + /** @knipclassignore */ getPointAt(pos: Point2D, radius: number): Point | null { return Glyphs.getPointAt(this, pos, radius); diff --git a/apps/desktop/src/renderer/src/lib/model/Segment.test.ts b/apps/desktop/src/renderer/src/lib/model/Segment.test.ts new file mode 100644 index 00000000..851b7db0 --- /dev/null +++ b/apps/desktop/src/renderer/src/lib/model/Segment.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect } from "vitest"; +import { Segment } from "./Segment"; +import type { LineSegment, QuadSegment, CubicSegment } from "@/types/segments"; +import { asPointId } from "@shift/types"; + +const pt = (id: string, x: number, y: number, pointType: "onCurve" | "offCurve" = "onCurve") => ({ + id: asPointId(id), + x, + y, + pointType, + smooth: false, +}); + +const line = (): LineSegment => ({ + type: "line", + points: { anchor1: pt("p1", 0, 0), anchor2: pt("p2", 100, 0) }, +}); + +const quad = (): QuadSegment => ({ + type: "quad", + points: { + anchor1: pt("p1", 0, 0), + control: pt("c1", 50, 100, "offCurve"), + anchor2: pt("p2", 100, 0), + }, +}); + +const cubic = (): CubicSegment => ({ + type: "cubic", + points: { + anchor1: pt("p1", 0, 0), + control1: pt("c1", 25, 100, "offCurve"), + control2: pt("c2", 75, 100, "offCurve"), + anchor2: pt("p2", 100, 0), + }, +}); + +describe("Segment", () => { + it("id uses anchor point ids", () => { + expect(new Segment(line()).id).toBe("p1:p2"); + expect(new Segment(quad()).id).toBe("p1:p2"); + expect(new Segment(cubic()).id).toBe("p1:p2"); + }); + + it("exposes type and anchors", () => { + const seg = new Segment(cubic()); + expect(seg.type).toBe("cubic"); + expect(seg.anchor1.id).toBe("p1"); + expect(seg.anchor2.id).toBe("p2"); + }); + + it("pointIds includes controls per variant", () => { + expect(new Segment(line()).pointIds).toEqual(["p1", "p2"]); + expect(new Segment(quad()).pointIds).toEqual(["p1", "c1", "p2"]); + expect(new Segment(cubic()).pointIds).toEqual(["p1", "c1", "c2", "p2"]); + }); + + it("toCurve returns matching curve type", () => { + expect(new Segment(line()).toCurve().type).toBe("line"); + expect(new Segment(quad()).toCurve().type).toBe("quadratic"); + expect(new Segment(cubic()).toCurve().type).toBe("cubic"); + }); + + it("bounds of a line are anchor extents", () => { + const b = new Segment(line()).bounds; + expect(b.min).toEqual({ x: 0, y: 0 }); + expect(b.max).toEqual({ x: 100, y: 0 }); + }); + + it("bounds of a cubic include control envelope", () => { + const b = new Segment(cubic()).bounds; + expect(b.min.x).toBeGreaterThanOrEqual(0); + expect(b.max.y).toBeGreaterThan(0); + }); + + describe("hitTest", () => { + it("hits a line within radius", () => { + const hit = new Segment(line()).hitTest({ x: 50, y: 2 }, 5); + expect(hit).not.toBeNull(); + expect(hit!.segmentId).toBe("p1:p2"); + expect(hit!.t).toBeCloseTo(0.5, 1); + expect(hit!.point.x).toBeCloseTo(50, 1); + }); + + it("misses when beyond radius", () => { + expect(new Segment(line()).hitTest({ x: 50, y: 10 }, 5)).toBeNull(); + }); + + it("rejects based on bounds cheaply", () => { + expect(new Segment(line()).hitTest({ x: 500, y: 500 }, 5)).toBeNull(); + }); + + it("hits a cubic near endpoints", () => { + const hit = new Segment(cubic()).hitTest({ x: 0, y: 0 }, 5); + expect(hit).not.toBeNull(); + }); + }); + + describe("hitTestMultiple", () => { + it("picks the closest hit", () => { + const a = new Segment(line()); + const b = new Segment({ + type: "line", + points: { anchor1: pt("p3", 0, 50), anchor2: pt("p4", 100, 50) }, + }); + const hit = Segment.hitTestMultiple([a, b], { x: 50, y: 1 }, 10); + expect(hit!.segmentId).toBe("p1:p2"); + }); + + it("returns null when nothing hits", () => { + expect(Segment.hitTestMultiple([new Segment(line())], { x: 50, y: 100 }, 5)).toBeNull(); + }); + + it("returns null for empty input", () => { + expect(Segment.hitTestMultiple([], { x: 0, y: 0 }, 5)).toBeNull(); + }); + }); + + describe("parse", () => { + it("returns empty for fewer than 2 points", () => { + expect(Segment.parse([], false)).toEqual([]); + expect(Segment.parse([pt("p1", 0, 0)], false)).toEqual([]); + }); + + it("parses onCurve pairs into line segments", () => { + const segments = Segment.parse([pt("p1", 0, 0), pt("p2", 100, 0)], false); + expect(segments).toHaveLength(1); + expect(segments[0]!.type).toBe("line"); + }); + + it("parses quad and cubic patterns", () => { + const q = Segment.parse( + [pt("p1", 0, 0), pt("c1", 50, 100, "offCurve"), pt("p2", 100, 0)], + false, + ); + expect(q).toHaveLength(1); + expect(q[0]!.type).toBe("quad"); + + const c = Segment.parse( + [ + pt("p1", 0, 0), + pt("c1", 25, 100, "offCurve"), + pt("c2", 75, 100, "offCurve"), + pt("p2", 100, 0), + ], + false, + ); + expect(c).toHaveLength(1); + expect(c[0]!.type).toBe("cubic"); + }); + + it("closes the contour with a wrap-around line", () => { + const segments = Segment.parse([pt("p1", 0, 0), pt("p2", 100, 0), pt("p3", 50, 87)], true); + expect(segments).toHaveLength(3); + expect(segments.every((s) => s.type === "line")).toBe(true); + expect(segments[2]!.anchor1.id).toBe("p3"); + expect(segments[2]!.anchor2.id).toBe("p1"); + }); + }); +}); diff --git a/apps/desktop/src/renderer/src/lib/model/Segment.ts b/apps/desktop/src/renderer/src/lib/model/Segment.ts new file mode 100644 index 00000000..7d9ba29c --- /dev/null +++ b/apps/desktop/src/renderer/src/lib/model/Segment.ts @@ -0,0 +1,170 @@ +import { Curve, Vec2, type Bounds, type CurveType, type Point2D } from "@shift/geo"; +import { parseContourSegments, segmentToCurve } from "@shift/font"; +import type { PointId, Point } from "@shift/types"; +import type { + SegmentPoint, + SegmentType, + LineSegment, + QuadSegment, + CubicSegment, +} from "@/types/segments"; +import type { SegmentId } from "@/types/indicator"; +import { asSegmentId } from "@/types/indicator"; + +export interface SegmentHitResult { + segment: Segment; + segmentId: SegmentId; + t: number; + point: Point2D; + distance: number; +} + +/** + * Segment — font-editing segment with point IDs. + * + * Bridges pure curves (`@shift/geo` — no IDs) and font segments (have IDs). + * Wraps a discriminated {@link SegmentType} and exposes id-aware operations + * as instance methods. + */ +export class Segment { + readonly #data: SegmentType; + + constructor(data: SegmentType) { + this.#data = data; + } + + /** Parse a contour's points into segment instances. */ + static parse(points: readonly Point[], closed: boolean): Segment[] { + return parseContourSegments({ points, closed }).map((geom) => new Segment(geom as SegmentType)); + } + + /** Find the closest-hit segment in a collection. */ + static hitTestMultiple( + segments: readonly Segment[], + pos: Point2D, + radius: number, + ): SegmentHitResult | null { + let best: SegmentHitResult | null = null; + + for (const segment of segments) { + const hit = segment.hitTest(pos, radius); + if (hit && (best === null || hit.distance < best.distance)) { + best = hit; + } + } + + return best; + } + + get type(): SegmentType["type"] { + return this.#data.type; + } + + get anchor1(): SegmentPoint { + return this.#data.points.anchor1; + } + + get anchor2(): SegmentPoint { + return this.#data.points.anchor2; + } + + get id(): SegmentId { + return asSegmentId(`${this.#data.points.anchor1.id}:${this.#data.points.anchor2.id}`); + } + + get pointIds(): PointId[] { + switch (this.#data.type) { + case "line": + return [this.#data.points.anchor1.id, this.#data.points.anchor2.id]; + case "quad": + return [ + this.#data.points.anchor1.id, + this.#data.points.control.id, + this.#data.points.anchor2.id, + ]; + case "cubic": + return [ + this.#data.points.anchor1.id, + this.#data.points.control1.id, + this.#data.points.control2.id, + this.#data.points.anchor2.id, + ]; + } + } + + get bounds(): Bounds { + return Curve.bounds(this.toCurve()); + } + + /** @knipclassignore */ + get length(): number { + return Curve.length(this.toCurve()); + } + + /** @internal Raw discriminated data for rendering / clipboard interop. */ + get raw(): SegmentType { + return this.#data; + } + + toCurve(): CurveType { + return segmentToCurve(this.#data); + } + + /** @knipclassignore */ + pointAt(t: number): Point2D { + return Curve.pointAt(this.toCurve(), t); + } + + /** @knipclassignore */ + closestPoint(pos: Point2D) { + return Curve.closestPoint(this.toCurve(), pos); + } + + /** @knipclassignore */ + splitAt(t: number): [CurveType, CurveType] { + return Curve.splitAt(this.toCurve(), t); + } + + /** @knipclassignore */ + sample(count: number): Point2D[] { + return Curve.sample(this.toCurve(), count); + } + + /** @knipclassignore Narrow to a line segment, or null if this isn't one. */ + asLine(): LineSegment | null { + return this.#data.type === "line" ? this.#data : null; + } + + /** Narrow to a quad segment, or null if this isn't one. */ + asQuad(): QuadSegment | null { + return this.#data.type === "quad" ? this.#data : null; + } + + /** Narrow to a cubic segment, or null if this isn't one. */ + asCubic(): CubicSegment | null { + return this.#data.type === "cubic" ? this.#data : null; + } + + hitTest(pos: Point2D, radius: number): SegmentHitResult | null { + const b = this.bounds; + const r = { x: radius, y: radius }; + const lo = Vec2.sub(b.min, r); + const hi = Vec2.add(b.max, r); + + if (pos.x < lo.x || pos.x > hi.x || pos.y < lo.y || pos.y > hi.y) return null; + + const closest = Curve.closestPoint(this.toCurve(), pos); + + if (closest.distance < radius) { + return { + segment: this, + segmentId: this.id, + t: closest.t, + point: closest.point, + distance: closest.distance, + }; + } + + return null; + } +} diff --git a/apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md b/apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md index 3fe5e710..c373a0b2 100644 --- a/apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md +++ b/apps/desktop/src/renderer/src/lib/reactive/docs/DOCS.md @@ -11,17 +11,24 @@ Fine-grained reactivity system providing automatic dependency tracking and effic - **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. ## Codemap ``` reactive/ signal.ts — signal, computed, effect, batch, untracked, isTracking - useSignal.ts — useSignalState (React bridge via useSyncExternalStore) + useSignal.ts — useSignalState, useSignalTrigger (React bridges) index.ts — public re-exports signal.test.ts — unit tests (vitest) ``` +Purpose-specific hooks for derived values live under `hooks/`: + +- `useSelectionBounds` — current selection bounds, pulled at render time. +- `useGlyphSidebearings` — current LSB/RSB, pulled at render time. +- `useGlyphXAdvance` — current xAdvance. + ## Key Types - `Signal` -- read-only reactive value. `.value` tracks dependencies; `.peek()` reads without tracking. diff --git a/apps/desktop/src/renderer/src/lib/reactive/index.ts b/apps/desktop/src/renderer/src/lib/reactive/index.ts index 59e040f5..597ffb0d 100644 --- a/apps/desktop/src/renderer/src/lib/reactive/index.ts +++ b/apps/desktop/src/renderer/src/lib/reactive/index.ts @@ -1,3 +1,3 @@ export { signal, computed, effect, batch, untracked, isTracking } from "./signal"; export type { Signal, WritableSignal, ComputedSignal, Effect } from "./signal"; -export { useSignalState } from "./useSignal"; +export { useSignalState, useSignalTrigger } from "./useSignal"; diff --git a/apps/desktop/src/renderer/src/lib/reactive/useSignal.ts b/apps/desktop/src/renderer/src/lib/reactive/useSignal.ts index 8adb3fad..603cdf63 100644 --- a/apps/desktop/src/renderer/src/lib/reactive/useSignal.ts +++ b/apps/desktop/src/renderer/src/lib/reactive/useSignal.ts @@ -23,3 +23,25 @@ export function useSignalState(signal: Signal): T { () => signal.peek(), ); } + +/** + * Subscribe to a signal purely for re-render side effects — returns nothing. + * Use when the component needs to re-render when a signal fires but doesn't + * need the value (e.g. subscribing to a coarse "something changed" signal + * while pulling derived values on demand elsewhere). + * + * Pass `null` to opt out of subscription — safe to call conditionally. + */ +export function useSignalTrigger(signal: Signal | null | undefined): void { + useSyncExternalStore( + (callback) => { + if (!signal) return () => {}; + const fx = effect(() => { + signal.value; + callback(); + }); + return () => fx.dispose(); + }, + () => (signal ? signal.peek() : null), + ); +} diff --git a/apps/desktop/src/renderer/src/lib/tools/core/ToolManager.test.ts b/apps/desktop/src/renderer/src/lib/tools/core/ToolManager.test.ts index af2b4380..820de7b9 100644 --- a/apps/desktop/src/renderer/src/lib/tools/core/ToolManager.test.ts +++ b/apps/desktop/src/renderer/src/lib/tools/core/ToolManager.test.ts @@ -80,12 +80,12 @@ describe("ToolManager", () => { it("should not interfere with zoom operations", () => { toolManager.activate("pen"); - const initialCursor = editor.getCursor(); + const initialCursor = editor.cursor; toolManager.handleKeyDown(createKeyboardEvent("keydown", { key: "Meta", metaKey: true })); toolManager.handleKeyUp(createKeyboardEvent("keyup", { key: "Meta" })); - expect(editor.getCursor()).toBe(initialCursor); + expect(editor.cursor).toBe(initialCursor); expect(toolManager.activeToolId).toBe("pen"); }); }); @@ -255,7 +255,7 @@ describe("ToolManager", () => { toolManager.handlePointerDown({ x: 0, y: 0 }, mods); - expect(editor.getCurrentModifiers()).toEqual(mods); + expect(editor.currentModifiers).toEqual(mods); }); it("updates currentModifiers on flushPointerMove", () => { @@ -270,7 +270,7 @@ describe("ToolManager", () => { toolManager.handlePointerMove({ x: 10, y: 10 }, mods); - expect(editor.getCurrentModifiers()).toEqual(mods); + expect(editor.currentModifiers).toEqual(mods); } finally { vi.stubGlobal("requestAnimationFrame", originalRAF); } @@ -283,7 +283,7 @@ describe("ToolManager", () => { createKeyboardEvent("keydown", { key: "Alt", altKey: true, shiftKey: false }), ); - expect(editor.getCurrentModifiers()).toEqual({ + expect(editor.currentModifiers).toEqual({ shiftKey: false, altKey: true, metaKey: false, @@ -296,7 +296,7 @@ describe("ToolManager", () => { toolManager.handleKeyUp(createKeyboardEvent("keyup", { key: "Alt", altKey: false })); - expect(editor.getCurrentModifiers()).toEqual({ + expect(editor.currentModifiers).toEqual({ shiftKey: false, altKey: false, metaKey: false, 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 4e79a0cc..e4cee650 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/Select.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/Select.ts @@ -75,10 +75,10 @@ export class Select extends BaseTool { }); } - const bbHandle = this.editor.getHoveredBoundingBoxHandle(); + const bbHandle = this.editor.hoveredBoundingBoxHandle; if (bbHandle) return boundingBoxHitResultToCursor(bbHandle); - if (this.editor.getCurrentModifiers().altKey && this.editor.getIsHoveringNode()) { + if (this.editor.currentModifiers.altKey && this.editor.isHoveringNode) { return { type: "copy" }; } 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 6c4a0858..0adff76d 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 @@ -19,7 +19,9 @@ export class BendCurve implements SelectHandlerBehavior { if (!hit || hit.type !== "segment" || hit.segment.type !== "cubic") return false; const { t, closestPoint, segmentId, segment } = hit; - const { control1, control2 } = segment.points; + const cubic = segment.asCubic(); + if (!cubic) return true; + const { control1, control2 } = cubic.points; this.#draft = ctx.editor.createDraft(); this.#hasChanges = false; @@ -57,7 +59,9 @@ export class BendCurve implements SelectHandlerBehavior { const { initialControlOne, initialControlTwo } = state.bend; const newCp1 = Vec2.add(initialControlOne, delta1); const newCp2 = Vec2.add(initialControlTwo, delta2); - const { control1, control2 } = segment.points; + const cubic = segment.asCubic(); + if (!cubic) return true; + const { control1, control2 } = cubic.points; const updates = [ { node: { kind: "point" as const, id: control1.id }, x: newCp1.x, y: newCp1.y }, diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ContourDoubleClick.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ContourDoubleClick.ts index fca2eeb0..56e77110 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ContourDoubleClick.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/ContourDoubleClick.ts @@ -4,7 +4,6 @@ import type { Editor } from "@/lib/editor/Editor"; import type { SelectHandlerBehavior, SelectState } from "../types"; import type { PointId } from "@shift/types"; import type { SegmentId } from "@/types/indicator"; -import { Segments as SegmentOps } from "@/lib/geo/Segments"; import { isSegmentHit } from "@/types/hitResult"; export class ContourDoubleClick implements SelectHandlerBehavior { @@ -31,9 +30,8 @@ export class ContourDoubleClick implements SelectHandlerBehavior { if (!glyph) return null; for (const contour of glyph.contours) { - const segments = SegmentOps.parse(contour.points, contour.closed); - for (const seg of segments) { - if (SegmentOps.id(seg) === segmentId) { + for (const segment of contour.segments()) { + if (segment.id === segmentId) { return contour.points.map((point) => point.id); } } 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 2b63a094..b4ee52b4 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 @@ -4,7 +4,6 @@ import type { Editor } from "@/lib/editor/Editor"; import type { SelectHandlerBehavior, SelectState } from "../types"; import type { PointId, AnchorId } from "@shift/types"; import type { SegmentId } from "@/types/indicator"; -import { Segments as SegmentOps } from "@/lib/geo/Segments"; import { getPointIdFromHit, isAnchorHit, isSegmentHit } from "@/types/hitResult"; function nextSelectionStateAfterToggle( @@ -123,7 +122,7 @@ export class Selection implements SelectHandlerBehavior { private selectSegment(editor: Editor, segmentId: SegmentId, additive: boolean): void { const segment = editor.getSegmentById(segmentId); if (!segment) return; - const pointIds = SegmentOps.getPointIds(segment); + const pointIds = segment.pointIds; if (additive) { editor.selection.add({ kind: "segment", id: segmentId }); @@ -145,7 +144,7 @@ export class Selection implements SelectHandlerBehavior { const segment = editor.getSegmentById(segmentId); if (!segment) return; - const pointIds = SegmentOps.getPointIds(segment); + const pointIds = segment.pointIds; for (const pointId of pointIds) { if (wasSelected) { diff --git a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/TextRunEdit.ts b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/TextRunEdit.ts index 79368eeb..7dd312aa 100644 --- a/apps/desktop/src/renderer/src/lib/tools/select/behaviors/TextRunEdit.ts +++ b/apps/desktop/src/renderer/src/lib/tools/select/behaviors/TextRunEdit.ts @@ -28,7 +28,7 @@ export class TextRunEdit implements SelectHandlerBehavior { glyphName: activeName, unicode: ctx.editor.getActiveGlyphUnicode(), }); - ctrl.setOriginX(ctx.editor.getDrawOffset().x); + ctrl.setOriginX(ctx.editor.drawOffset.x); textRunState = ctrl.state.value; } if (!textRunState) return false; 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 3f25fe50..b42dd367 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 @@ -7,7 +7,6 @@ import type { DragTarget } from "../types"; import type { ToolEventOf } from "../../core/GestureDetector"; import type { SelectHandlerBehavior, SelectState } from "../types"; import type { SegmentId } from "@/types/indicator"; -import { Segments as SegmentOps } from "@/lib/geo/Segments"; import { getPointIdFromHit, isAnchorHit, isSegmentHit } from "@/types/hitResult"; import type { DragSnapSession } from "@/lib/editor/snapping/types"; import type { GlyphDraft } from "@/types/draft"; @@ -179,7 +178,7 @@ export class Translate implements SelectHandlerBehavior { } if (isSegmentHit(hit)) { - const pointIds = SegmentOps.getPointIds(hit.segment); + const pointIds = hit.segment.pointIds; const isSelected = state.type === "selected" && editor.selection.isSelected({ kind: "segment", id: hit.segmentId }); @@ -284,7 +283,7 @@ export class Translate implements SelectHandlerBehavior { private selectSegment(editor: Editor, segmentId: SegmentId, additive: boolean): void { const segment = editor.getSegmentById(segmentId); if (!segment) return; - const pointIds = SegmentOps.getPointIds(segment); + const pointIds = segment.pointIds; if (additive) { editor.selection.add({ kind: "segment", id: segmentId }); 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 1beb195c..620c5beb 100644 --- a/apps/desktop/src/renderer/src/lib/tools/text/Text.ts +++ b/apps/desktop/src/renderer/src/lib/tools/text/Text.ts @@ -21,7 +21,7 @@ export class Text extends BaseTool { override activate(): void { const ctrl = this.editor.textRunController; const hasExistingRun = ctrl.length > 0; - const drawOffset = this.editor.getDrawOffset(); + const drawOffset = this.editor.drawOffset; const activeGlyphName = this.editor.getActiveGlyphName(); const activeUnicode = this.editor.getActiveGlyphUnicode(); const activeGlyph = diff --git a/apps/desktop/src/renderer/src/lib/tools/text/layout.ts b/apps/desktop/src/renderer/src/lib/tools/text/layout.ts index 9272d546..a2135500 100644 --- a/apps/desktop/src/renderer/src/lib/tools/text/layout.ts +++ b/apps/desktop/src/renderer/src/lib/tools/text/layout.ts @@ -1,9 +1,7 @@ import type { Point2D, FontMetrics } from "@shift/types"; import type { Bounds } from "@shift/geo"; import type { Font } from "@/lib/model/Font"; -import { isLikelyNonSpacingGlyphRef } from "@/lib/utils/unicode"; - -const NON_SPACING_EDITOR_ADVANCE = 600; +import { displayAdvance } from "@/lib/utils/unicode"; export interface GlyphRef { glyphName: string; @@ -120,11 +118,7 @@ export function computeTextLayout(glyphs: GlyphRef[], origin: Point2D, font: Fon export { NEWLINE_GLYPH_NAME }; function resolveEditorAdvance(glyph: GlyphRef, advance: number): number { - if (advance > 0) return advance; - if (!isLikelyNonSpacingGlyphRef(glyph.glyphName, glyph.unicode)) { - return advance; - } - return NON_SPACING_EDITOR_ADVANCE; + return displayAdvance(advance, glyph.glyphName, glyph.unicode); } function isWithinSlotVerticalBounds( diff --git a/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.test.ts b/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.test.ts deleted file mode 100644 index dfc5369c..00000000 --- a/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.test.ts +++ /dev/null @@ -1,117 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { getSegmentAwareBounds } from "./SelectionBounds"; -import { Bounds } from "@shift/geo"; -import type { GlyphSnapshot } from "@shift/types"; -import { asContourId, asPointId } from "@shift/types"; - -function makePoint(id: string, x: number, y: number, type: "onCurve" | "offCurve" = "onCurve") { - return { id: asPointId(id), x, y, pointType: type, smooth: false }; -} - -function makeSnapshot(points: ReturnType[]): GlyphSnapshot { - return { - unicode: 65, - name: "A", - xAdvance: 500, - contours: [ - { - id: asContourId("c1"), - points, - closed: false, - }, - ], - anchors: [], - compositeContours: [], - activeContourId: asContourId("c1"), - }; -} - -describe("getSegmentAwareBounds", () => { - it("returns null for empty selection", () => { - const snapshot = makeSnapshot([makePoint("p1", 0, 0), makePoint("p2", 100, 100)]); - const result = getSegmentAwareBounds(snapshot, []); - expect(result).toBeNull(); - }); - - it("calculates bounds for line segment (same as point AABB)", () => { - const snapshot = makeSnapshot([makePoint("p1", 0, 0), makePoint("p2", 100, 50)]); - const selectedPoints = [asPointId("p1"), asPointId("p2")]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).not.toBeNull(); - expect(result!.min.x).toBe(0); - expect(result!.min.y).toBe(0); - expect(result!.max.x).toBe(100); - expect(result!.max.y).toBe(50); - }); - - it("calculates segment-aware bounds for quadratic curve with vertical bulge", () => { - const snapshot = makeSnapshot([ - makePoint("p1", 0, 0), - makePoint("ctrl", 50, 200, "offCurve"), - makePoint("p2", 100, 0), - ]); - const selectedPoints = [asPointId("p1"), asPointId("ctrl"), asPointId("p2")]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).not.toBeNull(); - expect(result!.min.x).toBe(0); - expect(result!.min.y).toBe(0); - expect(result!.max.x).toBe(100); - expect(result!.max.y).toBe(100); - }); - - it("calculates segment-aware bounds for cubic S-curve", () => { - const snapshot = makeSnapshot([ - makePoint("p1", 0, 50), - makePoint("ctrl1", 0, 150, "offCurve"), - makePoint("ctrl2", 100, -50, "offCurve"), - makePoint("p2", 100, 50), - ]); - const selectedPoints = [ - asPointId("p1"), - asPointId("ctrl1"), - asPointId("ctrl2"), - asPointId("p2"), - ]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).not.toBeNull(); - expect(result!.min.x).toBe(0); - expect(result!.max.x).toBe(100); - expect(result!.min.y).toBeLessThan(50); - expect(result!.max.y).toBeGreaterThan(50); - }); - - it("uses point bounds for partially selected curves", () => { - const snapshot = makeSnapshot([ - makePoint("p1", 0, 0), - makePoint("ctrl", 50, 200, "offCurve"), - makePoint("p2", 100, 0), - ]); - const selectedPoints = [asPointId("p1"), asPointId("ctrl")]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).not.toBeNull(); - expect(result!.max.y).toBe(200); - }); - - it("returns null when selected points don't exist in snapshot", () => { - const snapshot = makeSnapshot([makePoint("p1", 0, 0)]); - const selectedPoints = [asPointId("nonexistent")]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).toBeNull(); - }); - - it("calculates center correctly", () => { - const snapshot = makeSnapshot([makePoint("p1", 0, 0), makePoint("p2", 100, 100)]); - const selectedPoints = [asPointId("p1"), asPointId("p2")]; - const result = getSegmentAwareBounds(snapshot, selectedPoints); - - expect(result).not.toBeNull(); - const center = Bounds.center(result!); - expect(center.x).toBe(50); - expect(center.y).toBe(50); - }); -}); diff --git a/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.ts b/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.ts deleted file mode 100644 index 685c75a2..00000000 --- a/apps/desktop/src/renderer/src/lib/transform/SelectionBounds.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { Segment } from "@/lib/geo/Segment"; -import { Bounds } from "@shift/geo"; -import type { PointId } from "@shift/types"; -import type { Glyph } from "@/lib/model/Glyph"; - -/** - * Compute a tight bounding box for the current selection that accounts for - * curve geometry. When every point of a segment is selected, the segment's - * full bezier bounds are used (which may extend beyond the on-curve points). - * Points that belong to only partially-selected segments are treated as - * isolated coordinates. - * - * Returns `null` when `selectedPointIds` is empty. - */ -export function getSegmentAwareBounds( - glyph: Glyph, - selectedPointIds: readonly PointId[], -): Bounds | null { - if (selectedPointIds.length === 0) return null; - - const selectedSet = new Set(selectedPointIds); - - const segBounds: (Bounds | null)[] = []; - const processedSegments = new Set(); - const pointsInFullSegments = new Set(); - - for (const { segment } of Segment.iterateGlyph(glyph.contours)) { - const segmentPointIds = Segment.getPointIds(segment); - const allSelected = segmentPointIds.every((id) => selectedSet.has(id)); - - if (allSelected) { - const segKey = segmentPointIds.join(":"); - if (!processedSegments.has(segKey)) { - processedSegments.add(segKey); - for (const id of segmentPointIds) { - pointsInFullSegments.add(id); - } - segBounds.push(Segment.bounds(segment)); - } - } - } - - const isolatedPoints: { x: number; y: number }[] = []; - for (const contour of glyph.contours) { - for (const point of contour.points) { - if (selectedSet.has(point.id) && !pointsInFullSegments.has(point.id)) { - isolatedPoints.push(point); - } - } - } - - const ptBounds = Bounds.fromPoints(isolatedPoints); - return Bounds.unionAll([...segBounds, ptBounds]); -} diff --git a/apps/desktop/src/renderer/src/lib/transform/index.ts b/apps/desktop/src/renderer/src/lib/transform/index.ts index 15b0ed4a..64f7bb2e 100644 --- a/apps/desktop/src/renderer/src/lib/transform/index.ts +++ b/apps/desktop/src/renderer/src/lib/transform/index.ts @@ -41,9 +41,6 @@ export { anchorToPoint } from "./anchor"; // Zoom-from-wheel (viewport zoom sensitivity) export { zoomMultiplierFromWheel, type ZoomFromWheelOptions } from "./zoomFromWheel"; -// Selection bounds utilities -export { getSegmentAwareBounds } from "./SelectionBounds"; - // Commands for undo/redo (re-export from commands/transform) export { RotatePointsCommand, diff --git a/apps/desktop/src/renderer/src/lib/utils/unicode.ts b/apps/desktop/src/renderer/src/lib/utils/unicode.ts index d1056918..62a2cb1a 100644 --- a/apps/desktop/src/renderer/src/lib/utils/unicode.ts +++ b/apps/desktop/src/renderer/src/lib/utils/unicode.ts @@ -72,9 +72,27 @@ export function isNonSpacingMarkCodepoint(codepoint: number | null): boolean { } /** - * Heuristic detection for non-spacing glyph refs used in editor-only visual layout. + * Heuristic check for non-spacing glyphs (combining marks) — combines the + * Unicode Mn/Me categories with a glyph-name token match (`comb` / `mark`). + * Used in editor-only visual layout to give combining glyphs a visible advance. */ -export function isLikelyNonSpacingGlyphRef(glyphName: string, unicode: number | null): boolean { +export function isNonSpacingGlyph(glyphName: string, unicode: number | null): boolean { if (isNonSpacingMarkCodepoint(unicode)) return true; return /(?:^|[._-])(comb|mark)(?:$|[._-])/i.test(glyphName); } + +/** + * Editor-only advance used when a non-spacing glyph reports zero xAdvance — + * without this, combining marks would render with no visible width. + */ +export const NON_SPACING_EDITOR_ADVANCE = 600; + +/** + * Advance used for on-canvas layout (guides, text runs). Matches the glyph's + * xAdvance except for non-spacing glyphs at zero, which get a visible fallback. + */ +export function displayAdvance(advance: number, glyphName: string, unicode: number | null): number { + if (advance > 0) return advance; + if (!isNonSpacingGlyph(glyphName, unicode)) return advance; + return NON_SPACING_EDITOR_ADVANCE; +} diff --git a/apps/desktop/src/renderer/src/perf/interaction.bench.ts b/apps/desktop/src/renderer/src/perf/interaction.bench.ts index 802bcd15..cf8dcb0d 100644 --- a/apps/desktop/src/renderer/src/perf/interaction.bench.ts +++ b/apps/desktop/src/renderer/src/perf/interaction.bench.ts @@ -12,7 +12,6 @@ */ import { bench, describe } from "vitest"; -import { Glyphs } from "@shift/font"; import { createPointMark, type PointScale } from "@/testing/pointMark"; const DRAG_FRAMES = 30; @@ -22,7 +21,7 @@ function setupDrag(scale: PointScale) { pm.editor.selectTool("select"); const glyph = pm.editor.currentGlyph!; - const firstPoint = Glyphs.getAllPoints(glyph)[0]; + const firstPoint = glyph.allPoints[0]; const startX = firstPoint.x; const startY = firstPoint.y; diff --git a/apps/desktop/src/renderer/src/types/hitResult.ts b/apps/desktop/src/renderer/src/types/hitResult.ts index 2518146c..80caa49d 100644 --- a/apps/desktop/src/renderer/src/types/hitResult.ts +++ b/apps/desktop/src/renderer/src/types/hitResult.ts @@ -14,7 +14,7 @@ * @module */ import type { Point2D, PointId, ContourId, Point, Contour, AnchorId } from "@shift/types"; -import type { Segment } from "./segments"; +import type { Segment } from "@/lib/model/Segment"; import type { SegmentId } from "./indicator"; import type { BoundingBoxHitResult } from "./boundingBox"; diff --git a/apps/desktop/src/renderer/src/types/segments.ts b/apps/desktop/src/renderer/src/types/segments.ts index ce178136..9ca6f4f4 100644 --- a/apps/desktop/src/renderer/src/types/segments.ts +++ b/apps/desktop/src/renderer/src/types/segments.ts @@ -51,6 +51,8 @@ export type CubicSegment = { }; /** - * A segment in a contour - either a line or bezier curve. + * Raw discriminated data for a segment in a contour — line, quad, or cubic. + * The public-facing API is the `Segment` class in `lib/model/Segment.ts`, + * which wraps this data and provides id-aware operations. */ -export type Segment = LineSegment | QuadSegment | CubicSegment; +export type SegmentType = LineSegment | QuadSegment | CubicSegment; diff --git a/apps/desktop/src/renderer/src/types/selection.ts b/apps/desktop/src/renderer/src/types/selection.ts index 73bacbf3..8efceb99 100644 --- a/apps/desktop/src/renderer/src/types/selection.ts +++ b/apps/desktop/src/renderer/src/types/selection.ts @@ -1,4 +1,4 @@ -import type { PointId, ContourId, AnchorId } from "@shift/types"; +import type { PointId, ContourId, AnchorId, Point2D } from "@shift/types"; import type { SegmentId } from "./indicator"; import type { Glyph } from "@/lib/model/Glyph"; import type { SelectionMode } from "./editor"; @@ -9,7 +9,7 @@ import { type Signal, type ComputedSignal, } from "@/lib/reactive/signal"; -import { Glyphs } from "@shift/font"; +import { Bounds, type Bounds as BoundsType } from "@shift/geo"; /** Discriminated reference to any selectable entity. */ export type Selectable = @@ -47,6 +47,7 @@ export class Selection { readonly #$segmentIds: WritableSignal>; readonly #$mode: WritableSignal; readonly #$derived: ComputedSignal; + readonly #$bounds: ComputedSignal; constructor(glyph: Signal) { this.#$pointIds = signal>(new Set()); @@ -65,17 +66,19 @@ export class Selection { const contourIds = new Set(); const contourTotalCounts = new Map(); - for (const { point, contour } of Glyphs.points(g)) { - pointToContour.set(point.id, contour.id); - contourTotalCounts.set(contour.id, (contourTotalCounts.get(contour.id) ?? 0) + 1); - - if (pointIds.has(point.id)) { - contourIds.add(contour.id); - const list = contourToPoints.get(contour.id); - if (list) { - list.push(point.id); - } else { - contourToPoints.set(contour.id, [point.id]); + for (const contour of g.contours) { + for (const point of contour.points) { + pointToContour.set(point.id, contour.id); + contourTotalCounts.set(contour.id, (contourTotalCounts.get(contour.id) ?? 0) + 1); + + if (pointIds.has(point.id)) { + contourIds.add(contour.id); + const list = contourToPoints.get(contour.id); + if (list) { + list.push(point.id); + } else { + contourToPoints.set(contour.id, [point.id]); + } } } } @@ -89,6 +92,27 @@ export class Selection { return { contourIds, fullySelectedContourIds, pointToContour, contourToPoints }; }); + + // Point-based bounds. Cheap per call (single pass over contour points), + // cached via ComputedSignal so repeated reads within a frame are free. + // Not exposed as `$bounds` — consumers that want live updates use the + // `useSelectionBounds` React hook, which subscribes to raw inputs and + // pulls this value at render time. Subscribing directly to this + // computed would force the iteration to run on every input signal + // fire (every drag frame), which is the footgun we're avoiding. + this.#$bounds = computed(() => { + const ids = this.#$pointIds.value; + const g = glyph.value; + if (ids.size === 0 || !g) return null; + + const selected: Point2D[] = []; + for (const contour of g.contours) { + for (const point of contour.points) { + if (ids.has(point.id)) selected.push(point); + } + } + return Bounds.fromPoints(selected); + }); } get pointIds(): ReadonlySet { @@ -153,6 +177,14 @@ export class Selection { return this.#$derived.value.contourIds.size; } + /** + * Axis-aligned bounding box of the currently selected points. + * Pull at read time; for React live display use `useSelectionBounds()`. + */ + get bounds(): BoundsType | null { + return this.#$bounds.value; + } + /** Raw signals for React hooks that need Signal. */ get $pointIds(): Signal> { return this.#$pointIds; diff --git a/packages/geo/src/Curve.ts b/packages/geo/src/Curve.ts index 021c21b2..5ddc3f81 100644 --- a/packages/geo/src/Curve.ts +++ b/packages/geo/src/Curve.ts @@ -155,10 +155,6 @@ export const Curve = { return Vec2.perp(tangent); }, - // ============================================ - // Closest Point (for hit testing) - // ============================================ - /** * Find closest point on curve to a test point. * Uses Newton-Raphson refinement for accuracy. @@ -181,10 +177,6 @@ export const Curve = { return Curve.closestPoint(curve, point).distance; }, - // ============================================ - // Properties - // ============================================ - startPoint(curve: CurveType): Point2D { return curve.p0; }, @@ -220,10 +212,6 @@ export const Curve = { } }, - // ============================================ - // Subdivision - // ============================================ - /** * Split curve at parameter t using De Casteljau's algorithm. */ @@ -260,10 +248,6 @@ export const Curve = { return points; }, - // ============================================ - // Type Guards - // ============================================ - isLine(curve: CurveType): curve is LineCurve { return curve.type === "line"; }, @@ -277,10 +261,6 @@ export const Curve = { }, } as const; -// ============================================ -// Quadratic Implementation -// ============================================ - function quadraticPointAt(curve: QuadraticCurve, t: number): Point2D { const mt = 1 - t; const mt2 = mt * mt; @@ -380,10 +360,6 @@ function newtonRaphsonQuadratic(curve: QuadraticCurve, point: Point2D, initialT: return t; } -// ============================================ -// Cubic Implementation -// ============================================ - function cubicPointAt(curve: CubicCurve, t: number): Point2D { const mt = 1 - t; const mt2 = mt * mt; @@ -532,10 +508,6 @@ function cubicSplitAt(curve: CubicCurve, t: number): [CubicCurve, CubicCurve] { return [Curve.cubic(curve.p0, p01, p012, p0123), Curve.cubic(p0123, p123, p23, curve.p1)]; } -// ============================================ -// Line Implementation -// ============================================ - function lineClosestPoint(curve: LineCurve, point: Point2D): ClosestPointResult { const v = Vec2.sub(curve.p1, curve.p0); const w = Vec2.sub(point, curve.p0); @@ -555,10 +527,6 @@ function lineClosestPoint(curve: LineCurve, point: Point2D): ClosestPointResult return { t, point: closest, distance: Vec2.dist(point, closest) }; } -// ============================================ -// Utility -// ============================================ - function curveLength(curve: QuadraticCurve | CubicCurve, subdivisions: number): number { let length = 0; let prevPoint = Curve.pointAt(curve, 0); diff --git a/scripts/oxlint/shift-plugin.mjs b/scripts/oxlint/shift-plugin.mjs index d501e36d..e2b2019f 100644 --- a/scripts/oxlint/shift-plugin.mjs +++ b/scripts/oxlint/shift-plugin.mjs @@ -18,7 +18,7 @@ const CONTOURS_ALLOWED = [ "lib/model/", "packages/font/", "rendering/", // render passes iterate contours to draw them - "SelectionBounds.ts", // segment-aware bounds needs contour structure + "types/selection.ts", // selection-bounds computed unions per-contour bounds "hit/composite.ts", // component contour bounds check "Editor.ts", // coordinator-level structural traversal "clipboard/", // ClipboardContent is not a Glyph, different type @@ -74,15 +74,19 @@ export default { /** * Ban direct .contours iteration in app code. * - * Use Glyphs.findPoints / Glyphs.points from @shift/font instead - * of raw `for (const contour of glyph.contours)` loops. + * Use instance methods on the Glyph / Contour domain classes: + * - glyph.allPoints / glyph.point(id) / glyph.points(ids) + * - glyph.segments() / contour.segments() + * - glyph.contour(id) + * Raw `glyph.contours.map(...)` / `for (const c of glyph.contours)` usually + * hides a reduction that belongs on the class. */ "no-raw-contour-access": { meta: { type: "suggestion", messages: { noRawContours: - "Do not iterate .contours directly. Use Glyphs.findPoints / Glyphs.points from @shift/font.", + "Do not iterate .contours directly. Use glyph.allPoints / glyph.point(id) / glyph.segments() / contour.segments() on the domain classes.", }, schema: [], }, @@ -105,6 +109,238 @@ export default { }, }, + /** + * Ban `getFoo()` / `isFoo()` methods that just return `this.#signal.value` + * (or `.peek()`). Use a value getter + optional `$foo` signal getter + * instead — matches the Glyph/Contour convention: + * + * // ❌ + * public getIsHoveringNode(): boolean { + * return this.#isHoveringNode.value; + * } + * + * // ✅ + * public get isHoveringNode(): boolean { + * return this.#isHoveringNode.value; + * } + * public get $isHoveringNode(): Signal { + * return this.#isHoveringNode; + * } + */ + "no-get-signal-value-method": { + meta: { + type: "suggestion", + messages: { + useGetter: + "Method `{{name}}` just reads a signal value. Use `get {{suggested}}(): T` + optional `get ${{suggested}}(): Signal` instead.", + }, + schema: [], + }, + create(context) { + const filename = context.getFilename(); + + if (filename.includes(".test.") || filename.includes("testing/")) return {}; + + function suggestedName(methodName) { + if (/^get[A-Z]/.test(methodName)) { + return methodName.charAt(3).toLowerCase() + methodName.slice(4); + } + if (/^is[A-Z]/.test(methodName)) { + // keep "is" prefix for boolean getters: isPreviewMode → previewMode? or isHoveringNode stays? + // The Editor rename uses: isPreviewMode (was method) → previewMode (is getter) + // But isHoveringNode stays as isHoveringNode. + // Heuristic: if what follows "is" is a noun/phrase, drop "is". Can't tell statically. + // Just return as-is and let humans decide. + return methodName; + } + return methodName; + } + + function isSignalValueReturn(body) { + if (!body || body.type !== "BlockStatement") return false; + if (body.body.length !== 1) return false; + const stmt = body.body[0]; + if (stmt.type !== "ReturnStatement" || !stmt.argument) return false; + const ret = stmt.argument; + if (ret.type !== "MemberExpression") return false; + if (!ret.property || ret.property.type !== "Identifier") return false; + if (ret.property.name !== "value" && ret.property.name !== "peek") return false; + // Return object must itself be a MemberExpression (this.#foo or this.$foo) + return ret.object && ret.object.type === "MemberExpression"; + } + + return { + MethodDefinition(node) { + if (node.kind !== "method") return; // skip get/set accessors + if (!node.key || node.key.type !== "Identifier") return; + + const name = node.key.name; + if (!/^(get|is)[A-Z]/.test(name)) return; + + const fn = node.value; + if (!fn || !fn.body) return; + + if (!isSignalValueReturn(fn.body)) return; + + context.report({ + node: node.key, + messageId: "useGetter", + data: { name, suggested: suggestedName(name) }, + }); + }, + }; + }, + }, + + /** + * Prefer instance methods on the Glyph / Contour domain classes over the + * static namespaces from @shift/font when the receiver is a class instance. + * + * Swaps: + * Glyphs.findPoint(glyph, id) → glyph.point(id) + * Glyphs.findPoints(glyph, ids) → glyph.points(ids) + * Glyphs.findContour(glyph, id) → glyph.contour(id) + * Glyphs.getAllPoints(glyph) → glyph.allPoints + * Glyphs.points(glyph) → iterate glyph.allPoints (or glyph.segments()) + * Contours.withNeighbors(contour) → contour.withNeighbors() + * Contours.canClose(c, pos, r) → c.canClose(pos, r) + * + * Static calls are still correct when operating on plain snapshots (e.g. + * drag handlers reading from a GlyphSnapshot), so the allowlist covers + * bridge/commands/behaviors that work with snapshot data. + */ + "prefer-instance-method-on-glyph": { + meta: { + type: "suggestion", + messages: { + useInstance: + "`{{callee}}` operates on Glyph/Contour domain instances. Call the instance method instead: {{suggestion}}.", + }, + schema: [], + }, + create(context) { + const filename = context.getFilename(); + + const PREFER_INSTANCE_ALLOWED = [ + "lib/model/", // domain classes delegate to these internally + "packages/font/", // implementation + "bridge/", // operates on GlyphSnapshot, not class instances + "commands/", // undo/redo snapshots + "behaviors/", // drag handlers read base snapshots + "testing/", + "types/engine.ts", + ]; + + if (isAllowedFile(filename, PREFER_INSTANCE_ALLOWED)) return {}; + if (filename.includes(".test.")) return {}; + + const SWAPS = { + Glyphs: { + findPoint: "glyph.point(id)", + findPoints: "glyph.points(ids)", + findContour: "glyph.contour(id)", + getAllPoints: "glyph.allPoints", + getPointAt: "glyph.getPointAt(pos, radius)", + points: "iterate glyph.allPoints / glyph.segments()", + }, + Contours: { + withNeighbors: "contour.withNeighbors()", + canClose: "contour.canClose(pos, radius)", + }, + }; + + return { + CallExpression(node) { + const callee = node.callee; + if (!callee || callee.type !== "MemberExpression") return; + const obj = callee.object; + const prop = callee.property; + if (!obj || obj.type !== "Identifier") return; + if (!prop || prop.type !== "Identifier") return; + + const swaps = SWAPS[obj.name]; + if (!swaps) return; + + const suggestion = swaps[prop.name]; + if (!suggestion) return; + + context.report({ + node: callee, + messageId: "useInstance", + data: { callee: `${obj.name}.${prop.name}`, suggestion }, + }); + }, + }; + }, + }, + + /** + * Ban raw segment parsing in app code. + * + * `Segment.parse(points, closed)` and `parseContourSegments({...})` are + * the low-level parser primitives. App code should use the reactive + * generators instead: + * - contour.segments() for a single contour + * - glyph.segments() for all contours in a glyph + * + * The generators wrap `parseContourSegments` and yield `Segment` class + * instances, which is what callers actually want. + */ + "no-raw-segment-parse": { + meta: { + type: "suggestion", + messages: { + useGenerator: + "Do not call `{{callee}}` directly. Use contour.segments() / glyph.segments() generators on the domain classes.", + }, + schema: [], + }, + create(context) { + const filename = context.getFilename(); + + const SEGMENT_PARSE_ALLOWED = [ + "lib/model/", // Segment.parse + Contour.segments wrap parseContourSegments here + "packages/font/", // parseContourSegments lives here + "testing/", + ]; + + if (isAllowedFile(filename, SEGMENT_PARSE_ALLOWED)) return {}; + if (filename.includes(".test.")) return {}; + + return { + // Segment.parse(...) + CallExpression(node) { + const callee = node.callee; + if (!callee) return; + + if ( + callee.type === "MemberExpression" && + callee.object?.type === "Identifier" && + callee.object.name === "Segment" && + callee.property?.type === "Identifier" && + callee.property.name === "parse" + ) { + context.report({ + node: callee, + messageId: "useGenerator", + data: { callee: "Segment.parse" }, + }); + return; + } + + // parseContourSegments(...) + if (callee.type === "Identifier" && callee.name === "parseContourSegments") { + context.report({ + node: callee, + messageId: "useGenerator", + data: { callee: "parseContourSegments" }, + }); + } + }, + }; + }, + }, + /** * Ban parameters named *Id that are typed as plain `string`. *