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

Discussion for the next major release (2.0.0) #17

Open
felselva opened this issue Nov 13, 2017 · 17 comments
Open

Discussion for the next major release (2.0.0) #17

felselva opened this issue Nov 13, 2017 · 17 comments

Comments

@felselva
Copy link
Owner

felselva commented Nov 13, 2017

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 felselva changed the title Discussion for the next stable release (2.0.0) Discussion for the next major release (2.0.0) Nov 13, 2017
@paladin-t
Copy link

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

@felselva
Copy link
Owner Author

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.

@cxong
Copy link
Contributor

cxong commented Jan 5, 2018

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?

@paladin-t
Copy link

paladin-t commented Jan 5, 2018

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);

@felselva
Copy link
Owner Author

felselva commented Jan 5, 2018

@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 */

@cxong
Copy link
Contributor

cxong commented Jan 9, 2018

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.

@r-lyeh-archived
Copy link

r-lyeh-archived commented Jan 9, 2018

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]);
}

@felselva
Copy link
Owner Author

felselva commented Jan 9, 2018

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 felselva self-assigned this Jan 10, 2018
@cxong
Copy link
Contributor

cxong commented Jan 10, 2018

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.

@felselva
Copy link
Owner Author

felselva commented Jan 10, 2018

@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?

@cxong
Copy link
Contributor

cxong commented Jan 11, 2018

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

@felselva
Copy link
Owner Author

felselva commented Jan 11, 2018

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.

@cxong
Copy link
Contributor

cxong commented Jan 12, 2018

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)

@felselva
Copy link
Owner Author

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.

@Ankush-p
Copy link

Ankush-p commented May 4, 2018

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.

@felselva
Copy link
Owner Author

felselva commented May 4, 2018

@Ankush-p
Copy link

Ankush-p commented May 4, 2018

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;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants