moderngl / moderngl-window

A cross platform utility library for ModernGL making window creation and resource loading simple
MIT License
245 stars 60 forks source link

Drop Pyrr and replace it with PyGLM #198

Closed henri-gasc closed 5 days ago

henri-gasc commented 1 month ago

Continuation of #176 and #177.

All the tests run successfully, but the examples are not correct (just run examples/cube_model to see it) and I do not understand how to solve it.
To create a matrix from Euler angles, you should use [roll, pitch, yaw] for Pyrr, and [pitch, yaw, roll] for PyGLM. However, if we use those, the results are different.

We can manage to get the same quaternions by using pyrr.quaternion.create_from_inverse_of_eulers((roll, pitch, yaw))) and q = glm.quat(glm.vec3(yaw, pitch, roll)); [q[2], q[1], q[3], q[0]]. As you can see, neither the method nor the arguments are what the docs tell us to use. We need this, otherwise both the quaternions and resulting 4x4 matrices are different.

If you want to see for yourself:

import glm, pyrr, numpy

pitch, yaw, roll = numpy.radians((10, 20, 30))
pyrr_qua = pyrr.quaternion.create_from_inverse_of_eulers((roll, pitch, yaw))
q = glm.quat(glm.vec3(yaw, pitch, roll))
glm_qua = [q[2], q[1], q[3], q[0]]

print("We have almost the same output:")
print(f"Pyrr: {pyrr_qua}")
print(f"GLM: {glm_qua}")

# And how we cannot have the same output
print(f"Pyrr: ", pyrr.quaternion.create_from_eulers((roll, pitch, yaw)))
print(f"GLM 1: {glm.quat(glm.vec3(pitch, yaw, roll))}")
print(f"GLM 2: {glm.quat(glm.vec3(pitch, roll, yaw))}")
print(f"GLM 3: {glm.quat(glm.vec3(roll, pitch, yaw))}")
print(f"GLM 4: {glm.quat(glm.vec3(roll, yaw, pitch))}")
print(f"GLM 5: {glm.quat(glm.vec3(yaw, roll, pitch))}")
print(f"GLM 6: {glm.quat(glm.vec3(yaw, pitch, roll))}")
einarf commented 1 month ago

Great. I will take a look soon. 👍

henri-gasc commented 1 month ago

The problem may come from the way numpy (Pyrr) and PyGLM handle matrices and their indices

import glm, numpy

a = numpy.eye(4)
b = glm.mat4()
(a == b).all() # == True

a[0, 1] = 1; b[0, 1] = 1
(a == b).all() # == False

# They differ in (0, 1) and (1, 0)
print(a)
print(b)
henri-gasc commented 6 days ago

ping

henri-gasc commented 5 days ago

Pyrr is, for lack of a better word, a mess. pyrr.Matrix44.from_eulers(...) and pyrr.Matrix44.from_quaternion(pyrr.Quaternion.from_eulers(...) does not give out the same matrix. That is because Quaternion.from_eulers output something strange.

roll, pitch, yaw = np.radians((60, 30, 20))

rot = pyrr.Matrix44.from_eulers((roll, pitch, yaw))
print(rot.quaternion)
print(pyrr.Quaternion.from_eulers((roll, pitch, yaw)))

# Output is
[0.5145478  0.27270303 0.13687299 0.80133601]
[0.5145478  0.13687299 0.27270303 0.80133601]

Another problem also stems from the differences between the expected and got outputs. On this, from everything I could find, Pyrr is just wrong, the correct equations are not used. See this, compared with the versions on Wikipedia and used by GLM (keep in mind that Pyrr has w as the last element of the quaternion).

In short, Pyrr gives false result, and there will probably be a need to redo all examples and all codes using moderngl-window, as the output will be different after updating to PyGLM

henri-gasc commented 5 days ago

@einarf I finaly managed to fix the issue concerning the examples, and unlike what I said in my previous message, it does not seems like we need to update the code that depends on moderngl-window. Feel free to test this with other examples or applications.

einarf commented 5 days ago

Great. I go through things today 👍 (and other PRs)

einarf commented 4 days ago

This still need a substantial amount of work to sort out all kind of typing, bugs, docstrings and other weirdness. Still, it's for the best to just merge this to get the ball rolling.

henri-gasc commented 3 days ago

Before I do it and potentially waste my time, should/can I fix most mypy errors and warnings ?