From 2e723caba0328ff93dcb2379a1ba3ff73047a112 Mon Sep 17 00:00:00 2001 From: Shai Rubinstein Date: Mon, 16 Mar 2026 22:12:37 +0200 Subject: [PATCH] fix: kiwi serialization bugs causing broken auto-layout in Figma import - Zero transforms for auto-layout children (Figma computes positions) - Write bordersTakeSpace from strokesIncludedInLayout - Always write explicit lineHeight, defaulting to ceil(fontSize * 1.2) - Normalize font family in derivedTextData fontMetaData and fontName - Extend normalizeFontFamily to strip optical size suffixes (9pt, 18px) - Extract computeExportTransform() helper for lint compliance - Add 20 unit tests covering all fixes Fixes #134 --- packages/core/src/kiwi/kiwi-serialize.ts | 53 +- tests/engine/kiwi-serialize-fixes.test.ts | 582 ++++++++++++++++++++++ 2 files changed, 626 insertions(+), 9 deletions(-) create mode 100644 tests/engine/kiwi-serialize-fixes.test.ts diff --git a/packages/core/src/kiwi/kiwi-serialize.ts b/packages/core/src/kiwi/kiwi-serialize.ts index 4ef600a08..f3dafdfa6 100644 --- a/packages/core/src/kiwi/kiwi-serialize.ts +++ b/packages/core/src/kiwi/kiwi-serialize.ts @@ -2,6 +2,7 @@ export const FIG_KIWI_VERSION = 106 import { deflateSync, inflateSync } from 'fflate' +<<<<<<< HEAD:packages/core/src/kiwi/kiwi-serialize.ts import { getLoadedFontData, normalizeFontFamily, weightToStyle } from '../fonts' import { encodeVectorNetworkBlob } from '../vector' import { stringToGuid, VARIABLE_BINDING_FIELDS } from './kiwi-convert' @@ -9,6 +10,15 @@ import { stringToGuid, VARIABLE_BINDING_FIELDS } from './kiwi-convert' import type { SceneGraph, SceneNode, CharacterStyleOverride } from '../scene-graph' import type { Color, GUID } from '../types' import type { NodeChange, Paint, VariableConsumptionEntry } from './codec' +======= +import { weightToStyle, getLoadedFontData, normalizeFontFamily } from './fonts' +import { encodeVectorNetworkBlob } from './vector' +import { stringToGuid, VARIABLE_BINDING_FIELDS } from './kiwi/kiwi-convert' + +import type { NodeChange, Paint, VariableConsumptionEntry } from './kiwi/codec' +import type { SceneGraph, SceneNode, CharacterStyleOverride } from './scene-graph' +import type { Color, GUID, Matrix } from './types' +>>>>>>> 4df3671 (fix: kiwi serialization bugs causing broken auto-layout in Figma import):packages/core/src/kiwi-serialize.ts const fontDigestCache = new Map() @@ -170,11 +180,12 @@ function buildDerivedTextData( const addFont = (family: string, weight: number, italic: boolean) => { const style = weightToStyle(weight, italic) - const key = `${family}|${style}` + const normalized = normalizeFontFamily(family) + const key = `${normalized}|${style}` if (seen.has(key)) return seen.add(key) fontMeta.push({ - key: { family, style, postscript: '' }, + key: { family: normalized, style, postscript: '' }, fontLineHeight: 1.2, fontDigest: digestMap.get(key), fontStyle: italic ? 'ITALIC' : 'NORMAL', @@ -311,7 +322,10 @@ function serializeTextProps( nc.textAlignHorizontal = node.textAlignHorizontal nc.textUserLayoutVersion = 3 if (fontDigestMap) nc.derivedTextData = buildDerivedTextData(node, fontDigestMap) - if (node.lineHeight != null) nc.lineHeight = { value: node.lineHeight, units: 'PIXELS' } + // Figma needs explicit lineHeight to compute text bounding boxes. + // Without it (and without baselines/glyphs data), text gets 0 height. + const lh = node.lineHeight != null ? node.lineHeight : Math.ceil(node.fontSize * 1.2) + nc.lineHeight = { value: lh, units: 'PIXELS' } if (node.letterSpacing !== 0) nc.letterSpacing = { value: node.letterSpacing, units: 'PIXELS' } if (node.textDecoration !== 'NONE') { nc.textDecoration = node.textDecoration === 'UNDERLINE' ? 'UNDERLINE' : 'STRIKETHROUGH' @@ -332,6 +346,7 @@ function serializeLayoutProps(node: SceneNode, nc: KiwiNodeChange): void { nc.stackCounterAlignItems = node.counterAxisAlign if (node.layoutWrap === 'WRAP') nc.stackWrap = 'WRAP' if (node.counterAxisSpacing > 0) nc.stackCounterSpacing = node.counterAxisSpacing + nc.bordersTakeSpace = node.strokesIncludedInLayout } if (node.layoutPositioning === 'ABSOLUTE') nc.stackPositioning = 'ABSOLUTE' if (node.layoutGrow > 0) nc.stackChildPrimaryGrow = node.layoutGrow @@ -393,6 +408,30 @@ function serializeVariableBindings( if (entries.length > 0) nc.variableConsumptionMap = { entries } } +function computeExportTransform( + node: SceneNode, + graph: SceneGraph +): Matrix { + const sx = node.flipX ? -1 : 1 + const cos = Math.cos((node.rotation * Math.PI) / 180) + const sin = Math.sin((node.rotation * Math.PI) / 180) + + // Auto-layout children should have (0,0) transform — Figma computes + // their positions from the layout engine at render time. + const parent = node.parentId ? graph.getNode(node.parentId) : undefined + const isAutoLayoutChild = parent + && parent.layoutMode !== 'NONE' + && parent.layoutMode !== 'GRID' + && node.layoutPositioning !== 'ABSOLUTE' + + return { + m00: cos * sx, m01: -sin, + m02: isAutoLayoutChild ? 0 : node.x, + m10: sin * sx, m11: cos, + m12: isAutoLayoutChild ? 0 : node.y + } +} + export function sceneNodeToKiwi( node: SceneNode, parentGuid: GUID, @@ -407,9 +446,6 @@ export function sceneNodeToKiwi( const localID = localIdCounter.value++ const guid = { sessionID: 1, localID } nodeIdToGuid?.set(node.id, guid) - const sx = node.flipX ? -1 : 1 - const cos = Math.cos((node.rotation * Math.PI) / 180) - const sin = Math.sin((node.rotation * Math.PI) / 180) const fillPaints = node.fills.map(fillToKiwiPaint) const strokePaints = node.strokes.map((s) => ({ @@ -429,9 +465,9 @@ export function sceneNodeToKiwi( opacity: node.opacity, phase: 'CREATED', size: { x: node.width, y: node.height }, - transform: { m00: cos * sx, m01: -sin, m02: node.x, m10: sin * sx, m11: cos, m12: node.y }, + transform: computeExportTransform(node, graph), strokeWeight: node.strokes.length > 0 ? node.strokes[0].weight : 1, - strokeAlign: node.strokes.length > 0 ? node.strokes[0].align : 'INSIDE' + strokeAlign: node.strokes.length > 0 ? node.strokes[0].align : 'INSIDE', } if (node.independentStrokeWeights) { @@ -461,7 +497,6 @@ export function sceneNodeToKiwi( if (node.type === 'TEXT') serializeTextProps(node, nc, fontDigestMap) nc.frameMaskDisabled = !node.clipsContent - if (node.clipsContent) nc.clipsContent = true if (node.horizontalConstraint !== 'MIN') nc.horizontalConstraint = node.horizontalConstraint if (node.verticalConstraint !== 'MIN') nc.verticalConstraint = node.verticalConstraint diff --git a/tests/engine/kiwi-serialize-fixes.test.ts b/tests/engine/kiwi-serialize-fixes.test.ts new file mode 100644 index 000000000..71eba1a1e --- /dev/null +++ b/tests/engine/kiwi-serialize-fixes.test.ts @@ -0,0 +1,582 @@ +import { describe, test, expect, beforeAll } from 'bun:test' + +import { + SceneGraph, + sceneNodeToKiwi, + exportFigFile, + parseFigFile, + initCodec, +} from '@open-pencil/core' + +beforeAll(async () => { + await initCodec() +}) + +function pageId(graph: SceneGraph) { + return graph.getPages()[0].id +} + +const ROOT_GUID = { sessionID: 1, localID: 0 } + +function toKiwi(node: ReturnType, graph: SceneGraph) { + const blobs: Uint8Array[] = [] + return sceneNodeToKiwi(node, ROOT_GUID, 0, { value: 100 }, graph, blobs) +} + +// --------------------------------------------------------------------------- +// Fix 1 — Auto-layout children get (0,0) transforms +// --------------------------------------------------------------------------- + +describe('Fix 1: auto-layout child transforms', () => { + test('auto-layout child gets zero transform regardless of its x/y', () => { + const graph = new SceneGraph() + const parent = graph.createNode('FRAME', pageId(graph), { + name: 'AutoLayout', + x: 0, + y: 0, + width: 300, + height: 400, + layoutMode: 'VERTICAL', + itemSpacing: 8, + paddingTop: 16, + paddingLeft: 16, + paddingBottom: 16, + paddingRight: 16, + }) + + const child = graph.createNode('FRAME', parent.id, { + name: 'Child', + x: 50, + y: 100, + width: 200, + height: 60, + }) + + const blobs: Uint8Array[] = [] + // Serialize the parent — children are serialized recursively + const changes = sceneNodeToKiwi( + graph.getNode(parent.id)!, + ROOT_GUID, + 0, + { value: 100 }, + graph, + blobs + ) as Record[] + + // changes[0] = parent, changes[1] = child + const childNc = changes.find((nc) => nc.name === 'Child')! + expect(childNc).toBeDefined() + expect(childNc.transform.m02).toBe(0) + expect(childNc.transform.m12).toBe(0) + }) + + test('absolute-positioned child inside auto-layout keeps its real x/y', () => { + const graph = new SceneGraph() + const parent = graph.createNode('FRAME', pageId(graph), { + name: 'AutoLayout', + x: 0, + y: 0, + width: 300, + height: 400, + layoutMode: 'VERTICAL', + itemSpacing: 8, + }) + + const absChild = graph.createNode('FRAME', parent.id, { + name: 'AbsChild', + x: 75, + y: 120, + width: 50, + height: 50, + layoutPositioning: 'ABSOLUTE', + }) + + const blobs: Uint8Array[] = [] + const changes = sceneNodeToKiwi( + graph.getNode(parent.id)!, + ROOT_GUID, + 0, + { value: 100 }, + graph, + blobs + ) as Record[] + + const absNc = changes.find((nc) => nc.name === 'AbsChild')! + expect(absNc).toBeDefined() + expect(absNc.transform.m02).toBe(75) + expect(absNc.transform.m12).toBe(120) + }) + + test('child in non-auto-layout parent keeps its real x/y', () => { + const graph = new SceneGraph() + const parent = graph.createNode('FRAME', pageId(graph), { + name: 'PlainFrame', + x: 0, + y: 0, + width: 300, + height: 400, + // layoutMode defaults to 'NONE' + }) + + const child = graph.createNode('FRAME', parent.id, { + name: 'Child', + x: 30, + y: 45, + width: 100, + height: 80, + }) + + const blobs: Uint8Array[] = [] + const changes = sceneNodeToKiwi( + graph.getNode(parent.id)!, + ROOT_GUID, + 0, + { value: 100 }, + graph, + blobs + ) as Record[] + + const childNc = changes.find((nc) => nc.name === 'Child')! + expect(childNc).toBeDefined() + expect(childNc.transform.m02).toBe(30) + expect(childNc.transform.m12).toBe(45) + }) + + test('horizontal auto-layout child also gets zero transform', () => { + const graph = new SceneGraph() + const parent = graph.createNode('FRAME', pageId(graph), { + name: 'HorizontalLayout', + x: 0, + y: 0, + width: 600, + height: 100, + layoutMode: 'HORIZONTAL', + itemSpacing: 12, + }) + + graph.createNode('RECTANGLE', parent.id, { + name: 'Item', + x: 200, + y: 50, + width: 80, + height: 80, + }) + + const blobs: Uint8Array[] = [] + const changes = sceneNodeToKiwi( + graph.getNode(parent.id)!, + ROOT_GUID, + 0, + { value: 100 }, + graph, + blobs + ) as Record[] + + const itemNc = changes.find((nc) => nc.name === 'Item')! + expect(itemNc.transform.m02).toBe(0) + expect(itemNc.transform.m12).toBe(0) + }) +}) + +// --------------------------------------------------------------------------- +// Fix 2 — frameMaskDisabled always true +// --------------------------------------------------------------------------- + +describe('Fix 2: frameMaskDisabled is inverse of clipsContent', () => { + test('FRAME without clipsContent gets frameMaskDisabled=true', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'Frame', + x: 0, + y: 0, + width: 100, + height: 100, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].frameMaskDisabled).toBe(true) + }) + + test('FRAME with clipsContent=true gets frameMaskDisabled=false', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'Clipping', + x: 0, + y: 0, + width: 100, + height: 100, + clipsContent: true, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].frameMaskDisabled).toBe(false) + }) + + test('FRAME with clipsContent=false gets frameMaskDisabled=true', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'NoClip', + x: 0, + y: 0, + width: 100, + height: 100, + clipsContent: false, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].frameMaskDisabled).toBe(true) + }) + + test('clipsContent roundtrips through export/parse', async () => { + const graph = new SceneGraph() + graph.createNode('FRAME', pageId(graph), { + name: 'ClipFrame', + x: 0, + y: 0, + width: 100, + height: 100, + clipsContent: true, + }) + + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const frame = [...reimported.nodes.values()].find((n) => n.name === 'ClipFrame')! + expect(frame.clipsContent).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// Fix 3 — bordersTakeSpace writes strokesIncludedInLayout +// --------------------------------------------------------------------------- + +describe('Fix 3: bordersTakeSpace serialization', () => { + test('auto-layout frame with strokesIncludedInLayout=true gets bordersTakeSpace=true', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'LayoutWithBorders', + x: 0, + y: 0, + width: 300, + height: 200, + layoutMode: 'VERTICAL', + itemSpacing: 8, + strokesIncludedInLayout: true, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].bordersTakeSpace).toBe(true) + }) + + test('auto-layout frame with strokesIncludedInLayout=false gets bordersTakeSpace=false', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'LayoutNoBorders', + x: 0, + y: 0, + width: 300, + height: 200, + layoutMode: 'VERTICAL', + itemSpacing: 8, + strokesIncludedInLayout: false, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].bordersTakeSpace).toBe(false) + }) + + test('non-auto-layout frame does not get bordersTakeSpace', () => { + const graph = new SceneGraph() + const node = graph.createNode('FRAME', pageId(graph), { + name: 'PlainFrame', + x: 0, + y: 0, + width: 300, + height: 200, + strokesIncludedInLayout: true, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].bordersTakeSpace).toBeUndefined() + }) +}) + +// --------------------------------------------------------------------------- +// Fix 4 — lineHeight always written on text nodes +// --------------------------------------------------------------------------- + +describe('Fix 4: text lineHeight serialization', () => { + test('text node with explicit lineHeight uses that value', () => { + const graph = new SceneGraph() + const node = graph.createNode('TEXT', pageId(graph), { + name: 'ExplicitLH', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Inter', + fontWeight: 400, + fontSize: 16, + lineHeight: 24, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].lineHeight).toEqual({ value: 24, units: 'PIXELS' }) + }) + + test('text node without lineHeight defaults to ceil(fontSize * 1.2)', () => { + const graph = new SceneGraph() + const node = graph.createNode('TEXT', pageId(graph), { + name: 'DefaultLH', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Inter', + fontWeight: 400, + fontSize: 16, + // lineHeight not set — defaults to null + }) + + const changes = toKiwi(node, graph) as Record[] + // ceil(16 * 1.2) = ceil(19.2) = 20 + expect(changes[0].lineHeight).toEqual({ value: 20, units: 'PIXELS' }) + }) + + test('lineHeight default for odd fontSize', () => { + const graph = new SceneGraph() + const node = graph.createNode('TEXT', pageId(graph), { + name: 'OddSize', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Inter', + fontWeight: 400, + fontSize: 14, + }) + + const changes = toKiwi(node, graph) as Record[] + // ceil(14 * 1.2) = ceil(16.8) = 17 + expect(changes[0].lineHeight).toEqual({ value: 17, units: 'PIXELS' }) + }) + + test('lineHeight survives roundtrip through export/parse', async () => { + const graph = new SceneGraph() + graph.createNode('TEXT', pageId(graph), { + name: 'Roundtrip', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Inter', + fontWeight: 400, + fontSize: 16, + lineHeight: 28, + }) + + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const textNode = [...reimported.nodes.values()].find((n) => n.type === 'TEXT')! + expect(textNode.lineHeight).toBe(28) + }) +}) + +// --------------------------------------------------------------------------- +// Fix 5 — Font family normalization in derivedTextData +// --------------------------------------------------------------------------- + +describe('Fix 5: font family normalization in derivedTextData', () => { + test('optical size suffix stripped in roundtrip', async () => { + const graph = new SceneGraph() + graph.createNode('TEXT', pageId(graph), { + name: 'OpticalSize', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'DM Sans 9pt', + fontWeight: 400, + fontSize: 14, + }) + + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const textNode = [...reimported.nodes.values()].find((n) => n.type === 'TEXT')! + expect(textNode.fontFamily).toBe('DM Sans') + }) + + test('Variable suffix stripped in roundtrip', async () => { + const graph = new SceneGraph() + graph.createNode('TEXT', pageId(graph), { + name: 'Variable', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Roboto Variable', + fontWeight: 700, + fontSize: 14, + }) + + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const textNode = [...reimported.nodes.values()].find((n) => n.type === 'TEXT')! + expect(textNode.fontFamily).toBe('Roboto') + }) + + test('normal font family preserved unchanged', async () => { + const graph = new SceneGraph() + graph.createNode('TEXT', pageId(graph), { + name: 'Normal', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'Inter', + fontWeight: 400, + fontSize: 14, + }) + + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const textNode = [...reimported.nodes.values()].find((n) => n.type === 'TEXT')! + expect(textNode.fontFamily).toBe('Inter') + }) + + test('fontName in kiwi output uses normalized family', () => { + const graph = new SceneGraph() + const node = graph.createNode('TEXT', pageId(graph), { + name: 'KiwiFontName', + x: 0, + y: 0, + width: 100, + height: 20, + text: 'Hello', + fontFamily: 'DM Sans 18px', + fontWeight: 400, + fontSize: 14, + }) + + const changes = toKiwi(node, graph) as Record[] + expect(changes[0].fontName.family).toBe('DM Sans') + }) +}) + +// --------------------------------------------------------------------------- +// Integration: all fixes together in a realistic auto-layout component +// --------------------------------------------------------------------------- + +describe('Integration: auto-layout component with all fixes', () => { + test('full component roundtrip preserves layout structure', async () => { + const graph = new SceneGraph() + + // Parent auto-layout frame (like a card component) + const card = graph.createNode('FRAME', pageId(graph), { + name: 'StatCard', + x: 100, + y: 100, + width: 328, + height: 200, + layoutMode: 'VERTICAL', + itemSpacing: 8, + paddingTop: 16, + paddingLeft: 16, + paddingBottom: 16, + paddingRight: 16, + primaryAxisSizing: 'FIXED', + counterAxisSizing: 'FIXED', + clipsContent: true, + strokesIncludedInLayout: true, + fills: [{ type: 'SOLID', color: { r: 1, g: 1, b: 1, a: 1 }, opacity: 1, visible: true }], + }) + + // Title text + graph.createNode('TEXT', card.id, { + name: 'Title', + x: 16, + y: 16, + width: 296, + height: 20, + text: 'Total Active Users', + fontFamily: 'DM Sans 9pt', + fontWeight: 600, + fontSize: 14, + lineHeight: 20, + }) + + // Value text + graph.createNode('TEXT', card.id, { + name: 'Value', + x: 16, + y: 44, + width: 296, + height: 40, + text: '12,345', + fontFamily: 'Inter', + fontWeight: 700, + fontSize: 32, + lineHeight: 40, + }) + + // Verify kiwi output directly + const blobs: Uint8Array[] = [] + const changes = sceneNodeToKiwi( + graph.getNode(card.id)!, + ROOT_GUID, + 0, + { value: 100 }, + graph, + blobs + ) as Record[] + + const cardNc = changes[0] + const titleNc = changes.find((nc) => nc.name === 'Title')! + const valueNc = changes.find((nc) => nc.name === 'Value')! + + // Fix 1: children have zero transforms + expect(titleNc.transform.m02).toBe(0) + expect(titleNc.transform.m12).toBe(0) + expect(valueNc.transform.m02).toBe(0) + expect(valueNc.transform.m12).toBe(0) + + // Fix 2: frameMaskDisabled is inverse of clipsContent + expect(cardNc.frameMaskDisabled).toBe(false) // clipsContent=true → frameMaskDisabled=false + expect(titleNc.frameMaskDisabled).toBe(true) // text nodes don't clip + + // Fix 3: bordersTakeSpace + expect(cardNc.bordersTakeSpace).toBe(true) + + // Fix 4: lineHeight on text nodes + expect(titleNc.lineHeight).toEqual({ value: 20, units: 'PIXELS' }) + expect(valueNc.lineHeight).toEqual({ value: 40, units: 'PIXELS' }) + + // Fix 5: font family normalized + expect(titleNc.fontName.family).toBe('DM Sans') + expect(valueNc.fontName.family).toBe('Inter') + + // Roundtrip through export/parse + const exported = await exportFigFile(graph) + const reimported = await parseFigFile(exported.buffer as ArrayBuffer) + + const nodes = [...reimported.nodes.values()] + const cardNode = nodes.find((n) => n.name === 'StatCard')! + const titleNode = nodes.find((n) => n.name === 'Title')! + + expect(cardNode.layoutMode).toBe('VERTICAL') + expect(cardNode.clipsContent).toBe(true) + expect(titleNode.fontFamily).toBe('DM Sans') + expect(titleNode.lineHeight).toBe(20) + }) +})