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

Library for translating shaders from SPIRV to anything #2269

Open
kvark opened this issue Jul 25, 2018 · 12 comments

Comments

@kvark
Copy link
Member

commented Jul 25, 2018

Or, simply speaking, rewrite SPIRV-Cross in Rust.

Why?

  • inconvenient and potentially hazardous: external C dependency that is not that widely used
  • slow: a random profile of a 5 second dota benchmark shows us spending 25% of time translating shaders
  • limited: lack of Metal argument buffer support, doesn't run on Android

In the context of BAL #2249, we could also expose the translation logic in a more flexible way. Perhaps, it would help abstraction layers that originate from a different source than SPIR-V.

cc @antiagainst @grovesNL

@derekdreery

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2018

@anderejd

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2018

@kvark

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2018

We started prototyping this in https://github.com/gfx-rs/javelin
Any help is appreciated!

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2019

I'm confused, you say "a random profile of a 5 second dota benchmark shows us spending 25% of time translating shaders". It seems to me like translating shaders should be something that's done once, ie, will affect load time but not run time. Am I mistaken? Can this be changed?

@kvark

This comment has been minimized.

Copy link
Member Author

commented Jan 21, 2019

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

Ok, that makes sense. And load times are important, just less important. ;-) I'm certainly not a pro in terms of rich graphics, Unity is about where I start and stop there.

So my next question is, what can we do better than spirv-cross or shaderc or such? I guess we need to profile them. Our goals are somewhat more limited so we can target more specific backends, which is nice. And I definitely agree that Pure Rust(tm) makes life simpler and better. But assuming that prior art is slow just cause it was written by lame compiler devs seems hazardous, and assuming it's all FFI overhead almost as much. If we want to make something with better performance, the first step is to understand why performance is poor.

Maybe I should give that a go and see what's up. Do you have a good test case that's easy to set up and doesn't involve owning DOTA 2?

@grovesNL

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

FWIW another reason is that we'll have to ship a fairly large Emscripten C++ bundle wherever gfx-hal is used with the wasm target in the future, so even just having the SPIR-V to ESSL portion would be useful to keep everything in one Rust wasm module. I think replacing SPIRV-Cross entirely is a pretty large task, so it might be easier to start with the most important pieces and target a subset of shader functionality.

There are also some performance gains we could get by splitting up the compile/parse steps in the spirv_cross wrapper now that it seems to be supported in SPIRV-Cross, which might help performance a bit (wherever shaders can be recompiled without redoing the initial parse step).

I think Dota2 is free-to-play if you do want to test it there. Otherwise we'd probably have to create some test cases or try to find something in the Vulkan CTS.

@icefoxen

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2019

Hm, DOOM has a vulkan backend, maybe I can try to make it work on gfx-hal... ;-)

@kvark

This comment has been minimized.

Copy link
Member Author

commented Jan 22, 2019

@icefoxen that would be insanely useful 👍

So my next question is, what can we do better than spirv-cross or shaderc or such?

It's quite messy inside. Lots of mutable state, heaps on heaps of allocations, generally not the best-looking C++ code. And the very fact it's a C++ API as opposed to pure C API brings a ton of trouble to us (have a Vec? lay it out in memeory and then construct a std::vector to copy into...).

I tried to separate shader passing from compilation (which one you'd expect from their API), and eventually just gave up, because in C++ codebase it's hard to trace down all side effects in reasonable developer time.

And I clearly see that spirv-cross will be improved, especially with Google involvement. I just feel like it's still beneficial to us to have an alternative. I.e. when glutin was established, we already had good bindings to glfw, but still we all agree that it was the right move to make.

@jeffparsons

This comment has been minimized.

Copy link

commented Feb 21, 2019

If you'll forgive a stupid question...

Why is it that the necessity of runtime translation from SPIRV->everything is a foregone conclusion in pretty much every conversation along these lines?

Personally I'd be happy to invoke SPIRV-Cross at build time, then ship 5 (or however many) different sets of shaders for different targets. I assume I've missed something obvious that makes this a bad idea... 😅

@grovesNL

This comment has been minimized.

Copy link
Member

commented Feb 21, 2019

@jeffparsons Probably the biggest reason is because gfx-hal might need to transform shaders at runtime based on how they're being used. For example, specialization constants aren't supported on all GL versions, so they may be inlined as a constant into the shader at runtime whenever the constant value becomes known. Metal and Direct3D have similar kinds of concerns (besides specialization constants). It would also limit some use cases like gfx-portability, since they wouldn't be able to match Vulkan's API.

@jeffparsons

This comment has been minimized.

Copy link

commented Feb 21, 2019

@grovesNL Thanks for the explanation — that's super-clear and a pretty compelling set of reasons! 🙇 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.