-
-
Notifications
You must be signed in to change notification settings - Fork 479
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
Add vertex light support #382
base: master
Are you sure you want to change the base?
Conversation
Really solid work. Code looks all good to me. I tried it out and seems to work well from initial experiments. Could you push the tiling fail case? You could check in main.unity temporarily and we could revert it later, if that makes sense. |
Thanks. Done |
It may be because the light is set to Auto and unity is deciding to make it a pixel light. If i set the red light to be "Not Important" the issue goes away.. ? |
I just went through the frame debugger and Metal is a bit strange with lighting. I will have to test on Windows tomorrow. Hopefully, I can get the same as yours. |
Thanks for the above, I'm trying to wrap my head around things. I think that Unity will move lights between vertex and pixel depending on number of lights, importance, maybe intensity(?). And there is a limit of 4 lights per vertex (and the directional light seems to impact the result for me - when i turn it off the result changes - so it's counted in the 4?). If all that is true the story that my brain has constructed is that it is moving lights between vertex and pixel based on these factors, but this branch only has vertex, not pixel, so lights disappear. But I'm not 100% sure if that is consistent with what we're seeing / with the above? |
Thanks. Displacement tip is useful. After disabling attenuation: I have gone over the following again:
It is hard to say. The only thing I think I can do to confirm is to merge the pixel and vertex branch together, and max out the lights to see what happens. Not too difficult.
From what I have been experiencing, it hasn't been consistent with the manual. I didn't think Important lights would even render without pixel light support in the shader. I thought that Unity was populating the shader data based on pixel light logic since it knows we don't support extra passes (Not sure if that make sense. My ignorance might be showing). I have also seen the issue with lower amounts of lights. Lastly, when comparing Important and Not Important in the Frame Debugger, the light positions, world light and spherical harmonics are the same; the only thing changed is the light colours. I would think if the light disappears, then the light positions shouldn't include the same set of lights (although, it could be the case where a light is both a vertex light and SH leading me astray): I might have to make some reproducible cases with planes and a trimmed down shader. I'll let this sit in my mind for a bit. |
After testing in isolation, I will make a forum post or file a bug since the issue is unrelated to Crest. |
Finally got around to it: https://forum.unity.com/threads/vertex-lights-behaving-strangely.822300/ |
Update from Unity (12/03/2020):
|
Removing draft status. It could probably use more work which I can look into another time. Unity has given a conclusion:
We could implement it here and then recommend URP if anyone wants nice lighting. |
Multiplying gave bad results for ambient light in scattering. Works better with bubbles than addition.
6ef862b
to
f1c6e56
Compare
# Conflicts: # crest/Assets/Crest/Crest/Shaders/Ocean.shader # crest/Assets/Crest/Crest/Shaders/OceanEmission.hlsl # crest/Assets/Crest/Crest/Shaders/OceanFoam.hlsl # crest/Assets/Crest/Crest/Shaders/Underwater/UnderwaterCurtain.shader
c8c57e2
to
f5520cb
Compare
This will help address #357
There is also pixel light support with #383
Experimental support for vertex lights. Vertex lights are point lights which are set to Not Important or in some cases Automatic.
Night Time
About
Attenuation
The attenuation formula was taken from here. The
UNITY_LIGHT_ATTENUATION
doesn't work for vertex lights. TheShade4PointLights
, as discussed in the thread, didn't give good results.Shader Parameters
I've added some shader parameters so others for flexibility for the time being. Not sure if they will be merged. They are mostly there to separate the parameters for the primary direction light.
Notes
ShadeVertexLightsFull
cannot be used since it is for Legacy Vertex Lit.Issues
Tiling Artefacts
Tiling artefacts occur depending on the attenuation, lights overlapping and camera orientation. The frame debugger shows lighting values being filled. It could be the attenuation calculation or a limitation in vertex lighting. Further investigation required.
Underwater
I have come across this issue when trying to integrate a weather asset with cloud shadows. It is much worse at night. Not including lights might be the best solution. Or at least having it behind a toggle.
Missing