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: Per-vertex shading not working #43093

Closed
AlzoPalzo opened this issue Oct 26, 2020 · 25 comments · Fixed by #83360
Closed

Vulkan: Per-vertex shading not working #43093

AlzoPalzo opened this issue Oct 26, 2020 · 25 comments · Fixed by #83360

Comments

@AlzoPalzo
Copy link

Godot version:
v4.0.dev.calinou.e16729a8c

OS/device including version:
Windows 10, GTX 1070, Vulkan

Issue description:
In Godot 3.2 I used Tree models with their normals edited in blender in tandem with per vertex shading to create smoothly shaded trees. In Godot 4.0, with vulkan backend switching the shading between per vertex and per pixel has no effect on the tree models

Examples of the expected result are shown below where the trees on the left are shaded per pixel and the trees on the right are shaded per vertex.
treeShadingExample2

treeShadingExample

Steps to reproduce:
Add a directional Light or other lights to the models and in the material turn on the per vertex shading flag(3.2) or switch shading to per vertex(4.0)

Minimal reproduction project:

LowPolyTrees.zip

@Calinou Calinou added this to the 4.0 milestone Oct 26, 2020
@Calinou Calinou changed the title Per vertex shading not working in 4.0 (Vulkan) Vulkan: Per-vertex shading not working Mar 31, 2022
@r0-che
Copy link

r0-che commented Oct 15, 2022

vertex shading still does not work in 4.0

3
godot 3.5, vertex shading on the left
4
godot 4.0 beta, vertex shading on the left aswell

@AnalogFeelings
Copy link

AnalogFeelings commented Jan 16, 2023

It seems it's also happening in the gl_compatibility and mobile renderers. I tried using the shaders from godot-psx on all 3 renderers and none of them have vertex lighting, only pixel lighting.

(Windows 11, RTX 2060, Godot 4.0 Beta 12)

@zCubed3
Copy link
Contributor

zCubed3 commented Jan 17, 2023

I've noticed 2 things regarding vertex lighting in 4.X, I hope it can be of help to someone more talented with graphics programming. (I'll note these are just my observations, I know nothing about how Godot's rendering pipeline works)

  1. Looking at 3.X, I can see that scene.glsl contains the USE_VERTEX_LIGHTING flag. While in the master branch, scene.glsl (and its equivalents) are completely missing the flag.
  2. It also appears that from the C++ side of things that USE_VERTEX_LIGHTING is never set anymore. The flag doesn't exist anymore in material.h either?

@clayjohn
Copy link
Member

I've noticed 2 things regarding vertex lighting in 4.X, I hope it can be of help to someone more talented with graphics programming. (I'll note these are just my observations, I know nothing about how Godot's rendering pipeline works)

  1. Looking at 3.X, I can see that scene.glsl contains the USE_VERTEX_LIGHTING flag. While in the master branch, scene.glsl (and its equivalents) are completely missing the flag.
  2. It also appears that from the C++ side of things that USE_VERTEX_LIGHTING is never set anymore. The flag doesn't exist anymore in material.h either?

Vertex lighting hasn't been re implemented yet in 4.x

@zCubed3
Copy link
Contributor

zCubed3 commented Jan 17, 2023

👍 Thanks, good to know.

@pozitiffcat
Copy link

When it will be implemented? 20 fps on mobile devices with only one direct light is no good ((

@Calinou
Copy link
Member

Calinou commented Feb 2, 2023

When it will be implemented? 20 fps on mobile devices with only one direct light is no good ((

This won't be done for 4.0 due to time constraints.

If performance is an issue, consider decreasing the 3D rendering resolution using the Scaling 3D > Scale advanced project setting (0.5 is a good value for mobile). This will often benefit performance more than switching to vertex shading, which comes with a lot of limitations.

@TaraSophieDev

This comment was marked as off-topic.

@GithubPrankster
Copy link

Could this issue be looked into now that 4.0 is out and 4.1 is nearing? It reduces the feasibility of retro 3D work in the current iteration of the engine. I personally would be happier with a proper solution to #39513 but I'll take anything at this point.

@Calinou
Copy link
Member

Calinou commented May 27, 2023

Could this issue be looked into now that 4.0 is out and 4.1 is nearing?

There's no ETA for implementing this feature, and nobody has looked into it yet to my knowledge. Feature freeze for 4.1 is approaching, so this won't be merged before 4.2 at the earliest.

@clayjohn clayjohn modified the milestones: 4.1, 4.x Jun 13, 2023
@MicrotonalMatt
Copy link

Any update on ETA? I just tried with 4.2-dev4 and still not working. This and #4619 are deal breakers for moving my project from 3 to 4 unfortunately.

@Calinou
Copy link
Member

Calinou commented Sep 9, 2023

Any update on ETA?

Same as before, as rendering contributors are busy with other tasks currently. #66030 is also stalled because the PR author no longer has time to work on it.

@golddotasksquestions
Copy link

golddotasksquestions commented Oct 14, 2023

I just want to state that I think this needs to be given much higher priority.

One of Godots most prominent usecases is 3D retro games. PS1 style games. 3D games that rely heavily on classic vertex lighting. The internet is filled with free and paid tutorials and complete courses which teach how to create Godot games in that style. Many of Godots most famous games are made in this style. People come to Godot knowing it's not a AAA engine, but they expect to be able to do 3D retro games with per-vertex lighting.

This is not a replacement for per vertex lighting, but if you have to use Godot 4.X over Godot 3.X, you could try to use the Standard Material Diffuse mode Lambert Wrap or Toon . It may be enough for some usecases. If you try this, I recommend to also play with the DirectionalLight3D Directional Shadow settings to adjust the shadow gradients.

@AThousandShips
Copy link
Member

AThousandShips commented Oct 14, 2023

Anyone willing and able to fix this is welcome to, we can't order people to just fix one specific thing

@golddotasksquestions
Copy link

golddotasksquestions commented Oct 14, 2023

we can't order people to just fix one specific thing

a) Everyone knows people who contribute for free on their own time out of their own ambition can't be ordered or forced to work on anything specific. You can't tell me what to do with my free time, and neither can I. Insinuating anyone would assume otherwise is honestly pretty offending.

b) Also everyone knows the Godot core devs and maintainers are able promote specific issues, bring attention to them and ask for help or if anyone would like to take this on, and offer support. This has been done countless times before (for example on various social media channels of core devs and maintainers). Often very successfully.

c) Also everyone knows the Godot Foundation is able to hire people to work on on specific issues. This has also been done before countless times. Also everyone knows Godot Foundation has received a massive uptick in donations recently due to the Unity debacle.

@AThousandShips
Copy link
Member

AThousandShips commented Oct 14, 2023

I'm sorry if pointing that out is offensive, I meant no offense, but I see no other way of "prioritising" it if no one is willing or able to work on it

I don't see this as not being promoted or prioritised, seeing the attention it has and is getting

Have a nice day

@golddotasksquestions
Copy link

golddotasksquestions commented Oct 14, 2023

I don't see this as not being promoted or prioritised, seeing the attention it has and is getting

I don't see any of the core devs with a large following promoting this issue on their socials, or the foundation taking some of the money in their hands and offer to hire someone to fix it.

Such a promotion could be shared on the official Godot twitter with ~100K followers, and by Remi and Juan who has a large following with technically skilled devs. There could also be a separate tab on https://godot.foundation/ next to "Donations" saying "For hire", where prioritized Github issues are linked. They could also post and promote on the Godot subreddit with 142K subscribers and the Godot Discord server. In such posts links to the contributor chat rendering channel could be included and members of the rendering team mentioned to be addressed in the contributor chat for support/review of the PR.

So I think there is still a lot of room to officially prioritize & promote this issue.

@ywmaa
Copy link
Contributor

ywmaa commented Oct 14, 2023

@golddotasksquestions well I need this feature too, so I am going to try to do it, though I am new to rendering.

@ywmaa
Copy link
Contributor

ywmaa commented Oct 14, 2023

I looked at the material.cpp and material.h, I saw vertex shading parameters, so I guess I need to only do the rendering backend, right ?

@AnalogFeelings
Copy link

I looked at the material.cpp and material.h, I saw vertex shading parameters, so I guess I need to only do the rendering backend, right ?

I am pretty sure the entire framework for it is already done, but the shaders lack the code required to handle vertex lighting.

@ywmaa
Copy link
Contributor

ywmaa commented Oct 14, 2023

I looked at the material.cpp and material.h, I saw vertex shading parameters, so I guess I need to only do the rendering backend, right ?

I am pretty sure the entire framework for it is already done, but the shaders lack the code required to handle vertex lighting.

Yeah that's true, atleast for GLES3

I looked at the scene.glsl and it looks like it only needs to add the Vertex Shading pass and we are good to go (Working on it).

Does vulkan use this shader too ?

@Calinou
Copy link
Member

Calinou commented Oct 14, 2023

Does vulkan use this shader too ?

Forward+ uses scene_forward_clustered.glsl, Mobile uses scene_forward_mobile.glsl. Both include scene_forward_lights_inc.glsl.

@ywmaa
Copy link
Contributor

ywmaa commented Oct 15, 2023

My draft PR
#83360

@Calinou
Copy link
Member

Calinou commented May 15, 2024

I made MRPs to test shading mode performance in 3.x and 4.x:

This is a situation where per-vertex shading gives you nearly identical visuals, but can greatly improve performance in situations where you have lit particles close to the camera.
In games, there are two common situations where this occurs:

  • In a racing game, you race on a wet track with rain/water particles from other cars in front of you. This gets particuarly noticeable in interior cameras. An example of this is the F1 series, to the point where the most popular benchmark scenario is a rainy track with 20 cars and the player being last.
  • In a FPS, you walk through a corridor filled with smoke. An example of this is in Doom 2016 (the game uses per-vertex shading on subdivided quads for shading particles, with automatic subdivision depending on distance).

Many people use vertex shading for stylistic reasons, but personally I want it for performance reasons first and foremost. This is doubly true on mobile and integrated graphics – I only tested a top-end dedicated GPU here and the difference is already pretty noticeable. You can imagine it only gets that much more important on low-end GPUs 🙂

Results with the MRPs on a RTX 4090 in 4K for reference:

3.5.2 GLES3

Imgsli

Per-pixel Per-vertex Unshaded
1449 FPS (0.69 mspf) 1842 FPS (0.54 mspf) 1936 FPS (0.52 mspf)

4.3.dev5 Forward+

Imgsli

Per-pixel Per-vertex Unshaded
856 FPS (1.17 mspf) N/A 1875 FPS (0.53 mspf)

4.3.dev5 Compatibility

Imgsli

Per-pixel Per-vertex Unshaded
2105 FPS (0.48 mspf) N/A 3259 FPS (0.31 mspf)

If done well, a 4.x implementation of per-vertex shading should offer performance between per-pixel shading and unshaded (hopefully closer to unshaded than per-pixel), like in 3.x.

That said, if we choose to sample shadow maps on a per-pixel basis on per-vertex materials, this will incur a performance cost so it won't be as fast as in 3.x. Doing so will greatly improve vertex shading usability, as in 3.x, you can't see shadow maps on vertex-shaded materials. There is a separate "do not receive shadows" flag that already exists in BaseMaterial3D, so you could enable that if you really need to skip shadow map sampling.

@akien-mga
Copy link
Member

Fixed by #83360.

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

Successfully merging a pull request may close this issue.