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

Add command line args setting option for the GLSL validator executable #2

Merged
merged 4 commits into from
Nov 5, 2018

Conversation

w103csh
Copy link
Contributor

@w103csh w103csh commented Nov 3, 2018

I started using your extension for linting my shader code, and it worked great until I started using push constants in my shaders. Push constants are a SPIR-V only type, so it caused the extension to stop evaluating my shader code after it found a push constant block like this:

struct MaterialData {
    vec3 Ka;            // Ambient reflectivity
    vec3 Kd;            // Diffuse reflectivity
    vec3 Ks;            // Specular reflectivity
    float Shininess;    // Specular shininess factor
};

layout(push_constant) uniform PushBlock {
    MaterialData material;
} pushConstantsBlock;

The GLSL validator I am using supports code like above, but it requires the command line argument -V. I altered your extension so that it could handle command line arguments. This is what the pull request entails.

I thought I should make a PR in case you want the change.

  • Note - The format of the error messages when using the -V argument is slightly different. It includes the file name instead of just a number like so:
    ERROR: color.vert:15: '' : syntax error, unexpected IDENTIFIER
    Instead of:
    ERROR: 0:15: '' : syntax error, unexpected IDENTIFIER
    This is why the regex for the error parser was changed.

@w103csh
Copy link
Contributor Author

w103csh commented Nov 3, 2018

The correct command line argument for my example above is actually --spirv-val not -V for the validator I linked to in the initial PR comment. Just an FYI.

@ghost
Copy link

ghost commented Nov 5, 2018

Thanks for the pull request, I will test this out the next few days and then create an update on the marketplace. This could also fix #1

@hsimpson hsimpson merged commit 986cb0f into hsimpson:master Nov 5, 2018
@w103csh
Copy link
Contributor Author

w103csh commented Nov 7, 2018

Just downloaded from the marketplace and it seems to work right for the SPIR-V flag. Cheers!!!

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