Comfy-Org / Comfy-Org/ComfyUI_frontend
Migrate node.imgs and rendering state to centralized ECS store
- 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
Assessment
This issue has not been assessed yet.