JOML-CI / JOML

A Java math library for OpenGL rendering calculations
MIT License
729 stars 104 forks source link

Make these useful constants available externally #331

Closed jordan4ibanez closed 1 year ago

jordan4ibanez commented 1 year ago

Hi

httpdigest commented 1 year ago

Hi, thanks for the suggestion. I see no problem with making these public. They weren't public before, because java.lang.Math (which this org.joml.Math is just mirroring in interface) didn't contain those as well, so that you could import one or the other class'es members as you like. But if these additional constants are useful, we could break this "interchangeability". But then we should focus more on the actual naming of these constants, since making them public will then set their names in stone (for not breaking future clients relying on their names).

For that, I would suggest to change the names to the following scheme:

jordan4ibanez commented 1 year ago

Hmm yes, I will give this a nice thinking and make changes in a few hours.

jordan4ibanez commented 1 year ago

What should I do with PI_INV = 1.0 / PI ? I see this stands for Pi inverse, but should I manually spell it out as PI_INVERSE?

jordan4ibanez commented 1 year ago

Also, I will give the schema PI_TIMES_X a commit, so you can look at it and see if you like the feeling of it within the code.

jordan4ibanez commented 1 year ago

Also also, I found these two elements which do not have float counterparts. They are: PI_OVER_4 & PI_INV. Would you like for me to include float counterparts?

httpdigest commented 1 year ago

What should I do with PI_INV = 1.0 / PI ?

See the second bullet point in my list in my first answer above: "anything that is a fraction containing PI as numerator or denominator [...] uses the notation numerator_OVER_denominator"

Therefore: ONE_OVER_PI.

The term PI_INVERSE really only expresses that this value is the multiplicative inverse of PI. That's probably because you use PI only in a multiplicative way (as a factor) anyways. But ONE_OVER_PI should be a more operation-agnostic way of expressing what the value is.

Also also, I found these two elements which do not have float counterparts. They are: PI_OVER_4 & PI_INV. Would you like for me to include float counterparts?

Yes, please.

jordan4ibanez commented 1 year ago

Thank you for the clarification. It appears that everything is complete. Would you like for me to squash these commits?

httpdigest commented 1 year ago

No, thanks, not necessary. I've squashed and merged into the repo now. Thanks for the change!