Feat: std-smoothstep #1450
Feat: std-smoothstep #1450
Conversation
|
pkg.pr.new packages benchmark commit |
| const smoothstepScalar = (edge0: number, edge1: number, x: number): number => { | ||
| if (x <= edge0) return 0; | ||
| if (x >= edge1) return 1; | ||
| const t = clamp((x - edge0) / (edge1 - edge0), 0.0, 1.0); |
There was a problem hiding this comment.
I think that either this clamp is unnecessary, or the two ifs above it are
There was a problem hiding this comment.
Note that smoothstep is defined for edge0 > edge1. This means that the two ifs are actually incorrect.
| return t * t * (3 - 2 * t); | ||
| }; | ||
|
|
||
| type TernaryOp = (a: number, b: number, c: number) => number; |
There was a problem hiding this comment.
Could you move this entire section to be where UnaryOp and BinaryOp are?
| @@ -27,6 +28,49 @@ type v4 = wgsl.v4f | wgsl.v4h | wgsl.v4i | wgsl.v4u; | |||
|
|
|||
| type MatKind = 'mat2x2f' | 'mat3x3f' | 'mat4x4f'; | |||
|
|
|||
| const smoothstepScalar = (edge0: number, edge1: number, x: number): number => { | |||
There was a problem hiding this comment.
Can you move this to NumberOps?
There was a problem hiding this comment.
I didn't mean a new file named numberOps.ts, but the object NumberOps that already exists in vectorOps.ts. I guess it may be located in another file, but then please move the divInteger function to that file as well. You could also move clamp to the same place.
|
|
||
| it('works with vec2f', () => { | ||
| // all components less than edge0 | ||
| const result1 = smoothstep( |
There was a problem hiding this comment.
I think it is enough to include one of these in this test
| return t * t * (3 - 2 * t); | ||
| }; | ||
|
|
||
| export const clamp = (value: number, low: number, high: number) => |
There was a problem hiding this comment.
can this be reused from std?
There was a problem hiding this comment.
Using clamp from std would create a circular dependency numeric.ts -> vectorOps.ts -> numberOps.ts -> numeric.ts. I think that if we want to keep this file, we should move all existing JS definitions of such functions here (divInteger, clamp, smoothstep etc.), and then use them in vectorOps.ts and numeric.ts
| it('works with vec2f', () => { | ||
| // all components less than edge0 | ||
| const result1 = smoothstep( | ||
| vec2f(0.3, 0.5), | ||
| vec2f(0.8, 0.9), | ||
| vec2f(0.1, 0.2), | ||
| ); | ||
| expect(isCloseTo(result1, vec2f(0, 0))).toBe(true); | ||
|
|
||
| // all components greater than edge1 | ||
| const result2 = smoothstep( | ||
| vec2f(0.3, 0.5), | ||
| vec2f(0.8, 0.9), | ||
| vec2f(0.9, 1.0), | ||
| ); | ||
| expect(isCloseTo(result2, vec2f(1, 1))).toBe(true); | ||
|
|
||
| // components between edge0 and edge1 | ||
| const result3 = smoothstep( | ||
| vec2f(0.3, 0.5), | ||
| vec2f(0.8, 0.9), | ||
| vec2f(0.55, 0.7), | ||
| ); | ||
| expect(isCloseTo(result3, vec2f(0.5, 0.5))).toBe(true); | ||
|
|
||
| // mixed results | ||
| const result4 = smoothstep( | ||
| vec2f(0.3, 0.5), | ||
| vec2f(0.8, 0.9), | ||
| vec2f(0.2, 0.95), | ||
| ); | ||
| expect(isCloseTo(result4, vec2f(0, 1))).toBe(true); | ||
| }); | ||
|
|
||
| it('works with vec3f', () => { | ||
| const result = smoothstep( | ||
| vec3f(0.1, 0.2, 0.3), | ||
| vec3f(0.6, 0.7, 0.8), | ||
| vec3f(0.35, 0.7, 0.55), | ||
| ); | ||
| expect(isCloseTo(result, vec3f(0.5, 1, 0.5))).toBe(true); | ||
| }); | ||
|
|
||
| it('works with vec4f', () => { | ||
| const result = smoothstep( | ||
| vec4f(0.0, 0.1, 0.2, 0.3), | ||
| vec4f(1.0, 0.9, 0.8, 0.7), | ||
| vec4f(0.0, 0.5, 0.8, 0.7), | ||
| ); | ||
| expect(isCloseTo(result, vec4f(0, 0.5, 1, 1))).toBe(true); | ||
| }); |
There was a problem hiding this comment.
We should test for more than the 3 most obvious cases
…' into feat/std-smoothstep
No description provided.