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

Adds a texture mapping allowing us to combine sampler uniforms #4636

Merged
merged 5 commits into from
Sep 16, 2022

Conversation

GSterbrant
Copy link
Contributor

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.

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.
Copy link
Contributor

@willeastcott willeastcott left a 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.

Copy link
Member

@slimbuck slimbuck left a 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

src/graphics/program-lib/programs/standard.js Outdated Show resolved Hide resolved
Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fricken awesome!

@GSterbrant GSterbrant merged commit 949ee64 into main Sep 16, 2022
@GSterbrant GSterbrant deleted the gsterbrant_minimize_texture_samplers branch September 16, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants