fix: address review feedback for Range editor

This commit is contained in:
Terry Jia
2026-04-08 21:17:52 -04:00
parent b95636fc81
commit 2019704fc3
8 changed files with 68 additions and 133 deletions

View File

@@ -178,12 +178,11 @@ import { useRangeEditor } from '@/composables/useRangeEditor'
import type { ColorStop } from '@/lib/litegraph/src/interfaces'
import type { RangeValue } from '@/lib/litegraph/src/types/widgets'
import {
clamp,
gammaToPosition,
normalize,
positionToGamma
} from './rangeUtils'
import { normalize } from '@/utils/mathUtil'
import { clamp } from 'es-toolkit'
import { gammaToPosition, positionToGamma } from './rangeUtils'
const {
display = 'plain',
@@ -214,7 +213,8 @@ const { handleTrackPointerDown, startDrag } = useRangeEditor({
trackRef,
modelValue,
valueMin: toRef(() => valueMin),
valueMax: toRef(() => valueMax)
valueMax: toRef(() => valueMax),
showMidpoint: toRef(() => showMidpoint)
})
function onTrackPointerDown(e: PointerEvent) {

View File

@@ -1,6 +1,6 @@
<template>
<RangeEditor
:model-value="effectiveValue.value"
:model-value="effectiveValue"
:display="widget?.options?.display"
:gradient-stops="widget?.options?.gradient_stops"
:show-midpoint="widget?.options?.show_midpoint"
@@ -64,8 +64,8 @@ watch(upstreamValue, (upstream) => {
const effectiveValue = computed(() =>
isDisabled.value && upstreamValue.value
? { value: upstreamValue.value }
: { value: modelValue.value }
? upstreamValue.value
: modelValue.value
)
function onValueChange(value: RangeValue) {

View File

@@ -1,38 +1,6 @@
import { describe, expect, it } from 'vitest'
import {
constrainRange,
denormalize,
formatMidpointLabel,
gammaToPosition,
isRangeValue,
normalize,
positionToGamma
} from './rangeUtils'
describe('normalize', () => {
it('normalizes value to 0-1', () => {
expect(normalize(128, 0, 256)).toBe(0.5)
expect(normalize(0, 0, 255)).toBe(0)
expect(normalize(255, 0, 255)).toBe(1)
})
it('returns 0 when min equals max', () => {
expect(normalize(5, 5, 5)).toBe(0)
})
})
describe('denormalize', () => {
it('converts normalized value back to range', () => {
expect(denormalize(0.5, 0, 256)).toBe(128)
expect(denormalize(0, 0, 255)).toBe(0)
expect(denormalize(1, 0, 255)).toBe(255)
})
it('round-trips with normalize', () => {
expect(denormalize(normalize(100, 0, 255), 0, 255)).toBeCloseTo(100)
})
})
import { gammaToPosition, isRangeValue, positionToGamma } from './rangeUtils'
describe('positionToGamma', () => {
it('converts 0.5 to gamma 1.0', () => {
@@ -60,51 +28,6 @@ describe('gammaToPosition', () => {
})
})
describe('formatMidpointLabel', () => {
it('formats linear scale as decimal', () => {
expect(formatMidpointLabel(0.5, 'linear')).toBe('0.50')
})
it('formats gamma scale as gamma value', () => {
expect(formatMidpointLabel(0.5, 'gamma')).toBe('1.00')
})
})
describe('constrainRange', () => {
it('passes through valid range unchanged', () => {
const result = constrainRange({ min: 0.2, max: 0.8 })
expect(result).toEqual({ min: 0.2, max: 0.8, midpoint: undefined })
})
it('clamps values to default [0, 1]', () => {
const result = constrainRange({ min: -0.5, max: 1.5 })
expect(result.min).toBe(0)
expect(result.max).toBe(1)
})
it('clamps values to custom range', () => {
const result = constrainRange({ min: -10, max: 300 }, 0, 255)
expect(result.min).toBe(0)
expect(result.max).toBe(255)
})
it('enforces min <= max', () => {
const result = constrainRange({ min: 0.8, max: 0.3 })
expect(result.min).toBe(0.8)
expect(result.max).toBe(0.8)
})
it('preserves midpoint when present', () => {
const result = constrainRange({ min: 0.2, max: 0.8, midpoint: 0.5 })
expect(result.midpoint).toBe(0.5)
})
it('clamps midpoint to [0, 1]', () => {
const result = constrainRange({ min: 0.2, max: 0.8, midpoint: 1.5 })
expect(result.midpoint).toBe(1)
})
})
describe('isRangeValue', () => {
it('returns true for valid range', () => {
expect(isRangeValue({ min: 0, max: 1 })).toBe(true)

View File

@@ -2,21 +2,8 @@ import { clamp } from 'es-toolkit'
import type { RangeValue } from '@/lib/litegraph/src/types/widgets'
export { clamp }
export function normalize(value: number, min: number, max: number): number {
return max === min ? 0 : (value - min) / (max - min)
}
export function denormalize(
normalized: number,
min: number,
max: number
): number {
return min + normalized * (max - min)
}
export function positionToGamma(position: number): number {
// Avoid log2(0) = -Infinity and log2(1) = 0 (division by zero in gamma)
const clamped = clamp(position, 0.001, 0.999)
return -Math.log2(clamped)
}
@@ -25,28 +12,6 @@ export function gammaToPosition(gamma: number): number {
return Math.pow(2, -gamma)
}
export function formatMidpointLabel(
position: number,
scale: 'linear' | 'gamma'
): string {
if (scale === 'gamma') {
return positionToGamma(position).toFixed(2)
}
return position.toFixed(2)
}
export function constrainRange(
value: RangeValue,
valueMin: number = 0,
valueMax: number = 1
): RangeValue {
const min = clamp(value.min, valueMin, valueMax)
const max = clamp(Math.max(min, value.max), valueMin, valueMax)
const midpoint =
value.midpoint !== undefined ? clamp(value.midpoint, 0, 1) : undefined
return { min, max, midpoint }
}
export function isRangeValue(value: unknown): value is RangeValue {
if (typeof value !== 'object' || value === null || Array.isArray(value))
return false

View File

@@ -3,7 +3,7 @@ import type { Ref } from 'vue'
import { clamp } from 'es-toolkit'
import { denormalize, normalize } from '@/components/range/rangeUtils'
import { denormalize, normalize } from '@/utils/mathUtil'
import type { RangeValue } from '@/lib/litegraph/src/types/widgets'
type HandleType = 'min' | 'max' | 'midpoint'
@@ -13,13 +13,15 @@ interface UseRangeEditorOptions {
modelValue: Ref<RangeValue>
valueMin: Ref<number>
valueMax: Ref<number>
showMidpoint: Ref<boolean>
}
export function useRangeEditor({
trackRef,
modelValue,
valueMin,
valueMax
valueMax,
showMidpoint
}: UseRangeEditorOptions) {
const activeHandle = ref<HandleType | null>(null)
let cleanupDrag: (() => void) | null = null
@@ -28,10 +30,7 @@ export function useRangeEditor({
const el = trackRef.value
if (!el) return valueMin.value
const rect = el.getBoundingClientRect()
const normalized = Math.max(
0,
Math.min(1, (e.clientX - rect.left) / rect.width)
)
const normalized = clamp((e.clientX - rect.left) / rect.width, 0, 1)
return denormalize(normalized, valueMin.value, valueMax.value)
}
@@ -41,7 +40,7 @@ export function useRangeEditor({
const dMax = Math.abs(value - max)
let best: HandleType = dMin <= dMax ? 'min' : 'max'
const bestDist = Math.min(dMin, dMax)
if (midpoint !== undefined) {
if (midpoint !== undefined && showMidpoint.value) {
const midAbs = min + midpoint * (max - min)
if (Math.abs(value - midAbs) < bestDist) {
best = 'midpoint'

View File

@@ -355,7 +355,6 @@ export interface IWidgetRangeOptions extends IWidgetOptions {
midpoint_scale?: 'linear' | 'gamma'
value_min?: number
value_max?: number
histogram?: Uint32Array | null
}
export interface IRangeWidget extends IBaseWidget<

View File

@@ -1,9 +1,39 @@
import { describe, expect, it } from 'vitest'
import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
import { computeUnionBounds, gcd, lcm } from '@/utils/mathUtil'
import {
computeUnionBounds,
denormalize,
gcd,
lcm,
normalize
} from '@/utils/mathUtil'
describe('mathUtil', () => {
describe('normalize', () => {
it('normalizes value to 0-1', () => {
expect(normalize(128, 0, 256)).toBe(0.5)
expect(normalize(0, 0, 255)).toBe(0)
expect(normalize(255, 0, 255)).toBe(1)
})
it('returns 0 when min equals max', () => {
expect(normalize(5, 5, 5)).toBe(0)
})
})
describe('denormalize', () => {
it('converts normalized value back to range', () => {
expect(denormalize(0.5, 0, 256)).toBe(128)
expect(denormalize(0, 0, 255)).toBe(0)
expect(denormalize(1, 0, 255)).toBe(255)
})
it('round-trips with normalize', () => {
expect(denormalize(normalize(100, 0, 255), 0, 255)).toBeCloseTo(100)
})
})
describe('gcd', () => {
it('should compute greatest common divisor correctly', () => {
expect(gcd(48, 18)).toBe(6)

View File

@@ -1,4 +1,23 @@
import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
/**
* Linearly maps a value from [min, max] to [0, 1].
* Returns 0 when min equals max to avoid division by zero.
*/
export function normalize(value: number, min: number, max: number): number {
return max === min ? 0 : (value - min) / (max - min)
}
/**
* Linearly maps a normalized value from [0, 1] back to [min, max].
*/
export function denormalize(
normalized: number,
min: number,
max: number
): number {
return min + normalized * (max - min)
}
import type { Bounds } from '@/renderer/core/layout/types'
/** Simple 2D point or size as [x, y] or [width, height] */