From c1bfb9cc7a0ea7ed2e5b8065e265ca456173cefa Mon Sep 17 00:00:00 2001 From: bymyself Date: Mon, 20 Apr 2026 19:48:21 -0700 Subject: [PATCH] refactor: complete DPR migration to viewport system (ADR 0009 Accepted) Migrate all window.devicePixelRatio reads to use LGraphCanvas.dpr, the single source of truth set by applyViewport. LGraphCanvas: - Add dpr property, initialized in constructor, updated on resize() - Migrate 6 internal DPR reads: drawFrontCanvas compositing, drawBackCanvas transform reset, centerOnNode, renderInfo, processMouseDown hit-testing, LOD threshold calculation - Mark resize() as @deprecated External consumers: - litegraphService.getCanvasCenter(): use canvas.dpr - useMinimapViewport: use canvas.dpr - layoutStore: documented TODO (no canvas reference at this layer) - DragAndScale: documented exception (no canvas reference) app.ts: - Mark resizeCanvas() as @deprecated - Inline viewport calls in scheduler path - Set canvas.dpr after applyViewport canvasViewport.ts: - Export CanvasViewport type - applyViewport returns the viewport for chaining ADR 0009: Proposed -> Accepted --- docs/adr/0009-canvas-viewport-system.md | 8 +++--- src/lib/litegraph/src/DragAndScale.ts | 3 +++ .../src/LGraphCanvas.renderInfo.test.ts | 24 +++++------------ src/lib/litegraph/src/LGraphCanvas.ts | 26 +++++++++---------- src/renderer/core/canvas/canvasViewport.ts | 3 ++- src/renderer/core/layout/store/layoutStore.ts | 2 +- .../composables/useMinimapViewport.test.ts | 1 + .../minimap/composables/useMinimapViewport.ts | 2 +- src/renderer/extensions/minimap/types.ts | 1 + src/scripts/app.ts | 6 ++++- src/services/litegraphService.ts | 2 +- 11 files changed, 38 insertions(+), 40 deletions(-) diff --git a/docs/adr/0009-canvas-viewport-system.md b/docs/adr/0009-canvas-viewport-system.md index c05f276318..7ac09872f1 100644 --- a/docs/adr/0009-canvas-viewport-system.md +++ b/docs/adr/0009-canvas-viewport-system.md @@ -4,7 +4,7 @@ Date: 2026-04-20 ## Status -Proposed +Accepted ## Context @@ -48,6 +48,8 @@ A `devAssert(condition, message)` utility throws in DEV mode and `console.error` The existing `LGraphCanvas.resize()` method and `resizeCanvas()` in app.ts are both replaced by calls through the viewport system. Both paths collapse into one: measure → apply → draw. +`LGraphCanvas` stores a `dpr` property that is set whenever a viewport is applied. All internal DPR consumers (`drawFrontCanvas`, `drawBackCanvas`, `centerOnNode`, `renderInfo`, `processMouseDown` hit testing, LOD threshold calculation) read `this.dpr` instead of `window.devicePixelRatio`. External consumers with access to the canvas instance (e.g. `litegraphService`, minimap composables) also read `canvas.dpr`. The only code that reads `window.devicePixelRatio` directly is (a) the viewport measurement functions themselves, (b) `DragAndScale` which doesn't have access to the canvas instance, and (c) `layoutStore` which operates at a layer without a direct canvas reference. + The viewport system composes with the existing `CanvasScheduler` — the scheduler handles **when** (deferring until the canvas is visible), the viewport handles **what** (correct DPR-scaled dimensions applied atomically to both canvases). Neither modifies the other. ### Design Principles @@ -77,12 +79,10 @@ Following the ECS principles established in [ADR 0008](0008-entity-component-sys ### Negative -- All existing `window.devicePixelRatio` reads in LGraphCanvas need migration to use the viewport's `dpr` field. This migration is incremental and not blocking. - Adds a new abstraction layer that all canvas-sizing code must flow through. -- A lint rule banning direct `window.devicePixelRatio` reads in canvas code requires team awareness during the migration period. +- `DragAndScale` and `layoutStore` still read `window.devicePixelRatio` directly because they lack a reference to the canvas instance. A future refactor could thread the `dpr` value through, but the current exception is documented and stable. ## Notes - References [ADR 0008](0008-entity-component-system.md) for the design principles (plain data components, pure system functions, no methods on entities). - The `devAssert` utility is general-purpose and can be used beyond canvas sizing for any invariant that should be loud in development but non-fatal in production. -- Migration of existing DPR reads in LGraphCanvas (`centerOnNode`, `renderInfo`, `processMouseDown` hit testing, `drawBackCanvas` `setTransform`, font scaling) can be done incrementally in follow-up PRs. This ADR covers the foundation and the critical resize path. diff --git a/src/lib/litegraph/src/DragAndScale.ts b/src/lib/litegraph/src/DragAndScale.ts index d97d4f6fc4..5fa5b0b3a6 100644 --- a/src/lib/litegraph/src/DragAndScale.ts +++ b/src/lib/litegraph/src/DragAndScale.ts @@ -194,6 +194,9 @@ export class DragAndScale { ): void { //If element hasn't initialized (browser tab is in background) //it has a size of 300x150 and a more reasonable default is used instead. + // DPR is stable between viewport application and fit-to-bounds calls. + // DragAndScale intentionally reads window.devicePixelRatio directly + // because it doesn't have access to the viewport system. const [width, height] = this.element.width === 300 && this.element.height === 150 ? [1920, 1080] diff --git a/src/lib/litegraph/src/LGraphCanvas.renderInfo.test.ts b/src/lib/litegraph/src/LGraphCanvas.renderInfo.test.ts index 40cba2d056..3decc452c4 100644 --- a/src/lib/litegraph/src/LGraphCanvas.renderInfo.test.ts +++ b/src/lib/litegraph/src/LGraphCanvas.renderInfo.test.ts @@ -35,27 +35,15 @@ describe('LGraphCanvas.renderInfo', () => { expect(spy).not.toHaveBeenCalled() }) - it('uses canvas.height divided by devicePixelRatio as y fallback', () => { + it('uses canvas.height divided by dpr as y fallback', () => { lgCanvas.canvas.width = 1920 lgCanvas.canvas.height = 2160 + lgCanvas.dpr = 2 - const originalDPR = window.devicePixelRatio - Object.defineProperty(window, 'devicePixelRatio', { - value: 2, - configurable: true - }) + lgCanvas.renderInfo(ctx, 10, 0) - try { - lgCanvas.renderInfo(ctx, 10, 0) - - // lineCount = 5 (graph present, no info_text), lineHeight = 13 - // y = canvas.height / DPR - (lineCount + 1) * lineHeight - expect(ctx.translate).toHaveBeenCalledWith(10, 2160 / 2 - 6 * 13) - } finally { - Object.defineProperty(window, 'devicePixelRatio', { - value: originalDPR, - configurable: true - }) - } + // lineCount = 5 (graph present, no info_text), lineHeight = 13 + // y = canvas.height / DPR - (lineCount + 1) * lineHeight + expect(ctx.translate).toHaveBeenCalledWith(10, 2160 / 2 - 6 * 13) }) }) diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index b72f839103..2fc6f5b989 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -510,7 +510,7 @@ export class LGraphCanvas implements CustomEventDispatcher } const baseFontSize = LiteGraph.NODE_TEXT_SIZE // 14px - const dprAdjustment = Math.sqrt(window.devicePixelRatio || 1) //Using sqrt here because higher DPR monitors do not linearily scale the readability of the font, instead they increase the font by some heurisitc, and to approximate we use sqrt to say basically a DPR of 2 increases the readability by 40%, 3 by 70% + const dprAdjustment = Math.sqrt(this.dpr) //Using sqrt here because higher DPR monitors do not linearily scale the readability of the font, instead they increase the font by some heurisitc, and to approximate we use sqrt to say basically a DPR of 2 increases the readability by 40%, 3 by 70% // Calculate the zoom level where text becomes unreadable this._lowQualityZoomThreshold = @@ -757,6 +757,9 @@ export class LGraphCanvas implements CustomEventDispatcher /** Link rendering adapter for litegraph-to-canvas integration */ linkRenderer: LitegraphLinkAdapter | null = null + /** Device pixel ratio from the last applied viewport. Single source of truth for DPR. */ + dpr: number = 1 + /** If true, enable drag zoom. Ctrl+Shift+Drag Up/Down: zoom canvas. */ dragZoomEnabled: boolean = false /** The start position of the drag zoom and original read-only state. */ @@ -1953,6 +1956,7 @@ export class LGraphCanvas implements CustomEventDispatcher this.bgcanvas = document.createElement('canvas') this.bgcanvas.width = this.canvas.width this.bgcanvas.height = this.canvas.height + this.dpr = Math.max(window.devicePixelRatio ?? 1, 1) const ctx = element.getContext?.('2d') if (ctx == null) { @@ -2534,7 +2538,7 @@ export class LGraphCanvas implements CustomEventDispatcher // Set the width of the line for isPointInStroke checks const { lineWidth } = this.ctx this.ctx.lineWidth = this.connections_width + 7 - const dpi = Math.max(window?.devicePixelRatio ?? 1, 1) + const dpi = this.dpr // Try layout store for segment hit testing first (more precise) const hitSegment = layoutStore.queryLinkSegmentAtPoint({ x, y }, this.ctx) @@ -4821,7 +4825,7 @@ export class LGraphCanvas implements CustomEventDispatcher * centers the camera on a given node */ centerOnNode(node: LGraphNode): void { - const dpi = window?.devicePixelRatio || 1 + const dpi = this.dpr this.ds.offset[0] = -node.pos[0] - node.size[0] * 0.5 + @@ -5054,7 +5058,7 @@ export class LGraphCanvas implements CustomEventDispatcher if (this.bgcanvas == this.canvas) { this.drawBackCanvas() } else { - const scale = window.devicePixelRatio + const scale = this.dpr ctx.drawImage( this.bgcanvas, 0, @@ -5382,12 +5386,7 @@ export class LGraphCanvas implements CustomEventDispatcher const lineHeight = 13 const lineCount = (this.graph ? 5 : 1) + (this.info_text ? 1 : 0) x = x || 10 - y = - y || - this.canvas.height / - ((this.canvas.ownerDocument.defaultView ?? window).devicePixelRatio || - 1) - - (lineCount + 1) * lineHeight + y = y || this.canvas.height / this.dpr - (lineCount + 1) * lineHeight ctx.save() ctx.translate(x, y) @@ -5456,7 +5455,7 @@ export class LGraphCanvas implements CustomEventDispatcher // reset in case of error if (!this.viewport) { - const scale = window.devicePixelRatio + const scale = this.dpr ctx.restore() ctx.setTransform(scale, 0, 0, scale, 0, 0) } @@ -6523,8 +6522,8 @@ export class LGraphCanvas implements CustomEventDispatcher } /** - * Resizes the canvas to a given size, if no size is passed, then it tries to fill the parentNode. - * Uses the CanvasViewport system internally for DPR-aware sizing. + * @deprecated Use {@link measureViewport} + {@link applyViewport} from `canvasViewport.ts` instead. + * This method remains for legacy callers that rely on parent-element fallback sizing. */ resize(width?: number, height?: number): void { if (!width && !height) { @@ -6550,6 +6549,7 @@ export class LGraphCanvas implements CustomEventDispatcher return applyViewport(viewport, this.canvas, this.bgcanvas) + this.dpr = viewport.dpr this.setDirty(true, true) } diff --git a/src/renderer/core/canvas/canvasViewport.ts b/src/renderer/core/canvas/canvasViewport.ts index 4044609368..1ec1ff7d49 100644 --- a/src/renderer/core/canvas/canvasViewport.ts +++ b/src/renderer/core/canvas/canvasViewport.ts @@ -48,7 +48,7 @@ function applyViewport( viewport: CanvasViewport, fg: HTMLCanvasElement, bg: HTMLCanvasElement -): void { +): CanvasViewport { fg.width = viewport.physicalWidth fg.height = viewport.physicalHeight bg.width = viewport.physicalWidth @@ -58,6 +58,7 @@ function applyViewport( bg.getContext('2d')?.scale(viewport.dpr, viewport.dpr) currentGeneration = viewport.generation + return viewport } export { measureViewport, measureViewportFromElement, applyViewport } diff --git a/src/renderer/core/layout/store/layoutStore.ts b/src/renderer/core/layout/store/layoutStore.ts index 1cb27b06f3..9ce5ee5892 100644 --- a/src/renderer/core/layout/store/layoutStore.ts +++ b/src/renderer/core/layout/store/layoutStore.ts @@ -695,7 +695,7 @@ class LayoutStoreImpl implements LayoutStore { if (!segmentLayout) continue if (ctx && segmentLayout.path) { - // Match LiteGraph behavior: hit test uses device pixel ratio for coordinates + // TODO: Migrate to canvas.dpr once layoutStore has access to the active viewport const dpi = (typeof window !== 'undefined' && window?.devicePixelRatio) || 1 const hit = ctx.isPointInStroke( diff --git a/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts b/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts index 03a7fb0625..06290e5ba5 100644 --- a/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts +++ b/src/renderer/extensions/minimap/composables/useMinimapViewport.test.ts @@ -42,6 +42,7 @@ describe('useMinimapViewport', () => { width: 1600, height: 1200 } as HTMLCanvasElement, + dpr: 2, ds: { scale: 1, offset: [0, 0] diff --git a/src/renderer/extensions/minimap/composables/useMinimapViewport.ts b/src/renderer/extensions/minimap/composables/useMinimapViewport.ts index 4aec08bdf6..67fb3fe433 100644 --- a/src/renderer/extensions/minimap/composables/useMinimapViewport.ts +++ b/src/renderer/extensions/minimap/composables/useMinimapViewport.ts @@ -44,7 +44,7 @@ export function useMinimapViewport( if (!c) return const canvasEl = c.canvas - const dpr = window.devicePixelRatio || 1 + const dpr = c.dpr canvasDimensions.value = { width: canvasEl.clientWidth || canvasEl.width / dpr, diff --git a/src/renderer/extensions/minimap/types.ts b/src/renderer/extensions/minimap/types.ts index d23200a2e9..55189f3b7a 100644 --- a/src/renderer/extensions/minimap/types.ts +++ b/src/renderer/extensions/minimap/types.ts @@ -9,6 +9,7 @@ import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSche */ export interface MinimapCanvas { canvas: HTMLCanvasElement + dpr: number ds: { scale: number offset: [number, number] diff --git a/src/scripts/app.ts b/src/scripts/app.ts index f21aa2b6ec..177914dde2 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -971,9 +971,11 @@ export class ComfyApp { ) } + /** @deprecated Use {@link measureViewportFromElement} + {@link applyViewport} directly. */ private resizeCanvas(canvas: HTMLCanvasElement) { const viewport = measureViewportFromElement(canvas) applyViewport(viewport, canvas, this.canvas.bgcanvas) + this.canvas.dpr = viewport.dpr this.canvas?.draw(true, true) } @@ -1342,7 +1344,9 @@ export class ComfyApp { } canvasScheduler.schedule(() => { - this.resizeCanvas(this.canvasEl) + const vp = measureViewportFromElement(this.canvasEl) + applyViewport(vp, this.canvasEl, this.canvas.bgcanvas) + this.canvas.dpr = vp.dpr fitView() }) } catch (error) { diff --git a/src/services/litegraphService.ts b/src/services/litegraphService.ts index 832a355eed..9e8a472c59 100644 --- a/src/services/litegraphService.ts +++ b/src/services/litegraphService.ts @@ -938,7 +938,7 @@ export const useLitegraphService = () => { } function getCanvasCenter(): Point { - const dpi = Math.max(window.devicePixelRatio ?? 1, 1) + const dpi = app.canvas?.dpr ?? 1 const visibleArea = app.canvas?.ds?.visible_area if (!visibleArea) { return [0, 0]