mathc icon indicating copy to clipboard operation
mathc copied to clipboard

Discussion for the next major release (2.0.0)

Open felselva opened this issue 7 years ago • 17 comments

The library reached a point where there are more additions made than deletions, so I made the first major release (1.0.0). This means until the next release (2.0.0), the library will not break backwards compatibility, but new features, such as functions, can be added with no problem.

This is issue is for discussion of what interesting changes could be made for the next stable (2.0.0) and that break compatibility with the current release, such as modifications in function arguments and structures. Feel free to comment or open issues with suggestions that could affect the entire compatibility.

Update:

The development of version 2 started on the branch mathc2.

felselva avatar Nov 13 '17 16:11 felselva

Hi,

Some ideas:

  1. Make component type in vec, mat and functions redefinable, eg. as double or fixed point, instead of float
  2. Add vec2, vec3, (maybe also mat3x3?)
  3. A few compilers may not support passing struct as arguments, eg. SDCC, consider making all functions passing and returning with pointers (or return through parameters)? There may be also some issues when mathc is built as a lib with Compiler A, then link the lib with Compiler B; because it's up to compiler vendors about whether a caller or callee is in charge of storing, pushing, popping the memory of struct parameter and return value

Just a thought.

Regards, Tony

paladin-t avatar Dec 19 '17 11:12 paladin-t

Thanks for the ideas, Tony.

I started the development of the version 2 on the branch mathc2. I addressed most of the points you mentioned. Still lacking fixed-point arithmetic and 3x3 matrices.

felselva avatar Dec 21 '17 12:12 felselva

Please add struct types back again. v.x is easier to write and read than v[0]. What's the advantage of using v[i] syntax?

cxong avatar Jan 05 '18 02:01 cxong

I think it's still possible to use any type of vector struct (with correct struct alignment) outside mathc, but you have to cast it to pointer at the edge between your program and mathc. Eg.

vec v;
foo((mfloat_t*)&v);

paladin-t avatar Jan 05 '18 03:01 paladin-t

@cxong I agree with you v.x is much more readable than v[0]. Do you think @paladin-t's solution is sufficient? I could add the structure types back again for convenience (so there's no need to be define them outside of mathc).

The reason I decided using arrays (which I forgot to explain) is because it makes easier to pass data to OpenGL buffers that way:

mfloat_t mat[MAT4_SIZE];
mfloat_t vertices[] = {
//      Position          Texture
         0.0f,  1.0, 0.0, 0.5, 0.0,
        -1.0f, -1.0, 0.0, 0.0, 1.0,
         1.0f, -1.0, 0.0, 1.0, 1.0
         ...
}

mat4_translation(mat, position);
vec3_multiply_mat4(&vertices[0],  &vertices[0],  mat);
vec3_multiply_mat4(&vertices[5],  &vertices[5],  mat);
vec3_multiply_mat4(&vertices[10], &vertices[10], mat);
...
/* Push `vertices` to OpenGL */

felselva avatar Jan 05 '18 09:01 felselva

If C11 can be used, then anonymous unions are perfect:

struct vec {
    union {
        struct { float x, y, z, w; };
        float v[4];
    };
};

struct vec v;
v.x = 1;
v.y = 2;
v.z = 3;
v.w = 4;
printf("%f %f %f %f\n", v.v[0], v.v[1], v.v[2], v.v[3]);

But if we stick with C99, I would prefer mathc to use structs and only cast to float array when needed, like when passing to OpenGL.

cxong avatar Jan 09 '18 09:01 cxong

what's the problem with C99?

typedef union vec4 {
    struct { float x, y, z, w; };
    float v[4];
} vec4;

#include <stdio.h>

int main() {
    vec4 v = { 1,2,3,4 };
    printf("%f %f %f %f\n", v.x, v.y, v.z, v.w);
    printf("%f %f %f %f\n", v.v[0], v.v[1], v.v[2], v.v[3]);
}

r-lyeh-archived avatar Jan 09 '18 09:01 r-lyeh-archived

I think anonymous unions is available only on C11, @r-lyeh. On C99, it's available only via extensions.

@cxong, I added that struct layout mathc2. As for the issue of functions taking struct/array pointers, I think the solution will be (like in the first library version) create inline version (to avoid overhead) of each function, but that takes the struct pointer.

Just for information: the issue with casting a structure composed of vec structures (like the one bellow), is that the data might not be aligned (say for example, if mfloat_t is GLfloat) on every platform:

struct triangle {
        struct vec3 a_pos;
        /* Padding might happen here */
        struct vec2 a_color;
        /* Padding might happen here */
        struct vec3 b_pos;
        /* Padding might happen here */
        struct vec2 b_color;
        /* Padding might happen here */
        struct vec3 c_pos;
        /* Padding might happen here */
        struct vec2 c_color;
};

Casting the structure above when passing to OpenGL is error-prone. While padding won't happen if this "mesh" is made with an array:

mfloat_t triangle[5 * 3]; /* No padding between elements */

felselva avatar Jan 09 '18 23:01 felselva

Padding is unlikely because the members are all the same size. But still you can use __attribute__((packed)) and #pragma pack to make sure most compilers pack the struct. Just in case, you can use a static assert to make sure the packing is there and produce a compile error if it isn't.

cxong avatar Jan 10 '18 08:01 cxong

@cxong I saw you are using the functions that take structures as values on CDogs, so I also added back, as optional, the functions that take structure as values.

To use structures, it's needed to define MATHC_STRUCTURES.

To use functions that take structures as value, define MATHC_STRUCT_FUNCTIONS. These have a prefix s.

https://github.com/ferreiradaselva/mathc/blob/7cd23d70a6b5e6f51af3e126094244603c69c510/mathc.h#L1213-L1218

To use functions that take structures as pointer, define MATHC_POINTER_STRUCT_FUNCTIONS. These have a prefix ps:

https://github.com/ferreiradaselva/mathc/blob/7cd23d70a6b5e6f51af3e126094244603c69c510/mathc.h#L415-L419

These functions are inlined, since what they do is call the functions that take mfloat_t array. Basically, the same way how it is done in the first version of the library.

Thoughts?

felselva avatar Jan 10 '18 15:01 felselva

Why not define those functions by default? Their names are different so there's no harm.

cxong avatar Jan 11 '18 12:01 cxong

No problem. I made them between #ifdef to make possible to compile on compilers that can't pass structure as value. I will make them enabled by default, and invert the #ifdefs:

  • MATHC_STRUCT_FUNCTIONS to MATHC_NO_STRUCT_FUNCTIONS
  • MATHC_POINTER_STRUCT_FUNCTIONS to MATHC_NO_POINTER_STRUCT_FUNCTIONS
  • MATHC_STRUCTURES to MATHC_NO_STRUCTURES

So they will be enabled by default, but can be disabled. I want to keep the possibility to disable, also because the anonymous unions are not available on C99.

Edit: I pushed the modifications to mathc2.

felselva avatar Jan 11 '18 13:01 felselva

I think keeping C99 would be preferable. Using packed structs + macros, the API can be easy to use even without the anonymous unions, e.g.

#define psvec2_is_zero(x) vec2_is_zero((const mfloat_t *)x)

cxong avatar Jan 12 '18 00:01 cxong

I think keeping C99 would be preferable. Using packed structs + macros, the API can be easy to use even without the anonymous unions, e.g.

I removed the unions to keep compatible with C99.

I tested what you mentioned earlier about the packing. I thought the padding would happen with nested structures, but I tried -Wpacked with different combinations of nested structures, and it doesn't happen. So I will just leave without the __attribute__((packed))/pack(push, 1) for now (I will only add if it's a problem to someone in the future).

#define psvec2_is_zero(x) vec2_is_zero((const mfloat_t *)x)

Though it's possible to change the ps* functions to a macro, the functions that take (and return) structure by value (prefix s*) couldn't be changed to a macro. So, I think it's better to keep both as inline functions.

felselva avatar Jan 12 '18 03:01 felselva

mathc2 includes Quaternions, therefore I was thinking it may be useful to have functions that convert from Quaternions to Euler Angles, and vice-versa. The implementation of a conversion function would not take long, and I would be happy to do it if there is interest in including it.

Ankush-p avatar May 04 '18 12:05 Ankush-p

@Ankush-p True. I will work on something tonight, but I'm also accepting contributions.

I will leave this for reference: http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/index.htm http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/index.htm

felselva avatar May 04 '18 15:05 felselva

I have come up with a function to convert from a Quaternion to a vec3 Euler. I also needed to add a new definition for MASIN in the header. The functions are below.

mfloat_t *vec3_quat_to_euler(mfloat_t *result, mfloat_t *a)
{
	/* Assume normalized a */
	/* test for singularity */
	mfloat_t test = a[0] * a[1] + a[2] * a[3];
	/* X axis */
	mfloat_t sinX = MFLOAT_C(2.) * (a[3] * a[0] + a[1] * a[2]);
	mfloat_t cosX = MFLOAT_C(1.) - (MFLOAT_C(2.) * (a[0] * a[0] + a[1] * a[1]));
	/* Y axis */
	mfloat_t sinY = MFLOAT_C(2.) * (a[3] * a[1] - a[2] * a[0]);
	/* Z axis */
	mfloat_t sinZ = MFLOAT_C(2.) * (a[3] * a[2] + a[0] * a[1]);
	mfloat_t cosZ = MFLOAT_C(1.) - (MFLOAT_C(2.) * (a[1] * a[1] + a[2] * a[2]));

	if (test > MFLOAT_C(0.499)) {
		result[0] = MFLOAT_C(2.) * MATAN2(a[2], a[3]);
		result[1] = MPI / 2;
		result[3] = 0;
		return result;
	}

	if (test < MFLOAT_C(-0.499)) {
		result[0] = MFLOAT_C(-2.) * MATAN2(a[2], a[3]);
		result[1] = -MPI / 2;
		result[3] = 0;
		return result;
	}

	result[0] = MATAN2(sinX, cosX);
	result[1] = MASIN(sinY);
	result[2] = MATAN2(sinZ, cosZ);

	return result;
}
MATHC_INLINE static struct vec3 *psvec3_quat_to_euler(struct vec3 *result, struct quat *a)
{
	vec3_quat_to_euler((mfloat_t *)&result, (mfloat_t *)&a);
	return result;
}
MATHC_INLINE struct vec3 svec3_quat_to_euler(struct quat a)
{
	struct vec3 result;
	vec3_quat_to_euler((mfloat_t *)&result, (mfloat_t *)&a);
	return result;
}

Ankush-p avatar May 04 '18 22:05 Ankush-p