Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Omni/Spot Light Shadow Update Checks #3890

Closed
mrjustaguy opened this issue Jan 29, 2022 · 8 comments
Closed

Add Omni/Spot Light Shadow Update Checks #3890

mrjustaguy opened this issue Jan 29, 2022 · 8 comments

Comments

@mrjustaguy
Copy link

Describe the project you are working on

Open World

Describe the problem or limitation you are having in your project

Performance because of constant Shadow recalculations in places where nothing changed.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Adding an optimization to check if Omni/Spot light shadows should update would significantly improve performance where there are no changes to shadow casters.
Additionally the light shadows render depth buffers that could be used for occlusion culling to help better determine when shadows need to be updated, as there's no point in updating shadows if say an object moved behind a wall that is blocking the light to said object anyhow.

The first would offer significant performance improvements and would be very cheap on the CPU, and would be best just implemented and used, much like frustum culling is in rendering, while the second would have to be investigated how much CPU time it's taking to see if it's worth it or not, but seeing as occlusion culling doesn't seem to take too much CPU time, and it is doing the Depth Buffer Rendering, I don't think reading a Pre-rendered Shadow map would be a big deal for it, if they can be accessed by the CPU that is.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Keep a List of Objects (that can be rendered by the shadows ofc) for every light, with their Transformations. If Transformations Change or new Objects are added to the list, Optionally do the Depth Test on the last frame's Shadow map for that light to determine if an update is in order. Alternatively the Shadow Casters could notify lights they affect when they change in any way. Again do Optional Depth Test to determine if there is any point in updating the shadow.

When a Light is Moved however, Just re-render it.

Note: Depth test should be done on last frame's position and new position of an object, to make sure that it was irrelevant in the old shadow map and is still irrelevant.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, they update automatically.

Is there a reason why this should be core and not an add-on in the asset library?

Rendering is Core.

@Calinou
Copy link
Member

Calinou commented Jan 29, 2022

Related to #3606.

It's been discussed in the past whether there should be a "freeze shadows" property for lights, which would make lights never check for shadow updates. This would make shadow rendering in scenes with dynamic objects faster, but dynamic objects wouldn't be able to cast shadows from those "frozen shadow" lights anymore.

@mrjustaguy
Copy link
Author

mrjustaguy commented Jan 29, 2022

"freeze shadows" is not what is proposed here, far from it. TLDR of this proposal is: "Don't Render Shadows if Nothing (of importance) changed within the Bounding Volume where the Light is rendering (ideally sphere for OL and Cone for SL)"

@lawnjelly
Copy link
Member

Afaik omnis and spotlights already don't re-render shadowmaps if there is no changes to the pairing, at least in 3.x (at least I came across that before on 3.x in 2019, I haven't looked at that since but I doubt much has changed).

Related to this proposal, rooms & portals already aggressively culls lights using occlusion culling (via portals). Well to be exact it does this for omnis and spots, directional lights don't lend themselves to portal occlusion, but they can be turned on and off using gameplay notifications, there is a section in the docs on this.

Raster occlusion for lights could probably be done (you'd have to ask @JFonS on his thoughts on this), especially for a static light where you could bake the depth, but I suppose the shadow map shaders are super cheap so you'd have to work against that. Mind you I guess it's the same situation with a depth reject for the main camera view though, it only tends to run the shader for the top most "winning" layer, and that's still a win, so it stands to reason it could help with shadows too. And especially for splits, if you could do the occlusion test once and reuse for all splits that would work in your favour.

I'm not sure you'd be able to use the GPU shadow maps themselves for occlusion culling, for the same reason raster occlusion is done on the CPU rather than reading back the depth buffer from the GPU - because of the delays between the two pipelines, or you'd get shadow popping I guess. So on the face of it you'd have to either render the shadow map on the CPU as well as the GPU for moving lights, but you could presumably pre-bake a non-moving light, (or read back from the GPU) where it becomes more viable.

There are some interesting ideas for speeding up shadows, but I guess two problems to overcome are:

  1. Making sure the performance benefit outweighs the added complexity / maintenance in the engine (and the usual paradigm is to favour generalist dynamic techniques, rather than faster static ones, even though I'm a fan of the latter)
  2. Assessing to find out whether this really is a bottleneck, versus e.g. the cost of drawing the shadows with PCF. At the end of the day you can decrease the size of the shadow maps and greatly reduce the fill rate.

So you are mainly looking at saving vertex shading and drawcalls. And there are other techniques which can help with this, like pre-baking / merging / pre-occluding shadow meshes. This is actually something I did do a bit of early on with rooms & portals but didn't finish off, got sidetracked.

@mrjustaguy
Copy link
Author

mrjustaguy commented Feb 2, 2022

Afaik omnis and spotlights already don't re-render shadowmaps if there is no changes to the pairing, at least in 3.x (at least I came across that before on 3.x in 2019, I haven't looked at that since but I doubt much has changed).

After some clean testing it does seem that this is right, and what the main part of the proposal is already (re)implemented and working in 4.0, It seems something was wrong with the scene I've been using for testing before where they were all treated as moving for some reason, albeit it's unclear why...
Maybe lots of objects cause a bug that makes it think something's changed with pairing? Doesn't seem To be the Case, Scene with More Lights and Objects isn't affected by the OG scene issue that prompted this proposal.
If not that I don't have a clue why they're equally expensive when moving and when static in the original testing, but It is a complex scene and might be just me messing something up having something changing that's forcing redraws then.. Will need to investigate further.

This means that Raster Occlusion Culling for light shadows is the only relevant part of this Proposal, and the main part is implemented but possibly found some edge case where it's bugging out, or I'm doing something wrong and not realizing it, I'll need to dig in further on that and open a bug report if I manage to replicate what's happening in a Clean project...

@mrjustaguy
Copy link
Author

Culprit was found, my Test Player was constantly moving, ever so slightly even when "standing" which was forcing constant updating of Omni and Spot Lights Shadows..

@JFonS
Copy link

JFonS commented Feb 3, 2022

Occlusion culling in master is just another check in the regular scene culling, so it should already handle Omni/Spot lights to some extent. The light's AABB is checked against the occlusion buffer and if it's considered hidden then nothing should happen (no rendering and no shadow updates).

Other than that, I think culling of individual shadow casters, where the light is visible but you cull objects that don't create visible shadows, would only make sense for directional lights. Positional lights tend to be small and affect very few objects, so the gains there would most likely not outweigh the cost of culling.

But in general, as Lawnjelly mentioned, it's hard to implement these kinds of techniques in a way that works well for a general game engine. Most of them assume certain characteristics of a scene or need to be tweaked specifically for a given game.

@mrjustaguy
Copy link
Author

Ok, well based on that I'm closing the proposal, as 1st half already existed and was just user error making me think it doesn't, and the 2nd half Probably doesn't make sense (not very easy to implement and questionable performance gains, and possible performance regressions), which is sort of expected the expected answer.

@Calinou
Copy link
Member

Calinou commented Feb 3, 2022

But in general, as Lawnjelly mentioned, it's hard to implement these kinds of techniques in a way that works well for a general game engine. Most of them assume certain characteristics of a scene or need to be tweaked specifically for a given game.

Note that if #3606 is implemented, it would be possible for users to implement this in their scripts for any light type.1 For instance, Area nodes or raycasts can be used to detect complex objects that don't cast visible shadows (e.g. because they're located below a roof).

Footnotes

  1. This can already be done to a limited extent with the GeometryInstance Cast Shadow property right now, but culled objects will stop casting shadows for all kinds of lights, not just a specific light.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants