GLSL - per-fragment lighting

Question

I started lighting with several light sources. All the manuals that I saw without taking into account the distance between the light source and the object (for example https://learnopengl.com/Lighting/Basic-Lighting). So I wrote my shader, but I'm not sure about its correctness. Please, analyze this shader, and tell me what's wrong / not correct in it. I will be very grateful for any help! Below I bring the shader itself, and the results of its work for different values of n and k.

Fragment shader:

#version 130

precision mediump float;                    // Set the default precision to medium. We don't need as high of a
                                            // precision in the fragment shader.

#define MAX_LAMPS_COUNT 8                   // Max lamps count.

uniform vec3 u_LampsPos[MAX_LAMPS_COUNT];   // The position of lamps in eye space.
uniform vec3 u_LampsColors[MAX_LAMPS_COUNT];
uniform vec3 u_AmbientColor = vec3(1, 1, 1);
uniform sampler2D u_TextureUnit;
uniform float u_DiffuseIntensivity = 12;
uniform float ambientStrength = 0.1;
uniform int u_LampsCount;

varying vec3 v_Position;                    // Interpolated position for this fragment.
varying vec3 v_Normal;                      // Interpolated normal for this fragment.
varying vec2 v_Texture;                     // Texture coordinates.

// The entry point for our fragment shader.
void main() {
    float n = 2;
    float k = 2;

    float finalDiffuse = 0;
    vec3 finalColor = vec3(0, 0, 0);

    for (int i = 0; i<u_LampsCount; i++) {
        // Will be used for attenuation.
        float distance = length(u_LampsPos[i] - v_Position);

        // Get a lighting direction vector from the light to the vertex.
        vec3 lightVector = normalize(u_LampsPos[i] - v_Position);

        // Calculate the dot product of the light vector and vertex normal. If the normal and light vector are
        // pointing in the same direction then it will get max illumination.
        float diffuse = max(dot(v_Normal, lightVector), 0.1);

        // Add attenuation.
        diffuse = diffuse / (1 + pow(distance, n));

        // Calculate final diffuse for fragment
        finalDiffuse += diffuse;

        // Calculate final light color
        finalColor += u_LampsColors[i] / (1 + pow(distance, k));
    }

    finalColor /= u_LampsCount;

    vec3 ambient = ambientStrength * u_AmbientColor;

    vec3 diffuse = finalDiffuse * finalColor * u_DiffuseIntensivity;

    gl_FragColor = vec4(ambient + diffuse, 1) * texture2D(u_TextureUnit, v_Texture);
}

Vertex shader:

#version 130

uniform mat4 u_MVPMatrix;      // A constant representing the combined model/view/projection matrix.
uniform mat4 u_MVMatrix;       // A constant representing the combined model/view matrix.

attribute vec4 a_Position;     // Per-vertex position information we will pass in.
attribute vec3 a_Normal;       // Per-vertex normal information we will pass in.
attribute vec2 a_Texture;      // Per-vertex texture information we will pass in.

varying vec3 v_Position;       // This will be passed into the fragment shader.
varying vec3 v_Normal;         // This will be passed into the fragment shader.
varying vec2 v_Texture;        // This will be passed into the fragment shader.

void main() {
    // Transform the vertex into eye space.
    v_Position = vec3(u_MVMatrix * a_Position);

    // Pass through the texture.
    v_Texture = a_Texture;

    // Transform the normal's orientation into eye space.
    v_Normal = vec3(u_MVMatrix * vec4(a_Normal, 0.0));

    // gl_Position is a special variable used to store the final position.
    // Multiply the vertex by the matrix to get the final point in normalized screen coordinates.
    gl_Position = u_MVPMatrix * a_Position;
}

n=2 k=2

n=1 k=3

n=3 k=1

n=3 k=3

And if my shader is correct, then how do I name these parameters (n, k)?

Answer 1

By "correct" I assume you mean is the code working as well as it should. These lighting calculations are not by any means physically accurate. Unless you are going for full compatibility with old devices, I would recommend you use a higher glsl version which allows you to use in and out and some other useful glsl features. The current is version 450 and you are still using 130. The vertex shader looks ok, as it is only passing through values to the fragment shader.

As for the fragment shader there are is one optimisation you could make.

The calculation u_LampsPos[i] - v_Position doesn't have to be repeated twice. Do it once and do the length and normalize on the same result from one calculation.

The code is quite small so there is not much to go wrong glsl wise however I was wondering why you did: finalColor /= u_LampsCount;?

This didn't make sense to me.