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

WGSL const-declarations #2994

Closed
9SMTM6 opened this issue Aug 28, 2022 · 5 comments
Closed

WGSL const-declarations #2994

9SMTM6 opened this issue Aug 28, 2022 · 5 comments
Labels
type: question Further information is requested

Comments

@9SMTM6
Copy link
Contributor

9SMTM6 commented Aug 28, 2022

I'm sorry to raise this issue here, but I've been at trying to understand this for over an hour and am tired and in the coming week unlikely to be able to spend much time on this, and I've yet to understand the whole picture to where I am certain to adress this to the correct entities, so I'll have to let you finalize triage on this.

When trying to define constants according to (my understanding of) the current spec (version specific link for future reference) I get an error:

[2022-08-28T18:25:35Z ERROR wgpu::backend::direct] Handling wgpu errors as fatal by default
thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_shader_module

Shader '' parsing error: expected global item ('struct', 'let', 'var', 'type', ';', 'fn') or the end of the file, found 'const'
  ┌─ wgsl:5:1
  │
5 │ const WORKGROUP_BLOCK_SIZE = 6;
  │ ^^^^^ expected global item ('struct', 'let', 'var', 'type', ';', 'fn') or the end of the file

during that >1h that I mentioned I stumbled upon a few issues that were using const syntax, which really confused me, but once I stumbled upon this I think this cleared up for me somewhat. I'm just not sure and somewhat confused, that, if this is the case, noone else reported this elsewhere I could find: It appears that the official wgsl spec is out-of date?

While spending the time searching this, I stumbled upon a few other things I know I would wish to be there (this of course isn't a demand but just a statement of what I was looking for and could not find, and that I think would have been helpful):

  • an overview what of WGSL wgpu implements
    • there is this in the naga repo, but
      • I'm not entirely certain that you always use naga - that error for example doesnt refer to naga at all, if naga was in that pipeline it wasnt transparent to me, or rather it was transparent in the (ab)usage of that term that seems common in distributed computing
      • if the above is the case I would wish to find that list linked somewhere in wgpu documentation
      • i only stumbled upon this by accident
    • in any case, that list just includes raised issues, and will have a very detailed granularity, a bigger picture overview would be great

thats actually all I can remember now.

Well, in any case, I would appreciate if you could confirm above conjecture and, that being the case, ideally forward it so that it eventually gets adressed in a way visible in the spec, or if I missed something explain it to me.

I would be willing to open an issue regarding the "overview what of wgsl wgpu implements currently" point in the coming days, otherwise feel free to open one yourself or of course to reject this request.

@cwfitzgerald cwfitzgerald added the type: question Further information is requested label Aug 29, 2022
@cwfitzgerald
Copy link
Member

cwfitzgerald commented Aug 29, 2022

Thanks for filing, hopefully this will clear up your confusion.

I'm just not sure and somewhat confused, that, if this is the case, noone else reported this elsewhere I could find: It appears that the official wgsl spec is out-of date?

It appears to have been re-added after that. I believe this was a fairly recent addition, so we just don't support this yet (the naga issue is the right one).

an overview what of WGSL wgpu implements

In theory we're always targetting the latest spec, but it takes us time and effort to implement things, so we basically implement an arbitrary sub/superset of the spec. This isn't ideal, but that's the perils of targetting a moving specification.

I'm not entirely certain that you always use naga

wgpu on native will always use naga for all of it's shader related things (translation, validation, reflection). We don't specifically name naga in any of the errors as it's basically an internal implementation detail.

if the above is the case I would wish to find that list linked somewhere in wgpu documentation

PRs accepted :)

in any case, that list just includes raised issues, and will have a very detailed granularity, a bigger picture overview would be great

The problem is, especially with the spec still changing, it'll be a non-trivial amount of work to keep taht overview up to date, and at the end of the day, wgsl will stabilize and this all won't be a problem again.

@9SMTM6
Copy link
Contributor Author

9SMTM6 commented Aug 31, 2022

Thank you for taking the time, that clears things uo somewhat.

the naga issue is the right one

I didnt link to a naga issue I could find, but github tracked me mentioning this issue in the naga issue I would suspect to be the relevant one.

PRs accepted :)

I'll see if I can find one/ a few spots to out it in the documentation and create a PR until this evening:). Otherwise I'll probably get to this at the earliest on the weekend.

@9SMTM6
Copy link
Contributor Author

9SMTM6 commented Aug 31, 2022

2 questions came up:

  1. I dont have a lot of experience with github wikis, and the process seems to be project dependent. I would also like to add parts to the wiki where it fits, would I just edit the pages on here or what is the intended way to contribute to the wiki?

  2. I dont want to put in wrong information, but from what I've found its unclear: As far as I understand, if I target WASM then naga will not touch the shaders, correct? (Unless the browser uses Naga, like Firefox, of course). So in that case WGSL compliance is entirely up to the Browser, and it wont fail at eg. a naga validation failing something the browser might understand.

@9SMTM6
Copy link
Contributor Author

9SMTM6 commented Sep 3, 2022

I just edited the wiki, if that was not intended it would be a seperate issue.

Otherwise, #3009 updated documentation, I could not find a mistake other than the style one I brought up there and upend another PR to fix, but thats also unconnected to this, so this issue can be closed.

@9SMTM6 9SMTM6 closed this as completed Sep 3, 2022
@yutannihilation
Copy link
Contributor

I believe this was a fairly recent addition

Just for reference, it seems this was the change in April (recent-ish?).

gpuweb/gpuweb#2668

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants