-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Async shader compilation (KHR_parallel_shader_compile) #1855
Conversation
So if async defaults to true, won't this change the behavior of existing PlayCanvas projects out there? There'll be meshes popping into view instead of the main thread blocking until the shader is ready, no? |
Yes, by default it does.
The goal would be to make everyone benefit from it straight away. |
Yeah, that way sounds better. @mvaligursky - what do you think about all this? |
From engine usability point of view, I'm not very keen on a solution where object might sometimes not show because shaders are compiling. Async shader compilation is optimization, and as such it's not something I'd want users to have to worry about by exposing some material API to enable / disable it. Material interface is predominantly used by artists, and they will not understand the implications of this. It would lead to a hard to find bugs that depend on timing, on some low end devices perhaps. I would prefer a solution without public API for this. I'd be happy with per project "use async shader compilation" option but that would allow users to disable it while there are perhaps issues with the system, as a work around. In ideal world we'd integrate async shaders in a way it has no impact on engine users. I'm not familiar with engine enough to cover most cases, but some points of what I'm thinking would be a part of good solution:
Examples of possible issue with current implementation:
|
Valid points @mvaligursky.
Currently it is this flow:
So in case of non-preloaded, assets, setting them by code will not have an immediate effect. Same goes with created by code assets. Developer has to call
There are few cases for userflow with async shaders:
This will enforce sync compilation by engine. Same for post effects and other similar cases. This PR is pretty simple and naive, and only solves either 1st or 3rd user cases.
Same goes for The challenge here, is that pc.Shader - is throw away object, managed by material system. Each material can have many of them depending on flag states. Different ModelComponents with different flags that use same Material, will lead that Material to have multiple Shaders. So developer does not have explicit handle of shader during application update loop. It only has abstraction over it, in form of Material. There can't be a simple function |
I wanted to follow-up on a this PR, with an idea that came up from discussions with @slimbuck + @mvaligursky - it became apparent that we could expose a way for engine users to explicitly choose to use async compilation by defaulting to non-async when using the standard material interface: |
I should add that internally in the engine (once we have support) we would use the asynchronous shader compilation where possible - but with guarantees to developers that their projects will look and behave the same - and hopefully load faster and run with fewer stalls - a prime example (covered to a degree in @mvaligursky comments) would be while preloading a scene asynchronously (perhaps while the player is interacting with a basic start-up scene) we could also precompile the new scene's material shaders asynchronously (after any dependencies are loaded) - and any shaders not compiled by the time the new scene is switched to, will block until done (an alternative to blocking - which was also discussed - is to temporarily switch to use a slower 'uber' shader until a specific optimized shader is ready)? |
Also, to avoid having to recompile shader due to async loaded textures, we should create and attach some dummy 1x1 textures to them when material is created to build the appropriate shader right away instead of building different variations for each loaded resource. |
I think we already do this: #753 |
lightly related: #3559 |
I tried to add support for the extension to the engine, but I wasn't very convinced by the benefits, and so I've created a separate example to investigate: https://github.com/mvaligursky/webgl-parallel_shader_compile |
I added issues with the extension to both chromium and webkit |
This PR is now so old, it's going to be pretty much impossible to resolve the files even if we decided to merge. I think we should close it for now. Thanks for the initial work on this though, @Maksims! |
Fixes #1474
Restored code of: #1685
After so many years of using git, I still mess up :D
I confirm I have signed the Contributor License Agreement.