Shadow volume culling and tighter shadow caster culling

[lawnjelly]

Existing shadow caster culling takes no account of the camera. This PR adds the highly encapsulated class RenderingLightCuller which can cut down the casters in the shadow volume to only those which can cast shadows on the camera frustum.

Forward port of #82584

This is used to:

• More accurately defer dirty updates to shadows when the shadow volume does not intersect the camera frustum.
• Tighter cull shadow casters to the view frustum.

Lights dirty state is now automatically managed:

• Continuous (tighter caster culling)
• Static (all casters are rendered)

Explanation

You can see roughly how it works in this old video of mine (ignore the rooms and portals, that is a separate system):
https://www.youtube.com/watch?v=1WT5AXZlsDc

The blue lines from the light sources to the camera frustum show the extra culling planes.

How does it work?

At runtime, the routine checks each plane of the camera frustum, and finds whether it is facing either towards or away from the light (0 or 1). These bits for the 6 planes form a 6 bit number, which is the lookup.

The lookup tells us a list of corner points from the camera frustum which form a silhouette, which can be used to generate culling planes together with the light origin (3 points form a culling plane).

References:

http://lspiroengine.com/?p=153
http://www.terathon.com/gdc06_lengyel.pdf

Performance

In tests in TPS demo, without GI and just using shadows, in many areas this halves the number of drawcalls / vertex count, in some cases reduces drawcalls by a factor of 10x. This can lead to 10-300% increase in FPS (the increase in FPS depends on settings used, if fill rate is bottlenecking then improving shadows has less dramatic effect and vice versa).

In WroughtFlesh, which uses directional light only, I get a more modest 10% or so improvement if FPS, due to the tighter caster culling with the directional light. So it seems like the benefits are higher the more omnis / spots are used.

Notes

• This is based on a resurrection of [WIP] Tighter shadow caster culling #33340 . That PR's approach wasn't compatible with the dirty flag optimization for omnis and spots, but this PR automatically manages dynamic lights to handle tighter culling, while reverting to rendering all casters when used in static manner.
• This PR also adds the major optimization that shadow maps outside the view frustum don't need to be updated at all. There was some existing culling based on AABBs but this PR is far more accurate. This can lead to major performance gains where a lot of shadowed lights are in use (e.g. TPS demo).
• The project setting is initially for testing, it can detect changes at runtime (if you e.g. change project setting from script). If it works correctly there may be no need to have it switchable, as it should be virtually always a win and the calculations are very little, and outweighed by any gains.

Tighter caster culling and Multiple Cameras

There is one more situation in which tighter caster culling is problematic: when multiple viewports are in use, and the shadow volume intersects multiple cameras.
In this situation tighter caster culling will work - it will do a tight cull on the first camera, and a full render for the second camera. The problem is that it will do 2 shadow renders per frame instead of one.
The answer used here is to detect this situation (in detect_light_intersects_multiple_cameras()) and switch to a different mode light_intersects_multiple_cameras.
This reverts to the legacy approach of doing a full render on the first update. However, we still want to detect the situation where it changes back to a single camera. This is done by means of a timeout after a certain number of frames without a double update.

Directional Lights

Directional lights are handled separately, they are always updated, and with different shadow maps if multiple cameras are used with viewports. Therefore they can always do the tighter caster cull.

There are some differences in the directional light culling compared with #82584, as the culling in 4.x relies on a brute force approach.

• Production edit: This closes Improve DirectionalLight shadow rendering efficiency #57549.

[clayjohn]

I tested quickly on a high complexity scene and it spammed the following error:

E 0:00:51:0072   add_cull_plane: Condition "data.num_cull_planes >= MAX_CULL_PLANES" is true.
  <C++ Source>   servers/rendering/rendering_light_culler.cpp:229 @ add_cull_plane()

I'll debug further and see if I can find the cause. Despite the error, it appeared to reduce the shadow draw time in the scene from 30ms to 20 ms (on my intel integrated graphics)

[lawnjelly]

I tested quickly on a high complexity scene and it spammed the following error:

I'll take a look. This may be due to some slight differences in cameras from 3.x (bear in mind this is a straight port and has had much more testing on 3.x). 👍
Also could be one of the exception cases where a light is within the camera frustum. Can't seem to reproduce it so far, I bet this is a super simple bug! If you find out which light type it is for, or an MRP, let me know.

Ah the max may possibly need to be 17 rather than 16! 😀
I see that there is an extra edge added in _add_light_camera_planes(), on top of the max of 8 from the LUT.
But I'd like to reproduce this to confirm first.

Actually possible bug for the n_edges 8 case, just investigating.

[clayjohn]

Quick debugging check, changing MAX_PLANES to 32 makes the error go away.

The scene has multiple omnilights and a directionallight3D. The error spams while multiple omnilights are within the camera frustum. The error continues when the DirectionalLight3D is made not visible.

I'll try to make a reproduction project

[lawnjelly]

Also worth checking is if setting MAX_PLANES to 17 makes it go away.

I've just pushed a slightly updated version with the lookup table showing all 8 entries (There was a strange half finished bit for only writing 7 entries on purpose, but I wrote this 3 years ago so can only try and remember why I did that, maybe it was a bug. It did seem vulnerable to reading an incorrect 8th entry in the rare case of the light pointing towards that particular viewpoint of the camera frustum, which would explain why I hadn't spotted it.).

I'm not 100% sure this would have created your bug but it is just possible, so it is worth trying the latest version as well. I'll update the 3.x version of this PR as well once we have pinned this down as it is likely common to both.

I'll try and reproduce it on 3.x as it is likely present there too, and I don't have any test projects on master as I rarely use it.

[clayjohn]

Okay, I've prepared an MRP that reliably reproduces the issue.
clip-plane-mrp.zip

[lawnjelly]

Fantastic! Thanks so much for spending time on this. I can immediately see that light culler contains some garbage values so some kind of corruption is going on.

I think it's a race condition. Must be being used multithread. I'd have never found this without MRP. 😀
Yeah _scene_cull_threaded() there's the clue. 😁

This shouldn't be a big deal to fix but I'll mark as draft as I have to prioritize 3.6 now we have a deadline. The 3.6 PR should be the same but no threading issues so I think it should be fine, @Calinou has reviewed but it would be good to get a check and approval if it looks okay.

It's off and on-able so if there are any bugs they can be fixed during RC.

[lawnjelly]

This should be fixed now.

• Preparing the directional lights has been moved outside the multithreaded code, only the culling is performed there now, which should be thread safe.
• I also fixed it to work with multiple directional lights. These are prepared in advance, so the cull_directional_light() can chop and change between the different directional lights (this is the pattern currently used in the code).
• This is for 4.x only, none of these problems occur in 3.x.
• As before, the cull_directional_light() could be optimized more, but this should give a decent boost over vanilla un-culled light casters.
• I moved the tighter cull out of the cascade part of the directional shadow loop.
  This is more efficient, and should be faster even if there is no extra rejects, because the old IN_FRUSTUM() check for each cascade was not very efficient (duplicates work in each cascade). All the current IN_FRUSTUM() check there actually has to do is check the near and far planes, the side planes are already checked. This could be fixed up, but would be fine for a later PR, and the lion's share of the gains are already made by moving the tighter cull outside the cascade loop.

[Calinou]

Tested locally, it works as expected. Visuals look correct too from my testing in various demo projects.

Benchmark on tps-demo

OS: Fedora 38
CPU: Intel Core i9-13900K
GPU: GeForce RTX 4090 (NVIDIA 535.113.01)

The project is modified to disable V-Sync. The FPS reported is the highest FPS attained over a period of 10 seconds after loading the level. Unfortunately, unlike the 3.x branch, I can't notice any performance improvement with this PR.

Type	Before	After
4K Maximum Forward+	232	234
4K Minimum Forward+	961	962
720p Maximum Forward+1	579	568
720p Minimum Forward+1	1044	1033
4K Minimum Compatibility1	471	472
720p Minimum Compatibility1	466	476

This also applies to other demos like Truck Town and 3D platformer which I've tested as well.

Footnotes

1. CPU is the bottleneck, with GPU sitting at less than 50% utilization. ↩ ↩² ↩³ ↩⁴

[lawnjelly]

Unfortunately, unlike the 3.x branch, I can't notice any performance improvement with this PR.

Oh I must have made a booboo in the fixup then, as there should be an improvement. I'll do a debug to check it's working correctly (I don't have many test projects in 4.x).

Ah, unless it is your GPU which is not bottlenecked like most lesser GPUs lol. 😁 I'll check though.

UPDATE:
Done some testing and it does appear to be working for directional and omnis. I've improved the debug logging in the fixed version.

If you uncomment the following defines:

#define LIGHT_CULLER_DEBUG_DIRECTIONAL_LIGHT
#define LIGHT_CULLER_DEBUG_REGULAR_LIGHT

It will output the number of extra instances culled by directional lights, and regular lights (omnis / spots). Providing these are non-zero, then it is working.

It could be that @Calinou 's tests on 3.x have different bottlenecks (if you can force the iGPU to render that might be useful comparison). In particular, GLES is more likely to be bottlenecked by drawcalls in the shadow caster rendering than Vulkan. Also larger shadowmap resolution is more likely to be bottlenecked (but again high end GPU may not be affected much).

The actual final render is unaffected (i.e. reading the PCF shadowmap), so changing the screen size only acts to shift the bottlenecks. Small screen size is more likely to bottleneck on rendering the shadow casters (you could try a postage stamp sized final window), but with high end GPU this may not bottleneck things compared to e.g. physics or blocked threads.

[clayjohn]

I tested again on the same scene and can confirm that the errors are gone.

In a section of the level that is mostly outdoors, I noticed a decrease in draw calls by about 25% (2500 -> 1900).

However, in an indoor section with multiple omnilights, I noticed an increase in the number of draw calls (14,300 -> 14,700). My guess is that something is forcing the omnilights to update every frame instead of just when changed. I'll try to isolate the case and confirm the theory

[lawnjelly]

However, in an indoor section with multiple omnilights, I noticed an increase in the number of draw calls (14,300 -> 14,700). My guess is that something is forcing the omnilights to update every frame instead of just when changed. I'll try to isolate the case and confirm the theory

I'll have a look too in the mean time and see if I can work out what is going on. Could be a flip between tighter culling mode and full culling is causing a double shadowmap update somewhere. This can occur but it should recognise it the first time and stay in the full mode for some time.

If you can produce an MRP that would be super useful. In particular it would be useful to know whether this occurred continuously, or on a single frame (e.g. captured with renderdoc). Single frame with multiple cameras it is expected that there will be 2 shadowmap updates on the first occurrence (so drawcalls can blip for a single frame).

Discussion

The tighter cull itself takes place after regular culling, so that shouldn't be possibly to render more than before, so that suggests that any increase in draw call count must be due to the shadow map being rendered more than before.

So if there is a problem it's likely in the logic for the shadow_dirty_count. This isn't a lot of code but it's easy to miss possibilities and perhaps end up with shadow map being updated twice. It's also possible that there's another race condition I haven't thought of due to the multithreaded culling in 4.x.

It goes something like this (maybe I could change the count to set to an enum to make it clearer):
shadow_dirty_count

• 0 - not dirty
• 1 - full shadow map update required
• 2 - tighter cull shadow map update required

When the light / objects in the light are continously moving, (providing there is one camera only) 2 is getting set each frame and thus it is only doing a single tighter cull update.

If two or more cameras are detected it enters light_intersects_multiple_cameras mode, which persists for 60 frames after the last frame in which multiple cameras were detected for the light. In this mode when making dirty it always sets to 1, in which case a full update is done every time (it essentially updates shadow maps as before this PR).

The last case is where an object in tighter cull mode (2) with a single camera stops updating, the frame afterwards it will do a single full update (as the shadow_dirty_count decrements to 1) leaving the shadow map in a suitable state for long term use. (This situation is where the light and the casters stop moving, but the view may still move on subsequent frames, so we can't leave it long term with a stale tighter cull shadow map, we have to leave it in a state where all casters are in the shadow map.)

Worse situation

This means that in the situation of staggered updates (e.g. light moves every second frame, casters stay still), the shadow map will be updated more often than before. It will be updated once per frame - one tighter cull on frame 0 (on the frame of light movement) and one full cull on frame 1 (to create longterm shadowmap) then repeat on frame 2. Versus with tighter culling switched off, it would do full cull frame 0, and no shadowmap update in frame 1 etc.

I was aware of the above, but wasn't sure whether it would be a problem in practice. If it does occur in the wild it would be pretty easy to put in some logic to detect this and revert to full updates. But I'd like to examine the situation first to confirm this is what is causing it, it may be some other logic bug.

[lawnjelly]

After testing the large MRP @clayjohn mentioned above, my results are as follows:

Drawcalls (tight culling first, legacy second):

Outdoor 2530, 2873
Indoor 19990, 21440
Panoramic 3487, 4713
Free 1977, 7191
Character 1972, 2315

So improvement in all cases.

I haven't figured out yet how to move the indoor camera, these are the default views.

BTW I tested by switching on the LIGHT_CULLER_DEBUG_FLASH define, and changing the frequency to 64 (I got very low frame rate on my PC).
Then recorded the drawcalls given in the game.
Also I set LIGHT_CULLER_DEBUG_LOGGING, it tells you when it flashes the culling on and off.
Also I realised there was an out of date variable name in the debugging defined section, had to swap an active to light_culling_active (due to a variable rename) to get it to compile. I'll fix that in the PR and repush.

[clayjohn]

Tested again and can confirm a decrease of draw calls in all cases.

Let's go ahead with this for 4.3. It should be a nice performance improvement for many scenes. In the demo scene I tested it results in a noticeable increase in FPS for much of the level.

[akien-mga]

Thanks!

[mrjustaguy]

Tighter Shadow caster culling causes the shadows to disappear in some cases...

Run this test scene, and press home a handful of times to rotate the lights one increment at a time (end to rotate in reverse)
With tighter shadow caster culling enabled, shadows will disappear for a frame a few rotations in, only to pop back in, but this is not the case with it disabled
Shadows.zip

[lawnjelly]

Tighter Shadow caster culling causes the shadows to disappear in some cases...

This could be a bug, usually better reported in an issue. I'll try out the project. 👍

EDIT: I think I can see what you mean, there is some logic to handle the updates which may have a bug in this situation hence the flash off for a frame. If you can make the project more minimal that will be helpful, as having complex project makes it more difficult to debug and pin down. 👍

UPDATE:
I noticed it was only happening on Female27, which was at the edge of the screen. This made me wonder if the AABB was correct for that model. Sure enough, if I set a custom AABB, the flicker goes away. This could be simply the AABB being a point and a problem with the AABB code (rather than the shadow culling).

I can't say for definite yet, but I know there have been problems in the past with AABBs for skinned meshes, so it is a suspect. And of course if the AABB is garbage, the culling won't work properly.

UPDATE:
It isn't to do with skinned meshes, same thing happens with a box in that exact position.
It is definitely strange. I will probably have to investigate more tomorrow.

[mrjustaguy]

Sorry about that, was kind of in a rush yesterday and it was a bit late, The PR happened to be the 1st thing I was thinking about when I observed and quickly pinned down the issue.

What doesn't make sense is that it only disappears for 1 frame, only to reaper the next despite the fact there should be no update to lights that next frame, as they're static... So somehow they're dirtied one frame after the rotation, probably by the offending model (Female26 for me) and this has nothing to do with skinning as yea, a cube of roughly the AABB of the dummy also gets the same issue.

I'll open an issue now..