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

Report conflicting explicit bindings #93

Closed
nbentyNV opened this issue Jul 14, 2017 · 5 comments
Closed

Report conflicting explicit bindings #93

nbentyNV opened this issue Jul 14, 2017 · 5 comments

Comments

@nbentyNV
Copy link
Collaborator

nbentyNV commented Jul 14, 2017

Not Slang's fault, but it would be great if it will report conflicts in external bind locations.
For example:

layout(set = 0, binding = 0) uniform texture2D gTexture;
layout(set = 0, binding = 0) uniform sampler gSampler;

The shader compiles successfully. I don't know if it's a valid GLSL shader or not, perhaps the correct place for this issue is glslang.

@tangent-vector-personal

This has some overlap with #38.

I'd like to make this a warning (two parameters with different names and overlapping bindings), with an option to make it trigger an error. This hasn't bubbled up to the top of the priority list yet.

As far as I understand, this is valid GLSL. It is okay to define overlapping bindings, so long as a given compiler shader ("version") only uses one or the other (I don't know if that is a static constraint or a dynamic one).

@nbentyNV
Copy link
Collaborator Author

Is it valid even if the declarations are in the same shader?
Issue #96 both the constant-buffer and sampler are bound to the same location and are being used in the shader, I don't see why this is valid.

I'm not suggesting Slang is doing anything wrong. Either glslang has a bug or GLSL has a bug.

@tangent-vector-personal

Do we get errors reported from the driver?

I would need to comb the spec carefully to know what is or isn't valid.

@tangent-vector
Copy link
Contributor

I implemented a basic version of this in #118. I'll close this if/when I get confirmation that it seems to meet requirements (and doesn't give false positives).

@tangent-vector
Copy link
Contributor

I've seen this trigger now in cases where it should, and nobody has reported false positives (yet), so I'm going to consider this good.

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

No branches or pull requests

3 participants