-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Adds a texture mapping allowing us to combine sampler uniforms #4636
Conversation
All frontend chunks use the new textureMapping lookup to understand if the chunk needs to produce a new uniform sampler for its texture fetches or if the same texture is already declared as a sampler elsewhere.
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.
A great improvement that also actually removes code from the engine. Nice! Get at least one more review though before merging.
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.
Awesome PR @GSterbrant. I especially like that existing chunks are still supported (..right?).
Few things:
- please update chunk validation with updated API for frontend chunks
- please update chunk docs on developer site
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.
Fricken awesome!
Description
All frontend chunks use the new textureMapping lookup to understand if the chunk needs to output a new uniform sampler for its texture fetches or if the same texture is already declared as a sampler elsewhere. This solves a bug where some materials just fails to compile due to overuse of texture samplers.
For example, the glTF sample Stained Glass Lamp would fail to compile with clustered on using over 16 samplers, with this PR only uses 10.