Comfy-Org / Comfy-Org/ComfyUI_frontend

Migrate node.imgs and rendering state to centralized ECS store

Open
#9,242 0 comments 1 reaction 1 assignee Claimed by @christian-byrne View on GitHub
area:nodes enhancement in progress
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Summary

Migrate node-scoped mutable rendering state (`node.imgs`, `node.imageIndex`, `node.imageRects`, `node.pointerDown`, `node.overIndex`) from ad-hoc `LGraphNode` properties into centralized ECS-style stores keyed by `NodeLocatorId`.

This builds on the pattern established by `useWidgetValueStore` (widget value centralization) and `useNodeOutputStore` (output/preview lifecycle).

## Motivation

- **Stale references**: Passing `node` references across subgraph boundaries (e.g. promoted widget rendering) creates memory leaks and stale-reference bugs. PR #9198 works around this with transient `DrawWidgetOptions.previewImages`, but the root cause is node-scoped mutable state.
- **Lifecycle management**: `useNodeOutputStore` already manages output/preview lifecycle with `removeNodeOutputs`, `resetAllOutputsAndPreviews`, `revokeSubgraphPreviews`. A `loadedImages` cache should piggyback on this existing lifecycle.
- **Testability**: Pure rendering functions that read from stores are far easier to test than functions coupled to mutable node properties.

## Desired Architecture

### Store Design

Move all node-scoped mutable rendering state into stores keyed by `NodeLocatorId`:

```typescript
// New store (or extension of useNodeOutputStore)
interface NodeImageState {
imgs: HTMLImageElement[]
imageIndex: number | null
imageRects: ImageRect[]
pointerDown: PointerDownState | null
overIndex: number | null
}
```

### Pure Rendering

`renderPreview` becomes a pure system that reads components from stores and writes to canvas — no direct node property access.

### Backwards Compatibility via Property Projection

During migration, use property getters/setters on `node.imgs` (and related fields) that proxy to the ECS store — identical to the approach used for widget value store centralization:

```typescript
Object.defineProperty(node, 'imgs', {
get() { return nodeImageStore.getImages(this.locatorId) },
set(value) { nodeImageStore.setImages(this.locatorId, value) }
})
```

This allows all ~30 existing consumers to continue using `node.imgs` while data flows through the store. Consumers migrate incrementally.

### Writer Migration

`useNodeImage.onLoaded` writes loaded `HTMLImageElement[]` to the centralized store instead of (or in addition to) `node.imgs`. `syncLegacyNodeImgs` goes away once all consumers are migrated.

## Key Consumers to Migrate (~30 across ~12 files)

| File | Access Pattern |
|------|---------------|
| `useImagePreviewWidget.ts` (`renderPreview`) | reads `imgs`, `imageIndex`, `imageRects`, `pointerDown`, `overIndex` |
| `useImageMenuOptions.ts` | reads `imgs[imageIndex]` for Copy/Open/Save Image |
| `useMaskEditorLoader.ts` | reads `imgs`, `imageIndex` |
| `useMaskEditorSaver.ts` | writes `imgs` |
| `useMaskEditor.ts` | reads `imgs` |
| `maskeditor.ts` (extension) | reads `imgs` |
| `useNodeAnimatedImage.ts` | reads/writes `imgs`, `overIndex` |
| `useNodeCanvasImagePreview.ts` | reads `imgs` |
| `useNodeImage.ts` | writes `imgs`, `imageIndex` |
| `webcamCapture.ts` (extension) | writes `imgs` |
| `app.ts` (clipspace) | reads/writes `imgs`, `imageIndex` |
| `litegraphUtil.ts` | reads `imgs` |
| `imagePreviewStore.ts` | writes `imageIndex` |
| `imagePreview.ts` | writes `overIndex` |

## Migration Strategy

1. Create the store with getter/setter projection on node properties (zero consumer changes needed)
2. Migrate `useNodeImage.onLoaded` to write to store (single writer)
3. Incrementally migrate consumers to read from store directly
4. Remove getter/setter projection once all consumers are migrated
5. Remove `syncLegacyNodeImgs`

## Related

- PR #9198 — introduced `DrawWidgetOptions.previewImages` as a tactical workaround for subgraph promotion rendering
- `useWidgetValueStore` — precedent for ECS centralization with backwards-compatible property projection
- `useNodeOutputStore` (`imagePreviewStore.ts`) — existing lifecycle management for node outputs/previews

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-9242-Migrate-node-imgs-and-rendering-state-to-centralized-ECS-store-3136d73d365081dab3bdeeb662893483) by [Unito](https://www.unito.io)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.