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

Test textureSample WGSL validation #3478

Merged
merged 4 commits into from
Mar 13, 2024
Merged

Conversation

greggman
Copy link
Contributor

Looking for feedback / guidance since this is my first WGSL test - I'm not sure every case that should be tested.

textureSample takes a bunch of formats. I tested that the wrong type can't be passed to each parameter except for the texture (not sure how you'd pass an incorrect texture type since the function is determined from the texture type. I suppose I could test passing an external texture ? Similarly I didn't test not passing a sampler since there is only one type of sampler.

I wasn't exactly sure how to tell if types are compatible so I hacked in isCompatible. I wanted the test to at least have some success cases because if it only expects failure than any typo will cause it appear to be correctly failing all tests.

I also wasn't sure what to check for const-expression. The spec says the offset parameter must be a const-experssion so I made 3 variables c (a const) u an uniform, l a let expression based off something non-uniform. It works but I'm not sure if there is some more official way or if there are more types I should be testing. I was surprised a let expressions like let a = 1 is not considered const even though AFAICT it is actually const. I'm guessing that's in the spec but it seems unintuitive.

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

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

Made some suggestions for how you can use the Type / Value utilities to remove the string comparisons.

@ben-clayton
Copy link
Contributor

I was surprised a let expressions like let a = 1 is not considered const even though AFAICT it is actually const. I'm guessing that's in the spec but it seems unintuitive.

You want const a = 1 for that to be a const expression. A let and var usage is a runtime expression.

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

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

Great job. One nit, but feel free to ignore.

@greggman greggman merged commit 0771e27 into gpuweb:main Mar 13, 2024
1 check passed
@greggman greggman deleted the textureSample branch March 13, 2024 00:50
ben-clayton pushed a commit to ben-clayton/cts that referenced this pull request Mar 18, 2024
* Test `textureSample` WGSL validation
ben-clayton pushed a commit to ben-clayton/cts that referenced this pull request Mar 18, 2024
* Test `textureSample` WGSL validation
ben-clayton pushed a commit to ben-clayton/cts that referenced this pull request Mar 19, 2024
* Test `textureSample` WGSL validation
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.

2 participants