Repository navigation
fix: Matrix writers - #3178
fix: Matrix writers#3178aleksanderkatan wants to merge 7 commits into
Conversation
|
pkg.pr.new packages benchmark commit |
Bundle size comparison (
|
| 🟢 Decreased (max -0.20%) | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 6 | 343 | 0 | 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.
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.98, 2.06, 4.64, 7.21, 7.70, 12.38, 24.64, 27.69]
line [1.05, 2.04, 4.18, 6.82, 7.67, 12.72, 25.98, 24.41]
line [0.98, 1.91, 4.04, 6.83, 9.06, 11.40, 25.58, 25.85]
---
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.31, 0.53, 0.74, 0.89, 1.13, 1.18, 1.45, 1.60]
line [0.35, 0.55, 0.73, 0.82, 1.11, 1.21, 1.47, 1.60]
line [0.30, 0.50, 0.68, 0.86, 1.13, 1.21, 1.44, 1.67]
---
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.00, 2.67, 4.53, 8.52, 13.33, 26.43, 55.80, 115.93]
line [0.91, 2.31, 3.62, 7.31, 12.84, 26.81, 57.34, 112.01]
line [0.90, 2.34, 4.00, 7.10, 12.59, 26.38, 55.27, 113.19]
|
|
@pullfrog review |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor consistency suggestion inline.
Reviewed changes
mat3x3ffallback padding — the fallback writer (dataIO.ts) now distinguishes already-padded inputs (Float32Array/ matrix instances) from compact JS arrays and, for the latter, inserts a per-column0so the on-wire layout matches the compiled writer's strided layout.- Write-side
InferInputtyping —DataWriter,writeData, andwriteToArrayBuffernow acceptInferInput<T>instead ofInfer<T>; writer methods drop their explicit parameter annotations, andwgslTypesis imported as a value (rather thanimport type) soisMatInstancecan run at runtime. - New
noEvaltest file —tests/internal/arrayBufferIO.noEval.test.tsmocks the compiled writer out and mirrors the publicarrayBufferIO.test.ts, addingmat3x3farray / matrix-instance / typed-array cases.
Verified locally: tests/internal + tests/arrayBufferIO.test.ts + tests/buffer.test.ts pass (558 tests), and pnpm --filter typegpu test:types is clean.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
( ◦°^°◦) 🏴☠️ Ahoy, matey! I read the whole diff o' the matrix writers end to end, ran the two test files (26 tests, all pass), typechecked the core package, an' sent a probe to be sure the no-eval mock truly forces the fallback writer. The compiled writer now zeroes mat3x3f column padding, an' the fallback writer pads packed JS arrays — the hull be sound. Previous findin' be addressed; ship it!
Reviewed changes
- Compiled writer zeroes
mat3x3fpadding —buildWriter's matrix branch now iteratesmatSize * wgslElementStride(12 format3x3f) and emitswriteFloat32(..., 0)for the three column-padding floats (row >= matSize), which were previously left untouched. Index math for non-padding elements is unchanged. This closes the earlier review finding (divergence between compiled and fallback writers on reused buffers). - Fallback writer handles JS arrays —
dataWriters.mat3x3fnow distinguishesFloat32Array/matrix instances (already padded, iterate as-is) from packed JS arrays (emit 3 floats per column plus a0padding float). dataIOtype cleanup —writeData/writeToArrayBuffervalue params narrowed toInferInput,wgslswitched to a runtime namespace import, and a cast dropped inwriteToArrayBuffer.- Test coverage for both paths —
arrayBufferIO.test.tsgains mat3x3f override/padding assertions (compiled path), and a newtests/internal/arrayBufferIO.noEval.test.tsmocksgetCompiledWriterto exercise the fallback writer across vec/array/struct/mat/typed-array inputs.
Verified: npx vitest run tests/arrayBufferIO.test.ts tests/internal/arrayBufferIO.noEval.test.ts → 26 passing; npx tsc --noEmit in packages/typegpu → clean; a probe confirmed the vi.mock('../../src/data/compiledIO.ts') genuinely intercepts getCompiledWriter, so the no-eval suite really covers the fallback. Since only f32 matrices exist, the hardcoded 4-byte element stride is safe, and wgslTypes.ts has no import back into dataIO, so the new runtime namespace import introduces no cycle.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Compiled writer didn't fill padding with zeroes, and fallback writer didn't handle JS arrays separately