Skip to content

fix: TypeGPU functions using TGSL dependencies declaration order#1522

Merged
aleksanderkatan merged 9 commits intomainfrom
fix/tgpu-functions-declarations-order
Jul 22, 2025
Merged

fix: TypeGPU functions using TGSL dependencies declaration order#1522
aleksanderkatan merged 9 commits intomainfrom
fix/tgpu-functions-declarations-order

Conversation

@aleksanderkatan
Copy link
Contributor

@aleksanderkatan aleksanderkatan commented Jul 21, 2025 •
edited
Loading

The change allows for a function to reference another function defined later on.
This could potentially lead to people attempting to write recursive functions.

TypeScript screams when either of the following happens (something something any):

const bar = tgpu.fn([], d.f32)(() => foo() + 2);
const foo = tgpu.fn([], d.f32)(() => bar() - 2);
const bar = () => foo() + 2;
const foo = () => bar() - 2;

However, the following is fine:

let bar: TgpuFn; 
let foo: TgpuFn;
bar = tgpu.fn([], d.f32)(() => foo() + 2);
foo = tgpu.fn([], d.f32)(() => bar() - 2);

and in this case we get a stack overflow:

Resolution of the following tree failed: 
- <root>
- fn:bar
- foo: Resolution of the following tree failed: 
- call:foo
- fn:foo
- bar: Resolution of the following tree failed: 
- call:bar
- fn:bar
...

Should we try to detect this situation? This would probably require resolve to hold a set of items it is currently trying to resolve, it seems pretty doable. On the other hand, this is already illegal in wgsl, and it is not too difficult to know what is going on when seeing an error like this.

@aleksanderkatan aleksanderkatan linked an issue Jul 21, 2025 that may be closed by this pull request
TypeGPU functions using TGSL have to be declared after all of their dependencies #1502
Closed
@github-actions
Copy link

github-actions bot commented Jul 21, 2025 •
edited
Loading

pkg.pr.new

packages
Ready to be installed by your favorite package manager ⬇️

https://pkg.pr.new/software-mansion/TypeGPU/typegpu@e1ad1ea26105b86e2622b5e19e9fabd62d728fe3

https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/noise@e1ad1ea26105b86e2622b5e19e9fabd62d728fe3

https://pkg.pr.new/software-mansion/TypeGPU/unplugin-typegpu@e1ad1ea26105b86e2622b5e19e9fabd62d728fe3

benchmark
view benchmark

commit
view commit

@aleksanderkatan aleksanderkatan marked this pull request as ready for review July 21, 2025 15:39
@iwoplaza
Copy link
Collaborator

iwoplaza commented Jul 21, 2025

Should we try to detect this situation? This would probably require resolve to hold a set of items it is currently trying to resolve, it seems pretty doable. On the other hand, this is already illegal in wgsl, and it is not too difficult to know what is going on when seeing an error like this.

People coming from a non-WGSL background, or people that don't yet know recursion is not permitted in WGSL could look at that error and think it's a problem with the library. I think we should throw a descriptive error in this case. I'll make an issue for this.

@iwoplaza
Copy link
Collaborator

iwoplaza commented Jul 21, 2025

Here's the issue: #1523

iwoplaza
iwoplaza approved these changes Jul 21, 2025
View reviewed changes
Copy link
Collaborator

@iwoplaza iwoplaza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please! 💜

reczkok
reczkok approved these changes Jul 22, 2025
View reviewed changes
Copy link
Contributor

@reczkok reczkok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 🏗️

@aleksanderkatan aleksanderkatan merged commit 4b4d804 into main Jul 22, 2025
6 checks passed
@aleksanderkatan aleksanderkatan deleted the fix/tgpu-functions-declarations-order branch August 5, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iwoplaza iwoplaza iwoplaza approved these changes

@reczkok reczkok reczkok approved these changes

@lursz lursz Awaiting requested review from lursz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeGPU functions using TGSL have to be declared after all of their dependencies

3 participants

@aleksanderkatan @iwoplaza @reczkok