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

OpenGL : Refact half to float and float to half conversion #72908

Closed

Conversation

JonqsGames
Copy link
Contributor

@JonqsGames JonqsGames commented Feb 8, 2023

Fixes: #72882

Based on : https://stackoverflow.com/a/60047308

I think this solution is more robust than the previous one, still has some edge case for not normalized float but i don't think this is an issue since all float come from the same source.

Bugsquad edit
Also fixes: #66462
Fixes: #66459
Supersedes: #72329

@JonqsGames JonqsGames requested a review from a team as a code owner February 8, 2023 18:18
@Riteo
Copy link
Contributor

Riteo commented Feb 8, 2023

I think that there might be some licensing issues. All user contributions on SO are licensed under CC-BY-SA (unless stated otherwise obv), which is incompatible with MIT.

@JonqsGames
Copy link
Contributor Author

@Riteo sorry didn't knew that. Not sure how to handle that. Since i understand it, i could refactor to make it less like the original answer but i feel like it's cheating.

@bitsawer
Copy link
Member

bitsawer commented Feb 8, 2023

He links his research paper which his SO post is based on, but it's hard to say what kind of copyright it has. Some educational institutions can have pretty liberal licenses for their research data, but you'd have to dig a bit. Or maybe even ask the person directly if they or any of the co-authors are allowed to re-license the code snippets.

However, he also seems to run this project: https://github.com/ProjectPhysX/FluidX3D, which has this pretty limiting (custom?) license: https://github.com/ProjectPhysX/FluidX3D/blob/master/LICENSE.md. Seems there is similar code in this file: https://github.com/ProjectPhysX/FluidX3D/blob/master/src/kernel.cpp

@clayjohn
Copy link
Member

clayjohn commented Feb 8, 2023

We could always do a direct port of make_half_float() and half_to_float()

static _ALWAYS_INLINE_ uint16_t make_half_float(float f) {

static _ALWAYS_INLINE_ float halfptr_to_float(const uint16_t *h) {
union {
uint32_t u32;
float f32;
} u;
u.u32 = halfbits_to_floatbits(*h);
return u.f32;
}
static _ALWAYS_INLINE_ float half_to_float(const uint16_t h) {
return halfptr_to_float(&h);
}

@JonqsGames
Copy link
Contributor Author

@clayjohn i agree.
Having second thoughts, maybe could even simplify and only check the case of exponent == 0, the rest may be overengineering for that case since the half float limitation is explicit for Compatibility renderer.

@clayjohn
Copy link
Member

clayjohn commented Feb 8, 2023

@clayjohn i agree. Having second thoughts, maybe could even simplify and only check the case of exponent == 0, the rest may be overengineering for that case since the half float limitation is explicit for Compatibility renderer.

Indeed, there may be an easy workaround here. Also these functions are not exposed to users, they are just used to implement pack/unpackHalf2x16

@clayjohn
Copy link
Member

clayjohn commented Feb 8, 2023

Superseded by #72914

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