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

Generated light probes avoid StaticBody3Ds #83420

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grenappels
Copy link
Contributor

Problem

When the LightmapGI generates light probes, a lot of those light probes end up inside objects and obscured from any light at all. This used to be a huge problem because they would glitch and end up with odd colored lighting, but this was fixed recently, and now they just end up totally black.

This is still not ideal though, having lots of black light probes in your scene produces very weird lighting especially when they are close to the edges of objects:

black_probes
black_probes_2

black_probes_present_420.mp4

Solution

It's not perfect, but we can make an assumption that probably StaticBody3Ds are usually objects that are visually solid and blocking light, like walls and floors, and do an intersect_point check against them when deciding whether to place a generated light probe. In my case this solved the problem entirely:

black_probes_gone
black_probes_gone_2

black_probes_gone_420.mp4

Still, there might be situations where you don't want a static body to block generated light probes, like if you have invisible walls. For that case, I added a probe_ignore_layer variable that defaults to 0. If it's set to a layer number 1-32, any static bodies in that layer will not block generated light probes. If it is set to -1, then all layers are ignored and the current behavior will remain.

Additionally, while poking around in lightmap_gi, I noticed there is code to avoid generating probes too close to manually placed probes, but it was doing an is_equal_approx check against the two positions instead of a distance check, so this was never being triggered. I changed it to a distance check so the behavior is as expected now:

probes_by_approx
probes_by_dist_good

@jcostello
Copy link
Contributor

Excelent fix. But it requires for a mesh to live inside a static body? Why is that? does it need collision?

@grenappels
Copy link
Contributor Author

Thanks! And yes, I'm just grabbing all StaticBody3Ds in the scene and doing an intersect_point check against them. I know it's not going to account for geometry that obscures light but doesn't have a collider on it, but I don't know how to do an implimentation like that.

@jcostello
Copy link
Contributor

The PR works but is not the best way to check if its inside a mesh. In my test scene I created the collitions by using

image

The problem is that some part that seems to be inside a mesh still generate probes. It won't cause the black probes problem (maybe) but it will generate unecesary probes

Inside walls of sponza
image

@jcostello
Copy link
Contributor

Additionally, while poking around in lightmap_gi, I noticed there is code to avoid generating probes too close to manually placed probes, but it was doing an is_equal_approx check against the two positions instead of a distance check, so this was never being triggered. I changed it to a distance check so the behavior is as expected now:

This part works

@jcostello
Copy link
Contributor

In some parts I still see black probes generated

image

@akien-mga akien-mga added this to the 4.x milestone Oct 16, 2023
@akien-mga akien-mga requested a review from a team October 16, 2023 06:30
@akien-mga
Copy link
Member

CC @DarioSamo @Calinou

@grenappels
Copy link
Contributor Author

@jcostello thanks for testing! It seems like maybe intersect_point doesn't work for concave collision shapes because there is no "inside" to them.

Still, I think this solution is valuable to those using Qodot for example, which generates level geometry using all convex shapes.

I'll do some testing myself and update the documentation to mention this concave shapes exception when I get a moment.

exists = true;
break;
}
}

// check for collision with static geo, unless we're ignoring all layers
if (probes_ignore_layer != -1) {
auto space_state = get_world_3d()->get_direct_space_state();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that when you mark this ready for review you need to change this, as auto is not allowed

@jcostello
Copy link
Contributor

@jcostello thanks for testing! It seems like maybe intersect_point doesn't work for concave collision shapes because there is no "inside" to them.

Still, I think this solution is valuable to those using Qodot for example, which generates level geometry using all convex shapes.

I'll do some testing myself and update the documentation to mention this concave shapes exception when I get a moment.

Maybe you can split this part that works well in a separate PR

Additionally, while poking around in lightmap_gi, I noticed there is code to avoid generating probes too close to manually placed probes, but it was doing an is_equal_approx check against the two positions instead of a distance check, so this was never being triggered. I changed it to a distance check so the behavior is as expected now:

Also, maybe if a probe is behind back curl faces we can discard it, or if its pure black (maybe not the best), or maybe there is an algorithm to check inside a mesh

@Calinou
Copy link
Member

Calinou commented Oct 16, 2023

Thanks for opening a pull request 🙂 This helps a lot with visuals indeed!

I would prefer using the visual mesh instead of collision data. Also, if the collision data has to be convex, this unfortunately makes the PR useless for my use cases (convex shapes are usually not a good idea for static level collision). I'd wager a majority of Godot users won't be able to benefit from this PR due to the way it's currently implemented.

The distance check part looks good and could be implemented in its own PR.

@grenappels
Copy link
Contributor Author

I agree that using the visual mesh and accounting for concave shapes is probably necessary. I have some ideas for this, mostly inspired by how this unity asset seems to be working. I won't work on this myself yet, since my fix does work for my use case, but it's an idea if anyone wants to pick up where I'm leaving this off.

As for the proximity fix, I will make that a separate PR. Thanks for the feedback everyone!

@Calinou
Copy link
Member

Calinou commented Jan 22, 2025

Regarding how to detect concave collision shapes from each light probe, you should be able to replace intersect_point with several intersect_rays that are rotated in various directions, and detect if all of them have a collision normal that is Vector3(0, 0, 0) (make sure hit_from_inside is enabled in PhysicsRayQueryParameters3D).

I have a mostly working prototype here: https://github.com/Calinou/godot/tree/lightmapgi-probe-avoid-solid-geometry

It still generates some probes very close to the geometry's edges from the inside though. There are a few outstanding TODOs as well before this can be in a PR-able state. See the commit message for more details. If you can lend a hand, please do 🙂

Testing project: test_probe.zip

Screenshot_20250122_165954 png-lossy-q100 webp

Screenshot_20250122_165947 png-lossy-q100 webp

Screenshot_20250122_165941 png-lossy-q100 webp

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

Successfully merging this pull request may close these issues.

5 participants