Closed tetrapod00 closed 1 month ago
Excellent work, thank you so much for going the extra mile! The experience using the shader is great. Also, thank you for all the details.
For next PRs, please note that you don't have to bother taking many screenshots. But, I always test directly in Godot, take the time to read the code, and it's already very clear and even commented so it already makes reviewing very easy. So, you don't have to use more of your time to present things neatly on GitHub. I really appreciate your thoughtfulness and attention to detail.
3.5: 4.3 Compatibility: 4.3 Forward+:
Changes:
Sliders
Linear depth:
The new linear depth calculation looks like this:
Call site:
This is the same one used by the official Advanced Postprocessing tutorial, and the official VisualShader node
LinearSceneDepth
. It may be more expensive than the 3.x version. To be honest, I don't fully understand the 3.x version. Relying on the internals of the matrices, like the 3.x version was doing, may be wrong for code like this that also functions partially as a tutorial. I may come back and see if there's a simpler or more efficient way to get the linear depth in 4.3, but as is this functions.OUTPUT_IS_SRGB
is a proxy for whether the renderer is Compatibility or Forward+/Mobile. I confirmed with clayjohn that it for this purpose it won't change until 5.0. Ideally this would be an explicit#define
for the renderer, but that's not actually currently available. I am a little wary of putting a snippet like this, with the branching, into code that will be copied and pasted a lot. But as is, code that is copied and pasted just assumes Vulkan NDC space, which is incorrect.We need to check the renderer because Compatibility and Forward+/Mobile use different NDC spaces.
Compatibility, if we don't check, and assume Vulkan Forward+ NDC:
Compatibility if we check: