-
Notifications
You must be signed in to change notification settings - Fork 353
Fix mistakes in examples #1333
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
Fix mistakes in examples #1333
Conversation
kvark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one note, otherwise looks great!
We'll need to make it so CI checks these examples to load as correct WGSL modules.
1786298 to
3ab5a4b
Compare
And annotate examples that are expected to not compile with `expect-error`
3ab5a4b to
662fdc3
Compare
There's a bit of a chicken-and-egg problem with doing this. Whatever the reference compiler is, it'll need to be updated before the spec in order for the tests to pass. FWIW, the tool I'm writing for this can easily be adapted to check against your compiler. If you're interested in this, please shout. |
|
|
||
| <div class='example wgsl global-scope' heading='Entry Point'> | ||
| <xmp> | ||
| [[builtin(position)]] var<out> gl_Position : vec4<f32>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably stop calling these gl_xxx :)
@kvark - if you are interested, the tool has been committed and lives here
Anyway, give me a shout if you want any changes. I'm happy to make this more generic and generally useful. |
|
@ben-clayton nice! Can this tool live separately from Tint and be prepared to work with an abstract implementation instead of Tint specifically? I'd love to add support for |
Sure. Any suggestions of a better home? Given that it's a just single file, what about this repo?
The two things listed above are the only tint specific things - it simply shells the executable you specify with temporary wgsl files extracted from the spec. If you can let me know how you invoke the naga compiler, and what sort of error you get when attempting to compile a shader with no entry point, I can easily adjust it to work with both compilers. |
|
Imagine a project that is a WebGPU-based engine. They may want to accept WGSL and provide examples of it in their documentation. They may want to use your script to check them. We wouldn't want to require them to depend on the whole WebGPU repo, unless you expect them to just copy the script over and re-vendor it in their own repo?
There is nothing in WGSL that requires a shader module to have entry points. cd naga
cargo build --example convert --features wgsl-in
target/debug/examples/convert <file.wgsl> |
I think this is well beyond the scope of the tool I've written. The bulk of the code is to fetch the spec, parse the HTML as emitted by Bikeshed along with the bespoke class tags I've invented. If you want a generic tool, you'd likely gut most of that, leaving nothing more than something that generates a temporary file, invokes an executable N times, check result script. That can be accomplished in less than 100 lines of most scripting languages. I think attempting to support this use case will end up with far more code to support the use case than just writing the tool from scratch.
The validation rules appear to disagree - see: That said, if naga doesn't have any issues with compiling a shader with no entry point, then the workaround will not need to be used, so no changes will likely to be needed here.
Same with tint. We build an 'example' binary called
Awesome, exactly the same args as the tint exe. No changes required here. Sounds like the tool might work for you without any modifications. |
|
I don't understand why WGSL needs this validation rule, honestly. It's not helping anybody. |
* Move test device selection to an optional before clause The beforeAllSubcases function is an optional function which will be run once for each testcase, but not for subcases inside that testcase. This allows some common code to be performed ahead of time and shared between subcases. This is immediately useful so that device selection out of the device pool can be performed once and then shared for all of the subcases. Subcases should not use different devices and will be refactored to select the device in the beforeAllSubcases function, without relying on subparams. The before function operates on a SubcaseBatchState which each Fixture class may override. This state object is shared between all fixtures created for subcases within a single testcase. The test device is stored in this shared state so that now, reserving a device from the device pool occurs once for all subcases, it is shared between the subcases, and released at the end of the testcase. This means that if there are unexpected validation, OOM, or device lost errors, they will only be reported at testcase granularity, not subcase granularity.
And annotate examples that are expected to not compile with
expect-error