Fix copyFill missing gradientTransform/imageTransform, add copy tests
Deep-copy gradient and image transforms in copyFill to prevent shared-reference mutations. Add mutation-isolation tests for all copy helpers. Fix benchmark to use precomputed IDs and actual copyFills function.
This commit is contained in:
parent
66c80685bc
commit
7c584db1b7
|
|
@ -14,9 +14,9 @@ import type { Effect, Fill, GeometryPath, GradientStop, Stroke, StyleRun } from
|
||||||
|
|
||||||
export function copyFill(f: Fill): Fill {
|
export function copyFill(f: Fill): Fill {
|
||||||
const copy: Fill = { ...f, color: { ...f.color } }
|
const copy: Fill = { ...f, color: { ...f.color } }
|
||||||
if (f.gradientStops) {
|
if (f.gradientStops) copy.gradientStops = f.gradientStops.map(copyGradientStop)
|
||||||
copy.gradientStops = f.gradientStops.map(copyGradientStop)
|
if (f.gradientTransform) copy.gradientTransform = { ...f.gradientTransform }
|
||||||
}
|
if (f.imageTransform) copy.imageTransform = { ...f.imageTransform }
|
||||||
return copy
|
return copy
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
109
tests/engine/copy.test.ts
Normal file
109
tests/engine/copy.test.ts
Normal file
|
|
@ -0,0 +1,109 @@
|
||||||
|
import { describe, test, expect } from 'bun:test'
|
||||||
|
import {
|
||||||
|
copyFill,
|
||||||
|
copyFills,
|
||||||
|
copyStroke,
|
||||||
|
copyEffect,
|
||||||
|
copyStyleRun,
|
||||||
|
copyGeometryPaths
|
||||||
|
} from '../../packages/core/src/copy'
|
||||||
|
import type { Fill, Stroke, Effect, StyleRun, GeometryPath } from '@open-pencil/core'
|
||||||
|
|
||||||
|
describe('copy helpers — mutation isolation', () => {
|
||||||
|
test('copyFill: mutating copy does not affect original', () => {
|
||||||
|
const original: Fill = {
|
||||||
|
type: 'SOLID',
|
||||||
|
color: { r: 1, g: 0, b: 0, a: 1 },
|
||||||
|
opacity: 1,
|
||||||
|
visible: true,
|
||||||
|
}
|
||||||
|
const copy = copyFill(original)
|
||||||
|
copy.color.r = 0
|
||||||
|
copy.opacity = 0.5
|
||||||
|
expect(original.color.r).toBe(1)
|
||||||
|
expect(original.opacity).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyFill: gradient stops are deep copied', () => {
|
||||||
|
const original: Fill = {
|
||||||
|
type: 'GRADIENT_LINEAR',
|
||||||
|
color: { r: 0, g: 0, b: 0, a: 1 },
|
||||||
|
opacity: 1,
|
||||||
|
visible: true,
|
||||||
|
gradientStops: [
|
||||||
|
{ color: { r: 1, g: 0, b: 0, a: 1 }, position: 0 },
|
||||||
|
{ color: { r: 0, g: 0, b: 1, a: 1 }, position: 1 },
|
||||||
|
],
|
||||||
|
gradientTransform: { m00: 1, m01: 0, m02: 0, m10: 0, m11: 1, m12: 0 },
|
||||||
|
}
|
||||||
|
const copy = copyFill(original)
|
||||||
|
copy.gradientStops![0].color.r = 0
|
||||||
|
copy.gradientTransform!.m00 = 99
|
||||||
|
expect(original.gradientStops![0].color.r).toBe(1)
|
||||||
|
expect(original.gradientTransform!.m00).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyStroke: dash pattern is independent', () => {
|
||||||
|
const original: Stroke = {
|
||||||
|
color: { r: 0, g: 0, b: 0, a: 1 },
|
||||||
|
weight: 1,
|
||||||
|
opacity: 1,
|
||||||
|
visible: true,
|
||||||
|
align: 'CENTER',
|
||||||
|
dashPattern: [5, 3],
|
||||||
|
}
|
||||||
|
const copy = copyStroke(original)
|
||||||
|
copy.dashPattern!.push(99)
|
||||||
|
copy.color.g = 1
|
||||||
|
expect(original.dashPattern).toEqual([5, 3])
|
||||||
|
expect(original.color.g).toBe(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyEffect: offset and color are independent', () => {
|
||||||
|
const original: Effect = {
|
||||||
|
type: 'DROP_SHADOW',
|
||||||
|
color: { r: 0, g: 0, b: 0, a: 0.5 },
|
||||||
|
offset: { x: 4, y: 4 },
|
||||||
|
radius: 8,
|
||||||
|
spread: 0,
|
||||||
|
visible: true,
|
||||||
|
}
|
||||||
|
const copy = copyEffect(original)
|
||||||
|
copy.offset.x = 100
|
||||||
|
copy.color.a = 1
|
||||||
|
expect(original.offset.x).toBe(4)
|
||||||
|
expect(original.color.a).toBe(0.5)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyStyleRun: style object is independent', () => {
|
||||||
|
const original: StyleRun = {
|
||||||
|
start: 0,
|
||||||
|
length: 5,
|
||||||
|
style: { fontWeight: 700, fontSize: 24 },
|
||||||
|
}
|
||||||
|
const copy = copyStyleRun(original)
|
||||||
|
copy.style.fontWeight = 400
|
||||||
|
expect(original.style.fontWeight).toBe(700)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyGeometryPaths: Uint8Array is independent', () => {
|
||||||
|
const original: GeometryPath[] = [
|
||||||
|
{ windingRule: 'NONZERO', commandsBlob: new Uint8Array([1, 2, 3]) },
|
||||||
|
]
|
||||||
|
const copy = copyGeometryPaths(original)
|
||||||
|
copy[0].commandsBlob[0] = 99
|
||||||
|
expect(original[0].commandsBlob[0]).toBe(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('copyFills: array independence', () => {
|
||||||
|
const originals: Fill[] = [
|
||||||
|
{ type: 'SOLID', color: { r: 1, g: 0, b: 0, a: 1 }, opacity: 1, visible: true },
|
||||||
|
{ type: 'SOLID', color: { r: 0, g: 1, b: 0, a: 1 }, opacity: 0.5, visible: true },
|
||||||
|
]
|
||||||
|
const copies = copyFills(originals)
|
||||||
|
copies.push({ type: 'SOLID', color: { r: 0, g: 0, b: 1, a: 1 }, opacity: 1, visible: true })
|
||||||
|
copies[0].color.r = 0
|
||||||
|
expect(originals).toHaveLength(2)
|
||||||
|
expect(originals[0].color.r).toBe(1)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -3,6 +3,7 @@ import { readFileSync } from 'fs'
|
||||||
import { resolve } from 'path'
|
import { resolve } from 'path'
|
||||||
|
|
||||||
import { parseFigFile, exportFigFile, initCodec, SceneGraph } from '@open-pencil/core'
|
import { parseFigFile, exportFigFile, initCodec, SceneGraph } from '@open-pencil/core'
|
||||||
|
import { copyFills } from '../../packages/core/src/copy'
|
||||||
|
|
||||||
const FIXTURES = resolve(import.meta.dir, '../fixtures')
|
const FIXTURES = resolve(import.meta.dir, '../fixtures')
|
||||||
|
|
||||||
|
|
@ -40,9 +41,9 @@ group('SceneGraph operations', () => {
|
||||||
void nodes.length
|
void nodes.length
|
||||||
})
|
})
|
||||||
|
|
||||||
|
const sampleIds = [...materialGraph.getAllNodes()].slice(0, 1000).map((n) => n.id)
|
||||||
bench('getNode × 1000 (material3)', () => {
|
bench('getNode × 1000 (material3)', () => {
|
||||||
const ids = [...materialGraph.getAllNodes()].slice(0, 1000).map((n) => n.id)
|
for (const id of sampleIds) materialGraph.getNode(id)
|
||||||
for (const id of ids) materialGraph.getNode(id)
|
|
||||||
})
|
})
|
||||||
|
|
||||||
bench('createNode + deleteNode × 100', () => {
|
bench('createNode + deleteNode × 100', () => {
|
||||||
|
|
@ -85,13 +86,8 @@ group('structuredClone vs copy (fills)', () => {
|
||||||
for (let i = 0; i < 10_000; i++) structuredClone(sampleFills)
|
for (let i = 0; i < 10_000; i++) structuredClone(sampleFills)
|
||||||
})
|
})
|
||||||
|
|
||||||
bench('manual copy × 10k', () => {
|
bench('copyFills × 10k', () => {
|
||||||
for (let i = 0; i < 10_000; i++) {
|
for (let i = 0; i < 10_000; i++) copyFills(sampleFills)
|
||||||
sampleFills.map((f) => ({
|
|
||||||
...f,
|
|
||||||
color: { ...f.color }
|
|
||||||
}))
|
|
||||||
}
|
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue