-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
GLTF Punctual Lights don't import properly when Physical Light Attenuation is enabled #54596
Comments
Physical light attenuation only affects rendering, and it affects all point lights at once. Light properties stored within each node remain identical, as you can still adjust each light's attenuation factor individually. For performance reasons, there is no per-light setting for toggling physical light attenuation. (This is also the case in 4.0 where physical light attenuation is always enabled.) Therefore, it's expected that lights' default attenuation values don't change if you enable physical light attenuation then import a glTF scene. |
I get what you're saying. I've diagnosed the issue incorrect, however it still remains, if you turn off Physical Light Attenuation on a scene imported from GLTF, you basically see no punctual lights unless you drop the attenuation drastically. In the attached example an attenuation of 800 isn't visible, you have to drop it below 1.0 to see lights. Is it possibly due to the fact that the range is set to 4096 affecting the falloff or something? |
Typical light attenuation values for Godot are between 0.0 and 4.0. Values greater than 4.0 will fade out very quickly when not using physical light attenuation. There's the question of whether Godot should clamp glTF light attenuation values to reasonable values, but this is technically a compatibility-breaking change. cc @fire |
For reference, the gtlf file in question is this: https://github.com/KhronosGroup/glTF-Sample-Models/tree/adf2abe718a61a6859b29fab481bfbbdfc05df24/2.0/Lights Where the intensity of the point light is 5. When imported it becomes an attenuation of 800+. |
We didn't choose the values, they were curve fitted by @aaronfranke. I think you can only pick ones set of curves. Picked either the 3.x curve or the 4.0 curve. |
I guess the 4.0 curve should be used at import-time if physical light attenuation is enabled in the Project Settings. |
Here are where the formulas were changed for the 4.0 curve: https://github.com/godotengine/godot/pull/53109/files#diff-fdf33f6fcd2b129d540ebd0e70fd04982b68e279cebfd9e96036d2fd7561c31f |
So it looks like I may have to open an issue for this, as I'm still troubleshooting it, but I believe this infinite range is preventing lights from getting properly culled with the new rooms and portals. Here it says sprawling of lights is determined by "the dimensions and direction of the light". I assume it's taking into account the range value of omni lights to determine (at least partially) how many VisualInstances they will impact. I'm effectively seeing that none of my gltf imported lights are getting culled. |
@timshannon GLTF defines lights like physical lights which have infinite range, therefore the GLTF import code will assume an infinite range when importing the data. However, once this data is used to create a node, it will be clamped to the limits of Godot's nodes, so it will have a range of 4096. You can manually modify this value as needed for your game. |
That makes sense, however the problem is that range and attenuation are related, so it basically means there is no "usable" information coming over from GTLF if I want to use portal culling. I have to basically set all the lights by hand in Godot after setting them in Blender or other GLTF source. I'm not sure what the best solution is here. |
Just finished manually setting all the light ranges and attenuation to something that looks appropriate, and my FPS literally went from mid 70's to 600+. I'm sure the light range affects performance everywhere not just with culling. Am I completely misunderstanding importing lights from GLTF files? I get that it's "technically" correct to set it to 4096, but is it in any way useful in a game engine? |
As I was aiming for physical units lets poll my peers. |
This issue has gotten off track a bit, so if you want me to open a separate issue or proposal, let me know. Even something as simple as a warning that your light range is maxed out would better than the current situation. |
I would prefer having less physically accurate lights but with better performance. Huge attenuation values should be avoided – in fact, we should probably add a configuration warning if a light's attenuation is set to a value over 50. |
@timshannon If you want a solution that doesn't involve changing the lights in Godot, you could set the range property of the lights in the GLTF files. Godot's GLTF import code will automatically adjust the attenuation as appropriate. https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_lights_punctual/README.md#light-shared-properties If we try to automatically infer the range from the brightness, I'm not sure at what point we should make the light drop to zero. Users would likely need to adjust it either way. |
Fixed by #62747. |
Godot version
3.4-rc3
System information
Ubuntu 20.04.3 LTS
Issue description
When the new Physical Light Attenuation is enabled in a project, punctual lights from GLTF files don't show up properly. It appears the range and attenuation settings are set the same as if Physical Light Attenuation is not enabled. I would expect the GLTF importer to take the project settings into account, otherwise you need to manually set all lights for any new scene.
This may not be considered a bug, so can resubmit as a proposal if needed.
Steps to reproduce
Start a new project, enable "Use Physical Light Attenuation" in the project settings.
Import a GLTF file with lights (https://github.com/KhronosGroup/glTF-Sample-Models/tree/adf2abe718a61a6859b29fab481bfbbdfc05df24/2.0/Lights).
Minimal reproduction project
test.zip
The text was updated successfully, but these errors were encountered: