Fix proximity fade's world_pos is calculated incorrectly when in gl_compatibility#89966
Fix proximity fade's world_pos is calculated incorrectly when in gl_compatibility#89966jsjtxietian wants to merge 1 commit into
world_pos is calculated incorrectly when in gl_compatibility#89966Conversation
| if (OS::get_singleton()->get_current_rendering_method() == "gl_compatibility") { | ||
| code += " depth_tex = depth_tex * 2.0 - 1.0;"; | ||
| } |
There was a problem hiding this comment.
Could we make this part be introduced by the shader preprocessor, instead of a C++ condition? This way, if you convert the BaseMaterial3D to a ShaderMaterial (making its shader code fixed), it'll keep working if you switch rendering methods afterwards. This is also important to handle situations where a project can fall back to Compatibility if Vulkan isn't supported.
(Also, I thought we already had automatic depth correction when using the Compatibility rendering method, so we wouldn't have to do this.)
There was a problem hiding this comment.
Indeed it would be great if the depth_texture was driver agnostic, so the same shader code would work regardless of renderer. Or a preprocessor macro that converts it into 0-1 (and does nothing if Vulkan).
There was a problem hiding this comment.
I'm sorry for my previous explaination of this problem, let me put it more clearly: depth is 0-1 but the ndc is different in opengl and vulkan. It's introduced in #54285, it changed to vulkan way of ndc so it caused this issue.
Visual shader also use an if here, see #74910. But we'd better not use this approach as Calinou mentioned.
There was a problem hiding this comment.
In 4.4+ we now have a way to do this with preprocessor defines. I made a PR: #100220.
There was a problem hiding this comment.
In 4.4+ we now have a way to do this with preprocessor defines. I made a PR: #100220.
Seems pretty reasonable. Closing this pr now.
There was a problem hiding this comment.
Your approach may still end up being the right one, actually. It looks like preprocessor macros don't quite work right now in the generated shaders in materials. We'll see if that ends up being considered a bug or not.
There was a problem hiding this comment.
It looks like preprocessor macros don't quite work right now in the generated shaders in materials.
I suppose this can be tweaked a little? Not sure it's a bug or not, but worth investigating.
depth_tex is set incorrectly in gl_compatibility rendering methodworld_posx is calculated incorrectly when in gl_compatibility rendering method
world_posx is calculated incorrectly when in gl_compatibility rendering methodworld_pos is calculated incorrectly when in gl_compatibility
Fixes #89942
OpenGL ndc's z range is-1.0 to 1.0 by default while the Vulkan is 0.0 to 1.0.