Skip to content

Get rid of some more magic numbers #200

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mardy
Copy link
Contributor

@mardy mardy commented Apr 14, 2025

Please review it commit by commit, because the commit messages should help in understanding what is happening :-)

mardy added 2 commits April 14, 2025 18:55
Use the defined constants for indirect matrix indexes. Note that
the changes to the comparison operators are compensated by the +/-1
offset in the operand, so the end result is unaltered.
The old code suffered from some obfuscation due to how it was reverse
engineered:
- The 0x43300000 magix number is the binary representation of the upper
  32 bits of a double floating point number havint the exponent set to
  1075, which makes it so that the least significant bit of the lower 32
  bit word maps exactly to the unit 1.
- 4503599627370496.0F is 1 << 52, which in floating point representation
  is 0x43300000 00000000.
- 0.00097656250F is "1.0 / (1 << 10)"

In other words: multiplying by 0.00097656250F is equivalent to dividing
by 2^10, and this is the actual operation that the old code was
fulfilling. The subtraction between the double floating point numbers is
just an optimisation to convert the integer parameter into a float, so
we can safely ignore it and leave it all to the compiler.
Copy link
Member

@DacoTaco DacoTaco left a comment

Choose a reason for hiding this comment

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

lgtm.
i'd need to validate the first commit but i always have issues with gx stuff because thats stuff i know nothing about haha.

ill check it out later

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

Successfully merging this pull request may close these issues.

2 participants