Rewrite/Test rounding logic of numeric widgets (#2758)

src/composables/widgets/useFloatWidget.ts

@@ -1,11 +1,27 @@
 import type { LGraphNode } from '@comfyorg/litegraph'
 import type { INumericWidget } from '@comfyorg/litegraph/dist/types/widgets'
+import _ from 'lodash'
 
 import type { InputSpec } from '@/schemas/nodeDefSchema'
 import type { ComfyWidgetConstructor } from '@/scripts/widgets'
 import { useSettingStore } from '@/stores/settingStore'
 import { getNumberDefaults } from '@/utils/mathUtil'
 
+function onFloatValueChange(this: INumericWidget, v: number) {
+  this.value = this.options.round
+    ? _.clamp(
+        Math.round((v + Number.EPSILON) / this.options.round) *
+          this.options.round,
+        this.options.min ?? -Infinity,
+        this.options.max ?? Infinity
+      )
+    : v
+}
+
+export const _for_testing = {
+  onFloatValueChange
+}
+
 export const useFloatWidget = () => {
   const widgetConstructor: ComfyWidgetConstructor = (
     node: LGraphNode,
@@ -40,16 +56,7 @@ export const useFloatWidget = () => {
         widgetType,
         inputName,
         val,
-        function (this: INumericWidget, v: number) {
-          if (config.round) {
-            this.value =
-              Math.round((v + Number.EPSILON) / config.round) * config.round
-            if (this.value > config.max) this.value = config.max
-            if (this.value < config.min) this.value = config.min
-          } else {
-            this.value = v
-          }
-        },
+        onFloatValueChange,
         config
       )
     }

src/composables/widgets/useIntWidget.ts

@@ -10,6 +10,29 @@ import {
 import { useSettingStore } from '@/stores/settingStore'
 import { getNumberDefaults } from '@/utils/mathUtil'
 
+function onValueChange(this: INumericWidget, v: number) {
+  // For integers, always round to the nearest step
+  // step === 0 is invalid, assign 1 if options.step is 0
+  const step = this.options.step2 || 1
+
+  if (step === 1) {
+    // Simple case: round to nearest integer
+    this.value = Math.round(v)
+  } else {
+    // Round to nearest multiple of step
+    // First, determine if min value creates an offset
+    const min = this.options.min ?? 0
+    const offset = min % step
+
+    // Round to nearest step, accounting for offset
+    this.value = Math.round((v - offset) / step) * step + offset
+  }
+}
+
+export const _for_testing = {
+  onValueChange
+}
+
 export const useIntWidget = () => {
   const widgetConstructor: ComfyWidgetConstructor = (
     node: LGraphNode,
@@ -35,20 +58,7 @@ export const useIntWidget = () => {
     config.precision = 0
 
     const result = {
-      widget: node.addWidget(
-        widgetType,
-        inputName,
-        val,
-        function (this: INumericWidget, v: number) {
-          const s = this.options.step2 || 1
-          let sh = (this.options.min ?? 0) % s
-          if (isNaN(sh)) {
-            sh = 0
-          }
-          this.value = Math.round((v - sh) / s) * s + sh
-        },
-        config
-      )
+      widget: node.addWidget(widgetType, inputName, val, onValueChange, config)
     }
 
     if (inputData[1]?.control_after_generate) {

src/types/litegraph-augmentation.d.ts

@@ -17,6 +17,10 @@ declare module '@comfyorg/litegraph/dist/types/widgets' {
      * - If true or undefined, the value will be included in both the API workflow and graph state
      */
     serialize?: boolean
+    /**
+     * Rounding value for numeric float widgets.
+     */
+    round?: number
   }
 
   interface IBaseWidget {

tests-ui/tests/composables/widgets/useFloatWidget.test.ts

@@ -0,0 +1,78 @@
+import { _for_testing } from '@/composables/widgets/useFloatWidget'
+
+jest.mock('@/scripts/widgets', () => ({
+  addValueControlWidgets: jest.fn()
+}))
+
+jest.mock('@/stores/settingStore', () => ({
+  useSettingStore: () => ({
+    settings: {}
+  })
+}))
+
+const { onFloatValueChange } = _for_testing
+
+describe('useFloatWidget', () => {
+  describe('onFloatValueChange', () => {
+    let widget: any
+
+    beforeEach(() => {
+      // Reset the widget before each test
+      widget = {
+        options: {},
+        value: 0
+      }
+    })
+
+    it('should not round values when round option is not set', () => {
+      widget.options.round = undefined
+      onFloatValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(5.7)
+    })
+
+    it('should round values based on round option', () => {
+      widget.options.round = 0.5
+      onFloatValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(5.5)
+
+      widget.options.round = 0.1
+      onFloatValueChange.call(widget, 5.74)
+      expect(widget.value).toBe(5.7)
+
+      widget.options.round = 1
+      onFloatValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(6)
+    })
+
+    it('should respect min and max constraints after rounding', () => {
+      widget.options.round = 0.5
+      widget.options.min = 1
+      widget.options.max = 5
+
+      // Should round to 1 and respect min
+      onFloatValueChange.call(widget, 0.7)
+      expect(widget.value).toBe(1)
+
+      // Should round to 5.5 but be clamped to max of 5
+      onFloatValueChange.call(widget, 5.3)
+      expect(widget.value).toBe(5)
+
+      // Should round to 3.5 and be within bounds
+      onFloatValueChange.call(widget, 3.6)
+      expect(widget.value).toBe(3.5)
+    })
+
+    it('should handle Number.EPSILON for precision issues', () => {
+      widget.options.round = 0.1
+
+      // Without Number.EPSILON, 1.35 / 0.1 = 13.499999999999998
+      // which would round to 13 * 0.1 = 1.3 instead of 1.4
+      onFloatValueChange.call(widget, 1.35)
+      expect(widget.value).toBeCloseTo(1.4, 10)
+
+      // Test another edge case
+      onFloatValueChange.call(widget, 2.95)
+      expect(widget.value).toBeCloseTo(3, 10)
+    })
+  })
+})

tests-ui/tests/composables/widgets/useIntWidget.test.ts

@@ -0,0 +1,70 @@
+import { _for_testing } from '@/composables/widgets/useIntWidget'
+
+jest.mock('@/scripts/widgets', () => ({
+  addValueControlWidgets: jest.fn()
+}))
+
+jest.mock('@/stores/settingStore', () => ({
+  useSettingStore: () => ({
+    settings: {}
+  })
+}))
+
+const { onValueChange } = _for_testing
+
+describe('useIntWidget', () => {
+  describe('onValueChange', () => {
+    let widget: any
+
+    beforeEach(() => {
+      // Reset the widget before each test
+      widget = {
+        options: {},
+        value: 0
+      }
+    })
+
+    it('should round values based on step size', () => {
+      widget.options.step2 = 0.1
+      onValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(5.7)
+
+      widget.options.step2 = 0.5
+      onValueChange.call(widget, 7.3)
+      expect(widget.value).toBe(7.5)
+
+      widget.options.step2 = 1
+      onValueChange.call(widget, 23.4)
+      expect(widget.value).toBe(23)
+    })
+
+    it('should handle undefined step by using default of 1', () => {
+      widget.options.step2 = undefined
+      onValueChange.call(widget, 3.7)
+      expect(widget.value).toBe(4)
+    })
+
+    it('should account for min value offset', () => {
+      widget.options.step2 = 2
+      widget.options.min = 1
+      // 2 valid values between 1.6 are 1 and 3
+      // 1.6 is closer to 1, so it should round to 1
+      onValueChange.call(widget, 1.6)
+      expect(widget.value).toBe(1)
+    })
+
+    it('should handle undefined min by using default of 0', () => {
+      widget.options.step2 = 2
+      widget.options.min = undefined
+      onValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(6)
+    })
+
+    it('should handle NaN shift value', () => {
+      widget.options.step2 = 0
+      widget.options.min = 1
+      onValueChange.call(widget, 5.7)
+      expect(widget.value).toBe(6)
+    })
+  })
+})