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

Shadows for Sprite3D with Billboard enabled always face the Vector3(0, 0, 0) rotational degree angle #41420

Closed
finalwarp opened this issue Aug 21, 2020 · 19 comments · Fixed by #72638

Comments

@finalwarp
Copy link

finalwarp commented Aug 21, 2020

I saw other issues like #27738 that is similar, but it's not exactly the same.

Godot version:

3.2.2

OS/device including version:

Windows 10. GLES3

Issue description:

Hello,
When I have Sprite3D objects with Billboard enabled, the shadow is not facing the correct angle based on the DirectionalLight. It always faces the default Vector3(0, 0, 0) rotational degree angle of the Sprite3D, even if I rotate the object.
The image below has 8 Sprite3D.
Row 1 objects have Billboard enabled. Row 2 objects have Billboard disabled.
Column 1 have a rotational vector3 at (0, 0, 0) on both rows.
Column 2 have (0, 90, 0).
Column 3 have (0, 180, 0).
Column 4 have (0, 270, 0).
I would expect the shadow at the very least, to be drawn based on the objects rotational degree if Billboarding isn't supposed to work. I'm not very familiar with billboarding, but should the shadows behave like this?

image

Minimal reproduction project:

Billboard Shadow Test.zip

@berarma
Copy link
Contributor

berarma commented May 7, 2021

I also have this issue. When billboard is set the sprite always faces the current camera but also every light source when rendering shadows. This behavior doesn't agree with the documentation and will produce incorrect shadows most of the time.

There was a similar issue with billboard meshes that was fixed but not this case.

@clayjohn
Copy link
Member

clayjohn commented May 7, 2021

In 3D graphics, when calculating shadows you render the scene from the perspective of the light source and only right out the distance to each pixel. This creates a "shadowmap" which can be used during rendering to determine when a pixel is in shadow. The trouble is that billboarding (in SpatialMaterials or in Sprite3Ds) just makes the sprite face the camera. In the case of rendering the sprite, the camera is the regular camera, but when rendering the shadow the camera is from the perspective of the light source.

To fix this, I think we need to add a condition the the SpatialMaterial so that when billboarding is enabled the camera matrix gets passed in as a uniform. This way the same camera matrix can be used for regular rendering and shadow rendering.

@Calinou
Copy link
Member

Calinou commented May 16, 2021

See also #27738.

@Drakula44
Copy link

Hello. I am new to contributing to Godot. Could I take a look at this?

@clayjohn
Copy link
Member

@Drakula44 yes please, go right ahead!

@eruhlinteractive
Copy link
Contributor

To fix this, I think we need to add a condition the the SpatialMaterial so that when billboarding is enabled the camera matrix gets passed in as a uniform. This way the same camera matrix can be used for regular rendering and shadow rendering.

Hey there, I'm new to contributing to Godot and would like to give this issue a try using the approach mentioned by clayjohn (above). I believe that I have found where the billboard flags are being set, but I wasn't able to locate where the camera matrix is used to render shadows, or where that new uniform should be utilized. Any advice one where I could find this or how it should be applied?

@Calinou
Copy link
Member

Calinou commented Mar 24, 2022

Note that this issue should be tested in master to see if it's still present in the Vulkan renderer.

If it's still present in master, then check in https://github.com/godotengine/godot/blob/master/scene/resources/material.cpp which is where the StandardMaterial3D shader code is generated (formerly SpatialMaterial).

If the issue is not present in master, then check in the 3.x branch's https://github.com/godotengine/godot/blob/3.x/scene/resources/material.cpp which is where the SpatialMaterial shader code is generated.

The above shader code is in the Godot shader language, not raw GLSL. The raw GLSL rendering code for 3D materials is in:

@eruhlinteractive
Copy link
Contributor

Awesome thank you! I was able to recreate the issue in master, using both the converted reproduction project and a recreation of it from a fresh project file.

@Calinou Calinou added this to the 4.0 milestone Mar 24, 2022
@devloglogan
Copy link
Contributor

This is still an issue on Master and it appears nobody is working on it so I'll be taking a crack at this using clayjohn's recommended approach.

@Calinou
Copy link
Member

Calinou commented Aug 28, 2022

This is still an issue on Master and it appears nobody is working on it so I'll be taking a crack at this using clayjohn's recommended approach.

Sounds good to me 🙂

@KnedlikMCPE
Copy link

I might be taking a look at this later today if no one has anything against it and if it's still present - seems that no one is working on it since August

@devloglogan
Copy link
Contributor

It certainly has been a fast six weeks. I have been meaning to make more progress on this but if you beat me to an actual PR I take no issue with it!

@Neczesk
Copy link

Neczesk commented Nov 18, 2022

I noticed that no PR has been linked and it's been quiet so I'm going to take a look as well. If someone reads this in another 6 weeks and says the same thing, hello from the past!

@KnedlikMCPE
Copy link

Since this is fixed on master, is there still a reason to leave this issue open?

@clayjohn
Copy link
Member

clayjohn commented Feb 5, 2023

Since this is fixed on master, is there still a reason to leave this issue open?

This issue hasn't been fixed yet. Once a PR that fixes it is merged, it will be closed.

@ecmjohnson
Copy link
Contributor

I was going to close this issue with a 3.x fix, which I have working. I'm thinking to hold off on the PR for 3.x since any review change in master will also affect it due to the approach being identical

@Zireael07
Copy link
Contributor

FYI a 3.x fix won't close the issue - it has to be fixed in master to be closed.

@clayjohn
Copy link
Member

clayjohn commented Feb 7, 2023

I was going to close this issue with a 3.x fix, which I have working. I'm thinking to hold off on the PR for 3.x since any review change in master will also affect it due to the approach being identical

That's a good idea.

@7fantasy7
Copy link

May this be cherry-picked for 4.2?

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.