impr: Clean up vectorOps - #2848
Conversation
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.97, 1.88, 4.24, 7.00, 7.46, 12.57, 23.64, 23.44]
line [0.99, 1.89, 4.61, 6.57, 7.27, 10.57, 23.44, 23.40]
line [0.96, 1.94, 3.88, 6.03, 7.42, 12.95, 23.88, 25.90]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.28, 0.49, 0.68, 0.77, 1.05, 1.11, 1.34, 1.52]
line [0.29, 0.51, 0.70, 0.82, 1.08, 1.16, 1.37, 1.55]
line [0.34, 0.54, 0.69, 0.81, 1.11, 1.19, 1.43, 1.54]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.87, 2.24, 3.82, 7.45, 12.34, 26.53, 55.40, 111.60]
line [0.86, 2.22, 4.00, 7.09, 13.09, 26.79, 53.88, 110.27]
line [0.79, 2.20, 4.29, 7.34, 12.83, 26.92, 55.66, 111.11]
|
|
pkg.pr.new packages benchmark commit |
Bundle size comparison (
|
| 🟢 Decreased (max -29.83%) | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 115 | 209 | 0 | 0 |
import * as ... in PR vs import * as ... in target (did bundle size increase?):
Click to reveal the results table (115 entries).
| Test | tsdown |
|---|---|
| STATIC_allImports.ts | 296.17 kB ( |
| tgpu_resolveWithContext.ts | 270.50 kB ( |
| tgpu_bindGroupLayout.ts | 270.50 kB ( |
| tgpu_mutableAccessor.ts | 270.50 kB ( |
| tgpu_initFromDevice.ts | 270.50 kB ( |
| tgpu_vertexLayout.ts | 270.50 kB ( |
| tgpu_workgroupVar.ts | 270.50 kB ( |
| tgpu_fragmentFn.ts | 270.50 kB ( |
| tgpu_privateVar.ts | 270.50 kB ( |
| tgpu_computeFn.ts | 270.50 kB ( |
| tgpu_accessor.ts | 270.49 kB ( |
| tgpu_comptime.ts | 270.49 kB ( |
| tgpu_vertexFn.ts | 270.49 kB ( |
| tgpu_resolve.ts | 270.49 kB ( |
| tgpu_unroll.ts | 270.49 kB ( |
| tgpu_const.ts | 270.49 kB ( |
| tgpu_init.ts | 270.49 kB ( |
| tgpu_lazy.ts | 270.49 kB ( |
| tgpu_slot.ts | 270.49 kB ( |
| tgpu_fn.ts | 270.49 kB ( |
| STATIC_tgpu.ts | 270.49 kB ( |
| STATIC_std.ts | 96.96 kB ( |
| STATIC_d.ts | 77.36 kB ( |
| std_abs.ts | 55.86 kB ( |
| std_acos.ts | 55.86 kB ( |
| std_acosh.ts | 55.86 kB ( |
| std_asin.ts | 55.86 kB ( |
| std_asinh.ts | 55.86 kB ( |
| std_atan.ts | 55.86 kB ( |
| std_atan2.ts | 55.86 kB ( |
| std_atanh.ts | 55.86 kB ( |
| std_ceil.ts | 55.86 kB ( |
| std_clamp.ts | 55.86 kB ( |
| std_cos.ts | 55.86 kB ( |
| std_cosh.ts | 55.86 kB ( |
| std_countLeadingZeros.ts | 55.86 kB ( |
| std_countOneBits.ts | 55.86 kB ( |
| std_countTrailingZeros.ts | 55.86 kB ( |
| std_cross.ts | 55.86 kB ( |
| std_degrees.ts | 55.86 kB ( |
| std_determinant.ts | 55.86 kB ( |
| std_dot4I8Packed.ts | 55.86 kB ( |
| std_exp.ts | 55.86 kB ( |
| std_exp2.ts | 55.86 kB ( |
| std_extractBits.ts | 55.86 kB ( |
| std_faceForward.ts | 55.86 kB ( |
| std_firstLeadingBit.ts | 55.86 kB ( |
| std_firstTrailingBit.ts | 55.86 kB ( |
| std_floor.ts | 55.86 kB ( |
| std_fma.ts | 55.86 kB ( |
| std_insertBits.ts | 55.86 kB ( |
| std_inverseSqrt.ts | 55.86 kB ( |
| std_ldexp.ts | 55.86 kB ( |
| std_log.ts | 55.86 kB ( |
| std_log2.ts | 55.86 kB ( |
| std_max.ts | 55.86 kB ( |
| std_min.ts | 55.86 kB ( |
| std_normalize.ts | 55.86 kB ( |
| std_pow.ts | 55.86 kB ( |
| std_quantizeToF16.ts | 55.86 kB ( |
| std_radians.ts | 55.86 kB ( |
| std_reflect.ts | 55.86 kB ( |
| std_refract.ts | 55.86 kB ( |
| std_reverseBits.ts | 55.86 kB ( |
| std_round.ts | 55.86 kB ( |
| std_saturate.ts | 55.86 kB ( |
| std_sign.ts | 55.86 kB ( |
| std_sin.ts | 55.86 kB ( |
| std_sinh.ts | 55.86 kB ( |
| std_smoothstep.ts | 55.86 kB ( |
| std_sqrt.ts | 55.86 kB ( |
| std_step.ts | 55.86 kB ( |
| std_tan.ts | 55.86 kB ( |
| std_tanh.ts | 55.86 kB ( |
| std_transpose.ts | 55.86 kB ( |
| std_trunc.ts | 55.86 kB ( |
| std_distance.ts | 55.86 kB ( |
| std_dot4U8Packed.ts | 55.86 kB ( |
| std_fract.ts | 55.86 kB ( |
| std_frexp.ts | 55.86 kB ( |
| std_mix.ts | 55.86 kB ( |
| std_modf.ts | 55.86 kB ( |
| std_dot.ts | 55.85 kB ( |
| std_length.ts | 55.85 kB ( |
| std_rotateY4.ts | 38.98 kB ( |
| std_rotateZ4.ts | 38.98 kB ( |
| std_rotateX4.ts | 38.97 kB ( |
| std_scale4.ts | 38.97 kB ( |
| std_translate4.ts | 38.97 kB ( |
| std_add.ts | 38.14 kB ( |
| std_bitShiftLeft.ts | 38.14 kB ( |
| std_bitShiftRight.ts | 38.14 kB ( |
| std_div.ts | 38.14 kB ( |
| std_mod.ts | 38.14 kB ( |
| std_mul.ts | 38.14 kB ( |
| std_sub.ts | 38.14 kB ( |
| std_neg.ts | 38.14 kB ( |
| std_bitcast.ts | 35.90 kB ( |
| std_bitcastU32toF32.ts | 35.89 kB ( |
| std_bitcastU32toI32.ts | 35.89 kB ( |
| std_bitcastF32toU32.ts | 35.89 kB ( |
| std_ge.ts | 37.84 kB ( |
| std_gt.ts | 37.84 kB ( |
| std_isCloseTo.ts | 37.84 kB ( |
| std_le.ts | 37.84 kB ( |
| std_allEq.ts | 37.84 kB ( |
| std_eq.ts | 37.84 kB ( |
| std_lt.ts | 37.84 kB ( |
| std_ne.ts | 37.84 kB ( |
| std_not.ts | 37.84 kB ( |
| std_select.ts | 37.84 kB ( |
| std_and.ts | 37.83 kB ( |
| std_or.ts | 37.83 kB ( |
| std_all.ts | 37.83 kB ( |
| std_any.ts | 37.84 kB ( |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 260.96 kB ( |
| tgpu_initFromDevice.ts | 260.42 kB ( |
| tgpu_resolve.ts | 161.30 kB ( |
| tgpu_resolveWithContext.ts | 161.23 kB ( |
| tgpu_bindGroupLayout.ts | 62.30 kB ( |
| tgpu_mutableAccessor.ts | 57.02 kB ( |
| tgpu_accessor.ts | 57.02 kB ( |
| tgpu_privateVar.ts | 55.71 kB ( |
| tgpu_workgroupVar.ts | 55.71 kB ( |
| tgpu_const.ts | 55.13 kB ( |
| tgpu_lazy.ts | 54.93 kB ( |
| tgpu_fragmentFn.ts | 39.65 kB ( |
| tgpu_fn.ts | 39.60 kB ( |
| tgpu_vertexFn.ts | 39.47 kB ( |
| tgpu_computeFn.ts | 39.17 kB ( |
| tgpu_vertexLayout.ts | 28.30 kB ( |
| tgpu_comptime.ts | 15.91 kB ( |
| tgpu_unroll.ts | 1.75 kB ( |
| tgpu_slot.ts | 1.70 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
| } | ||
| throw new Error('Div called with invalid arguments.'); | ||
| verifyKind([lhs, rhs], numericKind); | ||
| return generalizeFn((a, b) => a / b, upCast([lhs, rhs])); |
There was a problem hiding this comment.
Previously integer vectors used a separate function for integer division. This works because vector constructors cast their elements.
|
@pullfrog review |
|
Run failed. View the logs →
|
There was a problem hiding this comment.
Important
This cleanup introduces two CPU regressions in the refactored paths: the deprecated vector bitcasts now return the wrong type and corrupted values, and isCloseTo crashes on mixed scalar/vector operands where the codegen path returns false. Both are pinned inline with confirmation from running the CPU implementations directly.
Reviewed changes — large cleanup that removes most hand-written per-kind overloads in data/vectorOps.ts in favor of a new data/generalizeFn.ts helper (generalizeFn / generalizeBoolFn / upCast), which applies an n-ary scalar function component-wise over vectors/matrices and rebuilds the result via a kind→constructor map:
- New
generalizeFn.tswith theapplyArgsengine,kind/constructor maps,upCast, and the overloadedgeneralizeFn/generalizeBoolFnexports. std/numeric.tscpu*helpers (abs, acos, atan2, clamp, exp2, fract, log, max, min, mix, pow, sign, smoothstep, sqrt, trig, …) rewritten throughgeneralizeFn.std/operators.tsadd/mul/div/mod/negrewritten throughgeneralizeFn(div/mod viaupCast).std/boolean.tseq/lt/or/isCloseTo/selectrewritten viageneralizeBoolFn/generalizeFn.- Deprecated
bitcastU32toF32/bitcastU32toI32/bitcastF32toU32rewritten viageneralizeFn. VectorOpsreduced to the non-generalizable set (all,length,dot,cross, matrix multiply, bit shifts);clamprenamed toclampScalarinnumberOps.ts; tests updated for div-by-zero and distance.
ℹ️ Behavior change worth confirming
std.div (and std.mod) on integer vectors previously returned the operand on division by zero (divInteger returned lhs when rhs === 0); the generalized version now throws a Finite-Math-Assumption error via the u32/i32 cast. The rewritten test (forbids division by 0) intentionally encodes this, so it appears deliberate, but it is a silent behavioral change to the public CPU path. Worth confirming nothing downstream relies on the old lenient result.
ℹ️ Nitpicks
data/numberOps.tsstill exportsdivInteger, which is now referenced nowhere after this refactor — it should be removed with the other deleted helpers (bitcast*impls were removed in the same file).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Important
The isCloseTo crash fix (upcasting via upCast) resolves the CPU crash from the prior review but leaves a CPU/GPU divergence on mixed scalar/vector operands that the new test now locks in: the CPU computes a real component-wise result while the GPU codegen still returns the literal false.
Reviewed changes — incremental delta since the prior pullfrog review (9b5e1eb → cf3e123):
- Reverted the deprecated
bitcastU32toF32/bitcastU32toI32/bitcastF32toU32back to their originalVectorOps.*per-kind CPU impls, fixing the wrong-type/corrupted-value regression. - Removed the now-unreferenced
divIntegerhelper fromdata/numberOps.ts(also addresses the prior nitpick). - Fixed the
isCloseTomixed scalar/vector CPU crash by upcasting operands viaupCastbeforegeneralizeBoolFn, and expandedtests/std/boolean/isCloseTo.test.tswith close/distant/precision/mixed cases.
⚠️ CPU/GPU divergence on mixed isCloseTo
The new works for mixed types test asserts CPU results for mixed scalar/vector operands that a compiled shader cannot reproduce — the GPU codegen returns false for every mixed case while the CPU now computes the real comparison. Details inline.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| expect(isCloseTo(vec2h(0, 0), vec2h(0, 9), 10)).toBe(true); | ||
| expect(isCloseTo(vec2h(0, 0), vec2h(0, 11), 10)).toBe(false); | ||
| it('works for mixed types', () => { | ||
| expect(isCloseTo(d.vec2f(0, 0) as d.v2f | number, 0)).toBe(true); |
There was a problem hiding this comment.
The works for mixed types block asserts CPU results for mixed scalar/vector operands — isCloseTo(d.vec2f(0, 0), 0) → true — but the GPU codegen in std/boolean.ts still returns the literal false for mixed operands (the else branch when exactly one side is a scalar). A compiled shader evaluating this therefore yields false where the CPU returns true: a silent CPU/GPU divergence that the crash-fix did not resolve.
Technical details
# Mixed scalar/vector isCloseTo diverges CPU vs GPU
## Affected sites
- packages/typegpu/tests/std/boolean/isCloseTo.test.ts:59-62 — new test asserts CPU-side mixed results (e.g. `isCloseTo(d.vec2f(0,0), 0)` -> `true`)
- packages/typegpu/src/std/boolean.ts:338-348 — codegen `else` branch returns literal `'false'` whenever exactly one operand is a scalar snippet
## Required outcome
CPU and GPU must agree on mixed scalar/vector `isCloseTo`.
## Suggested approach
Either (a) make the GPU codegen broadcast the scalar to a matching vec and emit the real comparison (WGSL mixed scalar/vector arithmetic is valid — the codegen's own comment cites the spec), or (b) scope mixed operands out and have the CPU path (and this test) return `false` to match the codegen. Option (a) matches the upcast semantics now implemented on the CPU.
## Open questions for the human
Is mixed scalar/vector `isCloseTo` an intended, supported semantic (fix the codegen), or out of scope (the CPU should match the GPU's `false`)?
No description provided.