Flip Y axis in SPIR-V frontend and backend

[kvark]

Fixes #651
Unblocks gfx-rs/gfx#3707

Our IR has the coordinate system that matches everything but Vulkan. So in order to correctly translate, we need to flip the Y of vertex outputs in both SPV-in and SPV-out.

This is a draft because I'm still trying to figure out what flags are needed to be exposed to control this.

[kvark]

Alright, here is the plan.
IR has the coordinate space matching DX12, Metal, OpenGL, but not SPIR-V.
I thought about different scenarios of where this is used, and concluded that both SPV-in and SPV-out need to be able to control this behavior. I think it's a good direction, since this can be considered extra work, anyway.

The SPIR-V backend would always do this adjust_coordinate_space = !features.contains(NDC_Y_UP).
All backends, if given SPIR-V on input, would do adjust_coordinate_space = !features.contains(NDC_Y_UP).

Let's consider the following scenarios:

1. gfx-rs is used by Vulkan-like clients, passing down SPIR-V. It gets converted to IR with Y-flipping, and then goes into the platform shaders like this. Or it gets straight into the Vulkan backend.
2. gfx-rs is used by wgpu, which enables NDC_Y_UP and provides SPIR-V. It is generated with adjust_coordinate_space == false. The backends would assume the SPIR-V coming in the Y-flipped form, so they will generate the IR with adjust_coordinate_space == false and go from there.
3. gfx-rs is used by wgpu, which provides Naga IR. The coordinate system is expected to match the NDC_Y_UP, but I don't think we need to actually check this. If SPIR-V needs to be generated, we do this with adjust_coordinate_space == false.

[Napokue]

Okay so, the thing is: why do we prefer one of the other?

I have read in gpuweb/gpuweb#416 that there possibly would be a discussion for getPreferredOrientation which let the user specifiy the orientation. Because of this I think we need to support the direction conversion at IR/proc level, not at front- and back-end level.

The IR would need to figure out if it needs to be flipped in the other direction. Be it up or down.

What we can do however, is having some metadata about what direction every back-ends expect, and also have this metadata passed from the front-end. If both match when analyzing, nothing has to be done. If they don't match, flip the direction.

By having this, we also support other front-ends that are not necessarily implemented at Naga, but for consumers that use their own kind of front-end (or maybe even back-end?).

Where it differs from your proposal is that you want to have the control at both the front-end and back-end, and what this thought is, is about having this control at proc level. This proc will have the responsibility to convert it.

[kvark]

I have read in gpuweb/gpuweb#416 that there possibly would be a discussion for getPreferredOrientation which let the user specifiy the orientation. Because of this I think we need to support the direction conversion at IR/proc level, not at front- and back-end level.

It's not clear (at this point) if getPreferredOrientation is going to affect anything, see:

I think the discussion of adding "getPreferredOrientation" can be set aside for the moment. It's not directly related to the choice of WebGPU's coordinate system.

The scheme you are describing is basically having a flag in crate::Module saying what coordinate system this is. And then every user of Naga would have to potentially run the processor to change the coordinate system. I think this extra responsibility is bearable, but there is another aspect of this path that I find concerning at the moment (see next paragraph).

Right now, the IR is generated once by a fronted, then validated, the extra information is generated (ModuleInfo and friends), which is then consumed by the backends. All the backends assume the module was validated, and most require ModuleInfo to be provided. Any processing on the IR happens as a part of the frontend work (e.g. adding "Return" statements automatically).

Once we start going from IR(1) to IR(2), we face the following complication. The processor itself has to assume the IR(1) is valid, and potentially use ModuleInfo. Otherwise, it would have to be extra careful to not accidentally panic, e.g. by using a Handle that is simply not represented in the module. But then the IR(2) that backends would see is changed, so we'd have to run the validator again in order to re-generate the ModuleInfo. This is quite a bit of extra work. It may not matter in practice, but it feels like a waste to me (and it's a lot of work).

So this is a part of a larger discussion: how do we do IR processing?

1. every processor expects a valid module, and then the result is re-validated (extra work!)
2. the validator is able to incrementally apply changes to the module (seems very difficult to implement)
3. all the processing happens as a part of frontent/backend logic (what we have today)

What we have today works so far. We may need to change it, so far it's not clear, but doing this for the Flip-Y issue doesn't sound like a good reason, since it's relatively minor. The code in this PR can be ported into a real IR processor if we decide this to be a way to go (most of the changes here are in tests, there isn't that much code).

[Napokue]

I totally agree with your concerning points. I didn't know about the use case that an IR itself can be potentially re-used for back-ends.

The best way forward would be indeed landing this PR, and when and if we encounter more of this cases we can consider going down one of the roads that we have been proposing.

I will review this PR, so we can land this. Thanks for your input!

[Napokue]

LGTM, thank you!

[kvark]

@Napokue thank you being vigilant!

[bschwind]

@kvark I currently have a build.rs file which consumes wgsl shader code and produces SPIR-V via naga. The build.rs step is mostly for faster shader compilation checking, but I had also assumed it was less work for wgpu-rs to consume SPIR-V compared to WGSL.

I noticed some of my scenes were drawing upside down and eventually found this issue. For my workflow, would you recommend:

1.) Passing naga::back::spv::WriterFlags::empty() to the SPIR-V writer instead of the default of naga::back::spv::WriterFlags::ADJUST_COORDINATE_SPACE
2.) Only validate WGSL in build.rs, and then pass the WGSL in again directly to wgpu
3.) Something else?

Essentially I want to write a wgpu-rs game which runs on mac/linux/windows, with shaders written in WGSL, and I'd like the game logic to always assume positive Y is "up" in normalized device coordinates. Thanks for your help!

[kvark]

I strongly recommend (2). There is substantially less work for wgpu/gfx to work with WGSL than to SPIR-V. We are still trying to figure out some of the nasty spots about SPIR-V control flow.

[bschwind]

There is substantially less work for wgpu/gfx to work with WGSL than to SPIR-V

Got it! I had no idea, bad assumption on my part. Thanks for the recommendation, and again for all your work on the rust graphics ecosystem :D