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

OccluderInstance3D baking does not ignore ShaderMaterials that are transparent #91030

Open
Tracked by #70533
Gamepro5 opened this issue Apr 22, 2024 · 8 comments · May be fixed by #93371
Open
Tracked by #70533

OccluderInstance3D baking does not ignore ShaderMaterials that are transparent #91030

Gamepro5 opened this issue Apr 22, 2024 · 8 comments · May be fixed by #93371

Comments

@Gamepro5
Copy link
Contributor

Tested versions

4.2.2-stable

System information

Windows 10 (I don't think this matters at all)

Issue description

According to @Calinou, the OccluderInstance3D only checks for if a SurfaceMaterial has the transparency flag. Instead it should read the cull mode defined in the Shader, which should be even easier to read, according to him.

This should also be applied to the Lightmapper.

CC @Calinou @clayjohn

Steps to reproduce

Create a MeshInstance3D with the shader included as the MRP as the material. Observe as it fails to be ignored by the OccluderInstance3D

Minimal reproduction project (MRP)

glass.zip

@Calinou Calinou changed the title OccluderInstance3D does not ignore ShaderMaterials that are transparent. OccluderInstance3D baking does not ignore ShaderMaterials that are transparent Apr 22, 2024
@github-project-automation github-project-automation bot moved this to For team assessment in Rendering Issue Triage May 23, 2024
@clayjohn clayjohn moved this from For team assessment to Up for grabs in Rendering Issue Triage May 23, 2024
@ChristopheClaustre
Copy link
Contributor

I have low contribution atm but as it is tagged as complexity low in Rendering Issue Triage and if it is okay with you, I will try to fix this.

@ChristopheClaustre
Copy link
Contributor

ChristopheClaustre commented Jun 11, 2024

According to @Calinou, the OccluderInstance3D only checks for if a SurfaceMaterial has the transparency flag. Instead it should read the cull mode defined in the Shader, which should be even easier to read, according to him.

I confirm that actually it only checks the transparency flag.
About the shader's cull mode I only found the BaseMaterial3D::CullMode which is :

enum CullMode {
	CULL_BACK,
	CULL_FRONT,
	CULL_DISABLED,
	CULL_MAX
};

It don't seem linked to shader transparency to me, can anyone explain me where I lost myself ?

@clayjohn clayjohn moved this from Up for grabs to In progress / Assigned in Rendering Issue Triage Jun 11, 2024
@clayjohn
Copy link
Member

It don't seem linked to shader transparency to me, can anyone explain me where I lost myself ?

I'm a bit confused about the way the OP is worded as well. Transparency and culling are two very different concepts. Right now OccluderInstances don't do any backface culling, so I'm not sure why the backing would take culling into account.

@ChristopheClaustre
Copy link
Contributor

I agree with the issue that there is a problem with occluder and the shader given as MRP.
The shader is transparent but the occluder doesn't ignore it and hide objects behind it from the camera POV.
So maybe we should ignore the cull mode and think of a way to tell if a shader will return alpha or not ?

Maybe @Calinou can explain us more about the discussion they had with OP ?

@Calinou
Copy link
Member

Calinou commented Jun 12, 2024

I don't see why cull mode was mentioned either in OP.

I think what OP wants is to have the occlusion culling baker read the ShaderMaterial's source code and check if there is anything that would force the material to be transparent, such as assigning ALPHA.

@ChristopheClaustre
Copy link
Contributor

I have the impression that it's quite hard to know if a shader code sets alpha to another value than one (at least for one pixel) and be exhaustive.
I checked the code that determine if shader should be done in alpha pass or not and its quite complex to determine... Will try to get the result of this choice in occluder and check if we should keep it or not for baking.

@Gamepro5
Copy link
Contributor Author

I apologize, I am not as well versed in shader terminology as I'd like to be. This PR was the result from a conversation I had in the Godot rocket chat about why a specific shader (glass.zip) isn't being ignored when it comes to occlusion culling. The shader imitates a transparency effect without directly setting it, so we assumed there was something else in the shader that was giving a transparency effect. I understood that it was CULL MODE but I may have been wrong. Either way, there is something about this shader that makes it see-through and it should be ignored automatically by the engine's occlusion culling system.

@ChristopheClaustre
Copy link
Contributor

Cull mode exist mainly to cull face when they are back to the camera to prevent some artefacts and reduce draw calls. You could use it to simulate a glass but only on one side of the glass. Weird 😅.

I read the shader and it simply sets the alpha in the fragment stage, so output will have transparency.
Then godot check the content of the shader and deduce if it should be done in the alpha pass or not to manage draw orders.

I am trying something. I will propose a PR soon I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress / Assigned
Development

Successfully merging a pull request may close this issue.

4 participants