Skip to content

Light::Query and fixed light loop for shadows2#10399

Open
boxrocket6803 wants to merge 1 commit intoFacepunch:masterfrom
boxrocket6803:light-query-v2
Open

Light::Query and fixed light loop for shadows2#10399
boxrocket6803 wants to merge 1 commit intoFacepunch:masterfrom
boxrocket6803:light-query-v2

Conversation

@boxrocket6803
Copy link
Copy Markdown
Contributor

Summary

The light query stuff from #10148 but now it works with shadows2, also returns the directional light as part of the loop.

Implementation Details

Took the opportunity to remove Light::Count and Light::From since they run significantly worse and anyone who was using them would have to edit their shaders anyway, they are replaced with Light::Query returning a LightRange struct and Light::Fetch, the struct stores the Cluster::Query result so that it doesn't need to be queried twice per light. Looping over lights now looks like

LightRange query = Light::Query( ScreenPosition );
for ( uint x = 0; x < query.Count; x++ )
{
	Light l = Light::Fetch( query, x, WorldPosition, ScreenPosition.xy );
	...
}

The directional light (if enabled) is also stuck into a Light struct and fetched as part of the loop, presumably doesn't run quite as well but not in any notable way and seems worth it to let the Light class maintain its ease of use.

Screenshots

image

Checklist

  • Code follows existing style and conventions
  • No unnecessary formatting or unrelated changes
  • Public APIs are documented (if applicable)
  • Unit tests added where applicable and all passing
  • I’m okay with this PR being rejected or requested to change 🙂

Light struct is now initialized for directional light and returned as part of loop rather than needing to deal with it separately, looping over lights now looks like this (faster)
```
LightRange query = Light::Query( ScreenPosition );
for ( uint x = 0; x < query.Count; x++ )
{
	Light l = Light::Fetch( query, x, WorldPosition, ScreenPosition.xy );
	...
}
```
@handsomematt
Copy link
Copy Markdown
Member

I don't really want the directional light in any form of loop at all. It's the global uniform sun light.

@boxrocket6803
Copy link
Copy Markdown
Contributor Author

In a scene with translucent additive planes stacked 34 high I got the following results:

Using the Light loop with directional light included

float4 MainPs( PixelInput i ) : SV_Target0
{
    float3 color = 0.0;
    float3 pos = i.vPositionWithOffsetWs + g_vCameraPositionWs;

    LightRange query = Light::Query( i.vPositionSs ); // with dir light
    for ( uint x = 0; x < query.Count; x++ )
    {
        Light l = Light::Fetch( query, x, pos, i.vPositionSs.xy );
        color += l.Color * l.Attenuation * max( dot( l.Direction, i.vNormalWs ), 0 ) * l.Visibility;
    }

    return float4(color, 1);
}

With some extra point lights
avg 38.97ms, max 43.03ms
image
And without
avg 9.14ms, max 10.95ms
image

Using the Light loop with directional light excluded, plus a separate block for the directional light

float4 MainPs( PixelInput i ) : SV_Target0
{
    float3 color = 0.0;
    float3 pos = i.vPositionWithOffsetWs + g_vCameraPositionWs;

    LightRange query = Light::Query( i.vPositionSs ); // without dir light
    for ( uint x = 0; x < query.Count; x++ )
    {
        Light l = Light::Fetch( query, x, pos, i.vPositionSs.xy );
        color += l.Color * l.Attenuation * max( dot( l.Direction, i.vNormalWs ), 0 ) * l.Visibility;
    }

    if ( g_DirectionalLightEnabled ) {
        color += g_DirectionalLightColor.rgb * max( dot( -g_DirectionalLightDirection, i.vNormalWs ), 0 ) * DirectionalLightShadow::GetVisibility( pos, i.vPositionSs.xy );
    }

    return float4(color, 1);
}

With some extra point lights
avg 39.55ms, max 43.54ms
image
And without
avg 9.25ms, max 13.01ms
image

I don't think it would be fair to say separated directional light runs worse because the numbers jump around a bit (especially when screen snip is starting) but I think I could confidently say it doesn't run any better, and it's nicer to be able to put the lighting code directly into the loop and not need to worry about the directional light. I can still remove it from the loop but I don't really see the benefit unless there's something I'm missing.

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

Successfully merging this pull request may close these issues.

2 participants