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

Avoid branch in half2float in gl_compatibility renderer #73332

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Feb 15, 2023

Fixes: #73320

This is a regression from #72914.

For some reason the radeon drivers really don't like the h_e == uint(0x0000) comparison. Replacing it with (h_e >> uint(10)) == uint(0) works though.

Also, when running it through Radeon GPUAnalyzer I noticed that this was actually generating a branch. So I made the code branchless. Logically this code does almost exactly the same thing as the old code (it can now return -0 but couldn't before) but it works on more hardware, so oh well.

CC @JonqsGames you may be interested in this

@clayjohn clayjohn added this to the 4.0 milestone Feb 15, 2023
@clayjohn clayjohn requested a review from a team as a code owner February 15, 2023 04:46
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

It's a kind of magic

@JonqsGames
Copy link
Contributor

Looks good ! Should we also avoid branch in float2half ?

uint float2half(uint f) {
	uint e = f & uint(0x7f800000);
	return ((f >> uint(16)) & uint(0x8000)) |
			uint((e > uint(0x38000000))) * (((e - uint(0x38000000)) >> uint(13)) & uint(0x7c00)) |
			((f >> uint(13)) & uint(0x03ff));
}

@clayjohn
Copy link
Member Author

Looks good ! Should we also avoid branch in float2half ?

uint float2half(uint f) {
	uint e = f & uint(0x7f800000);
	return ((f >> uint(16)) & uint(0x8000)) |
			uint((e > uint(0x38000000))) * (((e - uint(0x38000000)) >> uint(13)) & uint(0x7c00)) |
			((f >> uint(13)) & uint(0x03ff));
}

We could. I wasn't able to detect any issues with that code on the same device. So my preference is to not touch it. I don't want to risk triggering another driver bug

@akien-mga akien-mga merged commit aa6ec76 into godotengine:master Feb 15, 2023
@akien-mga
Copy link
Member

Thanks!

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 RC 2 completely gets rid of viewport lines on OpenGL.
3 participants