Remove magic 10% scale on numeric widget step (#643)

There are external code still dependent on the fact that
Widget.options.step is scaled 10x, so the 10x-ed value is still kept
there, while we use the new unscaled step2 within our code now.

Ref:
https://cs.comfy.org/search?q=context:global+%22step+/+10%22&patternType=keyword&sm=0
This commit is contained in:
Chenlei Hu
2025-02-27 16:23:53 -05:00
committed by GitHub
parent 0a09ecc7ac
commit c66ca2ae66
5 changed files with 73 additions and 9 deletions

View File

@@ -12,7 +12,14 @@ export interface IWidgetOptions<TValue = unknown> extends Record<string, unknown
marker_color?: CanvasColour
precision?: number
read_only?: boolean
/**
* @deprecated Use {@link IWidgetOptions.step2} instead.
* The legacy step is scaled up by 10x in the legacy frontend logic.
*/
step?: number
/** The step value for numeric widgets. */
step2?: number
y?: number
multiline?: boolean
// TODO: Confirm this
@@ -25,7 +32,7 @@ export interface IWidgetOptions<TValue = unknown> extends Record<string, unknown
export interface IWidgetSliderOptions extends IWidgetOptions<number> {
min: number
max: number
step: number
step2: number
slider_color?: CanvasColour
marker_color?: CanvasColour
}
@@ -33,7 +40,7 @@ export interface IWidgetSliderOptions extends IWidgetOptions<number> {
export interface IWidgetKnobOptions extends IWidgetOptions<number> {
min: number
max: number
step: number
step2: number
slider_color?: CanvasColour // TODO: Replace with knob color
marker_color?: CanvasColour
gradient_stops?: string

10
src/utils/widget.ts Normal file
View File

@@ -0,0 +1,10 @@
import type { IWidgetOptions } from "@/types/widgets"
/**
* The step value for numeric widgets.
* Use {@link IWidgetOptions.step2} if available, otherwise fallback to
* {@link IWidgetOptions.step} which is scaled up by 10x in the legacy frontend logic.
*/
export function getWidgetStep(options: IWidgetOptions<unknown>): number {
return options.step2 || ((options.step || 10) * 0.1)
}

View File

@@ -4,6 +4,7 @@ import type { IKnobWidget, IWidgetKnobOptions } from "@/types/widgets"
import { LGraphCanvas } from "@/LGraphCanvas"
import { clamp } from "@/litegraph"
import { CanvasMouseEvent } from "@/types/events"
import { getWidgetStep } from "@/utils/widget"
import { BaseWidget } from "./BaseWidget"
@@ -214,8 +215,8 @@ export class KnobWidget extends BaseWidget implements IKnobWidget {
}): void {
if (this.options.read_only) return
const { e } = options
const step = this.options.step
// Shift to move by 10% increments, there is no division by 10 due to the front-end multiplier
const step = getWidgetStep(this.options) * 10
// Shift to move by 10% increments
const maxMinDifference = (this.options.max - this.options.min)
const maxMinDifference10pct = maxMinDifference / 10
const step_for = {
@@ -243,9 +244,8 @@ export class KnobWidget extends BaseWidget implements IKnobWidget {
: (use_y
? step_for.delta_y
: step)
// HACK: For some reason, the front-end multiplies step by 10, this brings it down to the advertised value
// SEE: src/utils/mathUtil.ts@getNumberDefaults in front end
const deltaValue = adjustment * step_with_shift_modifier / 10
const deltaValue = adjustment * step_with_shift_modifier
const newValue = clamp(
this.value + deltaValue,
this.options.min,

View File

@@ -3,6 +3,8 @@ import type { LGraphNode } from "@/LGraphNode"
import type { CanvasMouseEvent } from "@/types/events"
import type { INumericWidget, IWidgetOptions } from "@/types/widgets"
import { getWidgetStep } from "@/utils/widget"
import { BaseWidget } from "./BaseWidget"
export class NumberWidget extends BaseWidget implements INumericWidget {
@@ -111,7 +113,7 @@ export class NumberWidget extends BaseWidget implements INumericWidget {
if (delta) {
// Handle left/right arrow clicks
let newValue = this.value + delta * 0.1 * (this.options.step || 1)
let newValue = this.value + delta * getWidgetStep(this.options)
if (this.options.min != null && newValue < this.options.min) {
newValue = this.options.min
}
@@ -161,7 +163,7 @@ export class NumberWidget extends BaseWidget implements INumericWidget {
if (delta && (x > -3 && x < width + 3)) return
let newValue = this.value
if (e.deltaX) newValue += e.deltaX * 0.1 * (this.options.step || 1)
if (e.deltaX) newValue += e.deltaX * getWidgetStep(this.options)
if (this.options.min != null && newValue < this.options.min) {
newValue = this.options.min

45
test/utils/widget.test.ts Normal file
View File

@@ -0,0 +1,45 @@
import type { IWidgetOptions } from "@/types/widgets"
import { describe, expect, test } from "vitest"
import { getWidgetStep } from "@/utils/widget"
describe("getWidgetStep", () => {
test("should return step2 when available", () => {
const options: IWidgetOptions<unknown> = {
step2: 0.5,
step: 20,
}
expect(getWidgetStep(options)).toBe(0.5)
})
test("should calculate from step when step2 is not available", () => {
const options: IWidgetOptions<unknown> = {
step: 20,
}
expect(getWidgetStep(options)).toBe(2) // 20 * 0.1 = 2
})
test("should use default step value of 10 when neither step2 nor step is provided", () => {
const options: IWidgetOptions<unknown> = {}
expect(getWidgetStep(options)).toBe(1) // 10 * 0.1 = 1
})
// Zero value is not allowed for step, fallback to 1.
test("should handle zero values correctly", () => {
const optionsWithZeroStep2: IWidgetOptions<unknown> = {
step2: 0,
step: 20,
}
expect(getWidgetStep(optionsWithZeroStep2)).toBe(2)
const optionsWithZeroStep: IWidgetOptions<unknown> = {
step: 0,
}
expect(getWidgetStep(optionsWithZeroStep)).toBe(1)
})
})