Repository: Zuzu-Typ/PyGLM (language: python)

Issue:
few enhancements suggestions in the API
Hello that's me again !

With time, I have been collecting many small notices I had on the types and functions present in this great module. Here I would like to report back my thoughts about possible nice improvements :slightly_smiling_face: 
I know some of the designs my suggestions might be more related to [glm](https://github.com/g-truc/glm) than to `pyglm` ... but still !

## 1. mat <-> quat conversion

The way to convert matrices to quaternion is very smooth:  `quat(mat3(...))`
However the conversion from quat to mat is weird in my mind:  `mat3_cast(quat(...))` or `mat4_cast(quat(...))`
I would think that if a `quat` can be created directly from a matrix, with no extra function, it should be the same back to `mat`
Also conversion `quat->mat` is non-ambiguous at contrary to `mat->quat` which can be subject to interpretation when `mat` is non orthogonal. So I would say if some of both conversions should be a separated function, it should be `mat->quat` and not `quat->mat`

## 2. affine transformation for vectors

Half the operations I do with `mat4` is transforming a `vec3`, but as they are not of compatible dimensions there is always the need to convert them. Could it be possible to have a function doing this multiplication ?
```python
>>> vec3(matrix * vec4(v,1))    # many conversions and parenthesis for just one vector
>>> hmul(matrix, v)   # all conversions made in C code, so much more efficient
```
(I thought to `hmul` for Homogeneous Multiplication because `affineMul` like `affineInverse` seemed too cumbersome)

As i suspect it could be useful in other dimensions so it could be an overloaded functions:
```python
hmul(mat4, vec3) -> vec3
hmul(mat3, vec2) -> vec2
hmul(mat2, vec1) -> vec1
```

## 3. have `glm.min/max` work on iterables, like the python builtin functions

I think it wouldn't change a lot efficiency, but do you think it could be possible to have `glm.min/max` work with iterables of vectors ?
Currently I am frequently in situation where I need to do this
```python
vmin = vec3(inf)
for p in points:
    vmin = glm.min(vmin, p)

# instead of
vmin = glm.min(points)
```

making it working on iterables would make `glm.min/max` perfect vectorized replacements for the builtins `min/max`

## 4. rotation and translation functions

I fail to understand the exact meaning of the current `glm.rotate` and `glm.translate` functions :thinking: 
I have been using it in the early times of [my module](https://github.com/jimy-byerley/pymadcad) but found them error prone and hard to read mixed in other codes.

seeing [this example](https://github.com/g-truc/glm/blob/master/manual.md#-11-using-global-headers), It seems to be meant to apply successive transformations to a matrix, but why should it be done by composing those functions (which is hard to read), rather than multiplying the transformation matrices. Affine matrices are meant for it after all ?
I would have imagined something like
```python
htranslation(offset:vec3) -> mat4     # returning a mat4 doing only a translation
hrotation(angle:float, axis:vec3) -> mat4  # returning a mat4 doing only a rotation
hrotation(quat) -> mat4
hrotationx(angle:float) -> mat4  # rotation around X
hrotationy(angle:float) -> mat4  # rotation around Y
hrotationz(angle:float) -> mat4  # rotation around Z

# same as in the above glm example (If I correctly understood it):
>>> view = hrotation(orientation.x, X) * hrotation(orientation.y, Y) * htranslation(translate)
```


# conclusion

I hope I haven't been too out of topic with my suggestions. Else what do you think about these ?

cheers, Byerley


Discussion / hints:
> ## 1. mat <-> quat conversion
> The way to convert matrices to quaternion is very smooth: `quat(mat3(...))`
> However the conversion from quat to mat is weird in my mind: `mat3_cast(quat(...))` or `mat4_cast(quat(...))`
> I would think that if a `quat` can be created directly from a matrix, with no extra function, it should be the same back to `mat`

Yeah. I agree. This would be more logical.



> ## 2. affine transformation for vectors
> Half the operations I do with `mat4` is transforming a `vec3`, but as they are not of compatible dimensions there is always the need to convert them. Could it be possible to have a function doing this multiplication ?

This seems like a more complex feature to implement.
I'll see what I can do.
Could you please make a separate Issue for this?



> ## 3. have `glm.min/max` work on iterables, like the python builtin functions
> I think it wouldn't change a lot efficiency, but do you think it could be possible to have `glm.min/max` work with iterables of vectors ?
> Currently I am frequently in situation where I need to do this
> 
> ```python
> vmin = vec3(inf)
> for p in points:
>     vmin = glm.min(vmin, p)
> 
> # instead of
> vmin = glm.min(points)
> ```

I think this would be a nice feature.
For the time being, here's a slight improvement:
```Python
import functools

functools.reduce(glm.min, points)
```



> ## 4. rotation and translation functions
> I fail to understand the exact meaning of the current `glm.rotate` and `glm.translate` functions 🤔

It's actually simpler than you might think.
`glm.rotate(m, angle, axis)` creates a rotation matrix around the given axis (e.g. `vec3(0, 1, 0)`), by the given `angle` and the input matrix `m` is multiplied by this newly created rotation matrix.
It's basically a shorthand for:
```Python

# The hrotation function you suggested:
hrotation = lambda angle, axis: glm.rotate(glm.mat4(), angle, axis)

def rotate(m, angle, axis):
    return m * hrotation(angle, axis)
```

I don't know though why there is no overloaded version of `rotate()` or `translate()` that omits the input matrix.

---

Thank you for the suggestions!
I'll have to see when I have enough free time to implement them (:

Cheers
--Zuzu_Typ--
I opened a separated issue for suggestion 2. as you asked :)

Fix patch:
diff --git a/PyGLM/type_methods/mat.h b/PyGLM/type_methods/mat.h
index 8e1435f..9f2e980 100644
--- a/PyGLM/type_methods/mat.h
+++ b/PyGLM/type_methods/mat.h
@@ -597,6 +597,13 @@ mat3x3_init(mat<3, 3, T> *self, PyObject *args, PyObject *)
 			self->super_type = PyGLM_Mat_PTI_Get0(3, 3, uint32, arg1);
 			return 0;
 		}
+
+		// quaternions
+		PyGLM_PTI_Init0(arg1, PyGLM_T_QUA | PyGLM_PTI_GetDT(T));
+		if (!PyGLM_PTI_IsNone(0)) {
+			self->super_type = glm::mat3_cast(PyGLM_Qua_PTI_Get0(T, arg1));
+			return 0;
+		}
 		PyErr_SetString(PyExc_TypeError, "invalid argument type(s) for mat3x3()");
 		return -1;
 	}
@@ -1138,6 +1145,13 @@ mat4x4_init(mat<4, 4, T> *self, PyObject *args, PyObject *)
 			self->super_type = PyGLM_Mat_PTI_Get0(4, 4, uint32, arg1);
 			return 0;
 		}
+
+		// quaternions
+		PyGLM_PTI_Init0(arg1, PyGLM_T_QUA | PyGLM_PTI_GetDT(T));
+		if (!PyGLM_PTI_IsNone(0)) {
+			self->super_type = glm::mat4_cast(PyGLM_Qua_PTI_Get0(T, arg1));
+			return 0;
+		}
 		PyErr_SetString(PyExc_TypeError, "invalid argument type(s) for mat4x4()");
 		return -1;
 	}


Test patch:
diff --git a/test/PyGLM_test.py b/test/PyGLM_test.py
index 9c5db69..bb572c1 100644
--- a/test/PyGLM_test.py
+++ b/test/PyGLM_test.py
@@ -442,6 +442,14 @@ def test_mat_types():
             for args in gen_args("#uM{C}{R}".format(C=C, R=R) + "V{R}".format(R=R)*C + "_" + "_".join(["M{C}{R}M{c}{r}".format(C=C, R=R, c=c, r=r) for c in range(2, 5) for r in range(2,5)]) + "__fFiI"):
                 fassert(type(args[0]), args[1:])
 
+    for args in gen_args("Q__f"):
+        fassert(glm.mat3, args)
+        fassert(glm.mat4, args)
+
+    for args in gen_args("Q__F"):
+        fassert(glm.dmat3, args)
+        fassert(glm.dmat4, args)
+
 ## quat
 def test_quat_types():
     for args in gen_args("#u-_V3_M33_M44_NV3_V3V3_NNNN_Q__f"): # need support for conversion constructors