mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
fix: accurate bounding rect for Vue nodes with footer
- Measure body (node-inner-wrapper) for node.size to exclude footer height, preventing size accumulation on Vue/legacy mode switching - Store footer height separately (_footerHeight) for boundingRect - Store collapsed width/height from DOM in ResizeObserver instead of relying on canvas text measurement - Skip _collapsed_width text measurement in Vue nodes mode since measure() ctx overwrites ResizeObserver values - Restore selectionBorder.ts to use createBounds (no per-frame DOM)
This commit is contained in:
116
browser_tests/assets/selection/subgraph-with-regular-node.json
Normal file
116
browser_tests/assets/selection/subgraph-with-regular-node.json
Normal file
@@ -0,0 +1,116 @@
|
||||
{
|
||||
"id": "selection-bbox-test",
|
||||
"revision": 0,
|
||||
"last_node_id": 3,
|
||||
"last_link_id": 1,
|
||||
"nodes": [
|
||||
{
|
||||
"id": 2,
|
||||
"type": "e5fb1765-9323-4548-801a-5aead34d879e",
|
||||
"pos": [300, 200],
|
||||
"size": [400, 200],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"name": "positive",
|
||||
"type": "CONDITIONING",
|
||||
"link": null
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"links": [1]
|
||||
}
|
||||
],
|
||||
"properties": {},
|
||||
"widgets_values": []
|
||||
},
|
||||
{
|
||||
"id": 3,
|
||||
"type": "EmptyLatentImage",
|
||||
"pos": [800, 200],
|
||||
"size": [400, 200],
|
||||
"flags": {},
|
||||
"order": 1,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"name": "latent",
|
||||
"type": "LATENT",
|
||||
"link": 1
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"links": null
|
||||
}
|
||||
],
|
||||
"properties": {},
|
||||
"widgets_values": [512, 512, 1]
|
||||
}
|
||||
],
|
||||
"links": [[1, 2, 0, 3, 0, "LATENT"]],
|
||||
"groups": [],
|
||||
"definitions": {
|
||||
"subgraphs": [
|
||||
{
|
||||
"id": "e5fb1765-9323-4548-801a-5aead34d879e",
|
||||
"version": 1,
|
||||
"state": {
|
||||
"lastGroupId": 0,
|
||||
"lastNodeId": 1,
|
||||
"lastLinkId": 0,
|
||||
"lastRerouteId": 0
|
||||
},
|
||||
"revision": 0,
|
||||
"config": {},
|
||||
"name": "Test Subgraph",
|
||||
"inputNode": {
|
||||
"id": -10,
|
||||
"bounding": [100, 200, 120, 60]
|
||||
},
|
||||
"outputNode": {
|
||||
"id": -20,
|
||||
"bounding": [500, 200, 120, 60]
|
||||
},
|
||||
"inputs": [
|
||||
{
|
||||
"id": "c5cc99d8-a2b6-4bf3-8be7-d4949ef736cd",
|
||||
"name": "positive",
|
||||
"type": "CONDITIONING",
|
||||
"linkIds": [],
|
||||
"pos": { "0": 200, "1": 220 }
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"id": "9bd488b9-e907-4c95-a7a4-85c5597a87af",
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"linkIds": [],
|
||||
"pos": { "0": 520, "1": 220 }
|
||||
}
|
||||
],
|
||||
"widgets": [],
|
||||
"nodes": [],
|
||||
"groups": [],
|
||||
"links": [],
|
||||
"extra": {}
|
||||
}
|
||||
]
|
||||
},
|
||||
"config": {},
|
||||
"extra": {
|
||||
"ds": {
|
||||
"scale": 1,
|
||||
"offset": [0, 0]
|
||||
}
|
||||
},
|
||||
"version": 0.4
|
||||
}
|
||||
@@ -4,7 +4,10 @@ import type {
|
||||
LGraph,
|
||||
LGraphNode
|
||||
} from '../../../src/lib/litegraph/src/litegraph'
|
||||
import type { NodeId } from '../../../src/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type {
|
||||
ComfyWorkflowJSON,
|
||||
NodeId
|
||||
} from '../../../src/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { ComfyPage } from '../ComfyPage'
|
||||
import { DefaultGraphPositions } from '../constants/defaultGraphPositions'
|
||||
import type { Position, Size } from '../types'
|
||||
@@ -111,6 +114,27 @@ export class NodeOperationsHelper {
|
||||
}
|
||||
}
|
||||
|
||||
async loadWithPositions(
|
||||
positions: Record<string, [number, number]>
|
||||
): Promise<void> {
|
||||
await this.page.evaluate(
|
||||
async ({ positions }) => {
|
||||
const data = window.app!.graph.serialize()
|
||||
for (const node of data.nodes) {
|
||||
const pos = positions[String(node.id)]
|
||||
if (pos) node.pos = pos
|
||||
}
|
||||
await window.app!.loadGraphData(
|
||||
data as ComfyWorkflowJSON,
|
||||
true,
|
||||
true,
|
||||
null
|
||||
)
|
||||
},
|
||||
{ positions }
|
||||
)
|
||||
}
|
||||
|
||||
async resizeNode(
|
||||
nodePos: Position,
|
||||
nodeSize: Size,
|
||||
|
||||
@@ -332,6 +332,16 @@ export class NodeReference {
|
||||
async isCollapsed() {
|
||||
return !!(await this.getFlags()).collapsed
|
||||
}
|
||||
async setCollapsed(collapsed: boolean) {
|
||||
await this.comfyPage.page.evaluate(
|
||||
([id, collapsed]) => {
|
||||
const node = window.app!.canvas.graph!.getNodeById(id)
|
||||
if (!node) throw new Error('Node not found')
|
||||
if (node.collapsed !== collapsed) node.collapse(true)
|
||||
},
|
||||
[this.id, collapsed] as const
|
||||
)
|
||||
}
|
||||
async isBypassed() {
|
||||
return (await this.getProperty<number | null | undefined>('mode')) === 4
|
||||
}
|
||||
|
||||
171
browser_tests/tests/selectionBoundingBox.spec.ts
Normal file
171
browser_tests/tests/selectionBoundingBox.spec.ts
Normal file
@@ -0,0 +1,171 @@
|
||||
import { expect } from '@playwright/test'
|
||||
import type { Page } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
|
||||
interface CanvasRect {
|
||||
x: number
|
||||
y: number
|
||||
w: number
|
||||
h: number
|
||||
}
|
||||
|
||||
interface MeasureResult {
|
||||
selectionBounds: CanvasRect | null
|
||||
nodeVisualBounds: Record<string, CanvasRect>
|
||||
}
|
||||
|
||||
// Must match the padding value passed to createBounds() in selectionBorder.ts
|
||||
const SELECTION_PADDING = 10
|
||||
|
||||
async function measureBounds(
|
||||
page: Page,
|
||||
nodeIds: string[]
|
||||
): Promise<MeasureResult> {
|
||||
return page.evaluate(
|
||||
({ ids, padding }) => {
|
||||
const canvas = window.app!.canvas
|
||||
const ds = canvas.ds
|
||||
|
||||
const selectedItems = canvas.selectedItems
|
||||
let minX = Infinity
|
||||
let minY = Infinity
|
||||
let maxX = -Infinity
|
||||
let maxY = -Infinity
|
||||
for (const item of selectedItems) {
|
||||
const rect = item.boundingRect
|
||||
minX = Math.min(minX, rect[0])
|
||||
minY = Math.min(minY, rect[1])
|
||||
maxX = Math.max(maxX, rect[0] + rect[2])
|
||||
maxY = Math.max(maxY, rect[1] + rect[3])
|
||||
}
|
||||
const selectionBounds =
|
||||
selectedItems.size > 0
|
||||
? {
|
||||
x: minX - padding,
|
||||
y: minY - padding,
|
||||
w: maxX - minX + 2 * padding,
|
||||
h: maxY - minY + 2 * padding
|
||||
}
|
||||
: null
|
||||
|
||||
const canvasEl = canvas.canvas as HTMLCanvasElement
|
||||
const canvasRect = canvasEl.getBoundingClientRect()
|
||||
const nodeVisualBounds: Record<string, CanvasRect> = {}
|
||||
|
||||
for (const id of ids) {
|
||||
const nodeEl = document.querySelector(
|
||||
`[data-node-id="${id}"]`
|
||||
) as HTMLElement | null
|
||||
if (!nodeEl) continue
|
||||
|
||||
const domRect = nodeEl.getBoundingClientRect()
|
||||
const footerEls = nodeEl.querySelectorAll(
|
||||
'[data-testid="subgraph-enter-button"], [data-testid="node-footer"]'
|
||||
)
|
||||
let bottom = domRect.bottom
|
||||
for (const footerEl of footerEls) {
|
||||
bottom = Math.max(bottom, footerEl.getBoundingClientRect().bottom)
|
||||
}
|
||||
|
||||
nodeVisualBounds[id] = {
|
||||
x: (domRect.left - canvasRect.left) / ds.scale - ds.offset[0],
|
||||
y: (domRect.top - canvasRect.top) / ds.scale - ds.offset[1],
|
||||
w: domRect.width / ds.scale,
|
||||
h: (bottom - domRect.top) / ds.scale
|
||||
}
|
||||
}
|
||||
|
||||
return { selectionBounds, nodeVisualBounds }
|
||||
},
|
||||
{ ids: nodeIds, padding: SELECTION_PADDING }
|
||||
) as Promise<MeasureResult>
|
||||
}
|
||||
|
||||
const SUBGRAPH_ID = '2'
|
||||
const REGULAR_ID = '3'
|
||||
const WORKFLOW = 'selection/subgraph-with-regular-node'
|
||||
|
||||
const REF_POS: [number, number] = [100, 100]
|
||||
const TARGET_POSITIONS: Record<string, [number, number]> = {
|
||||
'bottom-left': [50, 500],
|
||||
'bottom-right': [600, 500]
|
||||
}
|
||||
|
||||
type NodeType = 'subgraph' | 'regular'
|
||||
type NodeState = 'expanded' | 'collapsed'
|
||||
type Position = 'bottom-left' | 'bottom-right'
|
||||
|
||||
function getTargetId(type: NodeType): string {
|
||||
return type === 'subgraph' ? SUBGRAPH_ID : REGULAR_ID
|
||||
}
|
||||
|
||||
function getRefId(type: NodeType): string {
|
||||
return type === 'subgraph' ? REGULAR_ID : SUBGRAPH_ID
|
||||
}
|
||||
|
||||
test.describe('Selection bounding box', { tag: ['@canvas', '@node'] }, () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.workflow.loadWorkflow(WORKFLOW)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
})
|
||||
|
||||
test.afterEach(async ({ comfyPage }) => {
|
||||
await comfyPage.canvasOps.resetView()
|
||||
})
|
||||
|
||||
const nodeTypes: NodeType[] = ['subgraph', 'regular']
|
||||
const nodeStates: NodeState[] = ['expanded', 'collapsed']
|
||||
const positions: Position[] = ['bottom-left', 'bottom-right']
|
||||
|
||||
for (const type of nodeTypes) {
|
||||
for (const state of nodeStates) {
|
||||
for (const pos of positions) {
|
||||
test(`${type} node (${state}) at ${pos}: selection bounds encompass node`, async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const page = comfyPage.page
|
||||
const targetId = getTargetId(type)
|
||||
const refId = getRefId(type)
|
||||
|
||||
await comfyPage.nodeOps.loadWithPositions({
|
||||
[refId]: REF_POS,
|
||||
[targetId]: TARGET_POSITIONS[pos]
|
||||
})
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
await comfyPage.vueNodes.getNodeLocator(targetId).waitFor()
|
||||
await comfyPage.vueNodes.getNodeLocator(refId).waitFor()
|
||||
|
||||
if (state === 'collapsed') {
|
||||
const nodeRef = await comfyPage.nodeOps.getNodeRefById(targetId)
|
||||
await nodeRef.setCollapsed(true)
|
||||
}
|
||||
|
||||
await comfyPage.canvas.press('Control+a')
|
||||
await expect
|
||||
.poll(() => comfyPage.nodeOps.getSelectedGraphNodesCount())
|
||||
.toBe(2)
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
const result = await measureBounds(page, [refId, targetId])
|
||||
expect(result.selectionBounds).not.toBeNull()
|
||||
|
||||
const sel = result.selectionBounds!
|
||||
const vis = result.nodeVisualBounds[targetId]
|
||||
expect(vis).toBeDefined()
|
||||
|
||||
const selRight = sel.x + sel.w
|
||||
const selBottom = sel.y + sel.h
|
||||
const visRight = vis.x + vis.w
|
||||
const visBottom = vis.y + vis.h
|
||||
|
||||
expect(sel.x).toBeLessThanOrEqual(vis.x)
|
||||
expect(selRight).toBeGreaterThanOrEqual(visRight)
|
||||
expect(sel.y).toBeLessThanOrEqual(vis.y)
|
||||
expect(selBottom).toBeGreaterThanOrEqual(visBottom)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
@@ -422,6 +422,16 @@ export class LGraphNode
|
||||
* Updated by {@link LGraphCanvas.drawNode}
|
||||
*/
|
||||
_collapsed_width?: number
|
||||
/**
|
||||
* The height of the node when collapsed (including footer).
|
||||
* Set by ResizeObserver in Vue nodes mode.
|
||||
*/
|
||||
_collapsed_height?: number
|
||||
/**
|
||||
* The footer height in Vue nodes mode.
|
||||
* Set by ResizeObserver, used by measure() to extend boundingRect.
|
||||
*/
|
||||
_footerHeight?: number
|
||||
/**
|
||||
* Called once at the start of every frame. Caller may change the values in {@link out}, which will be reflected in {@link boundingRect}.
|
||||
* WARNING: Making changes to boundingRect via onBounding is poorly supported, and will likely result in strange behaviour.
|
||||
@@ -2090,18 +2100,18 @@ export class LGraphNode
|
||||
out[1] = this.pos[1] + -titleHeight
|
||||
if (!this.flags?.collapsed) {
|
||||
out[2] = this.size[0]
|
||||
out[3] = this.size[1] + titleHeight
|
||||
out[3] = this.size[1] + titleHeight + (this._footerHeight ?? 0)
|
||||
} else {
|
||||
if (ctx) ctx.font = this.innerFontStyle
|
||||
this._collapsed_width = Math.min(
|
||||
this.size[0],
|
||||
ctx
|
||||
? cachedMeasureText(ctx, this.getTitle() ?? '') +
|
||||
LiteGraph.NODE_TITLE_HEIGHT * 2
|
||||
: 0
|
||||
)
|
||||
if (ctx && !LiteGraph.vueNodesMode) {
|
||||
ctx.font = this.innerFontStyle
|
||||
this._collapsed_width = Math.min(
|
||||
this.size[0],
|
||||
cachedMeasureText(ctx, this.getTitle() ?? '') +
|
||||
LiteGraph.NODE_TITLE_HEIGHT * 2
|
||||
)
|
||||
}
|
||||
out[2] = this._collapsed_width || LiteGraph.NODE_COLLAPSED_WIDTH
|
||||
out[3] = LiteGraph.NODE_TITLE_HEIGHT
|
||||
out[3] = this._collapsed_height || LiteGraph.NODE_TITLE_HEIGHT
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -15,8 +15,9 @@ import { useDocumentVisibility } from '@vueuse/core'
|
||||
|
||||
import { useSharedCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import { app } from '@/scripts/app'
|
||||
import type { Bounds, NodeId } from '@/renderer/core/layout/types'
|
||||
import { LayoutSource } from '@/renderer/core/layout/types'
|
||||
import {
|
||||
@@ -139,25 +140,44 @@ const resizeObserver = new ResizeObserver((entries) => {
|
||||
const nodeId: NodeId | undefined =
|
||||
elementType === 'node' ? elementId : undefined
|
||||
|
||||
// Skip collapsed nodes — their DOM height is just the header, and writing
|
||||
// that back to the layout store would overwrite the stored expanded size.
|
||||
// Collapsed nodes: don't update layoutStore (preserve expanded size),
|
||||
// but sync the collapsed DOM width to litegraph for boundingRect.
|
||||
if (elementType === 'node' && element.dataset.collapsed != null) {
|
||||
if (nodeId) {
|
||||
const lgNode = app.graph?.getNodeById(nodeId)
|
||||
if (lgNode) {
|
||||
const body = element.querySelector(
|
||||
'[data-testid^="node-inner-wrapper"]'
|
||||
)
|
||||
lgNode._collapsed_width =
|
||||
body instanceof HTMLElement ? body.offsetWidth : element.offsetWidth
|
||||
lgNode._collapsed_height = element.offsetHeight
|
||||
}
|
||||
nodesNeedingSlotResync.add(nodeId)
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Use borderBoxSize when available; fall back to contentRect for older engines/tests
|
||||
// Border box is the border included FULL wxh DOM value.
|
||||
const borderBox = Array.isArray(entry.borderBoxSize)
|
||||
? entry.borderBoxSize[0]
|
||||
: {
|
||||
inlineSize: entry.contentRect.width,
|
||||
blockSize: entry.contentRect.height
|
||||
// Measure body (node-inner-wrapper) for node.size to exclude footer,
|
||||
// but store the full height (with footer) for boundingRect.
|
||||
const bodyEl = element.querySelector('[data-testid^="node-inner-wrapper"]')
|
||||
const measuredEl = bodyEl instanceof HTMLElement ? bodyEl : element
|
||||
const width = Math.max(0, measuredEl.offsetWidth)
|
||||
const height = Math.max(0, measuredEl.offsetHeight)
|
||||
const fullHeight = Math.max(0, element.offsetHeight)
|
||||
|
||||
// Store footer-inclusive height for boundingRect calculation
|
||||
if (nodeId) {
|
||||
const lgNode = app.graph?.getNodeById(nodeId)
|
||||
if (lgNode) {
|
||||
const footerExtra = fullHeight - measuredEl.offsetHeight
|
||||
if (footerExtra > 0) {
|
||||
lgNode._footerHeight = footerExtra
|
||||
} else {
|
||||
lgNode._footerHeight = undefined
|
||||
}
|
||||
const width = Math.max(0, borderBox.inlineSize)
|
||||
const height = Math.max(0, borderBox.blockSize)
|
||||
}
|
||||
}
|
||||
const nodeLayout = nodeId
|
||||
? layoutStore.getNodeLayoutRef(nodeId).value
|
||||
: null
|
||||
|
||||
Reference in New Issue
Block a user