Repository: HandmadeMath/HandmadeMath (c)

Issue:
Bug in HMM_Perspecive
Values in projection matrix are in the wrong places, apart from that if we decided to use tangens instead of cotangents in the [0][0] position aspect ratio also have to be inverted so:

 float TanThetaOver2 = HMM_TanF(FOV * (HMM_PI32 / 360.0f));
    
    Result.Elements[0][0] = 1.0f / AspectRatio / TanThetaOver2;
    Result.Elements[1][1] = 1.0f / TanThetaOver2;

Or just compute cotangent up front:

float CoTanThetaOver2 = 1.0f / HMM_TanF(FOV * (HMM_PI32 / 360.0f));
    
    Result.Elements[0][0] = CoTanThetaOver2 / AspectRatio;
    Result.Elements[1][1] = CoTanThetaOver2;


Discussion:
I've successfully used HMM_Perspective in a couple projects, and I recently ported the exact same math over to Rust for another project of mine without issue, so I'm surprised to hear that something wouldn't be working. Could you be more specific about which values are in the wrong place?
I also ran into this just today and was assuming I had to be wrong... I can confirm that at the very least the produced matrix isn't the same as GLM's. I don't know which one is technically "correct," but that they are different is odd. I'll do some revision-scrubbing to get the actual input/output values. (Applying the precomputed cotangent, as recommended by @neurotok above, got them in sync.)
The input values:
Aperture: `90.0f` (`M_PI / 2.0f` for GLM)
Aspect: `1024.0f / 768.0f`
Near: `200.0f`
Far: `0.001f`

The output 4x4 matrices are as follows. 

GLM:
```
[0.75,  0.0,  0.0  ,  0.0]
[0.0 ,  1.0,  0.0  ,  0.0]
[0.0 ,  0.0, -1.0  , -1.0]
[0.0 ,  0.0, -0.002,  0.0]
```

HMM at present:
```
[1.0,  0.0   ,  0.0  ,  0.0]
[0.0,  1.3333,  0.0  ,  0.0]
[0.0,  0.0   , -1.0  , -1.0]
[0.0,  0.0   , -0.002,  0.0]
```

HMM with precomputed cotangent patch:
```
[0.75,  0.0,  0.0  ,  0.0]
[0.0 ,  1.0,  0.0  ,  0.0]
[0.0 ,  0.0, -1.0  , -1.0]
[0.0 ,  0.0, -0.002,  0.0]
```
Well I found that while playing with several c math library for 3d (cglm, kazmath, linmath.h), and HMM only one that get dirrents results:

Heres my sample code:

#include <stdio.h>
#include <math.h>
#ifndef M_PI
#define M_PI 3.14159265358979323846
#endif
#include <kazmath/kazmath.h>
#include <cglm/cglm.h>
#include "linmath.h"
#define HANDMADE_MATH_IMPLEMENTATION
#include "HandmadeMath.h"
int main(void)
{

	/*
	float camPosX = 0.0f;
	float camPosY = 0.0f;
	float camPosZ = 150.0f;
	*/
	float fov = 60.0f;
	float perspective = 800.0f / 600.0f;
	float near = 0.1f;
	float faar = 100.0f;

	kmMat4 test; //, rotation;
	//kmMat4Translation(&test,camPosX, camPosY, -camPosZ);
	kmMat4PerspectiveProjection(&test, fov, perspective, near, faar);
	//kmMat4RotationX(&test, kmDegreesToRadians(45));
	//kmMat4RotationY(&rotation, kmDegreesToRadians(45));
	//kmMat4Multiply(&test, &test, &rotation);
	
	printf("kazmath:\n");

	for (int i = 0; i < 4; ++i) {
		for (int j = 0; j < 4; ++j) {
			printf("%4.2f ",test.mat[i * 4 + j]);
		}
		printf("\n");
	}

	printf("\n");

	mat4 modelMat = GLM_MAT4_IDENTITY_INIT;
	//glm_translate(modelMat, (vec3){-camPosX,-camPosY,-camPosZ});
	glm_perspective(glm_rad((int)fov), perspective, near, faar, modelMat);
	//glm_rotate_x(modelMat, glm_rad(45), modelMat);
	//glm_rotate_y(modelMat, glm_rad(45), modelMat);

	printf("cglm:\n");

	for (int i = 0; i < 4; ++i) {
		for (int j = 0; j < 4; ++j) {
			printf("%4.2f ",modelMat[i][j]);
		}
		printf("\n");
	}

	printf("\n");

	mat4x4 lmodel;
	mat4x4_identity(lmodel);
	mat4x4_perspective(lmodel, fov * M_PI / 180.0f, perspective , near, faar); 

	printf("linmath:\n");

	for (int i = 0; i < 4; ++i) {
		for (int j = 0; j < 4; ++j) {
			printf("%4.2f ",lmodel[i][j]);
		}
		printf("\n");
	}

	printf("\n");

	hmm_mat4 hmodel = HMM_Perspective(fov, perspective, near, faar);

	printf("Hand Made Math:\n");

	for (int i = 0; i < 4; ++i) {
		for (int j = 0; j < 4; ++j) {
			printf("%4.2f ",hmodel.Elements[i][j]);
		}
		printf("\n");
	}

	return 0;
}


And theres the result

kazmath:
1.30 0.00 0.00 0.00 
0.00 1.73 0.00 0.00 
0.00 0.00 -1.00 -1.00 
0.00 0.00 -0.20 0.00 

cglm:
1.30 0.00 0.00 0.00 
0.00 1.73 0.00 0.00 
0.00 0.00 -1.00 -1.00 
0.00 0.00 -0.20 0.00 

linmath:
1.30 0.00 0.00 0.00 
0.00 1.73 0.00 0.00 
0.00 0.00 -1.00 -1.00 
0.00 0.00 -0.20 0.00 

Hand Made Math:
1.73 0.00 0.00 0.00 
0.00 2.31 0.00 0.00 
0.00 0.00 -1.00 -1.00 
0.00 0.00 -0.20 0.00 

To be completely honest HMM is by far my favorite in terms of it's architecture :) I have an ambionion to add Inverse function to it but sadly I have no expirence with SIMD so that may take a while ;)

I just go over CG lectures and I'm pretty sure the correct perspective matrix goes something like that:

https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/gluPerspective.xml
FWIW, the matrix produced by HMM is *close* to the others under certain circumstances — in my 2D context, for instance, it just looks like the FOV is a bit off. If I hadn't been swapping in HMM as a replacement (feeding it the same values), I may not have noticed. That might account for why it hasn't been caught before . 
I’ve also used Handmade Math for several 3D projects and have had no issues with the perspective matrix. 

I’ll look into this today and see if we’re simply just using the “wrong” Algorithm. 
Well hmm. Looks like we might not be using the aspect ratio correctly in the [0][0] and [1][1] cells? The [0][0] cell doesn't even take the aspect into account at all, but for example the formulation laid out [here](https://www.songho.ca/opengl/gl_projectionmatrix.html) looks like this:

![image](https://user-images.githubusercontent.com/4707402/60921693-c0fee080-a260-11e9-88ee-9dd9debe2e8c.png)

(where `r` and `t` are the right and top boundaries)

The rest of the formulation looks the same (given a little algebra). This probably just hasn't looked very wrong because those terms of the matrix don't really affect things as much as the scaling based on Z (which the value in the last column does).

I knew I should have written proper tests for perspective projection!

@StrangeZak We should probably just use the Khronos formulation @neurotok linked above.

https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/gluPerspective.xml
I’m okay with that. Do you wanna do the update or I can as well.

At the same time we really need LH and RH versions of these functions.

On Tue, Jul 9, 2019 at 1:53 pm Ben Visness <notifications@github.com> wrote:

> @StrangeZak <https://github.com/StrangeZak> We should probably just use
> the Khronos formulation @neurotok <https://github.com/neurotok> linked
> above.
>
>
> https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/gluPerspective.xml
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/HandmadeMath/Handmade-Math/issues/100?email_source=notifications&email_token=AB7DZ5MQUGV3EBQQ4JLQUCLP6T3GJA5CNFSM4H7IA7D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZRPXZY#issuecomment-509803495>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB7DZ5OQMJQ4KXEI6434P6DP6T3GJANCNFSM4H7IA7DQ>
> .
>

I'll take care of it in a bit. I wanna see what happens at extreme aspect ratios now...
Haha, okay thanks!

On Tue, Jul 9, 2019 at 1:55 pm Ben Visness <notifications@github.com> wrote:

> I'll take care of it in a bit. I wanna see what happens at extreme aspect
> ratios now...
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/HandmadeMath/Handmade-Math/issues/100?email_source=notifications&email_token=AB7DZ5OTONRES5XREDQQLSLP6T3MJA5CNFSM4H7IA7D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZRP5LQ#issuecomment-509804206>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB7DZ5K3H6UKKNXV3XHSSC3P6T3MJANCNFSM4H7IA7DQ>
> .
>

Actually wait. Does the FOV in your program refer to the vertical or horizontal field of view? I think we might have ended up using a formulation that uses horizontal, while everything else uses vertical.
I think that's exactly what happened. See the following:

![image](https://user-images.githubusercontent.com/4707402/60923156-eb05d200-a263-11e9-8e5e-642353887ae5.png)
![image](https://user-images.githubusercontent.com/4707402/60923174-f3f6a380-a263-11e9-8d93-36afa462df2f.png)

So it's not "wrong", exactly, but it's certainly not what most people would expect. And it's also not what people want from their ultrawide monitors, I bet. 😛 

Here's using vertical FOV:

![image](https://user-images.githubusercontent.com/4707402/60923324-4d5ed280-a264-11e9-8a5c-d0bbb7355fd6.png)
![image](https://user-images.githubusercontent.com/4707402/60923308-459f2e00-a264-11e9-8b31-841707f22ae7.png)

Obviously for resolutions closer to square, this difference doesn't show up as much, which explains why it slipped by in our various projects. 

I could go either way on how we want to version this fix. On the one hand, it is "breaking" in the sense that people might have depended on a particular horizontal field of view. But on the other hand, probably no one did, because they expected vertical FOV like everywhere else.

Thoughts?
Honestly, this should be fixed "correctly" horizontal FOV doesn't make much sense. So im okay with fixing this even if it breaks something. Id just put a comment, or a message in the release notes regarding what we broke, and why. 

Patch:
diff --git a/HandmadeMath.h b/HandmadeMath.h
--- a/HandmadeMath.h
+++ b/HandmadeMath.h
@@ -1186,10 +1186,12 @@ HMM_INLINE hmm_mat4 HMM_Perspective(float FOV, float AspectRatio, float Near, fl
 {
     hmm_mat4 Result = HMM_Mat4();
 
-    float TanThetaOver2 = HMM_TanF(FOV * (HMM_PI32 / 360.0f));
+    // See https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/gluPerspective.xml
+
+    float Cotangent = 1.0f / HMM_TanF(FOV * (HMM_PI32 / 360.0f));
     
-    Result.Elements[0][0] = 1.0f / TanThetaOver2;
-    Result.Elements[1][1] = AspectRatio / TanThetaOver2;
+    Result.Elements[0][0] = Cotangent / AspectRatio;
+    Result.Elements[1][1] = Cotangent;
     Result.Elements[2][3] = -1.0f;
     Result.Elements[2][2] = (Near + Far) / (Near - Far);
     Result.Elements[3][2] = (2.0f * Near * Far) / (Near - Far);
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -10,6 +10,7 @@ To get started, go download [the latest release](https://github.com/HandmadeMath
 
 Version         | Changes        |
 ----------------|----------------|
+**1.10.0** | Made HMM_Perspective use vertical FOV instead of horizontal FOV for consistency with other graphics APIs. |
 **1.9.0** | Added SSE versions of quaternion operations. |
 **1.8.0** | Added fast vector normalization routines that use fast inverse square roots.
 **1.7.1** | Changed operator[] to take a const ref int instead of an int.


Test patch:
diff --git a/test/categories/Projection.h b/test/categories/Projection.h
--- a/test/categories/Projection.h
+++ b/test/categories/Projection.h
@@ -20,16 +20,16 @@ TEST(Projection, Perspective)
     {
         hmm_vec3 original = HMM_Vec3(5.0f, 5.0f, -15.0f);
         hmm_vec4 projected = HMM_MultiplyMat4ByVec4(projection, HMM_Vec4v(original, 1));
-        EXPECT_FLOAT_EQ(projected.X, 5.0f);
-        EXPECT_FLOAT_EQ(projected.Y, 10.0f);
+        EXPECT_FLOAT_EQ(projected.X, 2.5f);
+        EXPECT_FLOAT_EQ(projected.Y, 5.0f);
         EXPECT_FLOAT_EQ(projected.Z, 15.0f);
         EXPECT_FLOAT_EQ(projected.W, 15.0f);
     }
     {
         hmm_vec3 original = HMM_Vec3(5.0f, 5.0f, -5.0f);
         hmm_vec4 projected = HMM_MultiplyMat4ByVec4(projection, HMM_Vec4v(original, 1));
-        EXPECT_FLOAT_EQ(projected.X, 5.0f);
-        EXPECT_FLOAT_EQ(projected.Y, 10.0f);
+        EXPECT_FLOAT_EQ(projected.X, 2.5f);
+        EXPECT_FLOAT_EQ(projected.Y, 5.0f);
         EXPECT_FLOAT_EQ(projected.Z, -5.0f);
         EXPECT_FLOAT_EQ(projected.W, 5.0f);
     }