2
votes

In my application I add two lights. One at (0,0,2) and the second one at (2,0,0). Here's what I get (the x,y,z axes are represented respectively by the red, green & blue lines):

cube with only one light working

Notice how only the first light is working and the second is not. I made my application core-profile compliant to inspect the buffers with various tools like RenderDoc and NSight and both show me that the second light's data is present in the buffer (picture taken while running Nsight):

nsight capture

The positions seem to be correctly transfered to the gpu memory buffer. Here's the implementation of my fragment shader that uses a SSBO to handle multiple lights in my application:

#version 430

struct Light {
  vec3  position;
  vec3  color;
  float intensity;
  float attenuation;
  float radius;
};

layout (std140, binding = 0) uniform CameraInfo {
  mat4  ProjectionView; 
  vec3  eye;
};

layout (std430, binding = 1) readonly buffer LightsData {
    Light lights[];
};

uniform vec3  ambient_light_color;
uniform float ambient_light_intensity;

in  vec3 ex_FragPos;
in  vec4 ex_Color;
in  vec3 ex_Normal;
out vec4 out_Color;

void main(void)
{
    // Basic ambient light
    vec3 ambient_light = ambient_light_color * ambient_light_intensity;

    int i;
    vec3 diffuse = vec3(0.0,0.0,0.0);
    vec3 specular = vec3(0.0,0.0,0.0);
    for (i = 0; i < lights.length(); ++i) {
        Light wLight = lights[i];
        // Basic diffuse light
        vec3 norm = normalize(ex_Normal); // in this project the normals are all normalized anyway...
        vec3 lightDir = normalize(wLight.position - ex_FragPos);
        float diff = max(dot(norm, lightDir), 0.0);
        diffuse += diff * wLight.color;

        // Basic specular light
        vec3 viewDir = normalize(eye - ex_FragPos);
        vec3 reflectDir = reflect(-lightDir, norm);  
        float spec = pow(max(dot(viewDir, reflectDir), 0.0), 32);
        specular += wLight.intensity * spec * wLight.color;  
    }

    out_Color = ex_Color * vec4(specular + diffuse + ambient_light,1.0); 
}

Note that I've read the section 7.6.2.2 of the OpenGL 4.5 spec and that, if I understood correctly, my alignment should follow the size of the biggest member of my struct, which is a vec3 and my struct size is 36 bytes so everything should be fine here. I also tried different std version (e.g. std140) and adding some padding, but nothing fixes the issue with the second light. In my C++ code, I have those definitions to add the lights in my application:

light_module.h/.cc:

struct Light {
  glm::f32vec3  position;
  glm::f32vec3  color;
  float         intensity;
  float         attenuation;
  float         radius;
};
...
constexpr GLuint        LIGHTS_SSBO_BINDING_POINT = 1U;
std::vector<Light>      _Lights;
...
void AddLight(const Light &light) {
  // Add to _Lights
  _Lights.push_back(light);
UpdateSSBOBlockData(
  LIGHTS_SSBO_BINDING_POINT, _Lights.size()* sizeof(Light),
  static_cast<void*>(_Lights.data()), GL_DYNAMIC_DRAW);
}

shader_module.h/.cc:

using SSBOCapacity = GLuint;
using BindingPoint = GLuint;
using ID = GLuint;
std::map<BindingPoint, std::pair<ID, SSBOCapacity> >  SSBO_list;
...
void UpdateSSBOBlockData(GLuint a_unBindingPoint,
  GLuint a_unSSBOSize, void* a_pData, GLenum a_eUsage) {
  auto SSBO = SSBO_list.find(a_unBindingPoint);
  if (SSBO != SSBO_list.end()) {
    GLuint unSSBOID = SSBO->second.first;
    glBindBuffer(GL_SHADER_STORAGE_BUFFER, unSSBOID);
    glBufferData(GL_SHADER_STORAGE_BUFFER, a_unSSBOSize, a_pData, a_eUsage);
    glBindBuffer(GL_SHADER_STORAGE_BUFFER, 0); //unbind
  }
  else 
    // error handling...
}

Basically, I'm trying to update/reallocate the SSBO size with glBufferData each time a light is added in my app.

Now, since I'm having issues processing the second light data, I changed my fragment shader code to only execute the second light in my SSBO array by forcing i = 1 and looping until i < 2, but I get the following errors:

(50) : error C1068: ... or possible array index out of bounds
(50) : error C5025: lvalue in field access too complex
(56) : error C1068: ... or possible array index out of bounds
(56) : error C5025: lvalue in field access too complex

Lines 50 and 56 refer to diffuse += diff * wLight.color; and specular += wLight.intensity * spec * wLight.color; respectively. Is there really an out of bounds access even if I add my lights before the first draw call? Why is the shader compiling correctly when I'm using lights.length() instead of 2?

Finally, I've added a simple if (i == 1) in my for-loop to see if lights.length() is equal to 2, but it doesn't go in it. Yet the initial size of my buffer is 0 and then I add a light that sets the buffer size to 36 bytes and we can see that the first light works fine. Why is the update/reallocate not working the second time?

1

1 Answers

2
votes

So what I did was to add some padding at the end of the declaration of my struct on the C++ side only. The padding required was float[3] or 12 bytes, which sums up to 48 bytes. I'm still not sure why this is required, since the specifications state (as highlighted in this post)

  1. If the member is a structure, the base alignment of the structure is N, where N is the largest base alignment value of any of its members, and rounded up to the base alignment of a vec4. The individual members of this sub-structure are then assigned offsets by applying this set of rules recursively, where the base offset of the first member of the sub-structure is equal to the aligned offset of the structure. The structure may have padding at the end; the base offset of the member following the sub-structure is rounded up to the next multiple of the base alignment of the structure. [...]

When using the std430 storage layout, shader storage blocks will be laid out in buffer storage identically to uniform and shader storage blocks using the std140 layout, except that the base alignment and stride of arrays of scalars and vectors in rule 4 and of structures in rule 9 are not rounded up a multiple of the base alignment of a vec4.

My guess is that structures such as vec3 and glm::f32vec3 defined by glm are recursively rounded up to vec4 when using std430 and therefore my struct must follow the alignment of a vec4. If anyone can confirm this, it would be interesting since the linked post above deals with vec4 directly and not vec3.

Picture with both lights working :

second light working

EDIT:

After more investigation, it turns out that the last 3 fields of the Light struct (intensity, attenuation and radius) were not usable. I fixed this by changing the position and color from glm::f32vec3 to glm::vec4 instead. More information can be found in a similar post. I also left a single float for padding, because of the alignment mentioned earlier.