From 020c912a8d83a90052b7669ccfcf496ab059362c Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:53:00 +1100 Subject: [PATCH] Convert Links to ES6 Map & LLink Serialisation (#246) * Fix intermittent links bug - graph.links Map() Replaces graph.links with Map() Adds a Proxy to provide for...in and indexer access Temporarily uses merged Map+Record type, to ease downstream migration * nit - Remove redundant code * nit - Remove redundant null checks * Add Serializable interface, used in LLink Allows LLink to be serialised as an object rather than an array, bringing it inline with the rest of LiteGraph. --- src/LGraph.ts | 66 ++++++++++++-------------- src/LGraphCanvas.ts | 12 ++--- src/LGraphNode.ts | 94 +++++++++++++++++--------------------- src/LLink.ts | 45 +++++++++++++++--- src/MapProxyHandler.ts | 55 ++++++++++++++++++++++ src/types/serialisation.ts | 30 +++++++++++- 6 files changed, 200 insertions(+), 102 deletions(-) create mode 100644 src/MapProxyHandler.ts diff --git a/src/LGraph.ts b/src/LGraph.ts index 4ebe2ce4d..179f7bc67 100644 --- a/src/LGraph.ts +++ b/src/LGraph.ts @@ -6,6 +6,7 @@ import { LGraphCanvas } from "./LGraphCanvas" import { LGraphGroup } from "./LGraphGroup" import { type NodeId, LGraphNode } from "./LGraphNode" import { type LinkId, LLink, type SerialisedLLinkArray } from "./LLink" +import { MapProxyHandler } from "./MapProxyHandler" interface IGraphInput { name: string @@ -34,7 +35,20 @@ export class LGraph { static STATUS_RUNNING = 2 _version: number - links: Record + /** The backing store for links. Keys are wrapped in String() */ + _links: Map = new Map() + /** + * Indexed property access is deprecated. + * Backwards compatibility with a Proxy has been added, but will eventually be removed. + * + * Use {@link Map} methods: + * ``` + * const linkId = 123 + * const link = graph.links.get(linkId) + * // Deprecated: const link = graph.links[linkId] + * ``` + */ + links: Map & Record list_of_graphcanvas?: LGraphCanvas[] status: number last_node_id: number @@ -101,11 +115,18 @@ export class LGraph { constructor(o?: ISerialisedGraph) { if (LiteGraph.debug) console.log("Graph created") + /** @see MapProxyHandler */ + const links = this._links + MapProxyHandler.bindAllMethods(links) + const handler = new MapProxyHandler() + this.links = new Proxy(links, handler) as Map & Record + this.list_of_graphcanvas = null this.clear() if (o) this.configure(o) } + // TODO: Remove //used to know which types of connections support this graph (some graphs do not allow certain types) getSupportedTypes(): string[] { @@ -140,9 +161,6 @@ export class LGraph { //other scene stuff this._groups = [] - //links - this.links = {} //container with all the links - //iterations this.iteration = 0 @@ -416,7 +434,7 @@ export class LGraph { //for every connection for (let j = 0; j < output.links.length; j++) { const link_id = output.links[j] - const link = this.links[link_id] + const link = this._links.get(link_id) if (!link) continue //already visited link (ignore it) @@ -1126,8 +1144,7 @@ export class LGraph { * clears the triggered slot animation in all links (stop visual animation) */ clearTriggeredSlots(): void { - for (const i in this.links) { - const link_info = this.links[i] + for (const link_info of this._links.values()) { if (!link_info) continue if (link_info._last_time) @@ -1150,7 +1167,7 @@ export class LGraph { * @param {Number} link_id */ removeLink(link_id: LinkId): void { - const link = this.links[link_id] + const link = this._links.get(link_id) if (!link) return const node = this.getNodeById(link.target_id) @@ -1170,22 +1187,7 @@ export class LGraph { //pack link info into a non-verbose format const links: SerialisedLLinkArray[] = [] - for (const linkId in this.links) { - let link = this.links[linkId] - if (!link.serialize) { - //weird bug I havent solved yet - console.warn( - "weird LLink bug, link info is not a LLink but a regular object" - ) - // @ts-expect-error Refactor this shallow copy or add static factory - const link2 = new LLink() - for (const j in link) { - link2[j] = link[j] - } - this.links[linkId] = link2 - link = link2 - } - + for (const link of this._links.values()) { links.push(link.serialize()) } @@ -1224,25 +1226,17 @@ export class LGraph { // LEGACY: This was changed from constructor === Array //decode links info (they are very verbose) if (Array.isArray(data.links)) { - const links: LLink[] = [] + this._links.clear() for (const link_data of data.links) { - //weird bug - if (!link_data) { - console.warn("serialized graph link data contains errors, skipping.") - continue - } - // @ts-expect-error Refactor this shallow copy or add static factory - const link = new LLink() - link.configure(link_data) - links[link.id] = link + const link = LLink.createFromArray(link_data) + this._links.set(link.id, link) } - data.links = links } //copy all stored fields for (const i in data) { //links must be accepted - if (i == "nodes" || i == "groups") + if (i == "nodes" || i == "groups" || i == "links") continue this[i] = data[i] } diff --git a/src/LGraphCanvas.ts b/src/LGraphCanvas.ts index 75da85635..70d8d36f3 100644 --- a/src/LGraphCanvas.ts +++ b/src/LGraphCanvas.ts @@ -1812,7 +1812,7 @@ export class LGraphCanvas { this.connecting_links = [] for (const linkId of output.links) { - const link = this.graph.links[linkId] + const link = this.graph._links.get(linkId) const slot = link.target_slot const linked_node = this.graph._nodes_by_id[link.target_id] const input = linked_node.inputs[slot] @@ -1887,7 +1887,7 @@ export class LGraphCanvas { if (input.link !== null) { //before disconnecting - const link_info = this.graph.links[input.link] + const link_info = this.graph._links.get(input.link) const slot = link_info.origin_slot const linked_node = this.graph._nodes_by_id[link_info.origin_id] if (LiteGraph.click_do_break_link_to || (LiteGraph.ctrl_alt_click_do_break_link && e.ctrlKey && e.altKey && !e.shiftKey)) { @@ -3020,7 +3020,7 @@ export class LGraphCanvas { const input = node.inputs[j] if (!input || input.link == null) continue - const link_info = this.graph.links[input.link] + const link_info = this.graph._links.get(input.link) if (!link_info) continue const target_node = this.graph.getNodeById(link_info.origin_id) @@ -3354,8 +3354,8 @@ export class LGraphCanvas { //autoconnect when possible (very basic, only takes into account first input-output) if (node.inputs?.length && node.outputs && node.outputs.length && LiteGraph.isValidConnection(node.inputs[0].type, node.outputs[0].type) && node.inputs[0].link && node.outputs[0].links && node.outputs[0].links.length) { - const input_link = node.graph.links[node.inputs[0].link] - const output_link = node.graph.links[node.outputs[0].links[0]] + const input_link = node.graph._links.get(node.inputs[0].link) + const output_link = node.graph._links.get(node.outputs[0].links[0]) const input_node = node.getInputNode(0) const output_node = node.getOutputNodes(0)[0] if (input_node && output_node) @@ -4959,7 +4959,7 @@ export class LGraphCanvas { if (!input || input.link == null) continue const link_id = input.link - const link = this.graph.links[link_id] + const link = this.graph._links.get(link_id) if (!link) continue //find link info diff --git a/src/LGraphNode.ts b/src/LGraphNode.ts index e79e6e8f6..6582b23df 100644 --- a/src/LGraphNode.ts +++ b/src/LGraphNode.ts @@ -354,7 +354,7 @@ export class LGraphNode { if (this.inputs) { for (let i = 0; i < this.inputs.length; ++i) { const input = this.inputs[i] - const link = this.graph ? this.graph.links[input.link] : null + const link = this.graph ? this.graph._links.get(input.link) : null this.onConnectionsChange?.(NodeSlotType.INPUT, i, true, link, input) this.onInputAdded?.(input) } @@ -367,7 +367,7 @@ export class LGraphNode { continue } for (let j = 0; j < output.links.length; ++j) { - const link = this.graph ? this.graph.links[output.links[j]] : null + const link = this.graph ? this.graph._links.get(output.links[j]) : null this.onConnectionsChange?.(NodeSlotType.OUTPUT, i, true, link, output) } this.onOutputAdded?.(output) @@ -551,7 +551,7 @@ export class LGraphNode { if (this.outputs[slot].links) { for (let i = 0; i < this.outputs[slot].links.length; i++) { const link_id = this.outputs[slot].links[i] - const link = this.graph.links[link_id] + const link = this.graph._links.get(link_id) if (link) link.data = data } @@ -575,7 +575,7 @@ export class LGraphNode { if (this.outputs[slot].links) { for (let i = 0; i < this.outputs[slot].links.length; i++) { const link_id = this.outputs[slot].links[i] - this.graph.links[link_id].type = type + this.graph._links.get(link_id).type = type } } } @@ -592,7 +592,7 @@ export class LGraphNode { if (slot >= this.inputs.length || this.inputs[slot].link == null) return const link_id = this.inputs[slot].link - const link: LLink = this.graph.links[link_id] + const link = this.graph._links.get(link_id) //bug: weird case but it happens sometimes if (!link) return null @@ -621,7 +621,7 @@ export class LGraphNode { if (slot >= this.inputs.length || this.inputs[slot].link == null) return null const link_id = this.inputs[slot].link - const link = this.graph.links[link_id] + const link = this.graph._links.get(link_id) //bug: weird case but it happens sometimes if (!link) return null @@ -677,7 +677,7 @@ export class LGraphNode { if (!this.inputs) return null if (slot < this.inputs.length) { const slot_info = this.inputs[slot] - return this.graph.links[slot_info.link] + return this.graph._links.get(slot_info.link) } return null } @@ -694,7 +694,7 @@ export class LGraphNode { const input = this.inputs[slot] if (!input || input.link === null) return null - const link_info = this.graph.links[input.link] + const link_info = this.graph._links.get(input.link) if (!link_info) return null return this.graph.getNodeById(link_info.origin_id) @@ -713,7 +713,7 @@ export class LGraphNode { for (let i = 0, l = this.inputs.length; i < l; ++i) { const input_info = this.inputs[i] if (name == input_info.name && input_info.link != null) { - const link = this.graph.links[input_info.link] + const link = this.graph._links.get(input_info.link) if (link) return link.data } } @@ -785,7 +785,7 @@ export class LGraphNode { const r: LGraphNode[] = [] for (let i = 0; i < output.links.length; i++) { const link_id = output.links[i] - const link = this.graph.links[link_id] + const link = this.graph._links.get(link_id) if (link) { const target_node = this.graph.getNodeById(link.target_id) if (target_node) { @@ -962,7 +962,7 @@ export class LGraphNode { //to skip links if (link_id != null && link_id != id) continue - const link_info = this.graph.links[links[k]] + const link_info = this.graph._links.get(id) //not connected if (!link_info) continue @@ -1008,7 +1008,7 @@ export class LGraphNode { //to skip links if (link_id != null && link_id != id) continue - const link_info = this.graph.links[links[k]] + const link_info = this.graph._links.get(id) //not connected if (!link_info) continue @@ -1112,7 +1112,7 @@ export class LGraphNode { continue const links = this.outputs[i].links for (let j = 0; j < links.length; ++j) { - const link = this.graph.links[links[j]] + const link = this.graph._links.get(links[j]) if (!link) continue link.origin_slot -= 1 @@ -1186,7 +1186,7 @@ export class LGraphNode { for (let i = slot; i < this.inputs.length; ++i) { if (!this.inputs[i]) continue - const link = this.graph.links[this.inputs[i].link] + const link = this.graph._links.get(this.inputs[i].link) if (!link) continue link.target_slot -= 1 @@ -1797,7 +1797,8 @@ export class LGraphNode { // Allow legacy API support for searching target_slot by string, without mutating the input variables let targetIndex: number - if (!this.graph) { + const graph = this.graph + if (!graph) { //could be connected before adding it to a graph //due to link ids being associated with graphs console.log("Connect: Error, node doesn't belong to any graph. Nodes must be added first to a graph before connecting them.") @@ -1817,7 +1818,7 @@ export class LGraphNode { } if (target_node && typeof target_node === "number") { - target_node = this.graph.getNodeById(target_node) + target_node = graph.getNodeById(target_node) } if (!target_node) throw "target node is null" @@ -1869,7 +1870,7 @@ export class LGraphNode { if (!LiteGraph.isValidConnection(output.type, input.type)) { this.setDirtyCanvas(false, true) // @ts-expect-error Unused param - if (changed) this.graph.connectionChange(this, link_info) + if (changed) graph.connectionChange(this, link_info) return null } @@ -1881,13 +1882,13 @@ export class LGraphNode { //if there is something already plugged there, disconnect if (target_node.inputs[targetIndex]?.link != null) { - this.graph.beforeChange() + graph.beforeChange() target_node.disconnectInput(targetIndex) changed = true } if (output.links?.length) { if (output.type === LiteGraph.EVENT && !LiteGraph.allow_multi_output_for_events) { - this.graph.beforeChange() + graph.beforeChange() // @ts-expect-error Unused param this.disconnectOutput(slot, false, { doProcessChange: false }) changed = true @@ -1896,7 +1897,7 @@ export class LGraphNode { const nextId = LiteGraph.use_uuids ? LiteGraph.uuidv4() - : ++this.graph.last_link_id + : ++graph.last_link_id //create link class link_info = new LLink( @@ -1909,14 +1910,14 @@ export class LGraphNode { ) //add to graph links list - this.graph.links[link_info.id] = link_info + graph._links.set(link_info.id, link_info) //connect in output output.links ??= [] output.links.push(link_info.id) //connect in input target_node.inputs[targetIndex].link = link_info.id - if (this.graph) this.graph._version++ + graph._version++ //link_info has been created now, so its updated this.onConnectionsChange?.( @@ -1934,14 +1935,14 @@ export class LGraphNode { link_info, input ) - this.graph?.onNodeConnectionChange?.( + graph.onNodeConnectionChange?.( NodeSlotType.INPUT, target_node, targetIndex, this, slot ) - this.graph?.onNodeConnectionChange?.( + graph.onNodeConnectionChange?.( NodeSlotType.OUTPUT, this, slot, @@ -1950,8 +1951,8 @@ export class LGraphNode { ) this.setDirtyCanvas(false, true) - this.graph.afterChange() - this.graph.connectionChange(this) + graph.afterChange() + graph.connectionChange(this) return link_info } @@ -1989,7 +1990,7 @@ export class LGraphNode { for (let i = 0, l = output.links.length; i < l; i++) { const link_id = output.links[i] - const link_info = graph.links[link_id] + const link_info = graph._links.get(link_id) //is the link we are searching for... if (link_info.target_id == target_node.id) { @@ -1997,8 +1998,9 @@ export class LGraphNode { const input = target_node.inputs[link_info.target_slot] input.link = null //remove there - delete graph.links[link_id] //remove the link from the links pool //remove the link from the links pool - if (graph) graph._version++ + //remove the link from the links pool + graph._links.delete(link_id) + graph._version++ //link_info hasn't been modified so its ok target_node.onConnectionsChange?.( @@ -2016,10 +2018,8 @@ export class LGraphNode { output ) - // FIXME: Called twice. - graph?.onNodeConnectionChange?.(NodeSlotType.OUTPUT, this, slot) - graph?.onNodeConnectionChange?.(NodeSlotType.OUTPUT, this, slot) - graph?.onNodeConnectionChange?.(NodeSlotType.INPUT, target_node, link_info.target_slot) + graph.onNodeConnectionChange?.(NodeSlotType.OUTPUT, this, slot) + graph.onNodeConnectionChange?.(NodeSlotType.INPUT, target_node, link_info.target_slot) break } } @@ -2027,12 +2027,12 @@ export class LGraphNode { else { for (let i = 0, l = output.links.length; i < l; i++) { const link_id = output.links[i] - const link_info = graph.links[link_id] + const link_info = graph._links.get(link_id) //bug: it happens sometimes if (!link_info) continue target_node = graph.getNodeById(link_info.target_id) - if (graph) graph._version++ + graph._version++ if (target_node) { const input = target_node.inputs[link_info.target_slot] @@ -2047,11 +2047,9 @@ export class LGraphNode { link_info, input ) - // FIXME: Called twice. - graph?.onNodeConnectionChange?.(NodeSlotType.INPUT, target_node, link_info.target_slot) } //remove the link from the links pool - delete graph.links[link_id] + graph._links.delete(link_id) this.onConnectionsChange?.( NodeSlotType.OUTPUT, @@ -2060,8 +2058,8 @@ export class LGraphNode { link_info, output ) - graph?.onNodeConnectionChange?.(NodeSlotType.OUTPUT, this, slot) - graph?.onNodeConnectionChange?.(NodeSlotType.INPUT, target_node, link_info.target_slot) + graph.onNodeConnectionChange?.(NodeSlotType.OUTPUT, this, slot) + graph.onNodeConnectionChange?.(NodeSlotType.INPUT, target_node, link_info.target_slot) } output.links = null } @@ -2092,26 +2090,20 @@ export class LGraphNode { } const input = this.inputs[slot] - if (!input) { - return false - } + if (!input) return false const link_id = this.inputs[slot].link if (link_id != null) { this.inputs[slot].link = null //remove other side - const link_info = this.graph.links[link_id] + const link_info = this.graph._links.get(link_id) if (link_info) { const target_node = this.graph.getNodeById(link_info.origin_id) - if (!target_node) { - return false - } + if (!target_node) return false const output = target_node.outputs[link_info.origin_slot] - if (!(output?.links?.length > 0)) { - return false - } + if (!(output?.links?.length > 0)) return false //search in the inputs list for this link let i = 0 @@ -2122,7 +2114,7 @@ export class LGraphNode { } } - delete this.graph.links[link_id] //remove from the pool + this.graph._links.delete(link_id) if (this.graph) this.graph._version++ this.onConnectionsChange?.( diff --git a/src/LLink.ts b/src/LLink.ts index 1ec16fefd..353f6317b 100644 --- a/src/LLink.ts +++ b/src/LLink.ts @@ -1,23 +1,24 @@ import type { CanvasColour, ISlotType } from "./interfaces" import type { NodeId } from "./LGraphNode" +import type { Serialisable, SerialisableLLink } from "./types/serialisation" export type LinkId = number | string export type SerialisedLLinkArray = [LinkId, NodeId, number, NodeId, number, ISlotType] //this is the class in charge of storing link information -export class LLink { +export class LLink implements Serialisable { /** Link ID */ - id?: LinkId - type?: ISlotType + id: LinkId + type: ISlotType /** Output node ID */ - origin_id?: NodeId + origin_id: NodeId /** Output slot index */ - origin_slot?: number + origin_slot: number /** Input node ID */ - target_id?: NodeId + target_id: NodeId /** Input slot index */ - target_slot?: number + target_slot: number data?: number | string | boolean | { toToolTip?(): string } _data?: unknown /** Centre point of the link, calculated during render only - can be inaccurate */ @@ -46,6 +47,20 @@ export class LLink { this._pos = new Float32Array(2) //center } + /** @deprecated Use {@link LLink.create} */ + static createFromArray(data: SerialisedLLinkArray): LLink { + return new LLink(data[0], data[5], data[1], data[2], data[3], data[4]) + } + + /** + * LLink static factory: creates a new LLink from the provided data. + * @param data Serialised LLink data to create the link from + * @returns A new LLink + */ + static create(data: SerialisableLLink): LLink { + return new LLink(data.id, data.type, data.origin_id, data.origin_slot, data.target_id, data.target_slot) + } + configure(o: LLink | SerialisedLLinkArray) { if (Array.isArray(o)) { this.id = o[0] @@ -64,6 +79,10 @@ export class LLink { } } + /** + * @deprecated Prefer {@link LLink.asSerialisable} (returns an object, not an array) + * @returns An array representing this LLink + */ serialize(): SerialisedLLinkArray { return [ this.id, @@ -74,4 +93,16 @@ export class LLink { this.type ] } + + asSerialisable(): SerialisableLLink { + const copy: SerialisableLLink = { + id: this.id, + origin_id: this.origin_id, + origin_slot: this.origin_slot, + target_id: this.target_id, + target_slot: this.target_slot, + type: this.type + } + return copy + } } diff --git a/src/MapProxyHandler.ts b/src/MapProxyHandler.ts new file mode 100644 index 000000000..68c59bb83 --- /dev/null +++ b/src/MapProxyHandler.ts @@ -0,0 +1,55 @@ +/** Temporary workaround until downstream consumers migrate to Map. A brittle wrapper with many flaws, but should be fine for simple maps using int indexes. */ +export class MapProxyHandler implements ProxyHandler> { + getOwnPropertyDescriptor(target: Map, p: string | symbol): PropertyDescriptor | undefined { + const value = this.get(target, p) + if (value) return { + configurable: true, + enumerable: true, + value + } + } + + has(target: Map, p: string | symbol): boolean { + if (typeof p === "symbol") return false + + const int = parseInt(p, 10) + return target.has(!isNaN(int) ? int : p) + } + + ownKeys(target: Map): ArrayLike { + return [...target.keys()].map(x => String(x)) + } + + get(target: Map, p: string | symbol): any { + // Workaround does not support link IDs of "values", "entries", "constructor", etc. + if (p in target) return Reflect.get(target, p, target) + if (typeof p === "symbol") return + + const int = parseInt(p, 10) + return target.get(!isNaN(int) ? int : p) + } + + set(target: Map, p: string | symbol, newValue: any): boolean { + if (typeof p === "symbol") return false + + const int = parseInt(p, 10) + target.set(!isNaN(int) ? int : p, newValue) + return true + } + + deleteProperty(target: Map, p: string | symbol): boolean { + return target.delete(p as number | string) + } + + static bindAllMethods(map: Map): void { + map.clear = map.clear.bind(map) + map.delete = map.delete.bind(map) + map.forEach = map.forEach.bind(map) + map.get = map.get.bind(map) + map.has = map.has.bind(map) + map.set = map.set.bind(map) + map.entries = map.entries.bind(map) + map.keys = map.keys.bind(map) + map.values = map.values.bind(map) + } +} diff --git a/src/types/serialisation.ts b/src/types/serialisation.ts index 56a1e4291..20c4a2aac 100644 --- a/src/types/serialisation.ts +++ b/src/types/serialisation.ts @@ -1,4 +1,4 @@ -import type { Dictionary, INodeFlags, INodeInputSlot, INodeOutputSlot, Point, Rect, Size } from "@/interfaces" +import type { ISlotType, Dictionary, INodeFlags, INodeInputSlot, INodeOutputSlot, Point, Rect, Size } from "@/interfaces" import type { LGraph } from "@/LGraph" import type { IGraphGroupFlags, LGraphGroup } from "@/LGraphGroup" import type { LGraphNode, NodeId } from "@/LGraphNode" @@ -7,6 +7,18 @@ import type { LinkId, LLink } from "@/LLink" import type { TWidgetValue } from "@/types/widgets" import { RenderShape } from "./globalEnums" +/** + * An object that implements custom pre-serialization logic via {@link Serialisable.asSerialisable}. + */ +export interface Serialisable { + /** + * Prepares this object for serialization. + * Creates a partial shallow copy of itself, with only the properties that should be serialised. + * @returns An object that can immediately be serialized to JSON. + */ + asSerialisable(): SerialisableObject +} + /** Serialised LGraphNode */ export interface ISerialisedNode { title?: string @@ -38,7 +50,7 @@ export type ISerialisedGraph< last_link_id: LGraph["last_link_id"] last_reroute_id?: LGraph["last_reroute_id"] nodes: TNode[] - links: TLink[] | LLink[] + links: TLink[] groups: TGroup[] config: LGraph["config"] version: typeof LiteGraph.VERSION @@ -61,3 +73,17 @@ export interface IClipboardContents { nodes?: ISerialisedNode[] links?: TClipboardLink[] } +export interface SerialisableLLink { + /** Link ID */ + id: LinkId + /** Output node ID */ + origin_id: NodeId + /** Output slot index */ + origin_slot: number + /** Input node ID */ + target_id: NodeId + /** Input slot index */ + target_slot: number + /** Data type of the link */ + type: ISlotType +}