Hey guys, hopefully someone smarter than me can help me out.

I have some point lights. They have a radius. However, the light they emit doesn't seem to be constrained by this radius like I want, see this image:

(The red circles are what their radius should be. You can see how the light continues though, producing a very visible square shape.)

Here's the shader I use where I pass in each light one by one and render it to the scene:

#version 330

in vec4 v_color;
in vec2 v_texCoord;            // UV for the current fragment

uniform sampler2D u_texture;
uniform sampler2D u_normal;

uniform float lightRadius;  // Radius of light (pixels)
uniform vec3 lightPos;      // Position of point light (UV)
uniform vec4 lightColor;    // Color of light, a is intensity
uniform vec2 screenSize;    // Screen resolution (pixels)

layout(location = 0) out vec4 lightBuffer;

void main()
{
    // RGBA of diffuse color.
    vec4 diffuseRGBA = texture2D(u_texture, v_texCoord);

    // RGB of normal map, invert the y-axis for accuracy.
    vec3 normalRGB = texture2D(u_normal, v_texCoord).rgb;
    normalRGB.g = 1.0 - normalRGB.g;

    // The delta position of light
    vec3 lightDir = vec3(lightPos.xy - (gl_FragCoord.xy / screenSize.xy), lightPos.z);

    // Correct for aspect ratio and set size of light.
    float lightDiameter = lightRadius * 2;
    lightDir.x /= (lightDiameter / screenSize.x);
    lightDir.y /= (lightDiameter / screenSize.y);

    // Determine distance (used for attenuation) before normalizing lightDir.
    float distance = length(lightDir);

    // Normalize vectors.
    vec3 N = normalize(normalRGB * 2.0 - 1.0);
    vec3 L = normalize(lightDir);
    
    // Pre-multiply light color with intensity then perform "N dot L" to determine diffuse term.
    vec3 diffuse = (lightColor.rgb * lightColor.a) * max(dot(N, L), 0.0);

    // Calculate attenuation.
    float attenuation = 1.0 - (distance * distance);

    // The calculation which brings it all together.
    vec3 intensity = diffuse * attenuation;
    vec3 finalColor = diffuseRGBA.rgb * intensity;
    
    // Done.
    lightBuffer = v_color * vec4(finalColor, diffuseRGBA.a);
}

Can anyone shed some light on what's going on, appreciate it!

    lightDir.x /= (lightDiameter / screenSize.x);
    lightDir.y /= (lightDiameter / screenSize.y);

Why are you doing this? Aside from the fact that 2D lights should always point into the screen. I.e., x=0; y=0; z=1 (or -1 for OpenGL), you are making everything dependent on the size of the screen (I assume screenSize is your resolution here).

Nevermind. I think I misunderstood what lightDir is (was thinking of 3D light instead of 2D). Anyway, you should probably use the size of your quads for aspect correction, not the screen size.

Thanks for the reply.

That's an interesting idea. I don't have the code in front of me at the moment, I'll check it out later.

Meanwhile, I discovered another issue last night, in case you or anyone else can help me shed some light on it. Notice the "dead area" in the top-right area of all of the point lights:

I can't seem to figure out what would be causing that.

Any ideas? Thanks!

You attenuation looks strange. Shouldn't you be attenuating over the lights diameter or radius and not 1?

You also probably want to saturate/clamp that attenuation value so it doesn't go negative.

I think you want something like 1 / (distance*distance) so when your distance from the light very short you are near 1 and as you move away it gets weaker.

You need to check for the distance == 0.

If it wasn't painfully obvious, this is my first real experience with shader programming. Much of this code is a slightly modified version from this tutorial:

https://github.com/mattdesl/lwjgl-basics/wiki/ShaderLesson6#FragmentShader

I basically just removed ambient lights and attempted to shove in lights with sizes.

Anyway, I've been reading what you guys have said but still can't quite make any progress.

@snake5 - I think you might be right about the radius, I got rid of the diameter. Also, it's all 2D, the z component is already in the range of 0-1 so it doesn't need to be scaled. Also, I'm not sure how I'd go about dividing the diameter/radius out from the distance later, I've tried it a few ways to no avail and can't seem to wrap my head around it. (The lack of being able to print out/debug variables is really messing with me.)

@Eklipse - What you say makes sense. Like I mentioned, much of this code is from the tutorial above. I went and implemented it your way (1/ distance).

Here's the updated fragment shader with the minor tweaks:

#version 330

in vec4 v_color;
in vec2 v_texCoord;

uniform sampler2D u_texture;
uniform sampler2D u_normal;

uniform float lightRadius;  // Radius of light (screen coords)
uniform vec3 lightPos;      // Position of point light (UV coords)
uniform vec4 lightColor;    // Color of light, a is intensity
uniform vec2 screenSize;    // Screen resolution (screen coords)

layout(location = 0) out vec4 lightBuffer;

void main()
{
    // RGBA of diffuse color.
    vec4 diffuseRGBA = texture2D(u_texture, v_texCoord);

    // RGB of normal map, invert the y-axis for accuracy.
    vec3 normalRGB = texture2D(u_normal, v_texCoord).rgb;
    normalRGB.g = 1.0 - normalRGB.g;

    // The delta position of light
    vec3 lightDir = vec3(lightPos.xy - (gl_FragCoord.xy / screenSize.xy), lightPos.z);

    // Correct for aspect ratio and set size of light.
    lightDir.x /= (lightRadius / screenSize.x);
    lightDir.y /= (lightRadius / screenSize.y);

    // Determine distance (used for attenuation) before normalizing lightDir.
    float distance = length(lightDir);

    // Normalize vectors.
    vec3 N = normalize(normalRGB * 2.0 - 1.0);
    vec3 L = normalize(lightDir);
    
    // Pre-multiply light color with intensity then perform "N dot L" to determine diffuse term.
    vec3 diffuse = (lightColor.rgb * lightColor.a) * max(dot(N, L), 0.0);

    // Calculate attenuation.
    float attenuation = 1.0;
    if (distance != 0)
        attenuation = max(1.0 / distance, 0.0);

    // The calculation which brings it all together.
    vec3 intensity = diffuse * attenuation;
    vec3 finalColor = diffuseRGBA.rgb * intensity;
    
    // Done.
    lightBuffer = v_color * vec4(finalColor, diffuseRGBA.a);
}

And its output:

The radius is still obviously messed up. But these tweaks appears to make it clearer that the top-right corner of the lights being dark is not an error, but seems to be correct. It's the rest of the "corners" of the light that appear to incorrectly bright, if that makes sense?

Really appreciate all of the help guys. Sorry for the utter display of stupidity, for some reason this is giving me a hard time.

@vlj - You know, that actually worked. The lights are now displaying properly using (1.0 - distance) for attenuation. Seeing it, it does make sense now. Appreciate it a ton.

@phil_t - I'm trying to apply your advice, I'd love to have things simplified using just screen coordinates. As a result, I've attempted to abandon UV coordinates and am trying to work in just screen coordinates, however, I think I broke everything (no lights are being displayed). Here's the shader code where I've tried to adapt it to what you suggested:

#version 330

in vec4 v_color;
in vec2 v_texCoord;

uniform sampler2D u_texture;
uniform sampler2D u_normal;

uniform float lightRadius;  // Radius of light (screen coords)
uniform vec3 lightCoord;    // Position of point light (screen coords, z is 0-1)
uniform vec4 lightColor;    // Color of light, a is intensity

layout(location = 0) out vec4 lightBuffer;

void main()
{
    // RGBA of diffuse color.
    vec4 diffuseRGBA = texture2D(u_texture, v_texCoord);

    // RGB of normal map, invert the y-axis for accuracy.
    vec3 normalRGB = texture2D(u_normal, v_texCoord).rgb;
    normalRGB.g = 1.0 - normalRGB.g;
    
    // The delta position of the light.
    vec3 lightDir = vec3(lightCoord.xy - gl_FragCoord.xy, lightCoord.z); // lightCoord.z is 0-1, lightCoord.xy are screen coords
    
    // Distance to be used for attenuation.
    float distance = length(lightDir.xy) / lightRadius;
    
    // Attenuation.
    float attenuation = clamp(1.0 - distance, 0.0, 1.0);
    attenuation *= attenuation;

    // Normalize vectors.
    vec3 N = normalize(normalRGB * 2.0 - 1.0);
    vec3 L = normalize(lightDir);
    
    // Pre-multiply light color with intensity then perform "N dot L" to determine diffuse term.
    vec3 diffuse = (lightColor.rgb * lightColor.a) * max(dot(N, L), 0.0);
    
    // The calculation which brings it all together.
    vec3 intensity = diffuse * attenuation;
    vec3 finalColor = diffuseRGBA.rgb * intensity;
    
    // Done.
    lightBuffer = v_color * vec4(finalColor, diffuseRGBA.a);
}

That seems pretty close to what you said, if I had to guess, I'd say the fact that my lightCoord.z being 0-1 whereas lightCoord.xy being screen coordinates is what's messing with me.

Would that be the case? Really wouldn't know how to go about not using 0-1 for z though?