godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.83k stars 21.13k forks source link

`specular_disabled` render mode in shaders does not make the `SPECULAR` output zero #95102

Open MajorMcDoom opened 3 months ago

MajorMcDoom commented 3 months ago

Tested versions

Reproducible in 4.3.rc1

System information

Godot v4.3.rc1 - Windows 10.0.22631 - GLES3 (Compatibility) - NVIDIA GeForce GTX 1660 SUPER (NVIDIA; 32.0.15.6070) - Intel(R) Core(TM) i7-10700F CPU @ 2.90GHz (16 Threads)

Issue description

Screenshot 2024-08-03 121238 Setting the render mode specular_disabled does eliminate specular contribution from lights, but not reflected light. The SPECULAR output is non-zero in this case. If this is intended, the namings are very confusing and should be either changed or documented better.

Steps to reproduce

Open attached project, and look at the two balls side-by-side. Note that one is blacker than the other. ball1 has the black shader, and ball2 has the true_black shader. Both shaders have the render mode specular_disabled, but only true_black explicitly sets SPECULAR output to zero. ball still receives reflected light from the WorldEnvironment in the scene.

Minimal reproduction project (MRP)

bug-report.zip

Calinou commented 3 months ago

This is done by design. The specular render modes only affect direct light specular lobes, not reflected light. Changing this behavior would break visuals in existing projects, sometimes to a significant degree - on top of giving less control to users over the final appearance (since you wouldn't be able to disable specular lobes while keeping reflections anymore).

Set Metallic > Specular or SPECULAR to 0.0 to disable specular contribution entirely (all values below 0.02 do that): https://github.com/godotengine/godot/pull/63587

This is already documented here:

https://github.com/godotengine/godot/blob/3978628c6cc1227250fc6ed45c8d854d24c30c30/doc/classes/BaseMaterial3D.xml#L358-L361

And here:

https://github.com/godotengine/godot/blob/3978628c6cc1227250fc6ed45c8d854d24c30c30/doc/classes/BaseMaterial3D.xml#L265-L268

https://github.com/godotengine/godot-docs/pull/9691 updates the manual.

MajorMcDoom commented 3 months ago

godotengine/godot-docs#9691 updates the manual.

That PR definitely helps. Thanks!

Calinou commented 3 months ago

Reopening, as the associated documentation PR hasn't been merged yet.