fix(text): preserve imported FIG glyph outlines
Keep authoritative derived glyphs through layout, alignment, fill, and decoration changes while still invalidating them for actual shaping edits. Add scene-graph invalidation coverage and a Gold Preview fixture regression test. Co-authored-by: Joseph Cumines <joeycumines@gmail.com>
This commit is contained in:
parent
a703a48926
commit
eb55cb1025
|
|
@ -49,7 +49,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Keep the canvas rendering when hiding or showing the UI on wide HiDPI viewports by bounding retained scene backing allocations and falling back to direct rendering when CanvasKit rejects an offscreen surface.
|
||||
- Preserve Figma’s imported glyph outlines through layout and appearance updates so text keeps its intended weight and shape.
|
||||
- Scale proportion-constrained `.fig` instance geometry through fixed wrapper layers so imported logos and icons retain their intended size.
|
||||
- Match Figma auto-layout spacing, padding, min/max constraints, scalar variable bindings, CanvasKit-shaped generated text, imported text bounds, and nested instance geometry more closely.
|
||||
- Match Figma Plugin API vector path and network editing, including bounds, transforms, winding rules, region fills, validation, and handle mirroring. (#444)
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ import { CONTAINER_TYPES, createDefaultNode } from './node-defaults'
|
|||
import { updateNodePreview } from './preview'
|
||||
import { styleDetachmentChanges } from './shared-styles'
|
||||
import { markSourceFieldsEdited } from './source-metadata'
|
||||
import { TEXT_PICTURE_KEYS } from './text-picture'
|
||||
import { GLYPH_AFFECTING_KEYS, TEXT_PICTURE_KEYS } from './text-picture'
|
||||
import * as Variables from './variables'
|
||||
import { normalizeVectorNetwork } from './vector-network'
|
||||
|
||||
|
|
@ -309,6 +309,7 @@ export class SceneGraph {
|
|||
}
|
||||
|
||||
static TEXT_PICTURE_KEYS: ReadonlySet<string> = TEXT_PICTURE_KEYS
|
||||
static GLYPH_AFFECTING_KEYS: ReadonlySet<string> = GLYPH_AFFECTING_KEYS
|
||||
|
||||
static LAYOUT_AFFECTING_KEYS: ReadonlySet<string> = new Set([
|
||||
'x',
|
||||
|
|
@ -404,7 +405,8 @@ export class SceneGraph {
|
|||
if (node.type === 'TEXT') {
|
||||
const textChanged = Object.keys(changes).some((k) => TEXT_PICTURE_KEYS.has(k))
|
||||
if (node.textPicture && textChanged) node.textPicture = null
|
||||
if (node.figmaDerivedTextGlyphs && textChanged) node.figmaDerivedTextGlyphs = null
|
||||
const glyphChanged = Object.keys(changes).some((k) => GLYPH_AFFECTING_KEYS.has(k))
|
||||
if (node.figmaDerivedTextGlyphs && glyphChanged) node.figmaDerivedTextGlyphs = null
|
||||
}
|
||||
const entries = Object.entries(changes) as Array<[string, unknown]>
|
||||
changes = Object.fromEntries(
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { GLYPH_AFFECTING_KEYS, TEXT_PICTURE_KEYS } from './text-picture'
|
||||
import type { SceneNode } from './types'
|
||||
import { normalizeVectorNetwork } from './vector-network'
|
||||
|
||||
|
|
@ -42,25 +43,6 @@ const LAYOUT_AFFECTING_KEYS = new Set<string>([
|
|||
'textAutoResize'
|
||||
])
|
||||
|
||||
const TEXT_PICTURE_KEYS = new Set<string>([
|
||||
'text',
|
||||
'fontSize',
|
||||
'fontFamily',
|
||||
'fontWeight',
|
||||
'italic',
|
||||
'textAlignHorizontal',
|
||||
'textDirection',
|
||||
'textAlignVertical',
|
||||
'lineHeight',
|
||||
'letterSpacing',
|
||||
'textDecoration',
|
||||
'textCase',
|
||||
'styleRuns',
|
||||
'fills',
|
||||
'width',
|
||||
'height'
|
||||
])
|
||||
|
||||
export function updateNodePreview(
|
||||
graph: PreviewGraph,
|
||||
id: string,
|
||||
|
|
@ -76,7 +58,8 @@ export function updateNodePreview(
|
|||
if (node.type === 'TEXT') {
|
||||
const textChanged = Object.keys(changes).some((key) => TEXT_PICTURE_KEYS.has(key))
|
||||
if (node.textPicture && textChanged) node.textPicture = null
|
||||
if (node.figmaDerivedTextGlyphs && textChanged) node.figmaDerivedTextGlyphs = null
|
||||
const glyphChanged = Object.keys(changes).some((key) => GLYPH_AFFECTING_KEYS.has(key))
|
||||
if (node.figmaDerivedTextGlyphs && glyphChanged) node.figmaDerivedTextGlyphs = null
|
||||
}
|
||||
const normalizedChanges = changes.vectorNetwork
|
||||
? { ...changes, vectorNetwork: normalizeVectorNetwork(changes.vectorNetwork) }
|
||||
|
|
|
|||
|
|
@ -16,3 +16,16 @@ export const TEXT_PICTURE_KEYS: ReadonlySet<string> = new Set([
|
|||
'width',
|
||||
'height'
|
||||
])
|
||||
|
||||
/** Properties that change imported glyph outlines or per-glyph positioning. */
|
||||
export const GLYPH_AFFECTING_KEYS: ReadonlySet<string> = new Set([
|
||||
'text',
|
||||
'fontSize',
|
||||
'fontFamily',
|
||||
'fontWeight',
|
||||
'italic',
|
||||
'lineHeight',
|
||||
'letterSpacing',
|
||||
'textCase',
|
||||
'styleRuns'
|
||||
])
|
||||
|
|
|
|||
|
|
@ -24,6 +24,20 @@ describe('derived instance layout regressions', () => {
|
|||
layoutNodes = collectAllNodes(layoutGraph)
|
||||
})
|
||||
|
||||
test('retains imported glyph outlines through non-glyph layout updates', () => {
|
||||
const glyphCount = () =>
|
||||
layoutNodes.filter(
|
||||
(node) => node.type === 'TEXT' && (node.figmaDerivedTextGlyphs?.length ?? 0) > 0
|
||||
).length
|
||||
|
||||
expect(glyphCount()).toBe(43)
|
||||
|
||||
computeAllLayouts(layoutGraph)
|
||||
layoutNodes = collectAllNodes(layoutGraph)
|
||||
|
||||
expect(glyphCount()).toBe(43)
|
||||
})
|
||||
|
||||
test('preserves repeated badge overrides without moving sibling component wrappers', () => {
|
||||
const input = previewChild(layoutGraph, layoutNodes, 'Input')
|
||||
const inputRoot = childNamed(layoutGraph, input, '_input')
|
||||
|
|
|
|||
|
|
@ -2,8 +2,8 @@ import { describe, expect, test } from 'bun:test'
|
|||
|
||||
import { SceneGraph } from '@open-pencil/core'
|
||||
|
||||
describe('TEXT_PICTURE_KEYS membership', () => {
|
||||
test('contains text rendering properties', () => {
|
||||
describe('text rendering invalidation keys', () => {
|
||||
test('TEXT_PICTURE_KEYS contains rendered-picture properties', () => {
|
||||
const keys = SceneGraph.TEXT_PICTURE_KEYS
|
||||
for (const k of [
|
||||
'text',
|
||||
|
|
@ -27,7 +27,37 @@ describe('TEXT_PICTURE_KEYS membership', () => {
|
|||
}
|
||||
})
|
||||
|
||||
test('does NOT contain non-text properties', () => {
|
||||
test('GLYPH_AFFECTING_KEYS contains only properties that change imported glyphs', () => {
|
||||
const keys = SceneGraph.GLYPH_AFFECTING_KEYS
|
||||
for (const k of [
|
||||
'text',
|
||||
'fontSize',
|
||||
'fontFamily',
|
||||
'fontWeight',
|
||||
'italic',
|
||||
'lineHeight',
|
||||
'letterSpacing',
|
||||
'textCase',
|
||||
'styleRuns'
|
||||
]) {
|
||||
expect(keys.has(k)).toBe(true)
|
||||
}
|
||||
for (const k of [
|
||||
'x',
|
||||
'y',
|
||||
'width',
|
||||
'height',
|
||||
'fills',
|
||||
'textAlignHorizontal',
|
||||
'textDirection',
|
||||
'textAlignVertical',
|
||||
'textDecoration'
|
||||
]) {
|
||||
expect(keys.has(k)).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
test('TEXT_PICTURE_KEYS does NOT contain non-text properties', () => {
|
||||
const keys = SceneGraph.TEXT_PICTURE_KEYS
|
||||
for (const k of ['x', 'y', 'rotation', 'opacity', 'visible', 'name']) {
|
||||
expect(keys.has(k)).toBe(false)
|
||||
|
|
|
|||
|
|
@ -205,6 +205,38 @@ describe('updateNode', () => {
|
|||
expect(expectDefined(graph.getNode(textId), 'updated node').figmaDerivedTextGlyphs).toBeNull()
|
||||
})
|
||||
|
||||
test('figmaDerivedTextGlyphs survive picture-only text property changes', () => {
|
||||
const pictureOnlyChanges = [
|
||||
{ width: 120 },
|
||||
{ height: 24 },
|
||||
{ fills: [] },
|
||||
{ textAlignHorizontal: 'CENTER' as const },
|
||||
{ textDirection: 'RTL' as const },
|
||||
{ textAlignVertical: 'CENTER' as const },
|
||||
{ textDecoration: 'UNDERLINE' as const }
|
||||
]
|
||||
|
||||
for (const changes of pictureOnlyChanges) {
|
||||
const graph = new SceneGraph()
|
||||
const page = pageId(graph)
|
||||
const textId = graph.createNode('TEXT', page, {
|
||||
name: 'T',
|
||||
text: 'Imported text',
|
||||
width: 100,
|
||||
height: 20
|
||||
}).id
|
||||
const glyphs = [{ commandsBlob: new Uint8Array([4, 5, 6]), x: 0, y: 10, fontSize: 14 }]
|
||||
const textNode = expectDefined(graph.getNode(textId), 'text node')
|
||||
textNode.figmaDerivedTextGlyphs = glyphs
|
||||
|
||||
graph.updateNode(textId, changes)
|
||||
|
||||
expect(expectDefined(graph.getNode(textId), 'updated node').figmaDerivedTextGlyphs).toBe(
|
||||
glyphs
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
test('figmaDerivedTextGlyphs survive non-text property change on TEXT node', () => {
|
||||
const graph = new SceneGraph()
|
||||
const page = pageId(graph)
|
||||
|
|
|
|||
Loading…
Reference in a new issue