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

Use proper UV in cubemap downsampler raster (Fixes reflections in mobile renderer) #76692

Merged
merged 1 commit into from
May 3, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented May 3, 2023

Fixes: #55878

This removes bias in cubemap downsampling shader that resulted in the bottom of cubemaps being over represented

Hint: review with whitespace hidden as most of the changes here are whitespace related

Before:
Screenshot from 2023-05-03 01-07-33

After:
Screenshot from 2023-05-03 01-08-56

Forward+
Screenshot from 2023-05-03 01-07-57

This removes bias in cubemap downsampling shader that resulted in the bottom of cubemaps being over represented
@clayjohn clayjohn added this to the 4.1 milestone May 3, 2023
@clayjohn clayjohn requested a review from BastiaanOlij May 3, 2023 08:11
@clayjohn clayjohn requested a review from a team as a code owner May 3, 2023 08:11
Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

I never thought this would be the cause of the issue, but good catch 🙂

@mhilbrunner mhilbrunner merged commit dbd615a into godotengine:master May 3, 2023
@mhilbrunner
Copy link
Member

Thanks! ❤️

@akien-mga
Copy link
Member

Cherry-picked for 4.0.3.

@lostminds
Copy link

If I understand this correctly this will mean that scenes using environment lighting on the Mobile renderer will get brighter with this change (since the old darker lighting was a bug) and environments will need to be adjusted to get the 4.0.x appearance?

If so, perhaps it would be good to describe and warn about this in the change notes when 4.1 is released, as it might be unexpected for people working on projects using the Mobile renderer as the base rather than Forward + to have the light levels change in all their scenes without understanding why.

@clayjohn clayjohn deleted the Mobile-sky-dark branch May 19, 2023 18:35
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.

Vulkan Mobile backend: Environment lighting and reflections are twice as dark as they should be
6 participants