Skip to content

fix: GLSL version directive never being applied#1221

Open
cedrik-fuoco-adsk wants to merge 3 commits intoAcademySoftwareFoundation:mainfrom
cedrik-fuoco-adsk:fix-glsl-amd
Open

fix: GLSL version directive never being applied#1221
cedrik-fuoco-adsk wants to merge 3 commits intoAcademySoftwareFoundation:mainfrom
cedrik-fuoco-adsk:fix-glsl-amd

Conversation

@cedrik-fuoco-adsk
Copy link
Copy Markdown
Contributor

GLSL version directive never being applied

Linked issues

n/a

Summarize your change.

Remove #version 150 from inside #if __VERSION__ >= 150 blocks in all TwkGLF shaders, and inject it from C++ via glShaderSource so it is the true first token in the concatenated shader source.

  • BasicGLProgram.cpp: added basicGLVersionHeader() helper that
    returns "#version 150\n" for GL3+ or "" for GL2, and passes it as
    the first element of a 2-string array to glShaderSource for both
    vertex and fragment shaders.
  • All 28 .glsl files under TwkGLF/glsl/: removed the now-redundant
    #version 150 line from inside the #if __VERSION__ >= 150 block.
    The guards themselves are kept for GL2/GL3 feature selection.

Describe the reason for the change.

#version must be the first token in GLSL source per spec.
The previous code placed #version 150 inside a #if __VERSION__ >= 150 guard

Describe what you have tested and on which operating system.

Windows with AMD GPU.

Add a list of changes, and note any that might need special attention during the review.

If possible, provide screenshots.

N/A

…0 was placed inside #if __VERSION__ >= 150, but

  __VERSION__\ndefaults to 110 before any #version is processed, so the condition was\nalways false and shaders compiled as GLSL 110 on
  all contexts.\n\nInject #version 150 from C++ via glShaderSource string array so it\nbecomes the true first token as the spec requires.
  The existing\n#if __VERSION__ >= 150 guards in the shaders still handle GL2/GL3\nfeature selection correctly at runtime.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Copy link
Copy Markdown
Contributor

@bernie-laberge bernie-laberge left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants