-
Notifications
You must be signed in to change notification settings - Fork 143
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
vertex format: half float conversion #96
Conversation
shrekshao
commented
Feb 21, 2016
- This is based on the vertex format branch vertex format #88
- I used a straight-forward (not optimized) method to do the half float transition.
- The transition function is now built-in with the sample code.
- The correctness needs checking. Although I have gone through several cases including normalized, subnormal, and zero.
- I think I can wrap up the code for a utility only after the correctness checking and algorithm optimization. So it might not be done before GDC.
exponent < -1074 ? mantissa * Math.pow(2, -1074) * Math.pow(2, exponent + 1074) : mantissa * Math.pow(2, exponent); | ||
} | ||
|
||
function encodeFloat16(value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function has a lot of allocations that we can avoid by using file-scoped variables through closures instead of function scoped variables.
Also, let's move this to a separate .js file for now, and update #91 with a note to better test and package this (we'll want to write careful fine-grained unit tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. This is ugly. I was too urgent to push this. Let me go back and refine
On Sunday, February 21, 2016, Patrick Cozzi notifications@github.com
wrote:
In samples/geo_vertex_format.html
#96 (comment)
:
if (bits === 0) {
data.setFloat64(0, value \* Math.pow(2, 64));
bits = ((data.getUint32(0) >>> 20) & 0x7FF) - 64;
}
var exponent = bits - 1022,
mantissa = ldexp(value, -exponent);
return [mantissa, exponent];
}
function ldexp(mantissa, exponent) {
// avoid multiplying by infinity and zero
return exponent > 1023 ? mantissa \* Math.pow(2, 1023) \* Math.pow(2, exponent - 1023) :
exponent < -1074 ? mantissa \* Math.pow(2, -1074) \* Math.pow(2, exponent + 1074) : mantissa \* Math.pow(2, exponent);
}
function encodeFloat16(value) {
This function has a lot of allocations that we can avoid by using
file-scoped variables through closures instead of function scoped variables.Also, let's move this to a separate .js file for now, and update #91
#91 with a note to
better test and package this (we'll want to write careful fine-grained unit
tests).—
Reply to this email directly or view it on GitHub
https://github.com/WebGLSamples/WebGL2Samples/pull/96/files#r53568119.
Pack up the code to a new js in third-party, use closure. |
@pjcozzi I've wrapped up the code to a separate .js file and use closure. How are we going to handle this sample? Shall we close the sample for now and reopen it after we do strict unit test for the half float convert tool? |
@shrekshao I think it is fine to merge this sample before making a separate standalone utility for half floats. Just keep #91 open. |
|
||
var HALFFLOAT = HALFFLOAT || {}; | ||
|
||
(function(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did any code in this file come from another library? For example, do we need to update LICENSE.md with licenses for new third-party code?
Ready:
|
Looks great! We'd still want to do a bit more cleanup to make |
vertex format: half float conversion