feat(@typegpu/three): t3.attribute as a shorthand for t3.fromTSL(TSL.attribute('...', 'type'), d.type) - #2853
Conversation
…attribute('...', 'type'), d.type)
|
pkg.pr.new packages benchmark commit |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a typed t3.attribute helper to simplify creating Three.js TSL attribute nodes from TypeGPU schemas, and updates docs/examples to use it.
Changes:
- Introduced
attribute()helper in@typegpu/threeand exported it from the package entrypoint. - Added a Vitest test to verify WGSL schema → TSL node type mapping.
- Updated docs and example apps to use
t3.attributeinstead of manualfromTSL(TSL.attribute(...), ...).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/typegpu-three/tests/attribute.test.ts | Adds parameterized tests validating schema → TSL nodeType mapping and attribute name wiring. |
| packages/typegpu-three/src/attribute.ts | Implements the new attribute() helper around three/tsl + fromTSL. |
| packages/typegpu-three/src/index.ts | Exports attribute from the package barrel. |
| packages/typegpu-three/src/uniform.ts | Aligns d import style with other code (import { d } from 'typegpu'). |
| apps/typegpu-docs/src/examples/threejs/simple/index.ts | Migrates example to t3.attribute. |
| apps/typegpu-docs/src/examples/threejs/compute-cloth/index.ts | Migrates example to t3.attribute. |
| apps/typegpu-docs/src/content/docs/ecosystem/typegpu-three/index.mdx | Documents t3.attribute and clarifies equivalence with manual fromTSL(TSL.attribute(...)). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| dataType: TDataType, | ||
| ): TSLAccessor<TDataType, AttributeNode> { | ||
| const glslType = wgslTypeToGlslType[dataType.type as keyof typeof wgslTypeToGlslType]; | ||
|
|
| * Shorthand for `t3.fromTSL(attribute(...), ...)`. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const position = t3.attribute('position', d.vec3f); | ||
| * // Equivalent to: | ||
| * // const position = t3.fromTSL( | ||
| * // attribute('position', 'vec3'), |
| [d.f32, 'float'], | ||
| [d.u32, 'uint'], | ||
| [d.i32, 'int'], | ||
| [d.vec2f, 'vec2'], | ||
| [d.vec3u, 'uvec3'], | ||
| [d.vec4i, 'ivec4'], | ||
| ] as const)('maps the %s schema to the %s TSL type', (schema, tslType) => { | ||
| const accessor = attribute('foo', schema); | ||
|
|
||
| expect(accessor.node.nodeType).toBe(tslType); | ||
| expect(accessor.node.getAttributeName({} as NodeBuilder)).toBe('foo'); | ||
| }); |
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased | ❔ Unknown |
|---|---|---|---|
| 0 | 324 | 0 | 0 |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 274.51 kB ( |
| tgpu_initFromDevice.ts | 273.97 kB ( |
| tgpu_resolve.ts | 174.80 kB ( |
| tgpu_resolveWithContext.ts | 174.74 kB ( |
| tgpu_bindGroupLayout.ts | 74.47 kB ( |
| tgpu_mutableAccessor.ts | 69.19 kB ( |
| tgpu_accessor.ts | 69.19 kB ( |
| tgpu_privateVar.ts | 67.88 kB ( |
| tgpu_workgroupVar.ts | 67.88 kB ( |
| tgpu_const.ts | 67.30 kB ( |
| tgpu_lazy.ts | 67.10 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.
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.00, 2.15, 4.36, 7.32, 8.63, 15.51, 25.54, 28.35]
line [1.04, 2.10, 4.41, 6.70, 7.94, 12.74, 25.80, 27.33]
line [1.04, 2.14, 4.67, 7.26, 9.04, 11.89, 26.48, 27.79]
---
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.30, 0.54, 0.71, 0.89, 1.23, 1.23, 1.51, 1.67]
line [0.37, 0.60, 0.72, 0.86, 1.19, 1.28, 1.51, 1.68]
line [0.36, 0.57, 0.78, 0.89, 1.25, 1.34, 1.61, 1.73]
---
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.15, 2.61, 4.25, 8.33, 14.14, 28.07, 58.99, 122.59]
line [0.96, 2.38, 4.19, 7.71, 13.03, 27.32, 58.59, 120.42]
line [0.94, 2.48, 4.53, 7.79, 14.42, 29.08, 61.09, 124.22]
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The new t3.attribute API can silently build a float-typed attribute node (plus a startup console warning) for TypeGPU schemas that aren't in wgslTypeToGlslType.
Reviewed changes
attribute.ts— newt3.attribute(name, dataType)shorthand that maps a TypeGPU schema to a GLSL type and wraps it viafromTSL(TSL.attribute(name, glslType), dataType); exported fromindex.ts.uniform.ts— normalizes thedimport to thetypegpuroot (matchinginstancedArray.ts); no behavior change.- Examples
threejs/simpleandthreejs/compute-cloth— migrated attribute accessors fromt3.fromTSL(TSL.attribute(...), ...)tot3.attribute(...);TSLimport dropped where unused. - Docs — new
t3.attributeandt3.uniformsections on the@typegpu/threepage, withtwoslashsnippets. - Tests — new parameterized
attribute.test.tsasserting GLSL-type mapping and attribute name for 6 scalar/vector schemas.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| name: string, | ||
| dataType: TDataType, | ||
| ): TSLAccessor<TDataType, AttributeNode> { | ||
| const glslType = wgslTypeToGlslType[dataType.type as keyof typeof wgslTypeToGlslType]; |
There was a problem hiding this comment.
wgslTypeToGlslType has no entries for bool, f16/half-precision vectors, vec*b, or matrices, so passing any such schema yields glslType === undefined. TSL.attribute(name, undefined) then keeps a struct with no declared type, and fromTSL resolves it to 'float' against its geometry-less sharedBuilder — verified at runtime: t3.attribute('x', d.bool), d.f16, d.vec3h, and d.mat4x4f each emit Suspected type mismatch between TSL type 'f32' ... and TypeGPU type ... at create-time and produce a float-typed node. For a real half-float attribute this is a silently wrong vertex attribute, not an error. This also undercuts the docs claim that the TSL node type is "inferred from the TypeGPU schema" (index.mdx:127-128).
| name: string, | ||
| dataType: TDataType, | ||
| ): TSLAccessor<TDataType, AttributeNode> { | ||
| const glslType = wgslTypeToGlslType[dataType.type as keyof typeof wgslTypeToGlslType]; |
There was a problem hiding this comment.
Suggested fix: restrict TDataType to the mapped type union (d.F32 | d.U32 | d.I32 | d.Vec2f | d.Vec2u | d.Vec2i | d.Vec3f | d.Vec3u | d.Vec3i | d.Vec4f | d.Vec4u | d.Vec4i), so unmappable schemas become a compile-time error instead of a runtime warning plus a mis-typed node. If broader support is intended, handle the undefined case explicitly (as uniform does) and document which schemas are supported; add a test pinning the rejected cases either way.
No description provided.