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

Fix Mobile Renderer - Shadow Disabled and User Vertex Lighting flags #98187

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

EnlightenedOne
Copy link

@EnlightenedOne EnlightenedOne commented Oct 14, 2024

Fixes #98102 Forward Mobile Renderer fix. Corrects preprocessor directive, also removes redefinition of method names as parameters which must predate the ubershader work.

To test toggle the two shader flags in Mobile mode and watch the console to confirm no fireworks, visual checks look good:
USE_VERTEX_LIGHTING - uber shader bug
Shadows Disabled - preprocessor order bug

Adjusted both here as the issue crept in under the vertex lighting change and I wanted to confirm no gremlins with the two interacting, was confused when I couldn't switch on the vertex lighting at all.

…ncompatible with ubershader method definitions
@EnlightenedOne EnlightenedOne requested a review from a team as a code owner October 14, 2024 21:21
@EnlightenedOne EnlightenedOne changed the title Move preprocessor to end of shadow definition in mobile forward renderer Fix Mobile Renderer - Shadow Disabled and User Vertex Lighting flags broken Oct 14, 2024
@EnlightenedOne EnlightenedOne changed the title Fix Mobile Renderer - Shadow Disabled and User Vertex Lighting flags broken Fix Mobile Renderer - Shadow Disabled and User Vertex Lighting flags Oct 14, 2024
@Calinou Calinou added this to the 4.4 milestone Oct 15, 2024
@roalyr
Copy link

roalyr commented Oct 15, 2024

Thank you. Will pull this to recompile the fork as soon as it is merged. Leaving this comment so as to not to forget.

@DarioSamo
Copy link
Contributor

Confirmed the issue and tested the PR to confirm it fixes it. Changes look good to me.

@EnlightenedOne
Copy link
Author

Cool is anything else required to merge this?

@clayjohn clayjohn merged commit a2117f5 into godotengine:master Oct 17, 2024
20 checks passed
@clayjohn
Copy link
Member

Thank you! And congratulations on your first merged pull request!!

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

Successfully merging this pull request may close these issues.

Godot 4.4dev3: shader compilation error when "Disable Receive Shadows" is Enabled
5 participants