fix: align link hit-test DPR source with canvas in queryLinkSegmentAtPoint

processMouseDown's isPointInStroke fallback uses this.dpr, but
layoutStore.queryLinkSegmentAtPoint was reading raw window.devicePixelRatio.
With the DPR migration in place these two hit-test paths can now disagree
on low-DPR displays, making alt/shift-click link detection depend on which
path wins.

Thread an optional dpr argument through queryLinkSegmentAtPoint /
queryLinkAtPoint and the LayoutStore interface, and pass this.dpr from
processMouseDown so both hit-test paths share the canvas-owned DPR.
Window fallback is preserved for legacy callers without a canvas reference.

Addresses review feedback:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11498#pullrequestreview-4151044800
This commit is contained in:
bymyself
2026-05-01 23:11:37 -07:00
parent a49329e51a
commit 4ad66359f9
3 changed files with 32 additions and 9 deletions

View File

@@ -2540,8 +2540,15 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
this.ctx.lineWidth = this.connections_width + 7
const dpi = this.dpr
// Try layout store for segment hit testing first (more precise)
const hitSegment = layoutStore.queryLinkSegmentAtPoint({ x, y }, this.ctx)
// Try layout store for segment hit testing first (more precise).
// Pass this.dpr so the layout-store hit-test uses the same DPR as the
// isPointInStroke fallback below; otherwise the two paths can disagree
// on low-DPR displays (e.g. chromium-0.5x).
const hitSegment = layoutStore.queryLinkSegmentAtPoint(
{ x, y },
this.ctx,
this.dpr
)
for (const linkSegment of this.renderedPaths) {
const centre = linkSegment._pos

View File

@@ -661,10 +661,17 @@ class LayoutStoreImpl implements LayoutStore {
/**
* Query link segment at point (returns structured data)
*
* @param dpr Device pixel ratio used to map the CSS-space point into the
* canvas's device-pixel-scaled stroke space. Pass the active
* `LGraphCanvas.dpr` so this hit-test agrees with `processMouseDown`'s
* `isPointInStroke` fallback. Falls back to `window.devicePixelRatio`
* for legacy callers without a canvas reference.
*/
queryLinkSegmentAtPoint(
point: Point,
ctx?: CanvasRenderingContext2D
ctx?: CanvasRenderingContext2D,
dpr?: number
): { linkId: LinkId; rerouteId: RerouteId | null } | null {
// Determine tolerance from current canvas state (if available)
// - Use the caller-provided ctx.lineWidth (LGraphCanvas sets this to connections_width + padding)
@@ -695,9 +702,12 @@ class LayoutStoreImpl implements LayoutStore {
if (!segmentLayout) continue
if (ctx && segmentLayout.path) {
// TODO: Migrate to canvas.dpr once layoutStore has access to the active viewport
// Prefer the caller-supplied DPR (the active LGraphCanvas.dpr) so
// this hit-test stays in lockstep with processMouseDown's fallback
// path; fall back to window.devicePixelRatio for legacy callers.
const dpi =
(typeof window !== 'undefined' && window?.devicePixelRatio) || 1
dpr ??
((typeof window !== 'undefined' && window?.devicePixelRatio) || 1)
const hit = ctx.isPointInStroke(
segmentLayout.path,
point.x * dpi,
@@ -732,10 +742,11 @@ class LayoutStoreImpl implements LayoutStore {
*/
queryLinkAtPoint(
point: Point,
ctx?: CanvasRenderingContext2D
ctx?: CanvasRenderingContext2D,
dpr?: number
): LinkId | null {
// Invoke segment query and return just the linkId
const segment = this.queryLinkSegmentAtPoint(point, ctx)
const segment = this.queryLinkSegmentAtPoint(point, ctx, dpr)
return segment ? segment.linkId : null
}

View File

@@ -274,10 +274,15 @@ export interface LayoutStore {
queryNodesInBounds(bounds: Bounds): NodeId[]
// Hit testing queries for links, slots, and reroutes
queryLinkAtPoint(point: Point, ctx?: CanvasRenderingContext2D): LinkId | null
queryLinkAtPoint(
point: Point,
ctx?: CanvasRenderingContext2D,
dpr?: number
): LinkId | null
queryLinkSegmentAtPoint(
point: Point,
ctx?: CanvasRenderingContext2D
ctx?: CanvasRenderingContext2D,
dpr?: number
): { linkId: LinkId; rerouteId: RerouteId | null } | null
querySlotAtPoint(point: Point): SlotLayout | null
queryRerouteAtPoint(point: Point): RerouteLayout | null