Forward+ not working with custom shader

Hello! I’m building a game with a toon graphic style which requires many lights to be displayed, more than 8 per object (so I can’t use neither deferred nor Forward rendering). I updated Unity to 2022.2.10f1 and URP to 14.0.6, and set the rendering setting to Forward+. I had to adapt my hlsl code to make it work with Forward+, and I modified NedMakesGame’s Lighting.hlsl script (from a toon tutorial video) to make it work with Forward+:

#ifndef CUSTOM_LIGHTING_INCLUDED
#define CUSTOM_LIGHTING_INCLUDED


float GetLightIntensity(float3 color)
{
    return max(color.r, max(color.g, color.b));
}

void CalculateMainLight_float(float3 WorldPos, out float3 Direction, out float3 Color, out half DistanceAtten, out half ShadowAtten, out float Intensity)
{
#ifdef SHADERGRAPH_PREVIEW
    Direction = half3(0.5, 0.5, 0);
    Color = 1;
    Intensity = 0;
    DistanceAtten = 1;
    ShadowAtten = 1;
#else
#if SHADOWS_SCREEN
    half4 clipPos = TransformWorldToHClip(WorldPos);
    half4 shadowCoord = ComputeScreenPos(clipPos);
#else
    half4 shadowCoord = TransformWorldToShadowCoord(WorldPos);
#endif
    Light mainLight = GetMainLight(shadowCoord);
    Direction = mainLight.direction;
    Color = mainLight.color;
    DistanceAtten = mainLight.distanceAttenuation;
    ShadowAtten = mainLight.shadowAttenuation;
    Intensity = GetLightIntensity(mainLight.color);
#endif
}


#ifndef SHADERGRAPH_PREVIEW
// This function gets additional light data and calculates realtime shadows
Light GetAdditionalLightForToon(int pixelLightIndex, float3 worldPosition)
{
    // Convert the pixel light index to the light data index
    int perObjectLightIndex = GetPerObjectLightIndex(pixelLightIndex);
    // Call the URP additional light algorithm. This will not calculate shadows, since we don't pass a shadow mask value
    Light light = GetAdditionalPerObjectLight(perObjectLightIndex, worldPosition);
    // Manually set the shadow attenuation by calculating realtime shadows
    light.shadowAttenuation = AdditionalLightRealtimeShadow(perObjectLightIndex, worldPosition);
    return light;
}
#endif
void AddAdditionalLights_float(float Smoothness, float3 WorldPosition, float3 WorldNormal, float3 WorldView,
    float MainDiffuse, float MainSpecular, float3 MainColor, float3 ScreenPosition,
    out float Diffuse, out float Specular, out float3 Color)
{

    float mainIntensity = GetLightIntensity(MainColor);
    Diffuse = 0;
    Specular = 0;
    Color = MainColor;
  
  
    float totalDiffuse = 0; // Aggiunto per accumulare i valori di thisDiffuse

#ifndef SHADERGRAPH_PREVIEW
    InputData inputData = (InputData) 0;
    float highestDiffuse = Diffuse;
    float maxIntensity = 0;
    float intensity = 0;
    uint meshRenderingLayers = GetMeshRenderingLayer();
  
    inputData.normalizedScreenSpaceUV = ScreenPosition;
    inputData.positionWS = WorldPosition;
    uint lightsCount = GetAdditionalLightsCount();
    LIGHT_LOOP_BEGIN(lightsCount)
    Light light = GetAdditionalLight(lightIndex, WorldPosition);
    #ifdef _LIGHT_LAYERS
        if (IsMatchingLightLayer(light.layerMask, meshRenderingLayers))
#endif
    {
        half NdotL = saturate(dot(WorldNormal, light.direction));
        half atten = light.distanceAttenuation * light.shadowAttenuation * GetLightIntensity(light.color);
        half thisDiffuse = atten * NdotL;
        half thisSpecular = LightingSpecular(thisDiffuse, light.direction, WorldNormal, WorldView, 1, Smoothness);
        Diffuse += thisDiffuse;
        Specular += thisSpecular;
        intensity = GetLightIntensity(light.color);
  
        if (thisDiffuse > 0)
        {
            if (intensity > maxIntensity)
            {
                maxIntensity = intensity;
                Color = light.color;
            }
        }
    }
        LIGHT_LOOP_END
#endif
    }
#endif

As you can see I’m using the LIGHT_LOOP system and I changed the GetAdditionalPerObjectLight function, and it works. When there are few objects and lights.
If I have many objects and many lights, the game crashes with this error:

This is a big problem and it should mean that the GPU is somehow under great stress, but switching to Forward or Deferred rendering, with the same amount of objects and lights in the same scene, doesn’t make the game crash. So I find a bit weird that a feature that has been literally implemented to handle more lights can’t even handle as many lights as Forward or Deferred do. I guess I’m doing something wrong with the code…
In small scenes with few objects, the Forward+ works correctly even with 20/30 lights, as it should. The scene where the game crashes has thousand of objects (not all of them are nearby a light, I’d say that there are about 30-40 objects inside the lights ranges) and about 4-8 lights.
Could somebody help? My GPU drivers are updated and I’m using a RX480 4GB card.

Will this issue still happen if switching to URP’s default shader rather than custom shader graph?

If yes, I think you can submit a bug report with a reproduceable project.

No, it doesn’t.
Digging more, I managed to find what could be the problem: the LIGHT_LOOP might not be supported on Forward+.
I changed the code to have this:

#if USE_FORWARD_PLUS
    for(uint lightIndex = 0; lightIndex < min(URP_FP_DIRECTIONAL_LIGHTS_COUNT, MAX_VISIBLE_LIGHTS); lightIndex++)
    {
    Light light = GetAdditionalLight(lightIndex, WorldPosition);
        half NdotL = max(0, dot(WorldNormal, light.direction));
        half atten = light.distanceAttenuation * light.shadowAttenuation * GetLightIntensity(light.color);
        half thisDiffuse = atten * NdotL;
        thisSpecular += LightingSpecular(thisDiffuse, light.direction, WorldNormal, WorldView, 1, Smoothness);
        Diffuse += thisDiffuse;

        Color = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? light.color : Color;
        maxIntensity = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? GetLightIntensity(light.color) : maxIntensity;
    }
#endif

Now, the game doesn’t crash. But the problem is that there’s no light shown, because the loop takes a minimum between MAX_VISIBLE_LIGHTS and URP_FP_DIRECTIONAL_LIGHTS_COUNT, which should be 1 with only one directional light. If I, for example, set a static number instead of URP_FP_DIRECTIONAL_LIGHTS_COUNT, the lights are shown correctly and the game doesn’t crash, but for obvious reasons everything is a big buggy because the number will most likely be greater than the lights count in my scene, resulting in the lights staying visible even when they are not enabled.

The solution to this problem seems now to be finding the total additional lights count, so I can place it in the loop condition. GetAdditionalLightsCount(); doesn’t work with Forward+. Any alternative? Also looked for URP_FP_LIGHTS_COUNT but it doesn’t exist :frowning:

Then it should be a custom shader issue, you can refer to how Lit shader calculates lighting in Forward+ in “URP-Package/ShaderLibrary/Lighting.hlsl” (link is for 2023.2a).

Example from the link:

    #if defined(_ADDITIONAL_LIGHTS)
    uint pixelLightCount = GetAdditionalLightsCount();

    #if USE_FORWARD_PLUS
    for (uint lightIndex = 0; lightIndex < min(URP_FP_DIRECTIONAL_LIGHTS_COUNT, MAX_VISIBLE_LIGHTS); lightIndex++)
    {
        FORWARD_PLUS_SUBTRACTIVE_LIGHT_CHECK

        Light light = GetAdditionalLight(lightIndex, inputData, shadowMask, aoFactor);

#ifdef _LIGHT_LAYERS
        if (IsMatchingLightLayer(light.layerMask, meshRenderingLayers))
#endif
        {
            // Do custom lighting here.
        }
    }
    #endif

    LIGHT_LOOP_BEGIN(pixelLightCount)
        Light light = GetAdditionalLight(lightIndex, inputData, shadowMask, aoFactor);

#ifdef _LIGHT_LAYERS
        if (IsMatchingLightLayer(light.layerMask, meshRenderingLayers))
#endif
        {
            // Repeat the same custom lighting here.
        }
    LIGHT_LOOP_END
    #endif

It’s quite confusing and I’m also looking forward to the SRP shader documentation improvements.

1 Like

It’s the code which I’ve taken as an example for my shader. The only difference that I do is that I don’t use the four argument GetAdditionalLight method because I can’t have InputData and SurfaceData completely loaded without passing about 20 variables in my shader graph and spending hours setting them up, creating a huge spaghetti code. The rest, is pretty much the same. The LIGHT_LOOP works, in small scenes, but in the game it crashes with a D3D11 error as another post (without solution) in the forum reports. The for loop, doesn’t crash, but until I find a way to count all the lights in a scene I can’t use it.

You can replace it with the one that doesn’t handle shadows.

What I usually do to solve an unknown shader problem (for me) is to make it work first (maybe from an example) and then clean it up until I found out where the problem is.

You can also try reporting this as a bug and they should point out what went wrong later as long as it’s confirmed by QA.

Hello. Did you find solution to your problem? I’m noob when it comes to coding shaders - could you please give me some tutorials to match Unity Toon Shader with Forward+ ? I already research NedMakesGame channel thanks to you. Thank you.

Yes! First, you have to add _FORWARD_PLUS as a boolean keyword, like every other special keyword used in the shader. Be sure to use it in every custom shader that you’re using! 8950851--1228842--upload_2023-4-15_11-46-14.png

Second, use the LIGHT_LOOP_BEGIN and LIGHT_LOOP_END to loop between all the lights. My C# code is this:

#ifndef CUSTOM_LIGHTING_INCLUDED
#define CUSTOM_LIGHTING_INCLUDED


float max3(float a, float b, float c)
{
    return max(a, max(b, c));
}

float GetLightIntensity(float3 color)
{
    return max3(color.r, color.g, color.b);
}



void CalculateMainLight_float(float3 WorldPos, out float3 Direction, out float3 Color, out half DistanceAtten, out half ShadowAtten, out float Intensity)
{
#ifdef SHADERGRAPH_PREVIEW
    Direction = half3(0.5, 0.5, 0);
    Color = 1;
    Intensity = 0;
    DistanceAtten = 1;
    ShadowAtten = 1;
#else
#if SHADOWS_SCREEN
    half4 clipPos = TransformWorldToHClip(WorldPos);
    half4 shadowCoord = ComputeScreenPos(clipPos);
#else
    half4 shadowCoord = TransformWorldToShadowCoord(WorldPos);
#endif
    Light mainLight = GetMainLight(shadowCoord);
    Direction = mainLight.direction;
    Color = mainLight.color;
    DistanceAtten = mainLight.distanceAttenuation;
    ShadowAtten = mainLight.shadowAttenuation;
    Intensity = GetLightIntensity(mainLight.color);
#endif
}


void AddAdditionalLights_float(float Smoothness, float3 WorldPosition, float3 WorldNormal, float3 WorldView,
    float MainDiffuse, float MainSpecular, float3 MainColor, float3 ScreenPosition,
    out float Diffuse, out float Specular, out float3 Color)
{
    float mainIntensity = GetLightIntensity(MainColor);
    Diffuse = 0;
    Specular = 0;
    Color = MainColor;

#ifndef SHADERGRAPH_PREVIEW
    InputData inputData = (InputData) 0;
    SurfaceData surfaceData;
    AmbientOcclusionFactor aoFactor = CreateAmbientOcclusionFactor(inputData, surfaceData);
    half thisSpecular = 0;
    half maxIntensity = 0;
    uint meshRenderingLayers = GetMeshRenderingLayer();

    inputData.normalizedScreenSpaceUV = ScreenPosition;
    inputData.positionWS = WorldPosition;
    uint lightsCount = GetAdditionalLightsCount();
    half4 shadowMask = CalculateShadowMask(inputData);
    uint _AdditionalLightsDirectionalCount = 128;

#if USE_FORWARD_PLUS
    for(uint lightIndex = 0; lightIndex < min(URP_FP_DIRECTIONAL_LIGHTS_COUNT, MAX_VISIBLE_LIGHTS); lightIndex++)
    {
    Light light = GetAdditionalLight(lightIndex, WorldPosition);
        half NdotL = max(0, dot(WorldNormal, light.direction));
        half atten = light.distanceAttenuation * light.shadowAttenuation * GetLightIntensity(light.color);
        half thisDiffuse = atten * NdotL;
        thisSpecular += LightingSpecular(thisDiffuse, light.direction, WorldNormal, WorldView, 1, Smoothness);
        Diffuse += thisDiffuse;

        Color = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? light.color : Color;
        maxIntensity = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? GetLightIntensity(light.color) : maxIntensity;
    }
#endif
   
   
    LIGHT_LOOP_BEGIN(lightsCount)

    Light light = GetAdditionalLight(lightIndex, WorldPosition);
        half NdotL = max(0, dot(WorldNormal, light.direction));
        half atten = light.distanceAttenuation * light.shadowAttenuation * GetLightIntensity(light.color);
        half thisDiffuse = atten * NdotL;
        thisSpecular += LightingSpecular(thisDiffuse, light.direction, WorldNormal, WorldView, 1, Smoothness);
        Diffuse += thisDiffuse;

        Color = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? light.color : Color;
        maxIntensity = (thisDiffuse > 0 && GetLightIntensity(light.color) > maxIntensity) ? GetLightIntensity(light.color) : maxIntensity;
    LIGHT_LOOP_END

    Specular = thisSpecular;
#endif
}
#endif

It’s still based on NedMakesGames code. It still uses all the subshaders and most of the tutorial’s graph structure. You can see the product of my shader here: https://twitter.com/senfinecogames/status/1637766562378661888?s=20

6 Likes

Could the light count be passed from script and do a normal for loop ? I think that would be safer.

What this stupid code doing?

for (uint lightIndex = 0; lightIndex < min(URP_FP_DIRECTIONAL_LIGHTS_COUNT, MAX_VISIBLE_LIGHTS); lightIndex++)
{
                Light light = GetAdditionalLight(lightIndex, inputData, shadowMask, aoFactor);

this should never work because lightIndex is always 1 (with 1 dir light) and all other lights just ignored.

I’m trying to implement forward+, but looks like it cant work by design.

PS I also like this

cmd.SetGlobalVector(“_FPParams0”, math.float4(m_ZBinScale, m_ZBinOffset, m_LightCount, m_DirectionalLightCount));
#define URP_FP_PROBES_BEGIN ((uint)_FPParams0.z)

Again wtf, why “m_LightCount” named as “URP_FP_PROBES_BEGIN” ???

1 Like

No, Forward Plus requires the LIGHT_LOOP_BEGIN. It’s a special loop made to loop all the lights.

Well, first of all chill down, we’re here to help or get help in a 100% friendly way. I can understand that this is an uncommon problem with few resources online, I miself spent one/two months making this shader work. Months have passed and I don’t really remember perfectly how my shader works - it does, and my Forward+ shader works flawelssy, handling hundreds of lights.
As far as I can remeber, that loop is either used to get multiple additional lights, or is something that must be used anyway (even with one directional light) to get the main light with the Forward+ setting on. I guess it’s the second option because you can find “that stupid code” inside Unity’s code:

https://github.com/Unity-Technologies/Graphics/blob/master/Packages/com.unity.render-pipelines.universal/ShaderLibrary/Lighting.hlsl

This shader should also work as an example for you. There are a few more online iirc, but I’m not sure.

Forward+ is not easy to implement. Remember to add all the custom keywords:
9482389--1333633--upload_2023-11-20_10-34-35.png

Do never forget the Forward + keyword:
9482389--1333636--upload_2023-11-20_10-35-0.png

Those keywords all need to have the same Node settings (multicompile, global, all, and the two booleans down unchecked).

I’ve not worked with probes so I can’t help you with that last code. :frowning:

3 Likes

Thanks for the details, is very useful :slight_smile:

1 Like

You misunderstood me. I addressed this to the unity developer who wrote the code.
I’m writing volumetric light and use the code from the
https://github.com/Unity-Technologi…pelines.universal/ShaderLibrary/Lighting.hlsl

As I said before, this unity code (lighting.hlsl) doesn’t make sense.

Light mainLight = GetMainLight(inputData, shadowMask, aoFactor);

    MixRealtimeAndBakedGI(mainLight, inputData.normalWS, inputData.bakedGI, aoFactor);

    inputData.bakedGI *= surfaceData.albedo;

    LightingData lightingData = CreateLightingData(inputData, surfaceData);
#ifdef _LIGHT_LAYERS
    if (IsMatchingLightLayer(mainLight.layerMask, meshRenderingLayers))
#endif
    {
        lightingData.mainLightColor += CalculateBlinnPhong(mainLight, inputData, surfaceData);
    }

    #if defined(_ADDITIONAL_LIGHTS)
    uint pixelLightCount = GetAdditionalLightsCount();

    #if USE_FORWARD_PLUS
    for (uint lightIndex = 0; lightIndex < min(URP_FP_DIRECTIONAL_LIGHTS_COUNT, MAX_VISIBLE_LIGHTS); lightIndex++)
    {
        FORWARD_PLUS_SUBTRACTIVE_LIGHT_CHECK

        Light light = GetAdditionalLight(lightIndex, inputData, shadowMask, aoFactor);
#ifdef _LIGHT_LAYERS
        if (IsMatchingLightLayer(light.layerMask, meshRenderingLayers))
#endif
        {
            lightingData.additionalLightsColor += CalculateBlinnPhong(light, inputData, surfaceData);
        }
    }
    #endif

    LIGHT_LOOP_BEGIN(pixelLightCount)
        Light light = GetAdditionalLight(lightIndex, inputData, shadowMask, aoFactor);
#ifdef _LIGHT_LAYERS
        if (IsMatchingLightLayer(light.layerMask, meshRenderingLayers))
#endif
        {
            lightingData.additionalLightsColor += CalculateBlinnPhong(light, inputData, surfaceData);
        }
    LIGHT_LOOP_END
    #endif

    #if defined(_ADDITIONAL_LIGHTS_VERTEX)
    lightingData.vertexLightingColor += inputData.vertexLighting * surfaceData.albedo;
    #endif
  1. dir light computed using " Light mainLight = GetMainLight(inputData, shadowMask, aoFactor);"
    since direct light affects all objects in the scene, it makes no sense to use forward+ for dir lights.

  2. forward+ used only with the macro " #if defined(_ADDITIONAL_LIGHTS)"
    so “URP_FP_DIRECTIONAL_LIGHTS_COUNT” doesn’t make sense in this case.

  3. URP_FP_DIRECTIONAL_LIGHTS_COUNT always 1 (or 0), while “URP_FP_PROBES_BEGIN” works correctly.
    Not sure why it names as “URP_FP_PROBES_BEGIN”, because its visible additional light counts (m_LightCount).
    cmd.SetGlobalVector(“_FPParams0”, math.float4(m_ZBinScale, m_ZBinOffset, m_LightCount, m_DirectionalLightCount));

  4. code looks strange, because it compute additional lightings twice

#if USE_FORWARD_PLUS
//additionalCode
#endif

uint pixelLightCount = GetAdditionalLightsCount();
LIGHT_LOOP_BEGIN(pixelLightCount)
LIGHT_LOOP_END

although GetAdditionalLightsCount return 0 if forward+ enabled (and compiller just ignore it), but this code raises questions anyway.

  1. “LIGHT_LOOP_BEGIN” can handle “forward_plus” macro, but it’s never called in forward+.
#if USE_FORWARD_PLUS
    #define LIGHT_LOOP_BEGIN(lightCount) { \
    uint lightIndex; \
    ClusterIterator _urp_internal_clusterIterator = ClusterInit(inputData.normalizedScreenSpaceUV, inputData.positionWS, 0); \
    [loop] while (ClusterNext(_urp_internal_clusterIterator, lightIndex)) { \
        lightIndex += URP_FP_DIRECTIONAL_LIGHTS_COUNT; \
        FORWARD_PLUS_SUBTRACTIVE_LIGHT_CHECK
    #define LIGHT_LOOP_END } }
#else
    #define LIGHT_LOOP_BEGIN(lightCount) \
    for (uint lightIndex = 0u; lightIndex < lightCount; ++lightIndex) {
    #define LIGHT_LOOP_END }
#endif

This macro raises questions again.

  1. GetAdditionalLightsCount(); doesnt work with custompass (or postprocessing), because AdditionalLightsCount is set on per-object basis by unity’s rendering code.
    And there is no documentation why this doesnt work properly.

This foward+ code raises too much questions

1 Like

Stepping back to this and coming in searching for my own answer on how to properly loop lights in forward+, I came across this. While I haven’t found my answer, I noticed one of the problem is that you aren’t using GetMainLight() which always grabs the main directional light, which is why it was missing.

I know it’s over a year later, but I hope this helps you or anyone else that finds this.

2 Likes