Skip to content

HMM2.0#149

Merged
bvisness merged 39 commits intoHandmadeMath:2-2023from
dev-dwarf:lcf-hmm2.0
Jan 22, 2023
Merged

HMM2.0#149
bvisness merged 39 commits intoHandmadeMath:2-2023from
dev-dwarf:lcf-hmm2.0

Conversation

@dev-dwarf
Copy link
Copy Markdown
Contributor

@dev-dwarf dev-dwarf commented Dec 29, 2022

I was hired by @bvisness to work on the list of changes for HMM2.0. The tracking issue for all of the changes is: #147.

Changes

Here are the changes I made:

  • Renaming. Types and functions have been renamed as described in the 2.0 issue: HMM 2.0 Design #147. This affects almost every file in the project.
  • Angles. A consistent unit is used for angles both as inputs to functions and internally. The default is radians for both, but can be changed. To change the input unit to all HMM functions, define one of HMM_USE_DEGREE_INPUT or HMM_USE_TURN_INPUT. There are macros, HMM_AngleRad, HMM_AngleDeg, and HMM_AngleTurn that will convert an angle to the input representation, such as passing an FOV in degrees: HMM_Perspective_RH(HMM_AngleDeg(120), ... other args ...). There are also functions to convert angles in the input representation to any of the other units, HMM_ToRad, HMM_ToDeg, HMM_ToTurn.
  • Configuration. Added HMM_PROVIDE_MATH_FUNCTIONS, removed individual checks for each math function being defined. Users now define this and all of the HMM_SINF and other trig functions, or accept the default crt implementations. The atanf and atan2f functions were unused and have been removed. If users override the functions, they can also use a different unit for the angles in their functions by defining both of HMM_ANGLE_USER_TO_INTERNAL and HMM_ANGLE_INTERNAL_TO_USER.
  • HMM_PREFIX macro has been removed. Everything now uses an uppercase HMM_ prefix, so it should be easy to find+replace on your local copy.
  • Handedness. All functions that depended on the handedness of the coordinate system have both a left and right handed version, marked by a "_LH" or "_RH" suffix respectively.
  • 2x2 and 3x3 Matrices. Basic operations (Add, Sub, Mul, Div, Transpose) are implemented for each size.
  • Matrix Inverses. There are determinants and general inverses for each size of matrix. For 4x4 matrices there are inverses specific to each type of matrix we have a constructor for (Orthographic, Perspective, Rotation, etc). New tests were added for the inverses.
  • Overloads and _Generic. There are overloads for the new matrix sizes and the determinant and inverse functions. There is also a C11 _Generic macro for each function that was overloaded before.
  • FastNorm is default. The FastNorm functions are now just the Norm functions. Precision has declined in several places from this, but is still on par with the accuracy of GLM. Per: Vector normalization could easily be faster #87
  • SLerp. The Slerp implementation was not robust in cases when the quaternions were very close. It has also been changed to always take the shortest path between angles. Divide by zero when calling HMM_Slerp with two identity quaternions #135
  • HMM_LinearCombineSSE is now HMM_LinearCombineV4M4. It will use the SSE code when available but fallback to a slower implementation. This reduces the amount of special cases needed for various Mat4 functions, which previously used HMM_LinearCombineSSE when available and then each had their own implementation.

Update Tool

I also created a tool that can be used to update user's code to 2.0 automatically, which you can find in a separate repository here: https://github.com/dev-dwarf/HMM2.0UpdateTool. The tool I created does the following:

The tool is written in one .c file that compiles as C or C++, so any of our users should be able to compile it easily. The tool takes one or more filenames as arguments and updates their contents to 2.0. There are also .bat and .sh scripts to recursively run the tool on all the c/h/cpp/hpp files in a directory. The tool changes the files it touches so it should be used with source control. I can provide compiled executables for windows and linux, but someone else would have to handle mac.


Please take a look and let me know anything that needs changes. Sorry for the huge PR, didn't know a good way to split things up. Happy to answer any questions or make additional changes. Thanks for the opportunity!

By default if user is overriding trig functions assume their input and internal units are the same.
Generics enable by default when available (see lines 97-104). User can also force them by defining HANDMADE_MATH_C11_GENERICS

Also fixed some missed things w.r.t renaming. My old tool didn't catch cases like HMM_MultiplyVec3f needing to be HMM_MulV3F instead of HMM_MulV3f.
Also improved quaternion tests. More work could be done here, see discussion here about optimizing slerp: https://discord.com/channels/239737791225790464/489148972305350656/1055167647274246265
Also renamed Transpose to TransposeM4, so that we can have TransposeM2,M3
As can be seen from the tests, precision has declined quite a bit from using the FastNorm implementations for various things. We can only guarantee about 0.001f precision for anything where a norm happens now. If this is undesired we can change back easily.
TODO: Tests for simple 4x4 Inverses
@dev-dwarf
Copy link
Copy Markdown
Contributor Author

Per offline feedback from Ben I've added some commits so that test coverage for the additions is comprehensive. Additionally I've added a configuration option so that projection matrices can use a Z range of [0, 1] for NDCs, this will be helpful for supporting graphics APIs other than OpenGL (DirectX and Metal use this convention that I know of).

Copy link
Copy Markdown
Member

@bvisness bvisness left a comment

Choose a reason for hiding this comment

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

Looks great overall. I have a few suggested changes but nothing major. I'll test out the renaming script on my Mac and report back with how that goes.

Thanks for all your work on this!

HandmadeMath.h Outdated
Comment on lines +381 to +387
HMM_Vec2 Result;
float* Column = Elements[Index];

Result.Elements[0] = Column[0];
Result.Elements[1] = Column[1];

return (Result);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we not just do return Elements[Index];...? Is there something I'm missing?

Copy link
Copy Markdown
Member

@bvisness bvisness left a comment

Choose a reason for hiding this comment

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

A couple more things.

@bvisness bvisness mentioned this pull request Jan 20, 2023
Copy link
Copy Markdown
Contributor Author

@dev-dwarf dev-dwarf left a comment

Choose a reason for hiding this comment

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

Hi Ben, please see my comments and let me know if additional changes are required.

Comment on lines +1506 to +1509
Result.Columns[0] = HMM_V4V(HMM_AddV3(HMM_Cross(Matrix.Columns[1].XYZ, B32), HMM_MulV3F(C23, Matrix.Columns[1].W)), -HMM_DotV3(Matrix.Columns[1].XYZ, C23));
Result.Columns[1] = HMM_V4V(HMM_SubV3(HMM_Cross(B32, Matrix.Columns[0].XYZ), HMM_MulV3F(C23, Matrix.Columns[0].W)), +HMM_DotV3(Matrix.Columns[0].XYZ, C23));
Result.Columns[2] = HMM_V4V(HMM_AddV3(HMM_Cross(Matrix.Columns[3].XYZ, B10), HMM_MulV3F(C01, Matrix.Columns[3].W)), -HMM_DotV3(Matrix.Columns[3].XYZ, C01));
Result.Columns[3] = HMM_V4V(HMM_SubV3(HMM_Cross(B10, Matrix.Columns[2].XYZ), HMM_MulV3F(C01, Matrix.Columns[2].W)), +HMM_DotV3(Matrix.Columns[2].XYZ, C01));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I hope the name InvGeneral for these functions (rather than just Inv) helps to imply that they are not the best option for most cases.

Copy link
Copy Markdown
Member

@bvisness bvisness left a comment

Choose a reason for hiding this comment

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

This all looks great. I'm good to merge, but I want to gather feedback from a couple more people first. Will be resolving this in the near future. Thanks again @dev-dwarf!

@strangezakary
Copy link
Copy Markdown
Member

Just took a look through this, and everything looks great. Thanks so much for doing this!!!

@bvisness bvisness changed the base branch from master to 2-2023 January 22, 2023 23:33
@bvisness bvisness merged commit c24e4ff into HandmadeMath:2-2023 Jan 22, 2023
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.

3 participants