address review comments

This commit is contained in:
Benjamin Lu
2025-09-15 14:09:42 -07:00
parent 1dfa72cf19
commit 69f5391fce
13 changed files with 148 additions and 197 deletions

View File

@@ -1,6 +1,10 @@
import { useElementBounding } from '@vueuse/core'
import type { LGraphCanvas, Vector2 } from '@/lib/litegraph/src/litegraph'
import type { LGraphCanvas, Point } from '@/lib/litegraph/src/litegraph'
import { useCanvasStore } from '@/stores/graphStore'
let sharedConverter: ReturnType<typeof useCanvasPositionConversion> | null =
null
/**
* Convert between canvas and client positions
@@ -14,7 +18,7 @@ export const useCanvasPositionConversion = (
) => {
const { left, top, update } = useElementBounding(canvasElement)
const clientPosToCanvasPos = (pos: Vector2): Vector2 => {
const clientPosToCanvasPos = (pos: Point): Point => {
const { offset, scale } = lgCanvas.ds
return [
(pos[0] - left.value) / scale - offset[0],
@@ -22,7 +26,7 @@ export const useCanvasPositionConversion = (
]
}
const canvasPosToClientPos = (pos: Vector2): Vector2 => {
const canvasPosToClientPos = (pos: Point): Point => {
const { offset, scale } = lgCanvas.ds
return [
(pos[0] + offset[0]) * scale + left.value,
@@ -36,3 +40,10 @@ export const useCanvasPositionConversion = (
update
}
}
export function useSharedCanvasPositionConversion() {
if (sharedConverter) return sharedConverter
const lgCanvas = useCanvasStore().getCanvas()
sharedConverter = useCanvasPositionConversion(lgCanvas.canvas, lgCanvas)
return sharedConverter
}

View File

@@ -1,5 +1,6 @@
import { Ref } from 'vue'
import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
import { usePragmaticDroppable } from '@/composables/usePragmaticDragAndDrop'
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
@@ -27,16 +28,19 @@ export const useCanvasDrop = (canvasRef: Ref<HTMLCanvasElement>) => {
if (dndData.type === 'tree-explorer-node') {
const node = dndData.data as RenderedTreeExplorerNode
const conv = useSharedCanvasPositionConversion()
const basePos = conv.clientPosToCanvasPos([loc.clientX, loc.clientY])
if (node.data instanceof ComfyNodeDefImpl) {
const nodeDef = node.data
const pos = comfyApp.clientPosToCanvasPos([loc.clientX, loc.clientY])
const pos = [...basePos]
// Add an offset on y to make sure after adding the node, the cursor
// is on the node (top left corner)
pos[1] += LiteGraph.NODE_TITLE_HEIGHT
litegraphService.addNodeOnGraph(nodeDef, { pos })
} else if (node.data instanceof ComfyModelDef) {
const model = node.data
const pos = comfyApp.clientPosToCanvasPos([loc.clientX, loc.clientY])
const pos = basePos
const nodeAtPos = comfyApp.graph.getNodeOnPos(pos[0], pos[1])
let targetProvider: ModelNodeProvider | null = null
let targetGraphNode: LGraphNode | null = null
@@ -73,11 +77,7 @@ export const useCanvasDrop = (canvasRef: Ref<HTMLCanvasElement>) => {
}
} else if (node.data instanceof ComfyWorkflow) {
const workflow = node.data
const position = comfyApp.clientPosToCanvasPos([
loc.clientX,
loc.clientY
])
await workflowService.insertWorkflow(workflow, { position })
await workflowService.insertWorkflow(workflow, { position: basePos })
}
}
}

View File

@@ -8,7 +8,7 @@ import type {
INodeOutputSlot,
ISlotType,
LLink,
Vector2
Point
} from '@/lib/litegraph/src/litegraph'
import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events'
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
@@ -557,7 +557,7 @@ app.registerExtension({
}
)
function isNodeAtPos(pos: Vector2) {
function isNodeAtPos(pos: Point) {
for (const n of app.graph.nodes) {
if (n.pos[0] === pos[0] && n.pos[1] === pos[1]) {
return true

View File

@@ -1,45 +0,0 @@
/**
* Canvas Rect Cache (VueUse-based)
*
* Tracks the client-origin and size of the graph canvas container using
* useElementBounding, and exposes a small API to read the rect and
* subscribe to changes.
*
* Assumptions:
* - Document scrolling is disabled (body overflow: hidden)
* - Layout changes are driven by window resize and container/splitter changes
*/
import { useElementBounding } from '@vueuse/core'
import { shallowRef, watch } from 'vue'
// Target container element (covers the canvas fully and shares its origin)
const containerRef = shallowRef<HTMLElement | null>(null)
// Bind bounding measurement once; element may be resolved later
const { x, y, width, height, update } = useElementBounding(containerRef, {
windowResize: true,
windowScroll: false,
immediate: true
})
// Listener registry for external subscribers
const listeners = new Set<() => void>()
function ensureContainer() {
if (!containerRef.value) {
containerRef.value = document.getElementById(
'graph-canvas-container'
) as HTMLElement | null
if (containerRef.value) update()
}
}
// Notify subscribers when the bounding rect changes
watch([x, y, width, height], () => {
if (listeners.size) listeners.forEach((cb) => cb())
})
export function getCanvasClientOrigin() {
ensureContainer()
return { left: x.value || 0, top: y.value || 0 }
}

View File

@@ -2,10 +2,41 @@ import type { InjectionKey } from 'vue'
import type { Point } from '@/renderer/core/layout/types'
/**
* Lightweight, injectable transform state used by layout-aware components.
*
* Consumers use this interface to convert coordinates between LiteGraph's
* canvas space and the DOM's screen space, access the current pan/zoom
* (camera), and perform basic viewport culling checks.
*
* Coordinate mapping:
* - screen = (canvas + offset) * scale
* - canvas = screen / scale - offset
*
* The full implementation and additional helpers live in
* `useTransformState()`. This interface deliberately exposes only the
* minimal surface needed outside that composable.
*
* @example
* const state = inject(TransformStateKey)!
* const screen = state.canvasToScreen({ x: 100, y: 50 })
*/
interface TransformState {
/** Convert a screen-space point (CSS pixels) to canvas space. */
screenToCanvas: (p: Point) => Point
/** Convert a canvas-space point to screen space (CSS pixels). */
canvasToScreen: (p: Point) => Point
/** Current pan/zoom; `x`/`y` are offsets, `z` is scale. */
camera?: { x: number; y: number; z: number }
/**
* Test whether a node's rectangle intersects the (expanded) viewport.
* Handy for viewport culling and lazy work.
*
* @param nodePos Top-left in canvas space `[x, y]`
* @param nodeSize Size in canvas units `[width, height]`
* @param viewport Screen-space viewport `{ width, height }`
* @param margin Optional fractional margin (e.g. `0.2` = 20%)
*/
isNodeInViewport?: (
nodePos: ArrayLike<number>,
nodeSize: ArrayLike<number>,

View File

@@ -39,7 +39,10 @@ import {
type Size,
type SlotLayout
} from '@/renderer/core/layout/types'
import { sameBounds, samePoint } from '@/renderer/core/layout/utils/geometry'
import {
isBoundsEqual,
isPointEqual
} from '@/renderer/core/layout/utils/geometry'
import { SpatialIndexManager } from '@/renderer/core/spatial/SpatialIndex'
type YEventChange = {
@@ -414,8 +417,8 @@ class LayoutStoreImpl implements LayoutStore {
// Short-circuit if bounds and centerPos unchanged
if (
existing &&
sameBounds(existing.bounds, layout.bounds) &&
samePoint(existing.centerPos, layout.centerPos)
isBoundsEqual(existing.bounds, layout.bounds) &&
isPointEqual(existing.centerPos, layout.centerPos)
) {
// Only update path if provided (for hit detection)
if (layout.path) {
@@ -456,8 +459,8 @@ class LayoutStoreImpl implements LayoutStore {
if (existing) {
// Short-circuit if geometry is unchanged
if (
samePoint(existing.position, layout.position) &&
sameBounds(existing.bounds, layout.bounds)
isPointEqual(existing.position, layout.position) &&
isBoundsEqual(existing.bounds, layout.bounds)
) {
return
}
@@ -486,8 +489,8 @@ class LayoutStoreImpl implements LayoutStore {
if (existing) {
// Short-circuit if geometry is unchanged
if (
samePoint(existing.position, layout.position) &&
sameBounds(existing.bounds, layout.bounds)
isPointEqual(existing.position, layout.position) &&
isBoundsEqual(existing.bounds, layout.bounds)
) {
continue
}
@@ -617,8 +620,8 @@ class LayoutStoreImpl implements LayoutStore {
// Short-circuit if bounds and centerPos unchanged (prevents spatial index churn)
if (
existing &&
sameBounds(existing.bounds, layout.bounds) &&
samePoint(existing.centerPos, layout.centerPos)
isBoundsEqual(existing.bounds, layout.bounds) &&
isPointEqual(existing.centerPos, layout.centerPos)
) {
// Only update path if provided (for hit detection)
if (layout.path) {

View File

@@ -1,10 +1,10 @@
import type { Bounds, Point } from '@/renderer/core/layout/types'
export function samePoint(a: Point, b: Point): boolean {
export function isPointEqual(a: Point, b: Point): boolean {
return a.x === b.x && a.y === b.y
}
export function sameBounds(a: Bounds, b: Bounds): boolean {
export function isBoundsEqual(a: Bounds, b: Bounds): boolean {
return (
a.x === b.x && a.y === b.y && a.width === b.width && a.height === b.height
)

View File

@@ -40,7 +40,6 @@ import {
import { useErrorHandling } from '@/composables/useErrorHandling'
import { getSlotColor } from '@/constants/slotColors'
import { INodeSlot, LGraphNode } from '@/lib/litegraph/src/litegraph'
// DOM-based slot registration for arbitrary positioning
import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
import SlotConnectionDot from './SlotConnectionDot.vue'
@@ -86,7 +85,7 @@ watchEffect(() => {
useSlotElementTracking({
nodeId: props.nodeId ?? '',
index: props.index,
isInput: true,
type: 'input',
element: slotElRef
})
</script>

View File

@@ -41,7 +41,6 @@ import {
import { useErrorHandling } from '@/composables/useErrorHandling'
import { getSlotColor } from '@/constants/slotColors'
import type { INodeSlot, LGraphNode } from '@/lib/litegraph/src/litegraph'
// DOM-based slot registration for arbitrary positioning
import { useSlotElementTracking } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
import SlotConnectionDot from './SlotConnectionDot.vue'
@@ -88,7 +87,7 @@ watchEffect(() => {
useSlotElementTracking({
nodeId: props.nodeId ?? '',
index: props.index,
isInput: false,
type: 'output',
element: slotElRef
})
</script>

View File

@@ -5,25 +5,23 @@
* positions in a single batched pass, and caches offsets so that node moves
* update slot positions without DOM reads.
*/
import { type Ref, inject, nextTick, onMounted, onUnmounted, watch } from 'vue'
import { type Ref, onMounted, onUnmounted, watch } from 'vue'
import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
import { getCanvasClientOrigin } from '@/renderer/core/layout/dom/canvasRectCache'
import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier'
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
import type { Point, SlotLayout } from '@/renderer/core/layout/types'
import type { SlotLayout } from '@/renderer/core/layout/types'
type SlotEntry = {
el: HTMLElement
index: number
isInput: boolean
type: 'input' | 'output'
cachedOffset?: { x: number; y: number }
}
type NodeEntry = {
nodeId: string
screenToCanvas?: (p: Point) => Point
slots: Map<string, SlotEntry>
stopWatch?: () => void
}
@@ -47,45 +45,34 @@ function scheduleSlotLayoutSync(nodeId: string) {
function flushScheduledSlotLayoutSync() {
if (pendingNodes.size === 0) return
// Read container origin once from cache
const { left: originLeft, top: originTop } = getCanvasClientOrigin()
const conv = useSharedCanvasPositionConversion()
for (const nodeId of Array.from(pendingNodes)) {
pendingNodes.delete(nodeId)
syncNodeSlotLayoutsFromDOM(nodeId, originLeft, originTop)
syncNodeSlotLayoutsFromDOM(nodeId, conv)
}
}
function syncNodeSlotLayoutsFromDOM(
nodeId: string,
originLeft?: number,
originTop?: number
conv?: ReturnType<typeof useSharedCanvasPositionConversion>
) {
const node = nodeRegistry.get(nodeId)
if (!node) return
if (!node.screenToCanvas) return
const nodeLayout = layoutStore.getNodeLayoutRef(nodeId).value
if (!nodeLayout) return
// Compute origin lazily if not provided
let originL = originLeft
let originT = originTop
if (originL == null || originT == null) {
const { left, top } = getCanvasClientOrigin()
originL = left
originT = top
}
const batch: Array<{ key: string; layout: SlotLayout }> = []
for (const [slotKey, entry] of node.slots) {
const rect = entry.el.getBoundingClientRect()
const centerScreen = {
x: rect.left + rect.width / 2 - (originL ?? 0),
y: rect.top + rect.height / 2 - (originT ?? 0)
}
const centerCanvas = node.screenToCanvas(centerScreen)
const screenCenter: [number, number] = [
rect.left + rect.width / 2,
rect.top + rect.height / 2
]
const [x, y] = (
conv ?? useSharedCanvasPositionConversion()
).clientPosToCanvasPos(screenCenter)
const centerCanvas = { x, y }
// Cache offset relative to node position for fast updates later
entry.cachedOffset = {
@@ -101,7 +88,7 @@ function syncNodeSlotLayoutsFromDOM(
layout: {
nodeId,
index: entry.index,
type: entry.isInput ? 'input' : 'output',
type: entry.type,
position: { x: centerCanvas.x, y: centerCanvas.y },
bounds: {
x: centerCanvas.x - half,
@@ -141,7 +128,7 @@ function updateNodeSlotsFromCache(nodeId: string) {
layout: {
nodeId,
index: entry.index,
type: entry.isInput ? 'input' : 'output',
type: entry.type,
position: { x: centerCanvas.x, y: centerCanvas.y },
bounds: {
x: centerCanvas.x - half,
@@ -159,60 +146,54 @@ function updateNodeSlotsFromCache(nodeId: string) {
export function useSlotElementTracking(options: {
nodeId: string
index: number
isInput: boolean
type: 'input' | 'output'
element: Ref<HTMLElement | null>
}) {
const { nodeId, index, isInput, element } = options
const { nodeId, index, type, element } = options
// Get transform utilities from TransformPane
const transformState = inject(TransformStateKey)
onMounted(async () => {
onMounted(() => {
if (!nodeId) return
await nextTick()
const el = element.value
if (!el) return
const stop = watch(
element,
(el) => {
if (!el) return
// Ensure node entry
let node = nodeRegistry.get(nodeId)
if (!node) {
node = {
nodeId,
screenToCanvas: transformState?.screenToCanvas,
slots: new Map()
}
nodeRegistry.set(nodeId, node)
// Subscribe once per node to layout changes for fast cached updates
const nodeRef = layoutStore.getNodeLayoutRef(nodeId)
const stop = watch(
nodeRef,
(newLayout, oldLayout) => {
if (newLayout && oldLayout) {
const moved =
newLayout.position.x !== oldLayout.position.x ||
newLayout.position.y !== oldLayout.position.y
const resized =
newLayout.size.width !== oldLayout.size.width ||
newLayout.size.height !== oldLayout.size.height
// Ensure node entry
let node = nodeRegistry.get(nodeId)
if (!node) {
const entry: NodeEntry = {
nodeId,
slots: new Map()
}
nodeRegistry.set(nodeId, entry)
// Only update from cache on move-only changes.
// On resizes (or move+resize), let ResizeObserver resync slots from DOM accurately.
if (moved && !resized) {
const unsubscribe = layoutStore.onChange((change) => {
const op = change.operation
if (
op &&
op.entity === 'node' &&
op.nodeId === nodeId &&
op.type === 'moveNode'
) {
updateNodeSlotsFromCache(nodeId)
}
}
},
{ flush: 'post' }
)
node.stopWatch = () => stop()
}
})
entry.stopWatch = () => unsubscribe()
node = entry
}
// Register slot
const slotKey = getSlotKey(nodeId, index, isInput)
node.slots.set(slotKey, { el, index, isInput })
// Register slot
const slotKey = getSlotKey(nodeId, index, type === 'input')
node.slots.set(slotKey, { el, index, type })
// Seed initial sync from DOM
scheduleSlotLayoutSync(nodeId)
// Seed initial sync from DOM
scheduleSlotLayoutSync(nodeId)
// Stop watching once registered
stop()
},
{ immediate: true, flush: 'post' }
)
})
onUnmounted(() => {
@@ -221,12 +202,13 @@ export function useSlotElementTracking(options: {
if (!node) return
// Remove this slot from registry and layout
const slotKey = getSlotKey(nodeId, index, isInput)
const slotKey = getSlotKey(nodeId, index, type === 'input')
node.slots.delete(slotKey)
layoutStore.deleteSlotLayout(slotKey)
// If node has no more slots, clean up
if (node.slots.size === 0) {
// Stop the node-level watcher when the last slot is gone
if (node.stopWatch) node.stopWatch()
nodeRegistry.delete(nodeId)
}
@@ -237,9 +219,6 @@ export function useSlotElementTracking(options: {
}
}
export function syncNodeSlotLayoutsNow(
nodeId: string,
origin?: { left: number; top: number }
) {
syncNodeSlotLayoutsFromDOM(nodeId, origin?.left, origin?.top)
export function syncNodeSlotLayoutsNow(nodeId: string) {
syncNodeSlotLayoutsFromDOM(nodeId)
}

View File

@@ -8,23 +8,15 @@
* Supports different element types (nodes, slots, widgets, etc.) with
* customizable data attributes and update handlers.
*/
import { getCurrentInstance, inject, onMounted, onUnmounted } from 'vue'
import { getCurrentInstance, onMounted, onUnmounted } from 'vue'
import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
import { getCanvasClientOrigin } from '@/renderer/core/layout/dom/canvasRectCache'
import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
import type { Point } from '@/renderer/core/layout/types'
import type { Bounds, NodeId } from '@/renderer/core/layout/types'
import { syncNodeSlotLayoutsNow } from './useSlotElementTracking'
// Per-element conversion context
const elementConversion = new WeakMap<
HTMLElement,
{ screenToCanvas?: (p: Point) => Point }
>()
/**
* Generic update item for element bounds tracking
*/
@@ -66,14 +58,13 @@ const trackingConfigs: Map<string, ElementTrackingConfig> = new Map([
// Single ResizeObserver instance for all Vue elements
const resizeObserver = new ResizeObserver((entries) => {
// Canvas is ready when this code runs; no defensive guards needed.
const conv = useSharedCanvasPositionConversion()
// Group updates by type, then flush via each config's handler
const updatesByType = new Map<string, ElementBoundsUpdate[]>()
// Track nodes whose slots should be resynced after node size changes
const nodesNeedingSlotResync = new Set<string>()
// Read container origin once per batch via cache
const { left: originLeft, top: originTop } = getCanvasClientOrigin()
for (const entry of entries) {
if (!(entry.target instanceof HTMLElement)) continue
const element = entry.target
@@ -105,22 +96,13 @@ const resizeObserver = new ResizeObserver((entries) => {
// Screen-space rect
const rect = element.getBoundingClientRect()
let bounds: Bounds = { x: rect.left, y: rect.top, width, height }
// Convert position to canvas space (top-left), leave size as-is
// Note: ResizeObserver sizes are pre-transform; they already represent canvas units.
const ctx = elementConversion.get(element)
if (ctx?.screenToCanvas) {
const topLeftCanvas = ctx.screenToCanvas({
x: bounds.x - originLeft,
y: bounds.y - originTop
})
bounds = {
x: topLeftCanvas.x,
y: topLeftCanvas.y + LiteGraph.NODE_TITLE_HEIGHT,
width: Math.max(0, width),
height: Math.max(0, height - LiteGraph.NODE_TITLE_HEIGHT)
}
const [cx, cy] = conv.clientPosToCanvasPos([rect.left, rect.top])
const topLeftCanvas = { x: cx, y: cy }
const bounds: Bounds = {
x: topLeftCanvas.x,
y: topLeftCanvas.y + LiteGraph.NODE_TITLE_HEIGHT,
width: Math.max(0, width),
height: Math.max(0, height - LiteGraph.NODE_TITLE_HEIGHT)
}
let updates = updatesByType.get(elementType)
@@ -145,7 +127,7 @@ const resizeObserver = new ResizeObserver((entries) => {
// After node bounds are updated, refresh slot cached offsets and layouts
if (nodesNeedingSlotResync.size > 0) {
for (const nodeId of nodesNeedingSlotResync) {
syncNodeSlotLayoutsNow(nodeId, { left: originLeft, top: originTop })
syncNodeSlotLayoutsNow(nodeId)
}
}
})
@@ -175,22 +157,15 @@ export function useVueElementTracking(
appIdentifier: string,
trackingType: string
) {
// For canvas-space conversion: provided by TransformPane
const transformState = inject(TransformStateKey)
onMounted(() => {
const element = getCurrentInstance()?.proxy?.$el
if (!(element instanceof HTMLElement) || !appIdentifier) return
const config = trackingConfigs.get(trackingType)
if (!config) return // Set the data attribute expected by the RO pipeline for this type
if (!config) return
// Set the data attribute expected by the RO pipeline for this type
element.dataset[config.dataAttribute] = appIdentifier
// Remember transformer for this element
if (transformState?.screenToCanvas) {
elementConversion.set(element, {
screenToCanvas: transformState.screenToCanvas
})
}
resizeObserver.observe(element)
})
@@ -204,6 +179,5 @@ export function useVueElementTracking(
// Remove the data attribute and observer
delete element.dataset[config.dataAttribute]
resizeObserver.unobserve(element)
elementConversion.delete(element)
})
}

View File

@@ -12,10 +12,10 @@ import {
LGraphEventMode,
LGraphNode,
LiteGraph,
type Point,
RenderShape,
type Subgraph,
SubgraphNode,
type Vector2,
createBounds
} from '@/lib/litegraph/src/litegraph'
import type {
@@ -994,7 +994,7 @@ export const useLitegraphService = () => {
return node
}
function getCanvasCenter(): Vector2 {
function getCanvasCenter(): Point {
const dpi = Math.max(window.devicePixelRatio ?? 1, 1)
const [x, y, w, h] = app.canvas.ds.visible_area
return [x + w / dpi / 2, y + h / dpi / 2]

View File

@@ -2,7 +2,7 @@ import { toRaw } from 'vue'
import { t } from '@/i18n'
import { LGraph, LGraphCanvas } from '@/lib/litegraph/src/litegraph'
import type { SerialisableGraph, Vector2 } from '@/lib/litegraph/src/litegraph'
import type { Point, SerialisableGraph } from '@/lib/litegraph/src/litegraph'
import { useWorkflowThumbnail } from '@/renderer/thumbnail/composables/useWorkflowThumbnail'
import { ComfyWorkflowJSON } from '@/schemas/comfyWorkflowSchema'
import { app } from '@/scripts/app'
@@ -344,7 +344,7 @@ export const useWorkflowService = () => {
*/
const insertWorkflow = async (
workflow: ComfyWorkflow,
options: { position?: Vector2 } = {}
options: { position?: Point } = {}
) => {
const loadedWorkflow = await workflow.load()
const workflowJSON = toRaw(loadedWorkflow.initialState)