Skip to content

feat: Match up vertex and fragment locations in render pipeline #1377

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

Merged
merged 30 commits into from
Jul 3, 2025

Conversation

mhawryluk
Copy link
Contributor

closes #885
closes #1375

Copy link

github-actions bot commented Jun 18, 2025

pkg.pr.new

packages

pnpm i https://pkg.pr.new/software-mansion/TypeGPU/typegpu@1377
pnpm i https://pkg.pr.new/software-mansion/TypeGPU/typegpu@5f2ccb44e1bd036b119536d1da803391251901de

benchmark
view benchmark

commit
view commit

@mhawryluk mhawryluk changed the title feat: Match-up vertex output and fragment location indices in render pipeline feat: Match up vertex and fragment location indices in render pipeline Jun 18, 2025
@mhawryluk mhawryluk marked this pull request as ready for review June 23, 2025 10:28
@mhawryluk mhawryluk changed the title feat: Match up vertex and fragment location indices in render pipeline feat: Match up vertex and fragment locations in render pipeline Jun 23, 2025
@mhawryluk mhawryluk requested a review from iwoplaza June 23, 2025 11:17
Copy link
Contributor

@aleksanderkatan aleksanderkatan left a comment

Choose a reason for hiding this comment

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

Nice!
We need to sort out the vertex in and fragment out locations before merging though

).toStrictEqual({
a: d.location(5, d.vec4f),
b: d.location(1, d.vec4f),
c: d.location(6, d.vec4f),
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably same for vertex in, however our types don't even allow decorated schemas.

baz2: d.location(5, d.f32),
baz3: d.u32,
},
})`{}`.$name('vertexMain');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we try to generate valid wgsl in these tests? If so, we should always include builtin position in the vertex out and return something in the body

Comment on lines 613 to 616
console.warn(
`Mismatched custom location for key: ${key}, using location set on vertex output: ${
locations[key]
}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd include the mismatched location as well.

`Mismatched location between vertexFn output (${locations[key]}) and fragmentFn input (${customLocation}) for key ${key}, using the location set on vertex output.`

Maybe we could also include the names of the vertexFn and fragmentFn?

Comment on lines 59 to 73
if (typeof implementation === 'string') {
if (!isEntry) {
addArgTypesToExternals(
implementation,
argTypes,
(externals) => externalsToApply.push(externals),
);
addReturnTypeToExternals(
implementation,
returnType,
(externals) => externalsToApply.push(externals),
);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We could probably do this check in tgpuFn.ts, if we already move the else branch to entry functions

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.

Great work!

@mhawryluk mhawryluk requested a review from iwoplaza June 27, 2025 13:31
@iwoplaza iwoplaza changed the base branch from main to release July 3, 2025 16:22
@iwoplaza iwoplaza changed the base branch from release to main July 3, 2025 16:22
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.

Y E A H

@iwoplaza iwoplaza merged commit f43b7f1 into main Jul 3, 2025
6 checks passed
@iwoplaza iwoplaza deleted the fix/match-vertex-fragment branch July 3, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow pipelines to be passed as externals to tgpu.resolve Match-up vertex output and fragment input location indices when generating WGSL
4 participants