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

Add 3->4 upgrading information about NDC, light(), and reverse-z #9940

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

tetrapod00
Copy link
Contributor

@tetrapod00 tetrapod00 commented Sep 10, 2024

Resolves godotengine/godot#96704 by changing the docs.

  • Adds information about normalized device coordinates changing from 3.x/Compatibility to Forward+/Mobile.
  • Adds a note that shaders with custom lighting will need changes.
  • Links to reverse-z changes. As more and more 4.x versions are released, reverse-z will become the default, and most users finding this page will need to apply reverse-z changes if their shader needs it. We should not expect users to visit each minor version conversion page in sequence in order to find this information.

Summary of the original issue: a simplified version of the shader in https://github.com/paddy-exe/Godot-RealTimeCaustics was converted automatically from 3 to 4, then all the manual fixes mentioned here were applied. But the shader still has no visible output.
Two additional changes had to be made so the shader works in 4.3 Forward+:

  • Convert the NDC reconstruction from vec3 ndc = vec3(SCREEN_UV, depth) * 2.0 - 1.0; to vec3 ndc = vec3(SCREEN_UV* 2.0 - 1.0, depth);.
  • Change ALBEDO = vec3(0.0); to ALBEDO = vec3(1.0);. The shader is doing additive lighting using the light() function, and requires a change to ALBEDO to look similar to 3.x.

These kinds of changes cannot be supported by an automatic conversion tool, and they can't even be exhaustively listed. We can list common cases, and clarify the warning that advanced shaders will need more manual work.

I didn't actually use 3.x, so my knowledge of this is only from converting shaders.

@skyace65 skyace65 added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:shaders labels Sep 11, 2024
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flarkk's recommendations are good. Everything else looks good, so this can be merged once you make those changes

@tetrapod00
Copy link
Contributor Author

I think that's everything. Thank you for the reviews!

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@mhilbrunner mhilbrunner merged commit 85c8a47 into godotengine:master Sep 11, 2024
1 check passed
@mhilbrunner
Copy link
Member

mhilbrunner commented Sep 11, 2024

@tetrapod00 Thank you! Merged.

@tetrapod00 tetrapod00 deleted the shader-upgrading branch September 11, 2024 18:04
@mhilbrunner
Copy link
Member

Cherrypicked to 4.3 in #10346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:shaders
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specific light shader works in Godot 3 but not Godot 4
5 participants