7

In GLSL there seems to be linking error of shaders when I try to pass a uniform struct with a sampler2D attribute to a function which is forward declared. The code works if I remove forward declaration and move the function above main. Is this illegal code?

#version 330 core

in vec2 texcoords;
out vec4 color;

struct Material{
    sampler2D tex; // Sampler inside a struct
}; 

uniform Material material;

// Forward Declaration
vec4 add(Material m);

void main() {
    color = add(material);
}

// Function Definition
vec4 add(Material m) {
    return vec4(texture(m.tex, texcoords));
}

// C++
glUniform1f(glGetUniformLocation(shader, "material.tex"), 0);
glBindTexture(GL_TEXTURE_2D, texture);

EDIT: So after a bit of searching it appears to be a bug in AMD's driver. I personally use ATI Mobility Radeon HD 4670 which is pretty old, but it still runs OpenGL 3.3. On AMD's forums I found a similar post, and so it would be interesting to know how big this is on AMD's graphic cards. Because if you're developing on Intel or NVidia, then how are you to know your shaders won't compile on some AMD's graphic cards? Should we stay safe and not use prototypes on structs with samplers, or even go as far as not putting samplers in struct completely?... It's also worth noting that WebGL doesn't even allow for samplers inside structs.

Error Message:

Vertex shader(s) failed to link, fragment shader(s) failed to link.
unexpected error.
unexpected error.
Iggy
  • 4,767
  • 5
  • 23
  • 34
  • You said there is a linking error. Linking errors are caused by linking the vertex and fragment shader together so in that stage something is wrong. As far as I know this code looks correct by itself. Could you also post the vertex shader code? – Joey Dewd Aug 24 '14 at 20:36
  • By the way, you should use `glUniform1i` instead of `glUniform1f` since texture locations are integers, not floats. – Joey Dewd Aug 24 '14 at 20:36
  • Hey Joey, nice to see you use Stack Overflow too. This is actually something that was brought into my attention when I compiled your code from "Multiple lights" chapter. I'm specifically referring to Calc***Light() prototypes which require a material object which uses sampler2D's. – Iggy Aug 24 '14 at 20:42
  • 1
    Hey you're that Iggy! What a coincidence ;) The fragment shader looks fine by me though - function prototypes like these are allowed in GLSL. I see no reason why it is complaining (in fact, your source code even compiled perfectly here). – Joey Dewd Aug 24 '14 at 20:51
  • Weird! Could be my machine or drivers? – Iggy Aug 24 '14 at 20:55
  • Could be, don't know what's causing this behavior – Joey Dewd Aug 24 '14 at 21:05
  • Oh, I just tried this in my own GLSL code base and apparently it works. I think I'm using a slightly broken version of your shader.h, must be something wrong in the shader linking section. – Iggy Aug 24 '14 at 21:13
  • Interesting, could you perhaps pinpoint the source of where it fails, so I could fix it? (just succesfully compiled the same shader, with the exact shader.h source code) – Joey Dewd Aug 24 '14 at 21:17
  • ERROR::SHADER::PROGRA::LINKING_FAILED Fragment shader(s) failed to link, vertex shader(s) failed to link. unexpected error. unexpected error. – Iggy Aug 24 '14 at 21:20
  • Actually never mind, it still breaks on me with the same error message! > – Iggy Aug 24 '14 at 21:27
  • Aww, I'm afraid I can't help you much more right now. Try to further pinpoint where the issue's coming from and perhaps wait for someone to jump along that had similar issues – Joey Dewd Aug 24 '14 at 21:30

1 Answers1

7

This actually is not supposed to work because you cannot instantiate a struct that contains an opaque data type (sampler, image, atomic counter). It is acceptable to have a struct with an opaque type as a uniform, but you cannot implement the function add (...) by passing an instance of Material.

Data Type (GLSL) - Opaque Types

Variables of opaque types can only be declared in one of two ways. They can be declared at global scope, as a uniform​ variables. Such variables can be arrays of the opaque type. They can be declared as members of a struct, but if so, then the struct can only be used to declare a uniform​ variable (or to declare a member of a struct/array that itself a uniform variable). They cannot be part of a buffer-backed interface block or an input/output variable, either directly or indirectly.

This change to your code should work across all compliant implementations of OpenGL:

// Must be in, because you cannot assign values to an opaque type.
vec4 add (in sampler2D tex) {
  return vec4(texture(tex, texcoords));
}

This would also work, since it uses the sampler in the material.tex uniform:

vec4 add (void) {
  return vec4(texture(material.tex, texcoords));
}
Andon M. Coleman
  • 42,359
  • 2
  • 81
  • 106
  • You could also use a pre-processor macro to portably do what you're trying to do right now. Something to the effect of implementing `add (...)` the first way I wrote in my answer and then using `#define ADD(m) add(m.tex)`. Then you could use `ADD(material)` on pretty much any GLSL implementation without issues. – Andon M. Coleman Oct 26 '14 at 22:29
  • Interesting insight. Yet the instance of material that is being passed to `add(...)` is still a uniform; no instantiation is going on. I'm still puzzled by the fact that you can run `add(Material)` if it's defined on place. It only breaks when it's forward declared. – Iggy Oct 27 '14 at 00:49
  • The uniform is actually `material.tex` in this example. Structs as uniforms are basically just a namespace sort of thing. It definitely is interesting that it works without the forward declaration though. – Andon M. Coleman Oct 27 '14 at 00:58
  • Well structs as namespaces makes perfect sense. It's strange that forward declared version works on other graphic cards. It might be something completely different. Again worth noting that it's a linking error. – Iggy Oct 27 '14 at 01:18