Skip to content
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

Sequence initialization behaviors #153

Closed
esoma opened this issue Oct 19, 2021 · 2 comments · Fixed by #154
Closed

Sequence initialization behaviors #153

esoma opened this issue Oct 19, 2021 · 2 comments · Fixed by #154
Assignees

Comments

@esoma
Copy link

esoma commented Oct 19, 2021

Thanks for the quick turnaround on #151, it's refreshing to see a maintainer take action so quickly (no pressure on this one 😄).

I'm writing type stubs for pyglm and came across some behaviors I'm not sure are intentional or not regarding sequences. I'm not making any prescriptions here for what pyglm should do, but I'm noting these because they differ from how I've put the stubs together (because these behaviors are either difficult to describe with the python typing system or don't appear immediately useful in a typed context) and they seem like the kind of thing that might be artifacts of implementation.


Sequences used for initialization that contain items that are not __float__/__int__ compatible will be 0:

import glm
glm.vec2(('abc', None)) # vec2(0, 0)

This is contrary to non-sequence initialization where this is a TypeError:

import glm
glm.vec2('abc', None) # TypeError: invalid argument type(s) for vec()

Sequences used for initialization may be larger than the type's length, but only if the sequence has a length < 5:

import glm
glm.vec2([1, 2]) # vec2(1, 2)
glm.vec2([1, 2, 3]) # vec2(1, 2)
glm.vec2([1, 2, 3, 4]) # vec2(1, 2)
glm.vec2([1, 2, 3, 4, 5]) # TypeError: invalid argument type(s) for vec()

This rule also applies to matrix row/columns.


Following that, all matrix columns must have an equal size, even if they extend beyond what that matrix actually requires:

import glm
glm.mat2x2([
    [1, 2, 3],
    [10, 20, 30],
])
# [1][10]
# [2][20]

glm.mat2x2([
    [1, 2, 3],
    [10, 20],
]) # TypeError: invalid argument type(s) for mat2x2()

glm.mat2x2([
    [1, 2, 3],
    [10, 20, 30],
    [100, 200, 300],
])
# [1][10]
# [2][20]

glm.mat2x2([
    [1, 2,],
    [10, 20,],
    [100, 200, 300],
])  # TypeError: invalid argument type(s) for mat2x2()
@Zuzu-Typ
Copy link
Owner

Hiya ^^

Thanks for the quick turnaround on #151, it's refreshing to see a maintainer take action so quickly (no pressure on this one 😄).

You're very welcome (:

Okay, let me have a look..

Sequences used for initialization that contain items that are not __float__/__int__ compatible will be 0:

import glm
glm.vec2(('abc', None)) # vec2(0, 0)

Yeah, this is not intentional.
I was a little surprised when I found out why exactly this happens.

When you call one of PyGLM's functions, the first thing it needs to do is find out what kind of arguments you've supplied.
Each function has a different number of argument combinations that are valid.
For example, the vec2 constructor accepts (among other things) a vector (either vec or mvec) of length 2 or greater of the given number type (float in this case).

(https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/type_methods/vec.h#L138)

PyGLM_PTI_Init0(arg1, PyGLM_T_ANY_VEC | PyGLM_SHAPE_2 | PyGLM_SHAPE_3 | PyGLM_SHAPE_4 | PyGLM_PTI_GetDT(T));

Internally, this macro first checks whether or not the supplied argument is a PyGLM type (e.g.vec2 or mat4x4 or dquat) and if so, it uses a quick way of checking if the argument is valid for this function.
If it is not a PyGLM type (as would be the case here), it tries to interpret the given argument as a PyGLM type.
This is a pretty complicated process, so I'm going to skip a few details.
During this process it finds out that the argument is a tuple of length 2.
It knows that we're looking for a vector of length 2 and since the tuple is also length two, it assumes that the tuple represents a vec2 (this is still intentional).

https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/internal_functions/type_checkers.h#L1505

if ((accepted_types & PyGLM_SHAPE_2) && (accepted_types & PyGLM_T_ANY_VEC)) {

It then tries to find out what's inside the tuple (i.e. which data types are contained by the tuple).

https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/internal_functions/type_checkers.h#L1506-L1509

PyGLMSingleTypeHolder item1Out = PyGLMSingleTypeHolder(item1);
PyGLMSingleTypeHolder item2Out = PyGLMSingleTypeHolder(item2);

int out_type = PyGLMSingleTypeHolder::getMostImportantType(accepted_types, { item1Out.dtype, item2Out.dtype });

This might look a bit complicated, but it's actually not.
The PyGLMSingleTypeHolder class is used to find out which numeric data type we have:

https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/internal_functions/type_checkers.h#L61-L104

if (PyBool_Check(o)) {
	dtype = DType::BOOL;
	[...]
}
else if (PyFloat_Check(o)) {
	double value = PyFloat_AS_DOUBLE(o);
	if (value > FLT_MAX || (value != 0.0 && value < FLT_MIN && value > - FLT_MIN) || value < - FLT_MAX) { // value doesn't > fit in float
		dtype = DType::DOUBLE;
	        [...]
	}
	else {
		dtype = DType::FLOAT;
	        [...]
	}
}
else if (PyLong_Check(o)) {
	[...]
}
else {
	dtype = DType::NONE;
}

And indeed, it does find out that neither 'abc' nor None are numeric values. I.e. it sets the dtype to NONE.

The problem lies within the getMostImportantType function, which is supposed to find out which numeric type suits the valid argument types the best (so you don't have to use floats in your tuple, i.e. vec2( ( 1, 2 ) ) works equally well as vec2( ( 1.0, 2.0 ) ), even though both times the numbers will end up as floats).
Anyway, in that function there is a check that basically says if the function accepts floats, use floats:

https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/internal_functions/type_checkers.h#L124-L125

if (accepted_types & PyGLM_DT_FLOAT)
    return PyGLM_DT_FLOAT;

What this does is telling the program to convert whichever numeric type it has to float.
Since it doesn't actually have a numeric type at this point, it just uses the default (fallback) value of 0.0:

https://github.com/Zuzu-Typ/PyGLM/blob/master/PyGLM/internal_functions/type_checkers.h#L188-L205

float asFloat() {
	switch (dtype) {
	case DType::DOUBLE:
		return static_cast<float>(*(double*)data);
	case DType::FLOAT:
		return (*(float*)data);
	case DType::INT32:
		return static_cast<float>(*(long*)data);
	case DType::INT64:
		return static_cast<float>(*(long long*)data);
	case DType::UINT64:
		return static_cast<float>(*(unsigned long long*)data);
	case DType::BOOL:
		return static_cast<float>(*(bool*)data);
	default:
		return 0.0f;
	}
}

Thus, you get a vec2( 0, 0 ).

This is relatively easy to fix, but it might take a while.

Sorry, might have gotten a little overboard, but I hope it was interesting to you 😅

Sequences used for initialization may be larger than the type's length, but only if the sequence has a length < 5:

import glm
glm.vec2([1, 2]) # vec2(1, 2)
glm.vec2([1, 2, 3]) # vec2(1, 2)
glm.vec2([1, 2, 3, 4]) # vec2(1, 2)
glm.vec2([1, 2, 3, 4, 5]) # TypeError: invalid argument type(s) for vec()

This behavior is actually intentional. It is the same way as glm by g-truc handles it.
You will find that you can also do this with the vector classes (e.g. glm.vec2( glm.vec4(1, 2, 3, 4) ))
Interestingly, GLSL accepts this only for vec3(vec4()) (see https://www.khronos.org/opengl/wiki/Data_Type_%28GLSL%29#Vector_constructors).

Following that, all matrix columns must have an equal size, even if they extend beyond what that matrix actually requires:

import glm
glm.mat2x2([
    [1, 2, 3],
    [10, 20, 30],
])
# [1][10]
# [2][20]

glm.mat2x2([
    [1, 2, 3],
    [10, 20],
]) # TypeError: invalid argument type(s) for mat2x2()

glm.mat2x2([
    [1, 2, 3],
    [10, 20, 30],
    [100, 200, 300],
])
# [1][10]
# [2][20]

glm.mat2x2([
    [1, 2,],
    [10, 20,],
    [100, 200, 300],
])  # TypeError: invalid argument type(s) for mat2x2()

This is also intentional. This is also the way that glm does it.
I don't know how versatile this is in GLSL, but I've seen mat4( mat3() ) on the wiki.

The idea here is that you can create a smaller matrix from a bigger one, effectively cropping it and discarding all values that the smaller matrix can't fit.
It also works the other way around. You can create a bigger matrix from a small one. All values that would be undefined are padded with values from the identity matrix:

>>> print( mat4(mat2(1,2,3,4)) )
[            1 ][            3 ][            0 ][            0 ]
[            2 ][            4 ][            0 ][            0 ]
[            0 ][            0 ][            1 ][            0 ]
[            0 ][            0 ][            0 ][            1 ]

You can also do both - i.e. expand one way and crop the other:

>>> print( mat4x2(mat2x4(1,2,3,4,5,6,7,8)) )
[            1 ][            5 ][            0 ][            0 ]
[            2 ][            6 ][            0 ][            0 ]

Don't ask me what the use case would be though.

Hope this helps (:

Cheers
--Zuzu_Typ--

@esoma
Copy link
Author

esoma commented Oct 20, 2021

Sorry, might have gotten a little overboard, but I hope it was interesting to you 😅

For sure, thanks.

This behavior is actually intentional. It is the same way as glm by g-truc handles it.
You will find that you can also do this with the vector classes (e.g. glm.vec2( glm.vec4(1, 2, 3, 4) ))

This is also intentional. This is also the way that glm does it.
I don't know how versatile this is in GLSL, but I've seen mat4( mat3() ) on the wiki.

The idea here is that you can create a smaller matrix from a bigger one, effectively cropping it and discarding all values that the smaller matrix can't fit.

I find that all pretty intuitive. The odd part (for me) is the somewhat arbitrary limit on python sequences being limited to 4 items or less and the rejection of oddly shaped "matrices" composed of sequences. Those limitations in glm are imposed naturally, since the objects themselves can't break out of their assigned shapes, but python lists/tuples/etc can be whatever we want. That said, it's not like I have a use case for requesting those be allowed. I just found them a bit surprising while comparing type checker results 😉.

Zuzu-Typ added a commit that referenced this issue Oct 22, 2021
Zuzu-Typ added a commit that referenced this issue Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants