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

Fix same uniforms in fragment and vertex shaders not working in Metal #1808

Merged
merged 5 commits into from
Apr 12, 2024

Conversation

smilediver
Copy link
Contributor

Describe your changes

Context: in OpenGL API setting shader uniforms works at the program level, which means that same named uniforms in vertex and fragment shaders are treated as the same variable, so when you set a uniform it is set in both shaders at once. In Metal API uniforms are set for vertex and fragment shaders separately.

In current Axmol's Metal implementation if you have a same named uniform in vertex and fragment shaders, then only the vertex shader's variable is set, which is different from OpenGL. This PR fixes Metal part to match OpenGL semantics.

This PR may have breaking changes as it modifies UniformLocation and ShaderModuleMTL::getUniformLocation(). But probably it's not a common thing to access UniformLocation internals or calling ShaderModuleMTL::getUniformLocation().

Issue ticket number and link

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

@halx99
Copy link
Collaborator

halx99 commented Apr 10, 2024

@smilediver
Copy link
Contributor Author

Pushed fix. Also noticed a uniform type mismatch in positionNormalTexture.vert and colorNormalTexture.frag, so fixed that too.

@halx99 halx99 added this to the 2.1.3 milestone Apr 11, 2024
@halx99 halx99 added enhancement New feature or request bug Something isn't working labels Apr 11, 2024
@halx99 halx99 merged commit 74c6a4a into axmolengine:dev Apr 12, 2024
14 of 15 checks passed
@halx99
Copy link
Collaborator

halx99 commented Apr 12, 2024

cheers

@smilediver smilediver deleted the fix_metal_uniforms branch April 17, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants