From ef0cd18616a5bc2dd78b9822261f327d28429ef6 Mon Sep 17 00:00:00 2001 From: Rizumu Ayaka Date: Tue, 5 May 2026 23:13:26 +0800 Subject: [PATCH] refactor: address review feedback on Input device PR - Clean up onDetectedDeviceChange callback when canvas is replaced - Migrate legacy NavigationMode/MouseWheelScroll on first run - Drop unreachable !isTrackpad branch in trackpad pan path - Rename isStandardNavMode to isTrackpadWheelMode for clarity --- src/lib/litegraph/src/LGraphCanvas.ts | 13 ++---- .../composables/useLitegraphSettings.ts | 42 +++++++++++++++++-- .../core/canvas/useCanvasInteractions.ts | 8 ++-- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index 3c03cfc7b4..259fd18715 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -3929,15 +3929,10 @@ export class LGraphCanvas implements CustomEventDispatcher this.ds.changeScale(scale, [e.clientX, e.clientY]) } } else { - // Trackpads and mice work on significantly different scales - const factor = isTrackpad ? 0.18 : 0.008_333 - - if (!isTrackpad && e.shiftKey && e.deltaX === 0) { - this.ds.offset[0] -= e.deltaY * (1 + factor) * (1 / scale) - } else { - this.ds.offset[0] -= e.deltaX * (1 + factor) * (1 / scale) - this.ds.offset[1] -= e.deltaY * (1 + factor) * (1 / scale) - } + // Trackpad two-finger pan: outer condition guarantees isTrackpad here + const factor = 0.18 + this.ds.offset[0] -= e.deltaX * (1 + factor) * (1 / scale) + this.ds.offset[1] -= e.deltaY * (1 + factor) * (1 / scale) } this.graph.change() diff --git a/src/platform/settings/composables/useLitegraphSettings.ts b/src/platform/settings/composables/useLitegraphSettings.ts index 3abb5b2c2d..94c0087541 100644 --- a/src/platform/settings/composables/useLitegraphSettings.ts +++ b/src/platform/settings/composables/useLitegraphSettings.ts @@ -10,6 +10,34 @@ import { useSettingStore } from '@/platform/settings/settingStore' // eslint-disable-next-line import-x/no-restricted-paths import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' +/** + * One-time translation of the legacy `Comfy.Canvas.NavigationMode` + + * `Comfy.Canvas.MouseWheelScroll` pair into the new `Comfy.Graph.WheelInputMode` + * preference, preserving explicit choices made by users on previous versions. + * + * Idempotency is achieved by resetting NavigationMode to its default after + * migration: subsequent boots see `legacy` and exit early. + */ +async function migrateLegacyNavigationSettings( + settingStore: ReturnType +) { + const navMode = settingStore.get('Comfy.Canvas.NavigationMode') + if (navMode === 'legacy') return + + let migrated: 'mouse' | 'trackpad' | undefined + if (navMode === 'standard') { + migrated = 'trackpad' + } else if (navMode === 'custom') { + const wheelScroll = settingStore.get('Comfy.Canvas.MouseWheelScroll') + migrated = wheelScroll === 'panning' ? 'trackpad' : 'mouse' + } + + if (migrated && settingStore.get('Comfy.Graph.WheelInputMode') === 'auto') { + await settingStore.set('Comfy.Graph.WheelInputMode', migrated) + } + await settingStore.set('Comfy.Canvas.NavigationMode', 'legacy') +} + /** * Watch for changes in the setting store and update the LiteGraph settings accordingly. */ @@ -17,6 +45,8 @@ export const useLitegraphSettings = () => { const settingStore = useSettingStore() const canvasStore = useCanvasStore() + void migrateLegacyNavigationSettings(settingStore) + watchEffect(() => { const canvasInfoEnabled = settingStore.get('Comfy.Graph.CanvasInfo') if (canvasStore.canvas) { @@ -158,15 +188,21 @@ export const useLitegraphSettings = () => { /** * Mirror the canvas pointer's auto-detected device onto a reactive ref so * settings UI can show the current detection inside the "Auto" option. + * The cleanup detaches the handler from the previous pointer so a stale + * canvas instance can no longer mutate the shared ref after replacement. */ - watchEffect(() => { + watchEffect((onCleanup) => { const { canvas } = canvasStore if (!canvas) return + const { pointer } = canvas const { detectedInputDevice } = useInputDeviceDetection() - detectedInputDevice.value = canvas.pointer.detectedDevice - canvas.pointer.onDetectedDeviceChange = (device) => { + detectedInputDevice.value = pointer.detectedDevice + pointer.onDetectedDeviceChange = (device) => { detectedInputDevice.value = device } + onCleanup(() => { + pointer.onDetectedDeviceChange = undefined + }) }) watchEffect(() => { diff --git a/src/renderer/core/canvas/useCanvasInteractions.ts b/src/renderer/core/canvas/useCanvasInteractions.ts index 38907f2dd9..9ef16bbd64 100644 --- a/src/renderer/core/canvas/useCanvasInteractions.ts +++ b/src/renderer/core/canvas/useCanvasInteractions.ts @@ -14,7 +14,7 @@ export function useCanvasInteractions() { const canvasStore = useCanvasStore() const { getCanvas } = canvasStore - const isStandardNavMode = computed( + const isTrackpadWheelMode = computed( () => settingStore.get('Comfy.Graph.WheelInputMode') === 'trackpad' ) @@ -43,7 +43,7 @@ export function useCanvasInteractions() { const shouldForwardWheelEvent = (event: WheelEvent): boolean => !wheelCapturedByFocusedElement(event) || - (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) + (isTrackpadWheelMode.value && (event.ctrlKey || event.metaKey)) /** * Handles wheel events from UI components that should be forwarded to canvas @@ -53,13 +53,13 @@ export function useCanvasInteractions() { if (!shouldForwardWheelEvent(event)) return // In standard mode, Ctrl+wheel should go to canvas for zoom - if (isStandardNavMode.value && (event.ctrlKey || event.metaKey)) { + if (isTrackpadWheelMode.value && (event.ctrlKey || event.metaKey)) { forwardEventToCanvas(event) return } // In legacy mode, all wheel events go to canvas for zoom - if (!isStandardNavMode.value) { + if (!isTrackpadWheelMode.value) { forwardEventToCanvas(event) return }