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

Vulkan: Light3D Reverse Cull Face not reimplemented yet #50005

Closed
mrjustaguy opened this issue Jun 29, 2021 · 12 comments · Fixed by #77238
Closed

Vulkan: Light3D Reverse Cull Face not reimplemented yet #50005

mrjustaguy opened this issue Jun 29, 2021 · 12 comments · Fixed by #77238

Comments

@mrjustaguy
Copy link
Contributor

mrjustaguy commented Jun 29, 2021

Godot version

v4.0.dev.custom_build [7d88c1a87]

System information

Windows 10, Vulkan, Nvidia Quadro K2000 461.92

Issue description

Reverse Cull Face doesn't do anything

Steps to reproduce

Make a scene with Shadows in 3.3.2 stable, toggle Reverse Cull Face, observe changes, then go do the same on current Master branch, observe lack of any changes

Minimal reproduction project

Culling.zip

@Calinou
Copy link
Member

Calinou commented Jun 29, 2021

This may be intended. Now that shadow normal offset bias/shadow pancaking has been implemented in the master branch, there's little point to using Reverse Cull Face anymore (which turns shadow peter-panning into light bleeding). You can now use much lower bias values than you previously could (usually 10 times smaller), so there is barely any peter panning visible anymore. Note that DirectionalLight3D currently uses a default bias that's too high due to a bug.

If this is the case, we should remove the Reverse Cull Face property from Light3D.

@Calinou Calinou changed the title Reverse Face Culling Not Implemented Vulkan: Light3D Reverse Cull Face not implemented Jun 29, 2021
@Calinou Calinou added this to the 4.0 milestone Jun 29, 2021
@mrjustaguy
Copy link
Contributor Author

mrjustaguy commented Jun 29, 2021

Actually, Reverse Cull Face helps with reducing visible Shadow Acne with very low biases, as the Shadow acne appears in the parts that are occluded by the mesh and is generally far easier to get looking great with as little artifacting visible to the end user as possible.

Here's an example in GLES 2

RCF On
RCF On
RCF Off
RCF Off

Please note that The settings are set up this way to Intentionally show the worst case, with all bias set to 0 and things like that.

The artifacting with RCF Off is very easy to un-intentionally get when going far from the camera, or lowering shadow map resolution, things like that.. on the other hand RCF On in my experience doesn't seem to be having any major issues from all the testing I've done and it "Just Works"
Honestly, When I figured that out, I was kinda curious why it wasn't on by default for directional lights (and possibly other lights too) considering how hard it is to get any -visible to the end user- artifacting.

@mrjustaguy
Copy link
Contributor Author

mrjustaguy commented Jun 29, 2021

Ok Correction, just tested it with Omni Lights, and it also removes some other artifacting that plagues Omni lights at high radiuses with shadows, and low bias issues for all Lights, just have to give a slight negative bias to prevent visible light line around a shadowed object on the floor, same goes for the Spot lights... The slight negative bias also adds a small, very hard to notice dark rim onto the shadow casters.

@mrjustaguy
Copy link
Contributor Author

mrjustaguy commented Jun 29, 2021

Here all lights in GLES 3:
RCF On
RCF On all
RCF Off
Screenshot 2021-06-29 204615

Only notable RCF On artifacts are when 2 planes that are parallel but opposite to one another are also very close to one another and one is casting shadow to the other which causes the slight Light leak at the bottom of the cube, which can be fixed by adding a low negative bias, which adds a dark rim to the edges of the cube, I'm unaware of any other issues and this one can be fairly simply avoided by having shadow casters slightly elevated from what they're casting to...

Conclusion: Removing RCF Would be bad, as it's a very useful option, and should be considered for being on by default, for a better experience, and Documented better.

@Calinou
Copy link
Member

Calinou commented Jun 29, 2021

Do not use a bias value of 0 when Reverse Cull Face is disabled. We should add a node configuration warning for this 🙂

When Reverse Cull Face is disabled, you should use a slightly positive bias. When Reverse Cull Face is enabled, you should use a slightly negative bias. We can't change this behavior in 3.x as it would break compatibility with existing projects.

@mrjustaguy
Copy link
Contributor Author

mrjustaguy commented Jun 29, 2021

Yeah this Bias 0 is not how to use it with RCF disabled, but these same artifacts do appear often even when tweaking stuff around, and worst of all, might not be visible while you're looking close to the actual light source, and appears further away from it, or in the case of Directional shadows, it appears in farther splits.

Bias was set to 0 to have an extreme case of Shadow Acne easily, without having to search for all the countless edge cases that appear with RCF off.

Here's an example of such an edge case, with an omni light with high radius and a high bias (higher the bias the less acne, but also the worse the Peter Panning issue, which isn't that clearly visible in this case) showing shadow acne with RCF disabled
Screenshot 2021-06-29 212813

@Calinou
Copy link
Member

Calinou commented Jun 29, 2021

Conclusion: Removing RCF Would be bad, as it's a very useful option, and should be considered for being on by default, for a better experience, and Documented better.

As I said, I'm not advocating for Reverse Cull Face to be removed in 3.x. I'm just saying it doesn't make much sense in master because shadow pancaking makes the issue hardly a problem in the first place. You just set the bias value once and it works (almost) everywhere.

@mrjustaguy
Copy link
Contributor Author

While shadows have improved significantly in master, I disagree with that statement, especially for directional lights.
Directional Lights are really tough to pull off right, because they can very easily get Shadow Acne in the farther away splits, while being totally fine close up. Also Shadow Acne increases as Shadow resolution Decreases, which totally helps with ending up with Shadow Acne on your distant splits, and if a person is testing at High Shadow res, like 16k, as "high" shadow res, and doesn't do all the testing done for high on medium (8k) and low (4k) shadow res, users using said lower resolutions could have a significantly poorer experience due to Shadow Acne issues.

While Reverse Cull Face Might not be a Must have & Must use in 4.0, it doesn't mean it isn't useful, and that it should be removed, as the ease of use with it is undeniable, because instead of having to worry a ton about both Shadow Acne and Peter Panning, you instead worry mostly about the light/dark rims (depending on bias) around the shadow caster if the shadow caster is on or intersecting the shadow recipients.

What should be done with 4.0 regarding Reverse Cull Face is improving the documentation around it, explaining how it changes things, and why one should or shouldn't use it
Also as a side note, as bias is now positive only, with Reverse Cull Face it should multiply the bias by -1 to get a negative result from biasing.

@Calinou
Copy link
Member

Calinou commented Jun 30, 2021

Directional Lights are really tough to pull off right, because they can very easily get Shadow Acne in the farther away splits, while being totally fine close up.

This is what increasing Bias Split Scale is for 🙂

Also, godotengine/godot-proposals#2729 will help with this as lower shadow blur makes shadow acne less visible.

@mrjustaguy
Copy link
Contributor Author

Bias Split Scale isn't in 4.0, it's in 3.3.2, 4.0 Has Pancakes... Blur does help produce shadow acne, and it is also helping with the issue in the distant splits, but the main issue is that distant splits are just such low effective resolution that Shadow Acne just Creeps in real easy.

Without RCF you are balancing Peter Panning with Distant Shadow Acne, and either way you put it you get quite visible issues, be it the Peter Panning in your close up splits, or Shadow Acne far away and it's simply more complicated to get decent results and be sure that they're going to hold up well under many different viewing positions

With RCF no need to worry about Shadow Acne and Peter Panning (aside from the slight stuff visible close up, the rims between Shadow caster and shadowed mesh) and you're free to play toy around with all the different split locations, resolutions and stuff, and if you manage to get things looking artifact-less close up, it's almost guaranteed that far away things will still look great.

@Calinou
Copy link
Member

Calinou commented Jun 30, 2021

reduz said that Reverse Cull Face is intended to be supported in master:

<reduz> @Calinou it is not intended though, its likely a bug, there are cases where you still want to reverse it
<reduz> or make it double sided
<Calinou> I guess double-sided makes sense, but this is implemented on a per-mesh basis rather than a per-light basis (in `3.x` at least)
<reduz> @Calinou it kinda may make sense per light in some cases too

@Calinou Calinou added bug and removed discussion labels Jun 30, 2021
@Calinou Calinou changed the title Vulkan: Light3D Reverse Cull Face not implemented Vulkan: Light3D Reverse Cull Face not reimplemented yet Jun 30, 2021
@clayjohn clayjohn modified the milestones: 4.0, 4.x Feb 22, 2023
@markusneg
Copy link
Contributor

Just wanted to leave another use case in which revert cull faces are almost a Must-have: A 2D top-down scene with 3D geometries and shadows. Even with pancaking and well tuned bias settings, thin structures will leave a visible gap on the ground due to peter panning. With reverse cull face enabled, zero bias settings and ultra-precise shadows are possible because the shadow acne appears on the never visible back of the geometries. Of course this is only possible if no self-shadowing is needed (e.g. a terrain topology casting shadows onto itself).

@akien-mga akien-mga modified the milestones: 4.x, 4.1 May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants