-
-
Notifications
You must be signed in to change notification settings - Fork 36
feat: tgpu.comptime, tgpu.rawCodeSnippet and this allowed in TypeGPU shader functions
#1917
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
|
pkg.pr.new packages benchmark commit |
this allowed in TypeGPUthis allowed in TypeGPU shader functions
aleksanderkatan
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.
Great changes! 💻🕑
Please add one sentence about each comptime and rawCodeSnippet to the docs (maybe utils.mdx?) so that people know they exist
| type: TDataType, | ||
| origin: RawCodeSnippetOrigin | undefined = 'runtime', | ||
| ): TgpuRawCodeSnippet<TDataType> { | ||
| return new TgpuRawCodeSnippetImpl(expression, type, 'runtime'); |
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.
| return new TgpuRawCodeSnippetImpl(expression, type, 'runtime'); | |
| return new TgpuRawCodeSnippetImpl(expression, type, origin); |
| ast: ${embedJSON({ params, body, externalNames })}, | ||
| get externals() { return {${externalNames.join(', ')}}; }, | ||
| externals: () => ({${ | ||
| externalNames.map((e) => e === 'this' ? '"this": this' : e).join( |
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.
Can you add some tests for this?
| // Implementation | ||
| // -------------- | ||
|
|
||
| class TgpuRawCodeSnippetImpl<TDataType extends AnyData> |
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.
Please add some resolve tests for the raw code snippet ($uses, constant inlining etc.)
| get value(): InferGPU<TDataType> { | ||
| return this.$; | ||
| } |
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.
Do we still plan to deprecate and remove the value in favor of $? If so, maybe we can skip adding the value getter in new resources
| * }; | ||
| * ``` | ||
| */ | ||
| export function comptime<T extends (...args: never[]) => unknown>( |
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.
Please add tests for this...
|
|
||
| // get data generated by the plugin | ||
| const pluginData = getMetaData(implementation); | ||
|
|
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.
smol
| // Passing a record happens prior to version 0.9.0 | |
| // TODO: Support for this can be removed down the line |
| if (ctx.ignoreExternalDepth === 0) { | ||
| ctx.externalNames.add('this'); | ||
| } | ||
|
|
||
| return 'this'; |
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'm not sure what exactly happens here, and whether this is relevant, but this is a reserved word and cannot be used to name variables, struct props, structs or functions
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.
That's true! Could be simplified to return just "this"
Changes:
externalsis now an arrow function instead of a getter, so that it can capture thethisvalue from the outer scope.thisis treated like any other identifier, except for when generating the metadata object in unplugin. We cannot use shorthand syntax forthistgpu['~unstable'].comptimeis a way to create functions that are callable from within shader functions, but only allowed to execute at compile-time. They accept only compile-time known arguments, and throw otherwise.tgpu['~unstable'].rawCodeSnippetlets a JS-implemented function refer to a WGSL expression that TypeGPU doesn't have control over (like in the case of our Three.js integration)