Repository navigation
feat: JS vector swizzle assignment - #3204
aleksanderkatan wants to merge 2 commits into
Conversation
|
pkg.pr.new packages benchmark commit |
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 [1.15, 2.21, 4.88, 7.34, 8.19, 13.31, 26.45, 27.88]
line [1.03, 2.08, 4.70, 7.94, 9.31, 12.62, 29.09, 27.36]
line [1.05, 2.06, 4.65, 6.89, 7.93, 13.42, 26.40, 26.00]
---
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.33, 0.55, 0.78, 0.90, 1.23, 1.30, 1.52, 1.74]
line [0.31, 0.57, 0.77, 0.88, 1.20, 1.26, 1.56, 1.68]
line [0.35, 0.55, 0.78, 0.92, 1.52, 1.23, 1.49, 1.70]
---
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 [1.03, 2.15, 4.25, 7.50, 15.25, 28.11, 61.30, 120.90]
line [0.94, 2.56, 4.37, 8.02, 14.12, 28.70, 62.77, 125.02]
line [0.95, 2.34, 4.76, 8.00, 15.54, 29.77, 60.83, 120.67]
|
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased (max 0.37%) | ❔ Unknown |
|---|---|---|---|
| 0 | 84 | 265 | 0 |
No notable changes.
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Initial review of the JS vector swizzle assignment change.
- Runtime swizzle setters —
VecBasenow installs asetaccessor for every permutation swizzle (2, 3, and 4 components, in bothxyzwandrgbaspellings); repeated-component swizzles keep getter-only descriptors, so assigning to them throws aTypeError. - Writable swizzle types — new
Perm2/Perm3/Perm4helpers plus writable index signatures intersected intoSwizzle2/Swizzle3/Swizzle4, exposing exactly the permutation swizzles as assignable while repeated ones stayreadonly. - Tests —
swizzleAssignment.test.tscovers full, partial, and self assignment, repeated-component rejection, and the WGSL swizzle-assignment evaluation-order example.
( ◦°^°◦) 🏴☠️ Avast! I checked the swizzle setters, ran the full typegpu suite (2902 tests passin'), typechecked the package, and verified the WGSL evaluation-order example — the ship be seaworthy.
ℹ️ Writable swizzle types also reach TGSL, where assignment is not implemented yet
The Swizzle2/3/4 interfaces are shared by the v2f/v3f/v4f types used inside 'use gpu' functions, so TGSL swizzle assignment (e.g. let v = d.vec4f(); v.xy = d.vec2f(1, 2);) now typechecks but still fails at tgpu.resolve time. This looks intentional — the stacked #3205 adds the WGSL codegen (src/tgsl/swizzle.ts) — so no change is needed here; flagging the merge-ordering dependency only.
Technical details
# TGSL swizzle assignment typechecks but is rejected by the generator
## Affected sites
- `packages/typegpu/src/data/wgslTypes.ts:185` — writable `Perm*` index signatures are inherited by the `'use gpu'` vector types.
- `packages/typegpu/src/tgsl/accessProp.ts:199` — existing comment notes a swizzle LHS assignment "is not yet supported in WGSL".
## Observed behavior
- `let v = d.vec4f(); v.xy = d.vec2f(1, 2);` inside a `'use gpu'` fn typechecks, then throws `'v.xy = d.vec2f(1, 2)' is invalid, because the left side is not a reference to an existing value.` during resolution.
## Required outcome
- None for this PR if the stack merges in order (#3204 → #3205).
## Open questions for the human
- Confirm #3204 is expected to land before #3205, accepting the brief type/runtime gap on `main`.deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
bc8aaae to
3c28952
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
There was a problem hiding this comment.
ℹ️ Minor suggestions only.
Reviewed changes
packages/typegpu/src/data/vectorImpl.ts— the static block that defines 2/3/4-length swizzle accessors now builds one descriptor per component tuple and appends asetwhen the tuple is a permutation. The setter destructures the RHS into locals before writing, so self-assignment (vec.wzyx = vec) and overlapping regions are handled correctly; repeat swizzles stay getter-only and therefore throw. Verified against the tests.packages/typegpu/src/data/wgslTypes.ts— addsPerm2/3/4<T>helpers and intersectsSwizzle2/3/4with mutable maps keyed by those permutations, so only permutation swizzles become writable while repeat swizzles stayreadonly. Confirmed withtsc --noEmit(valid perms compile, repeats/length mismatch/mixed aliases error). These interfaces are shared with'use gpu'code, where codegen support arrives in #3205 of the stack.packages/typegpu/tests/swizzleAssignment.test.ts— new JS tests covering all permutation counts in bothxyzw/rgbavariants, partial assignment, repeat rejection, self-assignment, and WGSL evaluation order (#example-95005bab).npx vitest run tests/swizzleAssignment.test.tspasses (6/6).
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
43cb142 to
340b4fe
Compare

Claude is a coauthor because I asked it to split my changes into a stack of 3 🤷♀️