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

DirectionalLight3D currently has no Specular property #80737

Closed
jonathon-green opened this issue Aug 17, 2023 · 3 comments · Fixed by #83917
Closed

DirectionalLight3D currently has no Specular property #80737

jonathon-green opened this issue Aug 17, 2023 · 3 comments · Fixed by #83917

Comments

@jonathon-green
Copy link

jonathon-green commented Aug 17, 2023

Note: This issue was transferred from the godot_docs repository. What appeared to be an issue with the documentation turned out to be an issue with the feature itself. clayjohn has commented below a bit of background. Ultimately, the ghost documentation highlights that we removed a setting by mistake. The solution is to restore the setting.

Your Godot version: 4.1 Stable

Issue description: DirectionalLight3D missing Specular property, current documentation states this should exist

URL to the documentation page:
https://docs.godotengine.org/en/stable/tutorials/3d/lights_and_shadows.html
https://docs.godotengine.org/en/stable/tutorials/3d/global_illumination/faking_global_illumination.html

Additional information:

https://github.com/godotengine/godot-docs/blob/05fe756d9d1e6abab198da4721ba8f0407f0e45e/tutorials/3d/lights_and_shadows.rst?plain=1#L44

According to the text (from Godot Engine 4.1 - 3D Lights and Shadows, linked above) and the screenshot of common light properties above it, DirectionalLight3D (like Omni and Spot lights) should have a Specular property but currently doesn't (4.1 Stable). Not sure if this is a bug, a documentation issue or due to on going work.

directionallight3d_missingspecular
specular property missing from DirectionalLight3D

Due to this, currently unable to remove the specular blob from DirectionalLights when following "Godot Engine 4.1 - Faking global illumination". Which specifically mentions setting Specular: 0.0 on DirectionalLight3D.

https://github.com/godotengine/godot-docs/blob/05fe756d9d1e6abab198da4721ba8f0407f0e45e/tutorials/3d/global_illumination/faking_global_illumination.rst?plain=1#L48

@clayjohn
Copy link
Member

clayjohn commented Aug 17, 2023

Looking at the shader code, it looks like specular is forcibly set to 1.0 from within the shader for DirectionalLight3Ds. but not for positional lights.

The value of LIGHT_PARAM_SPECULAR is sent to the shader for both DirectionalLight3Ds and positional lights.

And light specular is disabled at the Light3D level for DirectionalLight3Ds

And here is where it was disabled. #45023

Reduz mentions a few properties that were removed for performance reasons, and a few properties that should be returned in follow up PRs. specular is mentioned in neither column. To me this looks like it was mistakenly removed. All the piping was left in place. I have a feeling it was partially removed during the refactor, but was ultimately not fully removed. I would just restore the specular property to DirectionalLight3Ds

Changes are needed in at least two places:

  1. Remove light_specular from here:
    https://github.com/godotengine/godot/blame/0511f9d9a7d56c742d87fafdcea8785d40ad14b3/scene/3d/light_3d.cpp#L539

  2. use directional_lights.data[i].specular here:


    and here:

    and here:
    light_compute(normal, normalize(directional_lights[i].direction), normalize(view), directional_lights[i].size, directional_lights[i].color * directional_lights[i].energy, true, 1.0, f0, roughness, metallic, 1.0, albedo, alpha,

@dheerajd5
Copy link

Hey, can I take up this issue?

@AndrewShobbrook
Copy link
Contributor

AndrewShobbrook commented Aug 18, 2023

I think I might've fixed the issue just through clayjohn's advice, but I can't figure out how to change the default value for it back to 1.0 (Default is 0.5 for some reason)

Also am I supposed to ask beforehand for something like this? I haven't contributed here before so sorry if I acted too early

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants