Skip to content
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

TSL: Respect types in WGSL Layouts #28669

Merged
merged 5 commits into from
Jun 17, 2024

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jun 16, 2024

Maybe Related: #28659 #28577

Description
The regex function in the parsing method of WGSLNodeFunction was not taking in account the <abc> part resulting in not respecting the type properties. For example the type of vec2<ivec2> would be converted to vec2 instead of ivec2.
This PR fixes the issue.

This contribution is funded by Utsubo

@RenaudRohlinger RenaudRohlinger added this to the r166 milestone Jun 16, 2024
@sunag
Copy link
Collaborator

sunag commented Jun 16, 2024

type of vec2 would be converted to vec2 instead of ivec2

Did you mean: vec2<i32> would be converted to ivec2 instead of vec2?

The PR broke the webgpu_tsl_interoperability, I think it is related to texture_2d<f32>

@RenaudRohlinger
Copy link
Collaborator Author

It's probably another issue unveiled from this fix that was luckily working?

The issue only appears with wgslFn and works fine in TSL, it seems to generates a vec4 var in the example instead of simply consuming it as a texture:
image

@sunag
Copy link
Collaborator

sunag commented Jun 17, 2024

The issue only appears with wgslFn and works fine in TSL, it seems to generates a vec4 var in the example instead of simply consuming it as a texture:

This can happen if the type is texture_2d<f32> instead of texture_2d. These sub-type are ignored WGSLNodeFunction to optimize the checking in WGSLNodeBuilder.

@cmhhelgeson
Copy link
Contributor

The issue only appears with wgslFn and works fine in TSL, it seems to generates a vec4 var in the example instead of simply consuming it as a texture:

This can happen if the type is texture_2d<f32> instead of texture_2d. These sub-type are ignored WGSLNodeFunction to optimize the checking in WGSLNodeBuilder.

Not just in texture_2d, but pretty much any type that is not a fundamental data type like a vector, primitive type, or matrix type. Additionally, for some reason, when a texture_2d was added as an input rather than a texture_2d, an additional mat3x3 uniform was added to the wgsl objectStruct. I haven't investigated why however.

@RenaudRohlinger
Copy link
Collaborator Author

From what I understand and saw on the WebGPU specs a condition as simple as:

if ( resolvedType.startsWith( 'texture' ) ) {

resolvedType = type.split( '<' )[ 0 ];

}

Should be enough to support all non-fundamental data (mostly texture/texture_storage) in the node builder.
Consequently I counter reverted the #28577 and now all examples seems to work with proper types handled.

/cc @sunag @cmhhelgeson

@sunag
Copy link
Collaborator

sunag commented Jun 17, 2024

Merging... thanks!

@sunag sunag merged commit 903d50d into mrdoob:dev Jun 17, 2024
11 checks passed
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.

3 participants