the Fiddler's picture

OpenTK.Math.Half

Project:The Open Toolkit library
Component:Code
Category:feature request
Priority:normal
Assigned:Inertia
Status:closed
Description

This type should provide an interface similar to IntPtr, with a methods, conversion operators and constructors that can pack/unpack floats and doubles.

I'm opening this task so we can keep track of progress.


Comments

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Inertia's picture

#51

It should all be in there. Considering the fact that you made me implement 4 Interfaces I didn't actually need, I'll wait for a review of the Half4 struct before working on Half2,3....

Will take a look at TextPrinter now.

AttachmentSize
OpenTK Half v7.rar9.9 KB
the Fiddler's picture

#52

Ok, I worked my way through Half4. Suggestions:

  • There really is no need for ref overloads, pushing 8 vs 16 bytes won't make a difference in front of the 4x Single -> Half conversions.
  • If you think these are necessary, at least swap the [CLSCompliant] declarations around :)
  • Would you really use "ToVector4(out Vector4 v)"..?
  • Needs those pesky interfaces... They are mostly necessary to maintain some form of parity with Single/Vector4, but they could be genuinely useful (e.g. sorting or comparing elements.)

Other than these, I think Half4 alright. Would you mind creating a branch so we can work directly on svn? Hunting for updates through the posts is getting unwieldy. Besides, this will help with testing.

Inertia's picture

#53

If you think these are necessary, at least swap the [CLSCompliant] declarations around :)
I KNEW IT! :p I'll mark ref/out as not compliant, but I think the situation is the same as with our ToSingle() debate. The functions work fine, they won't hurt anyone and at least I will use them instead of passing by value.

ISerializable
Already done, tested and works.

IEquatable[Half4]
Ok this makes sense.

IComparable[Half4]
When should this return -1? Take the length of the vector and compare that? Vector4 doesn't implement this interface, probably due to the same problem. The user has access through IComparable[Half] though and implement the desired comparison by hand.

[ToString/Parse]
I've already overriden ToString(), but I don't see why one would need Parse. Vector4 does not do this either. The user can parse the individual Half's from a string and reassemble the Half4.

[BitConverter]
Sure.

[branch]
Uhm .. actually ... I was about to ask you in pm if you would mind starting to delete some of the old branches, the tree is growing confusingly large. Half does currently not impact any other parts of the library, might aswell commit it to the trunk when Half2,3 are done and ship it with 0.9.2?

the Fiddler's picture

#54

[IComparable]
Vector4 doesn't implement this? This is... interesting. Ok, let's leave comparable out of the equation for the moment.

[Branches]
The openal branch can safely go, but the other contain stuff to be salvaged. I wouldn't call a tree with 5 branches as large, but maybe we should follow a more systematic approach (e.g. have personal folders with branches, mark stale / finished branches so they can be deleted in the future).

As for Half going to trunk, no problem (it doesn't impact other code). I just thought you wished to work on it some more before committing to svn.

Inertia's picture

#55

Added Equality/BitConverter stuff, fixed a problem with 0xFFFF reported as both Zero and NaN, changed CLSCompliance and made some more corrections in docu. Unless you have any other ideas what to add to Half4 I'd say it's good as-is and ready for Half2,3?

Edit: Do you care how the regions are arranged? Currently they're just in order of implementation.

AttachmentSize
OpenTK Half v8.rar11.82 KB
the Fiddler's picture

#56

Can't think of anything, seems great.

No, I don't really care how regions are arranged. I typically place a few big regions around logical blocks ("Public Members", "Private Members", interface implementations) and smaller ones to group function overloads. The point is to filter out unnecessary content when you are looking for something or are working on a specific part of the code.

Inertia's picture

#57

[regions]
Yep, but the whole struct has only 2 private methods, the rest is public. Since it's relatively small I think noone will have trouble finding something in there, just wanted to make sure you are happy with it. :P

[negative zero]
Where did you read that 0xFFFF is -0.0f? I cannot confirm it for the half type (it sure is ok for Single), shouldn't it be 0x8000? s:1 e:00000 m:0000000000

What brought me to that conclusion is the following code to deal with numbers that are too small to be represented

// E is less than -10.  The absolute value of f is less than Half.MinValue
// (f may be a small normalized float, a denormalized float or a zero).
//
// We convert f to a half zero with the same sign as f.
 
return (UInt16)sign;

Quote from ARB_half_float_pixel
(-1)^S * 0.0, if E == 0 and M == 0,

[removing need for additional license]
I've tried to rewrite what is left of the 2 methods that were taken from Ilm's source code (their license is actually more text than the functions .. ) but their code is already extremely efficient. Not sure whether I can go lower than that.

the Fiddler's picture

#59

[negative zero]
Sorry, my bad - highest bit set == 0x8000.

[license]
I am going to mail for clarifications (what happens when the functions are rewritten in another language?) I wouldn't be worried in any case, just wrap it in a region and forget it.

Inertia's picture

#60

[negative zero]
Ty, just needed a confirm.

[license]
Well I do not intend to remove the "would not have been possible without their work" credit in source code, in fact the explanation in the ARB specification did not make much sense to me until I've seen their implementation. But I think they do not want other projects to ride on ILM's fame, and we will have to answer that our Half implementation is derived from theirs when asked. Checking back with them is probably the best cause of action to avoid any problems with this.

the Fiddler's picture

#61

[license]
I searched through the mailing list archives of OpenEXR and the same issue has come up before. Quoting from the discussion:

"RE: [Openexr-devel] Exr.NET

Another slightly legal question, I have a 'Half' data-type which is a direct
port to C# of the Half data-type from the OpenExr distribution. I currently
have the following acknowledgement at the top of the source code:

Based on the C++ code originally written by Florian Kainz and Rod Bogart.

The questions being, is this enough? [...]"

"I'm not a lawyer, but I think that, generally, if the code is largely
derived from the original code, you should retain the license and change
the copyright holder to you, with a reference to the original copyright
holder/author as well."

Still not clear what "largely derived from the original code" constitutes...