From 85a0a6fa334397728453685672c4de7942711f40 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Feb 2026 16:18:12 +0000 Subject: [PATCH] perf: optimize node rendering by caching visible nodes Reduced redundant iterations over `app.graph._nodes` in `OffscreenRenderer`. Implemented `getVisibleNodes` to calculate visible nodes once per frame (O(N)) and reused this list for text, title, and image preview rendering passes (O(M)). This improves performance for large graphs by replacing multiple O(N) traversals with a single O(N) traversal followed by O(M) traversals where M << N. Added unit tests in `tests/unit/OffscreenRenderer.test.ts` to verify node visibility culling logic. Updated `.jules/bolt.md` with performance learnings. Co-authored-by: AEmotionStudio <163354043+AEmotionStudio@users.noreply.github.com> --- .jules/bolt.md | 4 + src/magnify-glass/OffscreenRenderer.ts | 81 ++++++++++--- tests/unit/OffscreenRenderer.test.ts | 158 +++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 14 deletions(-) create mode 100644 tests/unit/OffscreenRenderer.test.ts diff --git a/.jules/bolt.md b/.jules/bolt.md index 4a0a090..638a589 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -9,3 +9,7 @@ ## 2024-05-23 - Read-Write-Read Layout Thrashing **Learning:** Writing to the DOM (e.g., `element.style.top = ...`) immediately invalidates the layout. If you subsequently read a layout property (e.g., `getBoundingClientRect()`) in the same frame, the browser must force a synchronous layout recalculation. **Action:** In event handlers that move elements, Read all necessary dimensions first, then Perform all Writes. If a downstream method (like `updateMagnifiedView`) needs dimensions, pass the cached values instead of re-reading them. + +## 2026-02-09 - Redundant Node Iterations +**Learning:** Iterating `app.graph._nodes` multiple times per frame (e.g., once for text, once for titles, once for images) is a significant bottleneck in extensions, especially with large graphs. Each traversal is O(N). +**Action:** Calculate the visible subset of nodes ONCE per frame (`getVisibleNodes`) and reuse this list for all subsequent rendering passes. diff --git a/src/magnify-glass/OffscreenRenderer.ts b/src/magnify-glass/OffscreenRenderer.ts index 0f4e4c3..b61dee4 100644 --- a/src/magnify-glass/OffscreenRenderer.ts +++ b/src/magnify-glass/OffscreenRenderer.ts @@ -88,6 +88,41 @@ export class OffscreenRenderer { } } + /** + * Get a list of nodes visible within the source rectangle. + * This avoids iterating over all nodes multiple times per frame. + */ + private getVisibleNodes( + sourceCssX: number, + sourceCssY: number, + sourceCssWidth: number, + sourceCssHeight: number, + scale: number, + offset: [number, number] + ): any[] { + const graph = app?.graph; + if (!graph || !graph._nodes) return []; + + const visibleNodes = []; + for (const node of graph._nodes) { + if (!node.pos || !node.size) continue; + if (node.flags?.collapsed) continue; + + // Calculate node position in CSS pixels + const nodeCssX = node.pos[0] * scale + offset[0]; + const nodeCssY = node.pos[1] * scale + offset[1]; + const nodeCssWidth = node.size[0] * scale; + const nodeCssHeight = node.size[1] * scale; + + // Check overlap + if (nodeCssX + nodeCssWidth < sourceCssX || nodeCssX > sourceCssX + sourceCssWidth) continue; + if (nodeCssY + nodeCssHeight < sourceCssY || nodeCssY > sourceCssY + sourceCssHeight) continue; + + visibleNodes.push(node); + } + return visibleNodes; + } + /** * Detect if there are nodes with image previews in the capture region. * These nodes (Save Image, Preview Image, etc.) have images that are drawn @@ -191,11 +226,19 @@ export class OffscreenRenderer { const lgCanvas = app?.canvas; const currentScale = lgCanvas?.ds?.scale ?? 1.0; const currentOffset: [number, number] = lgCanvas?.ds?.offset ? [lgCanvas.ds.offset[0], lgCanvas.ds.offset[1]] : [0, 0]; - this.drawWidgetTextNatively(sourceX, sourceY, sourceWidth, sourceHeight, renderSize, currentScale, currentOffset); + + // OPTIMIZATION: Get visible nodes once + const sourceCssX = sourceX / dpr; + const sourceCssY = sourceY / dpr; + const sourceCssWidth = sourceWidth / dpr; + const sourceCssHeight = sourceHeight / dpr; + const visibleNodes = this.getVisibleNodes(sourceCssX, sourceCssY, sourceCssWidth, sourceCssHeight, currentScale, currentOffset); + + this.drawWidgetTextNatively(visibleNodes, sourceX, sourceY, sourceWidth, sourceHeight, renderSize, currentScale, currentOffset); // Draw node titles if emphasis is enabled if (this.config.accessibilityEnabled && this.config.nodeTitleEmphasis) { - this.drawNodeTitlesNatively(sourceX, sourceY, sourceWidth, sourceHeight, renderSize, currentScale, currentOffset); + this.drawNodeTitlesNatively(visibleNodes, sourceX, sourceY, sourceWidth, sourceHeight, renderSize, currentScale, currentOffset); } // NOTE: We do NOT call drawImagePreviewsNatively() here. @@ -346,15 +389,23 @@ export class OffscreenRenderer { // Note: For virtual zoom, we use targetScale and the CURRENT offset (after setZoom) // because lgCanvas.setZoom() modified the offset to zoom around the pivot const captureOffset: [number, number] = [lgCanvas.ds.offset[0], lgCanvas.ds.offset[1]]; - this.drawWidgetTextNatively(sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); + + // OPTIMIZATION: Get visible nodes once + const sourceCssX = sourceX / dpr; + const sourceCssY = sourceY / dpr; + const sourceCssWidth = sourceWidth / dpr; + const sourceCssHeight = sourceHeight / dpr; + const visibleNodes = this.getVisibleNodes(sourceCssX, sourceCssY, sourceCssWidth, sourceCssHeight, targetScale, captureOffset); + + this.drawWidgetTextNatively(visibleNodes, sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); // Draw node titles if emphasis is enabled if (this.config.accessibilityEnabled && this.config.nodeTitleEmphasis) { - this.drawNodeTitlesNatively(sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); + this.drawNodeTitlesNatively(visibleNodes, sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); } // Draw image/video previews natively on top of the captured canvas - this.drawImagePreviewsNatively(sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); + this.drawImagePreviewsNatively(visibleNodes, sourceX, sourceY, sourceWidth, sourceHeight, renderSize, targetScale, captureOffset); // Draw cursor preview overlay if enabled if (this.config.showCursorPreview) { @@ -400,6 +451,7 @@ export class OffscreenRenderer { * Draw node titles natively with accessibility styling. */ private drawNodeTitlesNatively( + visibleNodes: any[], sourceX: number, sourceY: number, sourceWidth: number, @@ -408,8 +460,7 @@ export class OffscreenRenderer { scale: number, offset: [number, number] ): void { - const graph = app?.graph; - if (!graph || !graph._nodes || !this.offscreenCtx) return; + if (!this.offscreenCtx) return; const ctx = this.offscreenCtx; const TITLE_HEIGHT = 30; @@ -425,7 +476,7 @@ export class OffscreenRenderer { const sourceCssWidth = sourceWidth / actualDpr; const sourceCssHeight = sourceHeight / actualDpr; - for (const node of graph._nodes) { + for (const node of visibleNodes) { if (!node.pos || !node.size) continue; if (node.flags?.collapsed) continue; @@ -497,6 +548,7 @@ export class OffscreenRenderer { * Draw widget text natively on the offscreen canvas. * This renders text content that would otherwise be lost since widgets are DOM elements. * + * @param visibleNodes - List of nodes to check for text widgets * @param sourceX - Source X position in backing pixels * @param sourceY - Source Y position in backing pixels * @param sourceWidth - Source width in backing pixels @@ -506,6 +558,7 @@ export class OffscreenRenderer { * @param offset - Canvas offset [x, y] used during capture */ private drawWidgetTextNatively( + visibleNodes: any[], sourceX: number, sourceY: number, sourceWidth: number, @@ -514,8 +567,7 @@ export class OffscreenRenderer { scale: number, offset: [number, number] ): void { - const graph = app?.graph; - if (!graph || !graph._nodes || !this.offscreenCtx) return; + if (!this.offscreenCtx) return; const ctx = this.offscreenCtx; const lgCanvas = app?.canvas; @@ -546,7 +598,7 @@ export class OffscreenRenderer { const sourceCssWidth = sourceWidth / actualDpr; const sourceCssHeight = sourceHeight / actualDpr; - for (const node of graph._nodes) { + for (const node of visibleNodes) { if (!node.widgets || !node.pos || !node.size) continue; if (node.flags?.collapsed) continue; @@ -711,6 +763,7 @@ export class OffscreenRenderer { * - node.imgs[] array (standard ComfyUI SaveImage/PreviewImage nodes) * - VHS-style DOM widget previews (video/image elements) * + * @param visibleNodes - List of nodes to check for image previews * @param sourceX - Source X position in backing pixels * @param sourceY - Source Y position in backing pixels * @param sourceWidth - Source width in backing pixels @@ -720,6 +773,7 @@ export class OffscreenRenderer { * @param offset - Canvas offset [x, y] used during capture */ private drawImagePreviewsNatively( + visibleNodes: any[], sourceX: number, sourceY: number, sourceWidth: number, @@ -728,8 +782,7 @@ export class OffscreenRenderer { scale: number, offset: [number, number] ): void { - const graph = app?.graph; - if (!graph || !graph._nodes || !this.offscreenCtx) return; + if (!this.offscreenCtx) return; const ctx = this.offscreenCtx; @@ -748,7 +801,7 @@ export class OffscreenRenderer { const TITLE_HEIGHT = 30; const WIDGET_MARGIN = 4; - for (const node of graph._nodes) { + for (const node of visibleNodes) { if (!node.pos || !node.size) continue; if (node.flags?.collapsed) continue; diff --git a/tests/unit/OffscreenRenderer.test.ts b/tests/unit/OffscreenRenderer.test.ts new file mode 100644 index 0000000..89cf8d2 --- /dev/null +++ b/tests/unit/OffscreenRenderer.test.ts @@ -0,0 +1,158 @@ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; + +// Create hoisted mocks +const { appMock } = vi.hoisted(() => { + return { + appMock: { + graph: { + _nodes: [] as any[] + }, + canvas: { + ds: { + scale: 1, + offset: [0, 0] + } + } + } + }; +}); + +// Attach to global +// @ts-ignore +globalThis.app = appMock; + +vi.mock('/scripts/app.js', () => ({ + app: appMock +})); + +import { OffscreenRenderer } from '../../src/magnify-glass/OffscreenRenderer'; +import { ConfigManager } from '../../src/magnify-glass/ConfigManager'; +import { MagnifierState } from '../../src/magnify-glass/MagnifierState'; + +describe('OffscreenRenderer Optimization', () => { + let renderer: OffscreenRenderer; + let configMock: ConfigManager; + let stateMock: MagnifierState; + let canvasMock: HTMLCanvasElement; + + beforeEach(() => { + vi.clearAllMocks(); + appMock.graph._nodes = []; + appMock.canvas.ds = { scale: 1, offset: [0, 0] }; + + // Mock ConfigManager + configMock = { + glassSize: 200, + zoomFactor: 2, + accessibilityEnabled: true, + nodeTitleEmphasis: true, + invertColors: false, + grayscaleMode: false, + forceDirectCapture: false, + fontScaleFactor: 100, + boldTextEnabled: false, + textGlowEnabled: false, + textOutlineEnabled: false, + highContrastMode: false, + showCursorPreview: false + } as unknown as ConfigManager; + + // Mock MagnifierState + stateMock = { + x: 100, // Cursor backing X + y: 100, // Cursor backing Y + canvasScale: 1 + } as unknown as MagnifierState; + + // Mock canvas creation to return a mock context + const originalCreateElement = document.createElement.bind(document); + vi.spyOn(document, 'createElement').mockImplementation((tagName) => { + if (tagName === 'canvas') { + const canvas = originalCreateElement(tagName) as HTMLCanvasElement; + canvas.getContext = vi.fn().mockReturnValue({ + clearRect: vi.fn(), + save: vi.fn(), + restore: vi.fn(), + drawImage: vi.fn(), + beginPath: vi.fn(), + roundRect: vi.fn(), + fill: vi.fn(), + stroke: vi.fn(), + fillText: vi.fn(), + measureText: vi.fn().mockReturnValue({ width: 10 }), + clip: vi.fn(), + scale: vi.fn(), + translate: vi.fn(), + moveTo: vi.fn(), + lineTo: vi.fn(), + closePath: vi.fn(), + strokeText: vi.fn(), + }); + return canvas; + } + return originalCreateElement(tagName); + }); + + renderer = new OffscreenRenderer(configMock, stateMock); + }); + + it('should identify visible nodes and render them', () => { + // Setup nodes + // Node 1: Visible (at 50, 50, size 100x100) + // Cursor at 100, 100. Glass size 200, zoom 2. Source size 100. + // Capture region: 50 to 150. + // Node 1 overlaps. + const visibleNode = { + pos: [50, 50], // Graph coords + size: [100, 100], + widgets: [ + { type: 'text', name: 'visible_widget', value: 'Visible Text', computedHeight: 50, last_y: 40 } + ] + }; + // Node 2: Not Visible (far away) + const invisibleNode = { + pos: [1000, 1000], + size: [100, 100], + widgets: [ + { type: 'text', name: 'invisible_widget', value: 'Invisible Text', computedHeight: 50, last_y: 40 } + ] + }; + + appMock.graph._nodes = [visibleNode, invisibleNode]; + + // Mock target canvas + canvasMock = document.createElement('canvas'); + canvasMock.width = 1000; + canvasMock.height = 800; + vi.spyOn(canvasMock, 'getBoundingClientRect').mockReturnValue({ + x: 0, y: 0, width: 1000, height: 800, top: 0, left: 0, right: 1000, bottom: 800, toJSON: () => {} + } as DOMRect); + + // Spy on private method drawWidgetTextNatively via cast + const drawSpy = vi.spyOn(renderer as any, 'drawWidgetTextNatively'); + + // Spy on context fillText to verify rendering + // Need to access the internal context. + const ctx = (renderer as any).offscreenCtx; + const fillTextSpy = vi.spyOn(ctx, 'fillText'); + + // Render + renderer.renderHighResRegion(canvasMock); + + // Verify drawWidgetTextNatively was called + expect(drawSpy).toHaveBeenCalled(); + + // Verify that visibleNodes list was correctly filtered and passed + const callArgs = drawSpy.mock.calls[0]; + const visibleNodes = callArgs[0] as any[]; + expect(visibleNodes.length).toBe(1); + expect(visibleNodes[0].widgets[0].value).toBe('Visible Text'); + + // Verify text was drawn for visible node + expect(fillTextSpy).toHaveBeenCalledWith(expect.stringContaining('Visible Text'), expect.any(Number), expect.any(Number)); + + // Verify text was NOT drawn for invisible node + expect(fillTextSpy).not.toHaveBeenCalledWith(expect.stringContaining('Invisible Text'), expect.any(Number), expect.any(Number)); + }); +});