-
-
Notifications
You must be signed in to change notification settings - Fork 48
feat: Add .rgba swizzles to vectors with validation #2133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit adds support for .rgba swizzles on all vector types (vec2, vec3, vec4), doubling the available swizzle properties while ensuring component variant mixing is properly prevented. ## Changes Made: ### vectorImpl.ts - Added all .rgba swizzle getters (length 2, 3, and 4) to VecBase class - Added individual .r, .g, .b, .a property getters and setters to Vec2, Vec3, Vec4 classes - Maintains same pattern as existing .xyzw swizzles (256 total rgba swizzle getters) ### accessProp.ts - Updated swizzle validation to detect and prevent mixing of xyzw and rgba components - Added regex validation to ensure swizzles are either all xyzw OR all rgba - Invalid mixed swizzles (e.g., .xrgy, .rgxw) now return undefined ### Testing - Added comprehensive tests for rgba swizzles in JS code (vector.test.ts) - Added GPU function tests to verify rgba swizzles work in 'use gpu' blocks - Created dedicated test suite (swizzleMixedValidation.test.ts) to validate: - Pure xyzw swizzles work correctly - Pure rgba swizzles work correctly - Mixed swizzles are properly rejected - Edge cases like invalid characters and length limits ## Key Features: - Full rgba swizzle support matching xyzw functionality - Component access: .r, .g, .b, .a map to .x, .y, .z, .w - Works with all vector types: vec2f/i/u/h/b, vec3f/i/u/h/b, vec4f/i/u/h/b - Proper validation prevents mixing (e.g., .xrgy is invalid) - Compile-time swizzle resolution in GPU code - Runtime swizzle support in JS code Closes #[ISSUE_NUMBER]
reczkok
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for the contribution :D I left some comments that should be addressed before I can approve this.
I also created #2141 to bring our existing swizzle creator up to date with this. I think it was used to create the rgba swizzles but not committed (since the order of the swizzle implementation is the same as it generates - could be a coincidence though)?
Co-authored-by: Konrad Reczko <[email protected]>
- Replace 'as any' type casts with @ts-expect-error comments for explicit error handling - Remove successful test assertions from error validation tests (they belong in separate tests) - Delete PR_DESCRIPTION.md as requested This makes tests more strict about TypeScript errors and cleaner.
iwoplaza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Left a few comments to address
| it('should NOT create properties for mixed xyzw and rgba swizzles', () => { | ||
| const vec = d.vec4f(1, 2, 3, 4); | ||
| // These mixed swizzle properties should not exist | ||
| expect((vec as any).xrgy).toBeUndefined(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer // ts-expect-error comments instead
| return vec.xyz; | ||
| }); | ||
|
|
||
| expect(() => tgpu.resolve([main])).not.toThrow(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary to check if it doesn't throw, as the test will fail if it does
| expect(() => tgpu.resolve([main])).not.toThrow(); |
| return vec.rgb; | ||
| }); | ||
|
|
||
| expect(() => tgpu.resolve([main])).not.toThrow(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| expect(() => tgpu.resolve([main])).not.toThrow(); |
| const propLength = propName.length; | ||
| if (isVec(target.dataType) && propLength >= 1 && propLength <= 4) { | ||
| // Validate swizzle: must be all xyzw OR all rgba, not mixed | ||
| const hasXYZW = /[xyzw]/.test(propName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file you do 2 checks per swizzle type, one to check whether it has that swizzle type, one to check whether it is only that swizzle type. I propose you only check the latter, and use that in both checks. I believe it should still be valid.
Overview
This PR implements support for
.rgbaswizzles on all vector types in TypeGPU, effectively doubling the available swizzle properties while maintaining type safety and preventing component variant mixing.Problem Statement
Previously, vectors only supported
.xyzwswizzles. For graphics programming and color manipulation, having.rgbaswizzles is essential for code readability and maintainability, as they provide semantic meaning when working with color values.Solution
Added complete
.rgbaswizzle support mirroring the existing.xyzwfunctionality, with robust validation to prevent mixing component variants (e.g.,.xrgyis invalid).Changes Made
1. TypeScript Type Definitions (
packages/typegpu/src/data/wgslTypes.ts)SwizzleRGBA2,SwizzleRGBA3,SwizzleRGBA4interfaces mirroring xyzw swizzles.r,.g,.b,.acomponent properties2. Vector Implementation (
packages/typegpu/src/data/vectorImpl.ts)VecBaseclass.r,.g,.b,.a) with getters and setters3. Access Property Validation (
packages/typegpu/src/tgsl/accessProp.ts)undefinedfor invalid mixed swizzles (e.g.,.xrgy)4. Comprehensive Tests
vector.test.ts: 50+ new tests covering:
swizzleMixedValidation.test.ts: 14 tests validating:
Test Results
✅ All 144 tests passing
✅ Zero TypeScript compilation errors
✅ Works with all vector types
Usage Examples
JavaScript/TypeScript