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

"ValueError: Invalid shader code" due to additional whitespaces #20

Closed
Vipitis opened this issue Feb 11, 2024 · 0 comments · Fixed by #21
Closed

"ValueError: Invalid shader code" due to additional whitespaces #20

Vipitis opened this issue Feb 11, 2024 · 0 comments · Fixed by #21
Labels
bug Something isn't working

Comments

@Vipitis
Copy link
Collaborator

Vipitis commented Feb 11, 2024

during some testing for #19 I run into ValueError: Invalid shader code. Now this does also show up in #15 but I checked most of those cases and they were indeed correctly invalid. These wouldn't even work on the website. But the checks in #15 use a filtered dataset to only try shadertoys that include a image pass, not anything else (like common, buffers).

But these cases are different, one example: https://www.shadertoy.com/view/3dGXDt
on the image tab, L223 has void mainImage (with more white spaces) but the shader_type method is explicitly looking for a string with a single white space to make sure it's glsl.

So the lazy fix would be to avoid the whole elif block and just default to glsl if it isn't wgsl. Shaders without the proper mainImage function will raise an error instead about missing entry point perhaps.

Other ideas would be some regex (which can catch comments, as the current implementation might) or AST parsing for a fitting function. But I feel like the whole wgpu pipeline will do most of that for us already. And it's not really the place, since the whole task of that function is to decide between wgsl and glsl.

Will test around a bit and hopefully submit a PR later today.

@Vipitis Vipitis added the bug Something isn't working label Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant