Add get_compilation_info

[stefnotch]

Connections
Fix #2130

Description
This adds an async get_compilation_info method to the wgpu crate, which either uses the native compilation info as reported by naga, or the compilation info that the browser's WebGPU backend reported.

The general idea is to note down all the relevant info when the compilation happens, and later extract them when the user calls get_compilation_info

• WebGPU backend with WGSL code: We store the Javascript promise, and later access it. We also store the source code, so that we can remap the reported offsets.
• WebGPU backend with other code: We translate the code to WGSL, and store the compilation and validation errors. And then we store the Javascript promise, so that we can also report any errors that the underlying WGSL compiler reports. Here, we don't keep the source code around, cause it'd be nonsensical. (Offsets are pointing at auto-generated WGSL code)
• Native backend: We immediately compile it, and store the errors.

Relevant bits about API design are

• Using UTF-8 for positions. We're purposefully deviating from the GPUCompilationInfo specification, which uses UTF-16 for reporting positions.
• For any given error, we're only reporting the main error location. If naga reports multiple locations for a single error, then the additional locations will be in the message.
  • For tooling that really wants to get all the error locations: Please directly use naga

Also moved the ShaderError struct from wgpu_core to naga, so that it can be used by both

• wgpu_core
• webgpu with certain feature flags

Any suggestions on how to simplify the code, or improve it are very appreciated. There are a few bits that feel like I'm duplicating code, or feel suboptimal.

Testing

Basic unit tests have been added.

Checklist

• Run cargo fmt.
• Run cargo clippy. If applicable, add:
  • --target wasm32-unknown-unknown
  • --target wasm32-unknown-emscripten I cannot get this to run locally without a million compile errors. I clearly missed some setup part.
• Run cargo xtask test to run tests.
• Add change to CHANGELOG.md. See simple instructions inside file.

[Wumpf]

Looking great already! Left a few comments on things to improve / check upon.
Very nicely documented api on wgpu 👍

Regarding tests: Yes plz! :). You can put them under tests/test s, maybe even part of tests/tests/shader, haven't checked if that makes sense. Would be great to have simple test cases for success, warning and error.

Also, please add a changelog entry. This is a great addition that shouldn't be missed there :)

[stefnotch]

By the way, if I run

cargo check --target wasm32-unknown-emscripten

I get the following output. Am I doing something wrong?

    Checking spirv v0.3.0+sdk-1.3.268.0
    Checking bytemuck v1.15.0
    Checking winit v0.29.15
    Checking player v0.19.3 (D:\Coding\GitHub\Libraries\wgpu\player)
error: The platform you're compiling for is not supported by winit
  --> C:\Users\Stefnotch\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winit-0.29.15\src\platform_impl\mod.rs:67:1
   |
67 | compile_error!("The platform you're compiling for is not supported by winit");
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...

[Wumpf]

uh yeah emscripten target seems to be broken again (much worse for me on trunk actually), ignore that for now. wasm32-unknown-unknown still seems to work fine (it also gets a lot more testing on ci)

[stefnotch]

I added tests for this 🎉 They're very barebones, but should be decent enough to verify that this feature isn't completely broken.

Out of curiosity, do those tests need to be gpu tests? Couldn't they theoretically run with a fake GPU backend?

[Wumpf]

Out of curiosity, do those tests need to be gpu tests? Couldn't they theoretically run with a fake GPU backend?

most of what ci runs is actually with software rasterizers by the respective apis. You're right that for these tests we don't actually need to go to a real backend but we're lacking the necessary mocking architecture to stub out wgpu_hal out of these.

[Wumpf]

huh, sorry for the long delay - I missed the updates on this

Thanks for the improvements, looking better already. But I'm still not happy about any of the "pick first" code in here. If any of these were to remain we need a good (documented) reason each, otherwise I have to assume that we have a bug where we're accidentally hiding errors.

[kpreid]

Using Unicode code units for the line_position.

/// Uses Unicode code units, which are more comprehensible for humans instead of UTF-16 offsets.

I think you mean code points here. Counting code units is encoding-dependent. But beyond that terminological nitpick: surely on basic principles this should be counting UTF-8 bytes (aka UTF-8 code units) instead? That's the only way a Rust string can be indexed/sliced efficiently.

If the goal is to mark spans by counting "columns", then Unicode code points do not do that either — you need to execute the unicode-width algorithm to approximate that, or ask your font/terminal for feedback to get it perfect. I don't think that should be baked into the API, so it's better to present what is maximally useful for slicing, and let the caller compute the width it cares about.

Don't count code points, because there are almost no use cases where it is the correct answer.

[stefnotch]

@kpreid Yes, my bad, I meant code points.

I would, for a brief moment, have argued that counting code points makes sense, since VSCode seems to do that. Except that that's not true, since VSCode has a special case for tabs \t. Surprisingly, VSCode doesn't have a special case for emojis that consist of multiple code points.

So I'm realising that it's bonkers and you absolutely have a point. And that the best possible option is outputting something that a computer can reasonably convert to whatever is most appropriate for a given case.

Though at that point, one would usually directly use the offset field instead?

For what it's worth, I did copy it from the internal naga SourceLocation struct. Maybe the internal struct should also be changed?

[stefnotch]

Okay, I fixed a few more issues. Importantly, I moved the ShaderError struct from wgpu_core to naga, so that it can be used by both

• wgpu_core
• webgpu with certain feature flags

I wonder if there's a simpler variation of those cfgs for conditional compilation than whatever I chose.

[Wumpf]

Looking really good now. A few cosmetic notes that would be nice to address.
Thanks @teoxoy for the input - now that this touched Naga stuff more directly I'll wait for your additional sign-off before merge.

Could you try to avoid rebasing on the next iteration? Rebasing may look nice on the history but makes it a lot harder to incrementally review - I can't tell github to show me the changes since last time and instead have to start from scratch

[stefnotch]

@kpreid @teoxoy I looked into the linePos (column number) a bit more. I also looked into the naga SourceLocation struct, since it also contains such info.

The naga::SourceLocation struct is mainly being used for emitting SpirV OpLine instructions. Those are for debug tools.
And inconveniently, the specification says exactly nothing about what the column in an OpLine is supposed to represent. It's just supposed to be a positive integer.

So I looked at what some tools do

• SPIRV-Cross: Always emits 0 for the column number.
• DirectXShaderCompiler: Uses the length of the string plus one. I couldn't definitively figure out which encoding is being used.
• Renderdoc: Couldn't figure out what it assumes, but it doesn't seem to be counting code points.

So my question: May I change the naga::SourceLocation.line_position to count UTF-8 code units instead of general Unicode code points? This only makes a difference when dealing with non-ASCII Unicode characters, so it doesn't actually come up very often.

[teoxoy]

May I change the naga::SourceLocation.line_position to count UTF-8 code units instead of general Unicode code points?

Sounds good to me. It makes things more consistent.

[stefnotch]

And it's ready for a (hopefully) final review. I'm glad that I got to learn a ton about wgpu internals.

Thank you for all the reviews and helping me get this PR into a decent shape. <3

[Wumpf]

all good from my side 👍

Thanks again for taking this feature on working through all these iterations! :)

[Wumpf]

uhoh, this went into 0.20 changelog but didn't get release with it