Fix DSD propagation through intermediate DSD-targeted clones

Replace BFS (which skipped DSD-modified nodes in visited set, breaking
the chain for deeper clones) with iterative convergence loop. Track
dsdSizeSet separately from dsdModified so nodes with only position/
geometry changes inherit size from their source.

Fix transitive override propagation: re-clone from source (not component)
to preserve componentId chain for DSD. Add syncChildrenDeep for deep
property propagation (stroke colors on nested vectors).

Reduces vector overflow in gold-preview.fig from 25 to 0 (visible,
unclipped).
This commit is contained in:
Danila Poyarkov 2026-03-04 22:47:32 +03:00
parent ec8fce1b09
commit f6a12cc736
3 changed files with 145 additions and 54 deletions

View file

@ -345,6 +345,7 @@ export function populateAndApplyOverrides(
function applyDerivedSymbolData() { function applyDerivedSymbolData() {
const dsdModified = new Set<string>() const dsdModified = new Set<string>()
const dsdSizeSet = new Set<string>()
for (const [ncId, nc] of changeMap) { for (const [ncId, nc] of changeMap) {
if (nc.type !== 'INSTANCE') continue if (nc.type !== 'INSTANCE') continue
@ -390,46 +391,50 @@ export function populateAndApplyOverrides(
if (Object.keys(updates).length > 0) { if (Object.keys(updates).length > 0) {
graph.updateNode(targetId, updates) graph.updateNode(targetId, updates)
dsdModified.add(targetId) dsdModified.add(targetId)
if (d.size) dsdSizeSet.add(targetId)
} }
} }
} }
// Propagate DSD changes through clone chains. Clones inherit size, // Propagate DSD changes through clone chains. Each clone should match
// position, and geometry from their source but the normal transitive // its source (componentId) for size/position/geometry. Iterate until
// sync already ran before DSD. // convergence so deeper clone levels receive updates even when
// intermediate clones were also directly DSD-targeted (e.g., a DSD
// entry set only geometry on an intermediate clone — its size must
// still be inherited from the source).
//
// Nodes whose size was explicitly set by DSD (in dsdSizeSet) keep
// their own values; nodes only touched for position/geometry inherit
// size from their source.
if (dsdModified.size > 0) { if (dsdModified.size > 0) {
const clonesOf = new Map<string, string[]>() const synced = new Set<string>()
for (const node of graph.getAllNodes()) { let changed = true
if (!node.componentId) continue while (changed) {
let arr = clonesOf.get(node.componentId) changed = false
if (!arr) { for (const node of graph.getAllNodes()) {
arr = [] if (!node.componentId || synced.has(node.id)) continue
clonesOf.set(node.componentId, arr) const source = graph.getNode(node.componentId)
} if (!source) continue
arr.push(node.id) if (!dsdModified.has(node.componentId) && !synced.has(node.componentId)) continue
} // Nodes with explicitly DSD-set size keep their values but still
// act as chain links so their clones are reached
const queue = [...dsdModified] if (dsdSizeSet.has(node.id)) {
const visited = new Set(dsdModified) synced.add(node.id)
for (let sourceId = queue.shift(); sourceId !== undefined; sourceId = queue.shift()) { changed = true
const source = graph.getNode(sourceId) continue
if (!source) continue }
const clones = clonesOf.get(sourceId)
if (!clones) continue
for (const cloneId of clones) {
if (visited.has(cloneId)) continue
visited.add(cloneId)
const clone = graph.getNode(cloneId)
if (!clone) continue
const cu: Partial<SceneNode> = {} const cu: Partial<SceneNode> = {}
if (source.width !== clone.width) cu.width = source.width if (source.width !== node.width) cu.width = source.width
if (source.height !== clone.height) cu.height = source.height if (source.height !== node.height) cu.height = source.height
if (source.x !== clone.x) cu.x = source.x if (source.x !== node.x) cu.x = source.x
if (source.y !== clone.y) cu.y = source.y if (source.y !== node.y) cu.y = source.y
if (source.fillGeometry !== clone.fillGeometry) cu.fillGeometry = structuredClone(source.fillGeometry) if (source.fillGeometry !== node.fillGeometry) cu.fillGeometry = structuredClone(source.fillGeometry)
if (source.strokeGeometry !== clone.strokeGeometry) cu.strokeGeometry = structuredClone(source.strokeGeometry) if (source.strokeGeometry !== node.strokeGeometry) cu.strokeGeometry = structuredClone(source.strokeGeometry)
if (Object.keys(cu).length > 0) graph.updateNode(cloneId, cu) if (Object.keys(cu).length > 0) {
queue.push(cloneId) graph.updateNode(node.id, cu)
}
synced.add(node.id)
changed = true
} }
} }
} }
@ -473,6 +478,35 @@ export function populateAndApplyOverrides(
return overriddenNodes return overriddenNodes
} }
function syncNodeProps(source: SceneNode, target: SceneNode) {
const updates: Partial<SceneNode> = {}
if (source.text !== undefined && source.text !== target.text) updates.text = source.text
if (source.visible !== undefined && source.visible !== target.visible) updates.visible = source.visible
if (source.opacity !== undefined && source.opacity !== target.opacity) updates.opacity = source.opacity
if (source.fills !== undefined && source.fills !== target.fills) updates.fills = structuredClone(source.fills)
if (source.strokes !== undefined && source.strokes !== target.strokes) updates.strokes = structuredClone(source.strokes)
if (source.effects !== undefined && source.effects !== target.effects) updates.effects = structuredClone(source.effects)
if (source.styleRuns !== undefined && source.styleRuns !== target.styleRuns) updates.styleRuns = structuredClone(source.styleRuns)
if (source.layoutGrow !== undefined && source.layoutGrow !== target.layoutGrow) updates.layoutGrow = source.layoutGrow
if (source.textAutoResize !== undefined && source.textAutoResize !== target.textAutoResize) updates.textAutoResize = source.textAutoResize
if (source.locked !== undefined && source.locked !== target.locked) updates.locked = source.locked
if (Object.keys(updates).length > 0) graph.updateNode(target.id, updates)
}
function syncChildrenDeep(sourceId: string, targetId: string) {
const src = graph.getNode(sourceId)
const tgt = graph.getNode(targetId)
if (!src || !tgt) return
const len = Math.min(src.childIds.length, tgt.childIds.length)
for (let i = 0; i < len; i++) {
const srcNode = graph.getNode(src.childIds[i])
const tgtNode = graph.getNode(tgt.childIds[i])
if (!srcNode || !tgtNode || srcNode.type !== tgtNode.type) continue
syncNodeProps(srcNode, tgtNode)
syncChildrenDeep(src.childIds[i], tgt.childIds[i])
}
}
function propagateOverridesTransitively(seeds: Set<string>) { function propagateOverridesTransitively(seeds: Set<string>) {
if (seeds.size === 0) return if (seeds.size === 0) return
@ -487,8 +521,25 @@ export function populateAndApplyOverrides(
arr.push(node.id) arr.push(node.id)
} }
// Also seed parent INSTANCE nodes of overridden children so their
// clones are visited by the BFS — deep overrides (e.g., stroke color
// on a Vector nested inside a check inside an _icon-xs) need the
// instance-level clone to be visited for syncChildrenDeep to propagate.
const expandedSeeds = new Set(seeds)
for (const seedId of seeds) {
let cur = graph.getNode(seedId)
while (cur?.parentId) {
const parent = graph.getNode(cur.parentId)
if (!parent) break
if (parent.type === 'INSTANCE' || parent.type === 'COMPONENT') {
expandedSeeds.add(parent.id)
}
cur = parent
}
}
const needsSync = new Set<string>() const needsSync = new Set<string>()
const queue = [...seeds] const queue = [...expandedSeeds]
for (let id = queue.pop(); id !== undefined; id = queue.pop()) { for (let id = queue.pop(); id !== undefined; id = queue.pop()) {
const clones = clonesOf.get(id) const clones = clonesOf.get(id)
if (!clones) continue if (!clones) continue
@ -500,7 +551,7 @@ export function populateAndApplyOverrides(
} }
const visited = new Set<string>() const visited = new Set<string>()
const syncQueue = [...seeds] const syncQueue = [...expandedSeeds]
for (let sourceId = syncQueue.shift(); sourceId !== undefined; sourceId = syncQueue.shift()) { for (let sourceId = syncQueue.shift(); sourceId !== undefined; sourceId = syncQueue.shift()) {
const clones = clonesOf.get(sourceId) const clones = clonesOf.get(sourceId)
if (!clones) continue if (!clones) continue
@ -519,24 +570,17 @@ export function populateAndApplyOverrides(
continue continue
} }
if (node.type === 'INSTANCE' && source.type === 'INSTANCE' && node.componentId) { syncNodeProps(source, node)
repopulateInstance(node.id, node.componentId) // For structural changes (instance swaps change child count/type),
} else { // re-clone from the SOURCE (not the component) to preserve the
// Only propagate explicitly-set properties — undefined values must // componentId chain for DSD propagation.
// not overwrite values set by other override phases. if (source.childIds.length !== node.childIds.length) {
const updates: Partial<SceneNode> = {} for (const childId of [...node.childIds]) graph.deleteNode(childId)
if (source.text !== undefined && source.text !== node.text) updates.text = source.text if (source.childIds.length > 0) {
if (source.visible !== undefined && source.visible !== node.visible) updates.visible = source.visible graph.populateInstanceChildren(node.id, sourceId)
if (source.opacity !== undefined && source.opacity !== node.opacity) updates.opacity = source.opacity }
if (source.name !== undefined && source.name !== node.name) updates.name = source.name } else if (source.childIds.length > 0 && node.childIds.length > 0) {
if (source.fills !== undefined && source.fills !== node.fills) updates.fills = structuredClone(source.fills) syncChildrenDeep(sourceId, cloneId)
if (source.strokes !== undefined && source.strokes !== node.strokes) updates.strokes = structuredClone(source.strokes)
if (source.effects !== undefined && source.effects !== node.effects) updates.effects = structuredClone(source.effects)
if (source.styleRuns !== undefined && source.styleRuns !== node.styleRuns) updates.styleRuns = structuredClone(source.styleRuns)
if (source.layoutGrow !== undefined && source.layoutGrow !== node.layoutGrow) updates.layoutGrow = source.layoutGrow
if (source.textAutoResize !== undefined && source.textAutoResize !== node.textAutoResize) updates.textAutoResize = source.textAutoResize
if (source.locked !== undefined && source.locked !== node.locked) updates.locked = source.locked
if (Object.keys(updates).length > 0) graph.updateNode(node.id, updates)
} }
syncQueue.push(cloneId) syncQueue.push(cloneId)

View file

@ -766,4 +766,48 @@ describe('edge cases', () => {
expect(iconChildren).toHaveLength(2) expect(iconChildren).toHaveLength(2)
expect(iconChildren.map((c) => c.name).sort()).toEqual(['PathB1', 'PathB2']) expect(iconChildren.map((c) => c.name).sort()).toEqual(['PathB1', 'PathB2'])
}) })
test('DSD propagates through intermediate clones that are also DSD-targeted', async () => {
const graph = await parseFigFile(readFileSync(resolve(__dirname, '../fixtures/gold-preview.fig')).buffer)
const thumb = [...graph.getAllNodes()].find((n) => n.name === 'Preview Thumbnail')
expect(thumb).toBeDefined()
let overflows = 0
function walk(id: string) {
const node = graph.getNode(id)
if (!node) return
if (node.type === 'VECTOR') {
const parent = graph.getNode(node.parentId)
if (parent?.type === 'INSTANCE' && parent.width > 0 && parent.height > 0) {
// Check visibility
let vis = true
let cur: typeof node | null = node
while (cur) {
if (!cur.visible) {
vis = false
break
}
cur = cur.parentId ? graph.getNode(cur.parentId) ?? null : null
}
// Check clipping
let clipped = false
cur = graph.getNode(parent.parentId)
while (cur) {
if (cur.clipsContent) {
clipped = true
break
}
cur = cur.parentId ? graph.getNode(cur.parentId) ?? null : null
}
if (vis && !clipped && node.width > parent.width * 1.2) {
overflows++
}
}
}
for (const cid of node.childIds) walk(cid)
}
walk(thumb?.id ?? '')
expect(overflows).toBe(0)
})
}) })

3
tests/fixtures/gold-preview.fig vendored Normal file
View file

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:8e54efe5f1e11a1bf3cce669a4de8e3fa42047f4325472da2d8cd3b152c2da33
size 550091