-
Notifications
You must be signed in to change notification settings - Fork 378
[WIP] Vue node resizing functionality #5587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { computed, watch } from 'vue' | |
|
||
import { useWorkflowService } from '@/platform/workflow/core/services/workflowService' | ||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' | ||
import { useLayoutSync } from '@/renderer/core/layout/sync/useLayoutSync' | ||
import { api } from '@/scripts/api' | ||
import { app as comfyApp } from '@/scripts/app' | ||
import { getStorageValue, setStorageValue } from '@/scripts/utils' | ||
|
@@ -12,13 +13,20 @@ import { useSettingStore } from '@/stores/settingStore' | |
export function useWorkflowPersistence() { | ||
const workflowStore = useWorkflowStore() | ||
const settingStore = useSettingStore() | ||
const { forceSyncAll } = useLayoutSync() | ||
|
||
const workflowPersistenceEnabled = computed(() => | ||
settingStore.get('Comfy.Workflow.Persist') | ||
) | ||
|
||
const persistCurrentWorkflow = () => { | ||
if (!workflowPersistenceEnabled.value) return | ||
|
||
// Force sync all layout changes to LiteGraph before serialization | ||
if (comfyApp.canvas) { | ||
forceSyncAll(comfyApp.canvas) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe instead of a forced sync, we can try to just update once a frame? something like: // useLayoutBatching.ts
import { reactive, onScopeDispose } from 'vue'
export function useLayoutBatching() {
const pendingUpdates = reactive(new Map<NodeId, NodeLayout>())
let rafId: number | null = null
function batchSync(nodeId: NodeId, layout: NodeLayout) {
// Always accumulate updates in reactive map
pendingUpdates.set(nodeId, layout)
// Schedule RAF if not already scheduled
if (rafId) return
rafId = requestAnimationFrame(processAllUpdates)
}
function processAllUpdates() {
// Create snapshot of updates to process
const updatesToProcess = new Map(pendingUpdates)
pendingUpdates.clear()
flushAllUpdates(updatesToProcess)
// Single canvas dirty call per frame
canvas.setDirty()
rafId = null
}
function flushAllUpdates(updates: Map<NodeId, NodeLayout>) {
updates.forEach((layout, nodeId) => {
updateNodeLayout(nodeId, layout)
})
}
function updateNodeLayout(nodeId: NodeId, layout: NodeLayout) {
// Update the Vue reactive store with new node dimensions
// Should call layoutStore.updateNodeLayout(nodeId, layout) or similar
// This keeps Vue state in sync with DOM changes
}
function cleanup() {
if (rafId !== null) {
cancelAnimationFrame(rafId)
rafId = null
}
pendingUpdates.clear()
}
// Auto-cleanup when scope is disposed
onScopeDispose(cleanup)
return {
batchSync,
cleanup // Expose for manual cleanup if needed
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that might be heavier than we need, since litegraph already does batching in its draw. |
||
} | ||
|
||
const workflow = JSON.stringify(comfyApp.graph.serialize()) | ||
localStorage.setItem('workflow', workflow) | ||
if (api.clientId) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1381,6 +1381,24 @@ class LayoutStoreImpl implements LayoutStore { | |
// Restore original source | ||
this.currentSource = originalSource | ||
} | ||
|
||
/** | ||
* Get all node layouts for syncing to external systems | ||
*/ | ||
getAllNodeLayouts(): Map<NodeId, NodeLayout | null> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't use a function like this as it just creates the potential for de-sync and race conditions. |
||
const results = new Map<NodeId, NodeLayout | null>() | ||
|
||
for (const [nodeId, ynode] of this.ynodes.entries()) { | ||
if (ynode) { | ||
const layout = yNodeToLayout(ynode) | ||
results.set(nodeId, layout) | ||
} else { | ||
results.set(nodeId, null) | ||
} | ||
} | ||
|
||
return results | ||
} | ||
} | ||
|
||
// Create singleton instance | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be moved into a separate PR, as it's not necessary for resizing to work - just ensures the newly sized nodes' dimensions are serialized/saved. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,9 @@ | |
:style="[ | ||
{ | ||
transform: `translate(${layoutPosition.x ?? position?.x ?? 0}px, ${(layoutPosition.y ?? position?.y ?? 0) - LiteGraph.NODE_TITLE_HEIGHT}px)`, | ||
zIndex: zIndex | ||
zIndex: zIndex, | ||
...(currentSize?.width && { width: `${currentSize.width}px` }), | ||
...(currentSize?.height && { height: `${currentSize.height}px` }) | ||
}, | ||
dragStyle | ||
]" | ||
|
@@ -122,6 +124,13 @@ | |
/> | ||
</div> | ||
</template> | ||
|
||
<!-- Resize handle --> | ||
<div | ||
v-if="!readonly" | ||
class="absolute bottom-0 right-0 w-3 h-3 cursor-se-resize opacity-0 hover:opacity-20 hover:bg-white transition-opacity duration-200" | ||
@pointerdown.stop="startResize" | ||
/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proposed solution would be something like: <script setup lang="ts">
import { useNodeResize } from './composables/useNodeResize'
interface Props {
nodeId: string
title: string
}
const props = defineProps<Props>()
const { nodeElement, width, height } = useNodeResize(props.nodeId)
function handleNodeClick() {
// Handle node selection, focus, etc.
// Should update node state without triggering resize
}
function handleContextMenu(event: MouseEvent) {
// Handle right-click menu for node operations
// Should prevent default browser context menu
event.preventDefault()
}
</script>
<template>
<div
ref="nodeElementRef"
:data-node-id="nodeId"
class="graph-node"
@click="handleNodeClick"
@contextmenu="handleContextMenu"
>
<!-- Node Content -->
<!-- Resize Handle -->
<div class="resize-handle" />
</div>
</template> |
||
</div> | ||
</template> | ||
|
||
|
@@ -151,6 +160,7 @@ import { useNodeOutputStore } from '@/stores/imagePreviewStore' | |
import { getNodeByLocatorId } from '@/utils/graphTraversalUtil' | ||
import { cn } from '@/utils/tailwindUtil' | ||
|
||
import { useNodeResize } from '../composables/useNodeResize' | ||
import { useVueElementTracking } from '../composables/useVueNodeResizeTracking' | ||
import NodeContent from './NodeContent.vue' | ||
import NodeHeader from './NodeHeader.vue' | ||
|
@@ -194,7 +204,7 @@ const emit = defineEmits<{ | |
'update:title': [nodeId: string, newTitle: string] | ||
}>() | ||
|
||
useVueElementTracking(nodeData.id, 'node') | ||
const tracking = useVueElementTracking(nodeData.id, 'node') | ||
|
||
// Inject selection state from parent | ||
const selectedNodeIds = inject(SelectedNodeIdsKey) | ||
|
@@ -282,6 +292,31 @@ onMounted(() => { | |
} | ||
}) | ||
|
||
// Resize with local state to avoid reactive loops | ||
const currentSize = ref<{ width: number; height: number } | null>(null) | ||
|
||
const { startResize } = useNodeResize( | ||
(newSize) => { | ||
// Update local state for immediate visual feedback | ||
currentSize.value = newSize | ||
}, | ||
{ | ||
minWidth: 200, | ||
minHeight: 100, | ||
maxWidth: 800, | ||
maxHeight: 600, | ||
transformState, | ||
onStart: () => tracking.pause(), // Pause automatic tracking | ||
onEnd: () => { | ||
// Sync with layout system once at the end | ||
if (currentSize.value) { | ||
resize(currentSize.value) | ||
} | ||
tracking.resume() // Resume automatic tracking | ||
} | ||
} | ||
) | ||
|
||
// Drag state for styling | ||
const isDragging = ref(false) | ||
const dragStyle = computed(() => ({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to find a better approach for this module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
https://vueuse.org/core/useElementSize/ export function useNodeResize(nodeId) {
const { batchSync } = useLayoutBatching()
const nodeElement = useTemplateRef<HTMLElement>('nodeElementRef')
const { width, height } = useElementSize(nodeElement)
watch([width, height], ([newWidth, newHeight]: [number, number]) => {
if (!newWidth || !newHeight) return
batchSync(nodeId, { width: newWidth, height: newHeight })
})
// Separate resize handle logic (not using useDraggable)
function attachResizeHandles() {
// Implement custom resize handles with mouse events
// This directly modifies DOM element size
// useElementSize will detect the changes and trigger batchSync
}
return { width, height, nodeElement }
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,143 @@ | ||
/** | ||
* Composable for node resizing functionality | ||
* | ||
* Provides resize handle interaction that integrates with the layout system. | ||
* Handles pointer capture, coordinate calculations, and size constraints. | ||
*/ | ||
import { ref } from 'vue' | ||
|
||
interface TransformState { | ||
screenToCanvas: (point: { x: number; y: number }) => { x: number; y: number } | ||
camera: { z: number } | ||
} | ||
|
||
interface UseNodeResizeOptions { | ||
/** Minimum width constraint */ | ||
minWidth?: number | ||
/** Minimum height constraint */ | ||
minHeight?: number | ||
/** Maximum width constraint */ | ||
maxWidth?: number | ||
/** Maximum height constraint */ | ||
maxHeight?: number | ||
/** Transform state for coordinate conversion */ | ||
transformState?: TransformState | ||
/** Called when resize starts */ | ||
onStart?: () => void | ||
/** Called when resize ends */ | ||
onEnd?: () => void | ||
} | ||
|
||
export function useNodeResize( | ||
resizeCallback: (size: { width: number; height: number }) => void, | ||
options: UseNodeResizeOptions = {} | ||
) { | ||
const { | ||
minWidth = 200, | ||
minHeight = 100, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we have some of these as constants somewhere? |
||
maxWidth = 800, | ||
maxHeight = 600, | ||
transformState, | ||
onStart, | ||
onEnd | ||
} = options | ||
|
||
// Resize state | ||
const isResizing = ref(false) | ||
const resizeStartPos = ref<{ x: number; y: number } | null>(null) | ||
const resizeStartSize = ref<{ width: number; height: number } | null>(null) | ||
|
||
const startResize = (event: PointerEvent) => { | ||
event.preventDefault() | ||
isResizing.value = true | ||
resizeStartPos.value = { x: event.clientX, y: event.clientY } | ||
|
||
// Call onStart callback (to pause tracking) | ||
onStart?.() | ||
|
||
// Get the current element dimensions | ||
const element = (event.target as HTMLElement).closest( | ||
'.lg-node' | ||
) as HTMLElement | ||
if (!element) return | ||
const rect = element.getBoundingClientRect() | ||
|
||
let startWidth = rect.width | ||
let startHeight = rect.height | ||
|
||
// If we have transform state, convert screen size to canvas size | ||
if (transformState?.screenToCanvas && transformState?.camera) { | ||
// Scale the size by the inverse of the zoom factor to get canvas units | ||
const scale = transformState.camera.z | ||
startWidth = rect.width / scale | ||
startHeight = rect.height / scale | ||
} | ||
|
||
resizeStartSize.value = { | ||
width: startWidth, | ||
height: startHeight | ||
} | ||
|
||
// Capture pointer | ||
const target = event.target as HTMLElement | ||
target.setPointerCapture(event.pointerId) | ||
|
||
// Add global listeners | ||
document.addEventListener('pointermove', handleResize) | ||
document.addEventListener('pointerup', endResize) | ||
} | ||
|
||
const handleResize = (event: PointerEvent) => { | ||
if (!isResizing.value || !resizeStartPos.value || !resizeStartSize.value) | ||
return | ||
|
||
let deltaX = event.clientX - resizeStartPos.value.x | ||
let deltaY = event.clientY - resizeStartPos.value.y | ||
|
||
// Convert screen deltas to canvas coordinates if transform state is available | ||
if (transformState?.screenToCanvas) { | ||
const mouseDelta = { x: deltaX, y: deltaY } | ||
const canvasOrigin = transformState.screenToCanvas({ x: 0, y: 0 }) | ||
const canvasWithDelta = transformState.screenToCanvas(mouseDelta) | ||
|
||
deltaX = canvasWithDelta.x - canvasOrigin.x | ||
deltaY = canvasWithDelta.y - canvasOrigin.y | ||
} | ||
|
||
const newWidth = Math.max( | ||
minWidth, | ||
Math.min(maxWidth, resizeStartSize.value.width + deltaX) | ||
) | ||
const newHeight = Math.max( | ||
minHeight, | ||
Math.min(maxHeight, resizeStartSize.value.height + deltaY) | ||
) | ||
|
||
// Call the provided resize callback | ||
resizeCallback({ width: newWidth, height: newHeight }) | ||
} | ||
|
||
const endResize = (event: PointerEvent) => { | ||
if (!isResizing.value) return | ||
|
||
// Call onEnd callback (to resume tracking) | ||
onEnd?.() | ||
|
||
isResizing.value = false | ||
resizeStartPos.value = null | ||
resizeStartSize.value = null | ||
|
||
// Release pointer | ||
const target = event.target as HTMLElement | ||
target.releasePointerCapture(event.pointerId) | ||
|
||
// Remove global listeners | ||
document.removeEventListener('pointermove', handleResize) | ||
document.removeEventListener('pointerup', endResize) | ||
} | ||
|
||
return { | ||
isResizing, | ||
startResize | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to other file, this can be moved to separate PR