-
Couldn't load subscription status.
- Fork 391
Layoutstore Minimap calculation #5547
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 6 commits
3c129e5
3c143e6
481c155
1f090c2
968c8ce
16051b9
a34dd13
4d55f87
db3e4c2
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 |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| import { useThrottleFn } from '@vueuse/core' | ||
| import { ref } from 'vue' | ||
| import { ref, watch } from 'vue' | ||
| import type { Ref } from 'vue' | ||
|
|
||
| import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' | ||
| import { layoutStore } from '@/renderer/core/layout/store/layoutStore' | ||
| import type { NodeId } from '@/schemas/comfyWorkflowSchema' | ||
| import { api } from '@/scripts/api' | ||
|
|
||
| import { MinimapDataSourceFactory } from '../data/MinimapDataSourceFactory' | ||
| import type { UpdateFlags } from '../types' | ||
|
|
||
| interface GraphCallbacks { | ||
|
|
@@ -28,6 +30,9 @@ export function useMinimapGraph( | |
| viewport: false | ||
| }) | ||
|
|
||
| // Track LayoutStore version for change detection | ||
| const layoutStoreVersion = layoutStore.getVersion() | ||
|
|
||
| // Map to store original callbacks per graph ID | ||
| const originalCallbacksMap = new Map<string, GraphCallbacks>() | ||
|
|
||
|
|
@@ -96,35 +101,44 @@ export function useMinimapGraph( | |
| let positionChanged = false | ||
| let connectionChanged = false | ||
|
|
||
| if (g._nodes.length !== lastNodeCount.value) { | ||
| // Use unified data source for change detection | ||
| const dataSource = MinimapDataSourceFactory.create(g) | ||
|
|
||
| // Check for node count changes | ||
| const currentNodeCount = dataSource.getNodeCount() | ||
| if (currentNodeCount !== lastNodeCount.value) { | ||
| structureChanged = true | ||
| lastNodeCount.value = g._nodes.length | ||
| lastNodeCount.value = currentNodeCount | ||
| } | ||
|
|
||
| for (const node of g._nodes) { | ||
| const key = node.id | ||
| const currentState = `${node.pos[0]},${node.pos[1]},${node.size[0]},${node.size[1]}` | ||
| // Check for node position/size changes | ||
| const nodes = dataSource.getNodes() | ||
| for (const node of nodes) { | ||
| const nodeId = node.id as NodeId | ||
Myestery marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const currentState = `${node.x},${node.y},${node.width},${node.height}` | ||
|
|
||
| if (nodeStatesCache.get(key) !== currentState) { | ||
| if (nodeStatesCache.get(nodeId) !== currentState) { | ||
| positionChanged = true | ||
| nodeStatesCache.set(key, currentState) | ||
| nodeStatesCache.set(nodeId, currentState) | ||
| } | ||
| } | ||
|
|
||
| const currentLinks = JSON.stringify(g.links || {}) | ||
| if (currentLinks !== linksCache.value) { | ||
| connectionChanged = true | ||
| linksCache.value = currentLinks | ||
| } | ||
|
|
||
| const currentNodeIds = new Set(g._nodes.map((n: LGraphNode) => n.id)) | ||
| // Clean up removed nodes from cache | ||
| const currentNodeIds = new Set(nodes.map((n) => n.id as NodeId)) | ||
Myestery marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| for (const [nodeId] of nodeStatesCache) { | ||
| if (!currentNodeIds.has(nodeId)) { | ||
| nodeStatesCache.delete(nodeId) | ||
| structureChanged = true | ||
| } | ||
| } | ||
|
|
||
| // TODO: update when Layoutstore tracks links | ||
| const currentLinks = JSON.stringify(g.links || {}) | ||
|
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 feel like this could probably be devolved into a chain of 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 is my main concern too. Can we safely do this without stringifying? 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 we can handle it in a followup since this came from existing implementation that is just being refactored here. |
||
| if (currentLinks !== linksCache.value) { | ||
| connectionChanged = true | ||
| linksCache.value = currentLinks | ||
| } | ||
|
|
||
| if (structureChanged || positionChanged) { | ||
| updateFlags.value.bounds = true | ||
| updateFlags.value.nodes = true | ||
|
|
@@ -140,6 +154,10 @@ export function useMinimapGraph( | |
| const init = () => { | ||
| setupEventListeners() | ||
| api.addEventListener('graphChanged', handleGraphChangedThrottled) | ||
|
|
||
| watch(layoutStoreVersion, () => { | ||
| void handleGraphChangedThrottled() | ||
| }) | ||
| } | ||
|
|
||
| const destroy = () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import type { LGraph } from '@/lib/litegraph/src/litegraph' | ||
| import { calculateNodeBounds } from '@/renderer/core/spatial/boundsCalculator' | ||
|
|
||
| import type { | ||
| IMinimapDataSource, | ||
| MinimapBounds, | ||
| MinimapGroupData, | ||
| MinimapLinkData, | ||
| MinimapNodeData | ||
| } from '../types' | ||
|
|
||
| /** | ||
| * Abstract base class for minimap data sources | ||
| * Provides common functionality and shared implementation | ||
| */ | ||
| export abstract class AbstractMinimapDataSource implements IMinimapDataSource { | ||
|
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 feels really heavy for what it is doing 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 it's good considering we will have graph data from different sources. It should be nice as we make changes to data model and providers |
||
| constructor(protected graph: LGraph | null) {} | ||
|
|
||
| // Abstract methods that must be implemented by subclasses | ||
| abstract getNodes(): MinimapNodeData[] | ||
| abstract getNodeCount(): number | ||
| abstract hasData(): boolean | ||
|
|
||
| // Shared implementation using calculateNodeBounds | ||
| getBounds(): MinimapBounds { | ||
| const nodes = this.getNodes() | ||
| if (nodes.length === 0) { | ||
| return { minX: 0, minY: 0, maxX: 100, maxY: 100, width: 100, height: 100 } | ||
| } | ||
|
|
||
| // Convert MinimapNodeData to the format expected by calculateNodeBounds | ||
| const compatibleNodes = nodes.map((node) => ({ | ||
| pos: [node.x, node.y], | ||
| size: [node.width, node.height] | ||
| })) | ||
|
|
||
| const bounds = calculateNodeBounds(compatibleNodes) | ||
| if (!bounds) { | ||
| return { minX: 0, minY: 0, maxX: 100, maxY: 100, width: 100, height: 100 } | ||
| } | ||
|
|
||
| return bounds | ||
| } | ||
|
|
||
| // Shared implementation for groups | ||
| getGroups(): MinimapGroupData[] { | ||
| if (!this.graph?._groups) return [] | ||
| return this.graph._groups.map((group) => ({ | ||
| x: group.pos[0], | ||
| y: group.pos[1], | ||
| width: group.size[0], | ||
| height: group.size[1], | ||
| color: group.color | ||
| })) | ||
| } | ||
|
|
||
| // TODO: update when Layoutstore supports links | ||
| getLinks(): MinimapLinkData[] { | ||
| if (!this.graph) return [] | ||
| return this.extractLinksFromGraph(this.graph) | ||
| } | ||
|
|
||
| protected extractLinksFromGraph(graph: LGraph): MinimapLinkData[] { | ||
| const links: MinimapLinkData[] = [] | ||
| const nodeMap = new Map(this.getNodes().map((n) => [n.id, n])) | ||
|
|
||
| for (const node of graph._nodes) { | ||
| if (!node.outputs) continue | ||
|
|
||
| const sourceNodeData = nodeMap.get(String(node.id)) | ||
| if (!sourceNodeData) continue | ||
|
|
||
| for (const output of node.outputs) { | ||
| if (!output.links) continue | ||
|
|
||
| for (const linkId of output.links) { | ||
| const link = graph.links[linkId] | ||
| if (!link) continue | ||
|
|
||
| const targetNodeData = nodeMap.get(String(link.target_id)) | ||
| if (!targetNodeData) continue | ||
|
|
||
| links.push({ | ||
| sourceNode: sourceNodeData, | ||
| targetNode: targetNodeData, | ||
| sourceSlot: link.origin_slot, | ||
| targetSlot: link.target_slot | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return links | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { layoutStore } from '@/renderer/core/layout/store/layoutStore' | ||
|
|
||
| import type { MinimapNodeData } from '../types' | ||
| import { AbstractMinimapDataSource } from './AbstractMinimapDataSource' | ||
|
|
||
| /** | ||
| * Layout Store data source implementation | ||
| */ | ||
| export class LayoutStoreDataSource extends AbstractMinimapDataSource { | ||
| getNodes(): MinimapNodeData[] { | ||
| const allNodes = layoutStore.getAllNodes().value | ||
| if (allNodes.size === 0) return [] | ||
|
|
||
| const nodes: MinimapNodeData[] = [] | ||
|
|
||
| for (const [nodeId, layout] of allNodes) { | ||
| // Find corresponding LiteGraph node for additional properties | ||
| const graphNode = this.graph?._nodes?.find((n) => String(n.id) === nodeId) | ||
|
|
||
| nodes.push({ | ||
| id: nodeId, | ||
| x: layout.position.x, | ||
| y: layout.position.y, | ||
| width: layout.size.width, | ||
| height: layout.size.height, | ||
| bgcolor: graphNode?.bgcolor, | ||
| mode: graphNode?.mode, | ||
| hasErrors: graphNode?.has_errors | ||
| }) | ||
| } | ||
|
|
||
| return nodes | ||
| } | ||
|
|
||
| getNodeCount(): number { | ||
| return layoutStore.getAllNodes().value.size | ||
| } | ||
|
|
||
| hasData(): boolean { | ||
| return this.getNodeCount() > 0 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import type { MinimapNodeData } from '../types' | ||
| import { AbstractMinimapDataSource } from './AbstractMinimapDataSource' | ||
|
|
||
| /** | ||
| * LiteGraph data source implementation | ||
| */ | ||
| export class LiteGraphDataSource extends AbstractMinimapDataSource { | ||
| getNodes(): MinimapNodeData[] { | ||
| if (!this.graph?._nodes) return [] | ||
|
|
||
| return this.graph._nodes.map((node) => ({ | ||
| id: String(node.id), | ||
| x: node.pos[0], | ||
| y: node.pos[1], | ||
| width: node.size[0], | ||
| height: node.size[1], | ||
| bgcolor: node.bgcolor, | ||
| mode: node.mode, | ||
| hasErrors: node.has_errors | ||
| })) | ||
| } | ||
|
|
||
| getNodeCount(): number { | ||
| return this.graph?._nodes?.length ?? 0 | ||
| } | ||
|
|
||
| hasData(): boolean { | ||
| return this.getNodeCount() > 0 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import type { LGraph } from '@/lib/litegraph/src/litegraph' | ||
| import { layoutStore } from '@/renderer/core/layout/store/layoutStore' | ||
|
|
||
| import type { IMinimapDataSource } from '../types' | ||
| import { LayoutStoreDataSource } from './LayoutStoreDataSource' | ||
| import { LiteGraphDataSource } from './LiteGraphDataSource' | ||
|
|
||
| /** | ||
| * Factory for creating the appropriate data source | ||
| */ | ||
| export class MinimapDataSourceFactory { | ||
| static create(graph: LGraph | null): IMinimapDataSource { | ||
| // Check if LayoutStore has data | ||
| const layoutStoreHasData = layoutStore.getAllNodes().value.size > 0 | ||
|
|
||
| if (layoutStoreHasData) { | ||
| return new LayoutStoreDataSource(graph) | ||
| } | ||
|
|
||
| return new LiteGraphDataSource(graph) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.