Skip to content

Conversation

@rms7326
Copy link

@rms7326 rms7326 commented Jul 22, 2025

Description

Using shader specialization constants to size global arrays doesn't work on the Mac.

We have a need in our vertex shader to size both a vertex attribute array and an array of gl_clipDistance by a defined value at shader compile time. We were using shader specialization constants to achieve this and on Linux and Windows with NVidia and AMD we seemed to have success even though the "Arrays inside a block" discussion in section "4.4.x Specialization-Constant Qualifier" of ARB_gl_spirv.txt made it sound like using specialization constants to size these array members in the shader is not supported. When we tried out the same code on the Mac we had very unreliable results and were not able to get clipping to work correctly if the vertex array or the gl_ClipDistance array was sized by a specialization constant.

Currently, valueless shader compiler defines are added to a std::set<std::string> container via the shader module's hints. This PR extents the syntax to allow name=value without changing the client API thereby providing an option for defines with simple values for use the in the shader code.

Fixes # (issue)
#1534

Type of change

Core VSG change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I extended the vsgclip example in the vsgExamples repository. See vsgExamples pull request 364.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

@robertosfield
Copy link
Collaborator

Thanks for the changes. I have done a first pass review. It's good see how localized the changes are.

However, I think it's a bit too clever for it's own good, with the capabilities being hidden in plain sight. You and I will know that the defines are now capable of being used this way but it won't be obvious to others without them reading an example.

I think the right solution will be to modify the public interface of defines to make it clear that it can be used with just a keyword or a <keyword, value>. The natural implementation would be to use a std::map<std::string, std::string> but as this would break backwards compatibility we'd need to provide a mechanism to provide that capability.

Rather implement something in the short term then come back an implement it in a different way further down the line I will undertake this work myself, but I think it needs to happen after 1.1.11.

@rms7326
Copy link
Author

rms7326 commented Jul 24, 2025

OK, thanks for considering it, Robert. Implementing a name/value system is pretty important for sophisticated shader generation, so I would appreciate it if this is higher on your list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants