diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 7795af8e9..008233091 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -1424,7 +1424,7 @@ export class ComfyPage { } async closeDialog() { - await this.page.locator('.p-dialog-close-button').click() + await this.page.locator('.p-dialog-close-button').click({ force: true }) await expect(this.page.locator('.p-dialog')).toBeHidden() } diff --git a/src/renderer/core/layout/store/layoutStore.ts b/src/renderer/core/layout/store/layoutStore.ts index ca5d67120..673344086 100644 --- a/src/renderer/core/layout/store/layoutStore.ts +++ b/src/renderer/core/layout/store/layoutStore.ts @@ -36,9 +36,19 @@ import { type Point, type RerouteId, type RerouteLayout, - type Size, type SlotLayout } from '@/renderer/core/layout/types' +import { + REROUTE_RADIUS, + boundsIntersect, + pointInBounds +} from '@/renderer/core/layout/utils/layoutMath' +import { makeLinkSegmentKey } from '@/renderer/core/layout/utils/layoutUtils' +import { + type NodeLayoutMap, + layoutToYNode, + yNodeToLayout +} from '@/renderer/core/layout/utils/mappers' import { SpatialIndexManager } from '@/renderer/core/spatial/SpatialIndex' type YEventChange = { @@ -48,9 +58,6 @@ type YEventChange = { const logger = log.getLogger('LayoutStore') -// Constants -const REROUTE_RADIUS = 8 - // Utility functions function asRerouteId(id: string | number): RerouteId { return Number(id) @@ -60,15 +67,6 @@ function asLinkId(id: string | number): LinkId { return Number(id) } -interface NodeLayoutData { - id: NodeId - position: Point - size: Size - zIndex: number - visible: boolean - bounds: Bounds -} - interface LinkData { id: LinkId sourceNodeId: NodeId @@ -91,15 +89,6 @@ interface TypedYMap { } class LayoutStoreImpl implements LayoutStore { - private static readonly NODE_DEFAULTS: NodeLayoutData = { - id: 'unknown-node', - position: { x: 0, y: 0 }, - size: { width: 100, height: 50 }, - zIndex: 0, - visible: true, - bounds: { x: 0, y: 0, width: 100, height: 50 } - } - private static readonly REROUTE_DEFAULTS: RerouteData = { id: 0, position: { x: 0, y: 0 }, @@ -109,7 +98,7 @@ class LayoutStoreImpl implements LayoutStore { // Yjs document and shared data structures private ydoc = new Y.Doc() - private ynodes: Y.Map> // Maps nodeId -> Y.Map containing NodeLayout data + private ynodes: Y.Map // Maps nodeId -> NodeLayoutMap containing NodeLayout data private ylinks: Y.Map> // Maps linkId -> Y.Map containing link data private yreroutes: Y.Map> // Maps rerouteId -> Y.Map containing reroute data private yoperations: Y.Array // Operation log @@ -155,7 +144,7 @@ class LayoutStoreImpl implements LayoutStore { this.rerouteSpatialIndex = new SpatialIndexManager() // Listen for Yjs changes and trigger Vue reactivity - this.ynodes.observe((event: Y.YMapEvent>) => { + this.ynodes.observe((event: Y.YMapEvent) => { this.version++ // Trigger all affected node refs @@ -184,16 +173,6 @@ class LayoutStoreImpl implements LayoutStore { }) } - private getNodeField( - ynode: Y.Map, - field: K, - defaultValue: NodeLayoutData[K] = LayoutStoreImpl.NODE_DEFAULTS[field] - ): NodeLayoutData[K] { - const typedNode = ynode as TypedYMap - const value = typedNode.get(field) - return value ?? defaultValue - } - private getLinkField( ylink: Y.Map, field: K @@ -227,7 +206,7 @@ class LayoutStoreImpl implements LayoutStore { get: () => { track() const ynode = this.ynodes.get(nodeId) - const layout = ynode ? this.yNodeToLayout(ynode) : null + const layout = ynode ? yNodeToLayout(ynode) : null return layout }, set: (newLayout: NodeLayout | null) => { @@ -242,7 +221,7 @@ class LayoutStoreImpl implements LayoutStore { timestamp: Date.now(), source: this.currentSource, actor: this.currentActor, - previousLayout: this.yNodeToLayout(existing) + previousLayout: yNodeToLayout(existing) }) } } else { @@ -260,7 +239,7 @@ class LayoutStoreImpl implements LayoutStore { actor: this.currentActor }) } else { - const existingLayout = this.yNodeToLayout(existing) + const existingLayout = yNodeToLayout(existing) // Check what properties changed if ( @@ -330,8 +309,8 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) - if (layout && this.boundsIntersect(layout.bounds, bounds)) { + const layout = yNodeToLayout(ynode) + if (layout && boundsIntersect(layout.bounds, bounds)) { result.push(nodeId) } } @@ -352,7 +331,7 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) + const layout = yNodeToLayout(ynode) if (layout) { result.set(nodeId, layout) } @@ -378,7 +357,7 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) + const layout = yNodeToLayout(ynode) if (layout) { nodes.push([nodeId, layout]) } @@ -389,7 +368,7 @@ class LayoutStoreImpl implements LayoutStore { nodes.sort(([, a], [, b]) => b.zIndex - a.zIndex) for (const [nodeId, layout] of nodes) { - if (this.pointInBounds(point, layout.bounds)) { + if (pointInBounds(point, layout.bounds)) { return nodeId } } @@ -561,16 +540,6 @@ class LayoutStoreImpl implements LayoutStore { return this.rerouteLayouts.get(rerouteId) || null } - /** - * Helper to create internal key for link segment - */ - private makeLinkSegmentKey( - linkId: LinkId, - rerouteId: RerouteId | null - ): string { - return `${linkId}:${rerouteId ?? 'final'}` - } - /** * Update link segment layout data */ @@ -579,7 +548,7 @@ class LayoutStoreImpl implements LayoutStore { rerouteId: RerouteId | null, layout: Omit ): void { - const key = this.makeLinkSegmentKey(linkId, rerouteId) + const key = makeLinkSegmentKey(linkId, rerouteId) const existing = this.linkSegmentLayouts.get(key) // Short-circuit if bounds and centerPos unchanged (prevents spatial index churn) @@ -629,7 +598,7 @@ class LayoutStoreImpl implements LayoutStore { * Delete link segment layout data */ deleteLinkSegmentLayout(linkId: LinkId, rerouteId: RerouteId | null): void { - const key = this.makeLinkSegmentKey(linkId, rerouteId) + const key = makeLinkSegmentKey(linkId, rerouteId) const deleted = this.linkSegmentLayouts.delete(key) if (deleted) { // Remove from spatial index @@ -693,7 +662,7 @@ class LayoutStoreImpl implements LayoutStore { rerouteId: segmentLayout.rerouteId } } - } else if (this.pointInBounds(point, segmentLayout.bounds)) { + } else if (pointInBounds(point, segmentLayout.bounds)) { // Fallback to bounding box test return { linkId: segmentLayout.linkId, @@ -733,7 +702,7 @@ class LayoutStoreImpl implements LayoutStore { // Check precise bounds for candidates for (const key of candidateSlotKeys) { const slotLayout = this.slotLayouts.get(key) - if (slotLayout && this.pointInBounds(point, slotLayout.bounds)) { + if (slotLayout && pointInBounds(point, slotLayout.bounds)) { return slotLayout } } @@ -969,7 +938,7 @@ class LayoutStoreImpl implements LayoutStore { } } - this.ynodes.set(layout.id, this.layoutToYNode(layout)) + this.ynodes.set(layout.id, layoutToYNode(layout)) // Add to spatial index this.spatialIndex.insert(layout.id, layout.bounds) @@ -987,7 +956,7 @@ class LayoutStoreImpl implements LayoutStore { return } - const size = this.getNodeField(ynode, 'size') + const size = yNodeToLayout(ynode).size const newBounds = { x: operation.position.x, y: operation.position.y, @@ -1016,7 +985,7 @@ class LayoutStoreImpl implements LayoutStore { const ynode = this.ynodes.get(operation.nodeId) if (!ynode) return - const position = this.getNodeField(ynode, 'position') + const position = yNodeToLayout(ynode).position const newBounds = { x: position.x, y: position.y, @@ -1053,7 +1022,7 @@ class LayoutStoreImpl implements LayoutStore { operation: CreateNodeOperation, change: LayoutChange ): void { - const ynode = this.layoutToYNode(operation.layout) + const ynode = layoutToYNode(operation.layout) this.ynodes.set(operation.nodeId, ynode) // Add to spatial index @@ -1187,7 +1156,7 @@ class LayoutStoreImpl implements LayoutStore { * Update node bounds helper */ private updateNodeBounds( - ynode: Y.Map, + ynode: NodeLayoutMap, position: Point, size: { width: number; height: number } ): void { @@ -1335,27 +1304,6 @@ class LayoutStoreImpl implements LayoutStore { } // Helper methods - private layoutToYNode(layout: NodeLayout): Y.Map { - const ynode = new Y.Map() - ynode.set('id', layout.id) - ynode.set('position', layout.position) - ynode.set('size', layout.size) - ynode.set('zIndex', layout.zIndex) - ynode.set('visible', layout.visible) - ynode.set('bounds', layout.bounds) - return ynode - } - - private yNodeToLayout(ynode: Y.Map): NodeLayout { - return { - id: this.getNodeField(ynode, 'id'), - position: this.getNodeField(ynode, 'position'), - size: this.getNodeField(ynode, 'size'), - zIndex: this.getNodeField(ynode, 'zIndex'), - visible: this.getNodeField(ynode, 'visible'), - bounds: this.getNodeField(ynode, 'bounds') - } - } private notifyChange(change: LayoutChange): void { this.changeListeners.forEach((listener) => { @@ -1367,24 +1315,6 @@ class LayoutStoreImpl implements LayoutStore { }) } - private pointInBounds(point: Point, bounds: Bounds): boolean { - return ( - point.x >= bounds.x && - point.x <= bounds.x + bounds.width && - point.y >= bounds.y && - point.y <= bounds.y + bounds.height - ) - } - - private boundsIntersect(a: Bounds, b: Bounds): boolean { - return !( - a.x + a.width < b.x || - b.x + b.width < a.x || - a.y + a.height < b.y || - b.y + b.height < a.y - ) - } - // CRDT-specific methods getOperationsSince(timestamp: number): LayoutOperation[] { const operations: LayoutOperation[] = [] diff --git a/src/renderer/core/layout/utils/layoutMath.ts b/src/renderer/core/layout/utils/layoutMath.ts new file mode 100644 index 000000000..786e261dd --- /dev/null +++ b/src/renderer/core/layout/utils/layoutMath.ts @@ -0,0 +1,21 @@ +import type { Bounds, Point } from '@/renderer/core/layout/types' + +export const REROUTE_RADIUS = 8 + +export function pointInBounds(point: Point, bounds: Bounds): boolean { + return ( + point.x >= bounds.x && + point.x <= bounds.x + bounds.width && + point.y >= bounds.y && + point.y <= bounds.y + bounds.height + ) +} + +export function boundsIntersect(a: Bounds, b: Bounds): boolean { + return !( + a.x + a.width < b.x || + b.x + b.width < a.x || + a.y + a.height < b.y || + b.y + b.height < a.y + ) +} diff --git a/src/renderer/core/layout/utils/layoutUtils.ts b/src/renderer/core/layout/utils/layoutUtils.ts new file mode 100644 index 000000000..408e2718d --- /dev/null +++ b/src/renderer/core/layout/utils/layoutUtils.ts @@ -0,0 +1,11 @@ +import type { LinkId, RerouteId } from '@/renderer/core/layout/types' + +/** + * Creates a unique key for identifying link segments in spatial indexes + */ +export function makeLinkSegmentKey( + linkId: LinkId, + rerouteId: RerouteId | null +): string { + return `${linkId}:${rerouteId ?? 'final'}` +} diff --git a/src/renderer/core/layout/utils/mappers.ts b/src/renderer/core/layout/utils/mappers.ts new file mode 100644 index 000000000..48b85dc53 --- /dev/null +++ b/src/renderer/core/layout/utils/mappers.ts @@ -0,0 +1,45 @@ +import * as Y from 'yjs' + +import type { NodeLayout } from '@/renderer/core/layout/types' + +export type NodeLayoutMap = Y.Map + +export const NODE_LAYOUT_DEFAULTS: NodeLayout = { + id: 'unknown-node', + position: { x: 0, y: 0 }, + size: { width: 100, height: 50 }, + zIndex: 0, + visible: true, + bounds: { x: 0, y: 0, width: 100, height: 50 } +} + +export function layoutToYNode(layout: NodeLayout): NodeLayoutMap { + const ynode = new Y.Map() as NodeLayoutMap + ynode.set('id', layout.id) + ynode.set('position', layout.position) + ynode.set('size', layout.size) + ynode.set('zIndex', layout.zIndex) + ynode.set('visible', layout.visible) + ynode.set('bounds', layout.bounds) + return ynode +} + +function getOr( + map: NodeLayoutMap, + key: K, + fallback: NodeLayout[K] +): NodeLayout[K] { + const v = map.get(key) + return (v ?? fallback) as NodeLayout[K] +} + +export function yNodeToLayout(ynode: NodeLayoutMap): NodeLayout { + return { + id: getOr(ynode, 'id', NODE_LAYOUT_DEFAULTS.id), + position: getOr(ynode, 'position', NODE_LAYOUT_DEFAULTS.position), + size: getOr(ynode, 'size', NODE_LAYOUT_DEFAULTS.size), + zIndex: getOr(ynode, 'zIndex', NODE_LAYOUT_DEFAULTS.zIndex), + visible: getOr(ynode, 'visible', NODE_LAYOUT_DEFAULTS.visible), + bounds: getOr(ynode, 'bounds', NODE_LAYOUT_DEFAULTS.bounds) + } +} diff --git a/tests-ui/tests/renderer/core/layout/utils/layoutMath.test.ts b/tests-ui/tests/renderer/core/layout/utils/layoutMath.test.ts new file mode 100644 index 000000000..3d768740e --- /dev/null +++ b/tests-ui/tests/renderer/core/layout/utils/layoutMath.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' + +import { + REROUTE_RADIUS, + boundsIntersect, + pointInBounds +} from '@/renderer/core/layout/utils/layoutMath' + +describe('layoutMath utils', () => { + describe('pointInBounds', () => { + it('detects inclusion correctly', () => { + const bounds = { x: 10, y: 10, width: 100, height: 50 } + expect(pointInBounds({ x: 10, y: 10 }, bounds)).toBe(true) + expect(pointInBounds({ x: 110, y: 60 }, bounds)).toBe(true) + expect(pointInBounds({ x: 9, y: 10 }, bounds)).toBe(false) + expect(pointInBounds({ x: 111, y: 10 }, bounds)).toBe(false) + expect(pointInBounds({ x: 10, y: 61 }, bounds)).toBe(false) + }) + + it('works with zero-size bounds', () => { + const zero = { x: 10, y: 20, width: 0, height: 0 } + expect(pointInBounds({ x: 10, y: 20 }, zero)).toBe(true) + expect(pointInBounds({ x: 10, y: 21 }, zero)).toBe(false) + expect(pointInBounds({ x: 9, y: 20 }, zero)).toBe(false) + }) + }) + + describe('boundsIntersect', () => { + it('detects intersection correctly', () => { + const a = { x: 0, y: 0, width: 10, height: 10 } + const b = { x: 5, y: 5, width: 10, height: 10 } + const c = { x: 11, y: 0, width: 5, height: 5 } + expect(boundsIntersect(a, b)).toBe(true) + expect(boundsIntersect(a, c)).toBe(false) + }) + + it('treats touching edges as intersecting', () => { + const a = { x: 0, y: 0, width: 10, height: 10 } + const d = { x: 10, y: 0, width: 5, height: 5 } // touches at right edge + expect(boundsIntersect(a, d)).toBe(true) + }) + }) + + describe('REROUTE_RADIUS', () => { + it('exports a sensible reroute radius', () => { + expect(REROUTE_RADIUS).toBeGreaterThan(0) + }) + }) +}) diff --git a/tests-ui/tests/renderer/core/layout/utils/layoutUtils.test.ts b/tests-ui/tests/renderer/core/layout/utils/layoutUtils.test.ts new file mode 100644 index 000000000..4c2dd87be --- /dev/null +++ b/tests-ui/tests/renderer/core/layout/utils/layoutUtils.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest' + +import { makeLinkSegmentKey } from '@/renderer/core/layout/utils/layoutUtils' + +describe('layoutUtils', () => { + describe('makeLinkSegmentKey', () => { + it('creates stable keys for null reroute', () => { + expect(makeLinkSegmentKey(10, null)).toBe('10:final') + expect(makeLinkSegmentKey(42, null)).toBe('42:final') + }) + + it('creates stable keys for numeric reroute ids', () => { + expect(makeLinkSegmentKey(10, 3)).toBe('10:3') + expect(makeLinkSegmentKey(42, 0)).toBe('42:0') + expect(makeLinkSegmentKey(42, 7)).toBe('42:7') + }) + }) +}) diff --git a/tests-ui/tests/renderer/core/layout/utils/mappers.test.ts b/tests-ui/tests/renderer/core/layout/utils/mappers.test.ts new file mode 100644 index 000000000..a4ab0cd6b --- /dev/null +++ b/tests-ui/tests/renderer/core/layout/utils/mappers.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import * as Y from 'yjs' + +import { + NODE_LAYOUT_DEFAULTS, + type NodeLayoutMap, + yNodeToLayout +} from '@/renderer/core/layout/utils/mappers' + +describe('mappers', () => { + it('yNodeToLayout reads from Yjs-attached map', () => { + const layout = { + id: 'node-1', + position: { x: 12, y: 34 }, + size: { width: 111, height: 222 }, + zIndex: 5, + visible: true, + bounds: { x: 12, y: 34, width: 111, height: 222 } + } + + const doc = new Y.Doc() + const ynode = doc.getMap('node') as NodeLayoutMap + ynode.set('id', layout.id) + ynode.set('position', layout.position) + ynode.set('size', layout.size) + ynode.set('zIndex', layout.zIndex) + ynode.set('visible', layout.visible) + ynode.set('bounds', layout.bounds) + + const back = yNodeToLayout(ynode) + expect(back).toEqual(layout) + }) + + it('yNodeToLayout applies defaults for missing fields', () => { + const doc = new Y.Doc() + const ynode = doc.getMap('node') as NodeLayoutMap + // Don't set any fields - they should all use defaults + + const back = yNodeToLayout(ynode) + expect(back.id).toBe(NODE_LAYOUT_DEFAULTS.id) + expect(back.position).toEqual(NODE_LAYOUT_DEFAULTS.position) + expect(back.size).toEqual(NODE_LAYOUT_DEFAULTS.size) + expect(back.zIndex).toEqual(NODE_LAYOUT_DEFAULTS.zIndex) + expect(back.visible).toEqual(NODE_LAYOUT_DEFAULTS.visible) + expect(back.bounds).toEqual(NODE_LAYOUT_DEFAULTS.bounds) + }) +})