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.

Sign up for GitHub

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

Jump to bottom

Fix albedo value wraparound in Compatibility render mode #92388

Merged
merged 1 commit into from
Jun 14, 2024
Merged

Fix albedo value wraparound in Compatibility render mode #92388

merged 1 commit into from
Jun 14, 2024

Conversation

sunfl0w
Copy link
Contributor

@sunfl0w sunfl0w commented May 26, 2024

Using the Compatibility renderer results in wrapped albedo values if a custom shader returns albedo values outside the expected range of [0,1].

This commit fixed this issue by clamping the albedo value to [0,1] right after the custom shader is executed.

Fixes #91919

@sunfl0w sunfl0w requested a review from a team as a code owner May 26, 2024 15:21
@Calinou
Copy link
Member

Calinou commented May 26, 2024

Is clamp() actually needed here, or would max(0.0, albedo) suffice here to avoid negative values?

@Calinou Calinou added bug topic:rendering topic:3d cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels May 26, 2024
@Calinou Calinou added this to the 4.3 milestone May 26, 2024
@sunfl0w sunfl0w force-pushed the fix-compatibility-albedo branch from 8cf72ad to d437f8e Compare May 26, 2024 18:03
@sunfl0w
Copy link
Contributor Author

sunfl0w commented May 26, 2024 •
edited
Loading

In my tests, the max() function was actually sufficient. Only negative numbers wraparound, resulting in an unwanted color impression.

I edited my commit accordingly and also fixed code formatting.

@sunfl0w
Fix albedo value wraparound in Compatibility render mode
0e56b64 
Using the Compatibility renderer results in wrapped albedo values if a custom shader returns albedo values outside the expected range of [0,1].

This commit fixed this issue by clamping the albedo value to [0,1] right after the custom shader is executed.

Fixes godotengine#91919
@sunfl0w sunfl0w force-pushed the fix-compatibility-albedo branch from d437f8e to 0e56b64 Compare June 13, 2024 11:33
@sunfl0w
Copy link
Contributor Author

sunfl0w commented Jun 13, 2024

Good to know. I updated the commit.

clayjohn
clayjohn approved these changes Jun 13, 2024
View reviewed changes
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.

Looks great! Thanks

@akien-mga akien-mga merged commit 4ce3e16 into godotengine:master Jun 14, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clayjohn clayjohn clayjohn approved these changes

Assignees
No one assigned
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:rendering topic:3d
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

Compatibility renderer breaks shader
4 participants
@sunfl0w @Calinou @akien-mga @clayjohn