gingerbill / gb Goto Github PK

Hey, as I'm continue to develop a game based on gb_math, I find that rect unions are pretty much quite useful. Would be nice to have them in standard gb_math. So far I'm using something like this:

gbRect2 gb_rect2_union(gbRect2 a, gbRect2 b)
{
	float tlx = gb_min(a.pos.x, b.pos.x);
	float tly = gb_min(a.pos.y, b.pos.y);
	float brx = gb_max(a.pos.x + a.dim.x, b.pos.x + b.dim.x);
	float bry = gb_max(a.pos.y + a.dim.y, b.pos.y + b.dim.y);
	return gb_rect2(gb_vec2(tlx, tly), gb_vec2(brx - tlx, bry - tly));
}

isize gb_random_range_isize(gbRandom *r, isize lower_inc, isize higher_inc) {
	u64 u = gb_random_gen_u64(r);
	isize i = *cast(isize *)&u; // may be negative if u64 has a bit pattern corresponding to a negative isize
	isize diff = higher_inc-lower_inc+1;
	i %= diff; // modulus of negative i can happen here
	i += lower_inc;
	return i; 
}

A possible fix could be:

isize gb_random_range_isize(gbRandom *r, isize lower_inc, isize higher_inc) {
	u64 u = gb_random_gen_u64(r);
	usize diff = higher_inc-lower_inc+1;
	u %= diff;
	u += lower_inc;
	return cast(isize)u;
}

But you may want to look into better debiasing or faster biased random number within range generation: https://www.pcg-random.org/posts/bounded-rands.html

void *ptr = GB_ALLOC(header_size + len + 1);
if (!init_str)
	memset(ptr, 0, header_size + len + 1);

If GB_ALLOC() return NULL and init_str is not NULL.

Only using gb.h, There are many compile problem.
VS2017 is Ok.
The inside demo in gb.h is outdate.

Noticed you had a TODO for this and I needed a slight variation of it anyway. Not well tested, but my variant seems to work.

gbQuat gb_quat_euler_angles(float pitch, float yaw, float roll) {
	gbQuat q, p, y, r;

	p.x = gb_sin(0.5f*pitch), p.y = 0, p.z = 0, p.w = gb_cos(0.5f*pitch);
	y.x = 0, y.y = gb_sin(0.5f*yaw), y.z = 0, y.w = gb_cos(0.5f*yaw);
	r.x = 0, r.y = 0, r.z = gb_sin(0.5f*roll), r.w = gb_cos(0.5f*roll);

	q.x =   y.w * p.x;
	q.y =   y.y * p.w;
	q.z = - y.y * p.x;
	q.w =   y.w * p.w;

	q.x =   q.x * r.w + q.y * r.z;
	q.y = - q.x * r.z + q.y * r.w;
	q.z =   q.w * r.z + q.z * r.w;
	q.w =   q.w * r.w - q.z * r.z;

	return q;
}

It looks like gb_float22_mul_vec2 expects a row-major matrix, instead of column-major. The gbMat struct is column-major, and the operator gbVec2 operator*(gbMat2 const& a, gbVec2 v) implies we're multiplying a matrix with a column vector (and not multiplying a row vector with a matrix).

void gb_float22_mul_vec2(gbVec2* out, float m[2][2], gbVec2 v) {
  out->x = m[0][0] * v.x + m[0][1] * v.y;
  out->y = m[1][0] * v.x + m[1][1] * v.y;
}

it think this should be:

void gb_float22_mul_vec2(gbVec2* out, float m[2][2], gbVec2 v) {
  out->x = m[0][0] * v.x + m[1][0] * v.y;
  out->y = m[0][1] * v.x + m[1][1] * v.y;
}

The same holds for gb_float33_mul_vec3

gb_float44_mul_vec4 is ok.

gb_quat_slerp seems to be fairly broken, it yields rather large results or nans/infs for well-formed input and does not agree at all with gb_quat_slerp_approx or gb_quat_nlerp.

For example, the following non-tricky interpolation results in a magnitude of over 2, and small angle actual data yielded magnitudes in the thousands in my application:

gbMat4 ma, mb;
gb_mat4_rotate(&ma, gb_vec3(1.0f, 0.0f, 0.0f), 0.3f);
gb_mat4_rotate(&mb, gb_vec3(1.0f, 0.0f, 0.0f), 1.2f);

gbQuat q, qa, qb;
gb_quat_from_mat4(&qa, &ma);
gb_quat_from_mat4(&qb, &mb);

gb_quat_slerp(&q, qa, qb, 0.2f);
float mq = gb_quat_mag(q);

If one contrasts the implementation with the Wikipedia example code the gb_math.h implementation seems to eventually differ mathematically in two places:
s1 = gb_sin(1.0f - t*angle); ought to be s1 = gb_sin(angle - t*angle);
gb_quat_mulf(&x, z, s1); ought to be gb_quat_mulf(&x, a, s1);

Once those two changes are in place, the output agrees with gb_quat_slerp_approx.

Would be nice to have mat inverse functions, they are quite useful for projecting screen coordinates back to world/model space. In most cases just having mat4 affine inverse will suffice.

I would like to point out that identifiers like "__GB_NAMESPACE_START" and "gb__string_realloc" do eventually not fit to the expected naming convention of the C++ language standard.
Would you like to adjust your selection for unique names?

// ...
float e[2];
} gbVec3;
// ...
should be:
// ...
float e[3];
} gbVec3;
// ...

At https://github.com/gingerBill/gb/blob/master/gb.h#L207, there is a check for if GNUC is defined, and if so the compiler is marked as gcc. However, clang also defines the GNUC macro and so is detected as being gcc; to fix this, put the clang check first.

As I mentioned here #14 there still a case that gb_mat4_inverse transposes the result for me, aka

 --------- inverting 
0.220 0.000 0.000 345.000
0.000 -0.220 0.000 365.200
0.000 0.000 -1.000 0.000
0.000 0.000 0.000 1.000
 --------- result from gb_mat4_inverse
4.545 -0.000 0.000 -0.000
-0.000 -4.545 -0.000 0.000
0.000 -0.000 -1.000 -0.000
-1568.182 1660.000 -0.000 1.000
 --------- should be 
4.545 -0.000 0.000 -1568.182
-0.000 -4.545 -0.000 1660.000
0.000 -0.000 -1.000 -0.000
-0.000 0.000 -0.000 1.000

So my code to fix it looks like this now:

gb_mat4_inverse(&inv_vpvw, &vpvw);
gb_mat4_transpose(&inv_vpvw);

Would be nice to fix it in upstream :)

Well, technically it's partially broken, but there is a mismatch in row-column major implementations.
To reproduce try this one:

gbVec4 translate = gb_vec4(100.0f, 200.0f, 300.0f, 1.0f);
gbVec4 in = gb_vec4(0.0f, 0.0f, 0.0f, 1.0f);
gbVec4 out;
gbMat4 mat;
gb_mat4_translate(&mat, translate.xyz);
gb_mat4_mul_vec4(&out, &mat, in);
printf("%f %f %f %f\n", in.x, in.y, in.z, in.w);
printf("%f %f %f %f\n", out.x, out.y, out.z, out.w);

Prints:

0.000000 0.000000 0.000000 1.000000
0.000000 0.000000 0.000000 1.000000

When it's definitely should print something else in a second line ;)

I was looking over the various math functions and noticed that there is a lot of error in the implementation of the Logarithm and Exponential functions. This is after applying the Intrinsic-free version of the square root function.

// The Square-Root Function.
sqrtf(x);
// Fast and Simple alternative... 
expf(0.5f*logf(x));

// the Power function
powf(a, b);
// alternative method:
expf(b*logf(a));

Now as far as improving the Accuracy of the Exponential and Logarithm Functions. I suggest looking into the work completed by Ping Tak Peter Tang in the following series of articles from the early 1990s and late 1980s.

• Table-driven implementation of the Expm1 function in IEEE floating-point arithmetic. ACM Transactions on Mathematical Software 18(2): 211-222 (1992)
• Table-lookup algorithms for elementary functions and their error analysis. IEEE Symposium on Computer Arithmetic 1991: 232-236
• Accurate and efficient testing of the exponential and logarithm functions. ACM Transactions on Mathematical Software 16(3): 185-200 (1990)
• Table-driven implementation of the exponential function in IEEE floating-point arithmetic. ACM Transactions on Mathematical Software ( 15(2): 144-157 (1989)

Edit: Added note about alternative method for calling the pow function.

gb_remainder uses round. this is bad, because it means gb_remainder(1.5, 2.0) = -0.5, and gb_mod(2.5, 2.0) = 2.5. this is contrary to how the C standard library modf / remainder functions work.

edit: it looks like gb_remainder is correct; but gb_mod is wrong.

The byte 0x80 is no a valid utf-8 but the function doen't return GB_RUNE_INVALID

gb_memcopy returns a pointer that is equal to dest+n where n is the number of bytes copied.

This causes certain certain allocation functions to fail, like gb_alloc_str, gb_alloc_str_len, and gb_alloc_copy.

It seems that the instruction rep movsb increments the pointers tht are passed in.

There are tons of functions in gb_math with declaration similar to gb_mat4_mul(gbMat4 *out, gbMat4 *m1, gbMat4 *m2). Would be nice to have a clarification if out can point to either of inputs or not. Because looks like most of functions are safe to use when we reuse one of inputs as output, but I'm not 100% sure :)

In gb_ucs2_to_utf8, in the case of surrogate UTF-6 chars, there is only a check for the first u16.
else if (*str >= 0xd800 && *str < 0xdc00)

I believe it should also check that the second u16 is valid:
if (Src[1] < 0xDC00 || Src[1] >= 0xE000) return NULL;

It should work fine with UCS2 as, in theory, surrogate are not part of the encoding.
It might not check properly in case of UTF-16.

Bug here: https://github.com/gingerBill/gb/blob/master/gb.hpp#L980

https://github.com/gingerBill/gb/blob/master/gb.h#L7957-L7959

Pretty sure this always produces incorrect results since libc functions return 0 on success.

Reading gb_math.h source sometimes is really confusing.
Usual math notation for matrix is Aij where i is row and j is column:

A00 A01 A02 A03
A10 A11 A12 A13
A20 A21 A22 A23
A30 A31 A32 A33

Note that this notation is memory-layout free, this is just how matrices are written down on paper in generic science/engineering notation.

Considering this:

typedef union gbMat4 {
	struct { gbVec4 x, y, z, w; };
	gbVec4 col[4];
	float e[16];
} gbMat4;

One can make an assumption that layout is column-major, awesome. But then reading something like this:

gbFloat4 *gb_float44_m(gbMat4 *m)    { return (gbFloat4 *)m; }
...
gbFloat4 *m = gb_float44_m(out);
...
m[0][0] = 1.0f / (aspect*tan_half_fovy);
m[1][1] = 1.0f / (tan_half_fovy);
m[2][2] = -(z_far + z_near) / (z_far - z_near);
m[2][3] = -1.0f;
m[3][2] = -2.0f*z_far*z_near / (z_far - z_near);

Means that m is still column-major, so it's m[j][i] notation instead of m[i][j] as accepted in generic mathematics. We can test it like this :

uint8_t a[16] = // column-major
{
	0x0, 0x1, 0x2, 0x3,
	0x4, 0x5, 0x6, 0x7,
	0x8, 0x9, 0xa, 0xb,
	0xc, 0xd, 0xe, 0xf
};
typedef uint8_t column_t[4];
column_t * c = (column_t*)a;
printf("%x\n", c[2][1]); // prints 9, which means 2 is column, 1 is row

So would be nice to write down some comments in a lib saying this, and maybe introduce some _M(i, j) macro just so algorithms are written down in more mathematics friendly notation, which should enable less error-prone code.

I know this is not built with immediate Mac support, but I thought I should draw your attention to an error I've received when building with GB.

error: invalid use of a cast in a inline asm context requiring an l-value: remove the cast or build with -fheinous-gnu-extensions

The trace says line 3622 of gb.h underlining cast(u8 *gb_restrict)dest and cast(u8 *gb_restrict)source

In https://github.com/gingerBill/gb/blob/master/gb.h line 8971
I believe this should be set to 24 and not 32 :-
pfd.cColorBits = 32;
As according to https://msdn.microsoft.com/en-us/library/windows/desktop/dd368826(v=vs.85).aspx and other sources, the size does not include the alpha bits, only the RGB part. (And when I try similar and call DescribePixelFormat it's given me a format with it set to 24 anyway.

Probably no big deal even if true, but seemed worth mentioning.

The Windows version of gbMutex still depends on CRITICAL_SECTION, which is in windows.h.

// Mutex
typedef struct gbMutex {
#if defined(GB_SYSTEM_WINDOWS)
	CRITICAL_SECTION win32_critical_section;
#else
	pthread_mutex_t pthread_mutex;
	pthread_mutexattr_t pthread_mutexattr;
#endif
} gbMutex;

By mistake I've passed 45.0f to gb_mat4_perspective and everything was looking fine.
But when I've enabled GB_MATH_NO_MATH_H suddenly nothing worked anymore.

Apparently tanf does some wrap arounds inside, and current emulation doesn't do that.
After some googling I found that original code is probably coming from some old DSP code ( for example this ) and then idlib math included it with some improvements here.

Not a problem for me, but would be nice to fix it at some point 👍

This is a demo, vs2017 is ok.

`
#define GB_IMPLEMENTATION
#include "gb.h"
#include <stdio.h>

int main(int argc, char **argv)
{
gbAllocator allocator = gb_heap_allocator();
gbString str = gb_string_make(allocator ,"Hello");

gbString other_str = gb_string_make_length(allocator,", ", 2);

str = gb_string_append(str, other_str);
str = gb_string_appendc(str, "world!");

gb_printf("%s\n", str); // Hello, world!

gb_printf("str length = %d\n", gb_string_length(str));

str = gb_string_set(str, "Potato soup");
gb_printf("%s\n", str); // Potato soup

str = gb_string_set(str, "Hello");
other_str = gb_string_set(other_str, "Pizza");
if (gb_string_are_equal(str, other_str))
	gb_printf("Not called\n");
else
	gb_printf("Called\n");

str = gb_string_set(str, "Ab.;!...AHello World       ??");
str = gb_string_trim(str, "Ab.;!. ?");
gb_printf("%s\n", str); // "Hello World"

gb_string_free(str);
gb_string_free(other_str);

return 0;

}
`

I noticed that the mask for the second and third byte in gb_endian_swap32() are the same. Here is the line: https://github.com/gingerBill/gb/blob/master/gb.h#L5524

line 100 of gb_math.h

Hello! I am not a pro at setting up my C environment and am spoiled by all in one IDEs.

When I try to #include "gb.h" I get an error regardless of what #define I try.

In file included from game.c:4:
gb.h:204:3: error: #error This operating system is not supported
gb.h:214:3: error: #error Unknown compiler
gb.h:250:3: error: #error Unknown CPU Type
gb.h:307:20: fatal error: dlfcn.h: No such file or directory
compilation terminated.

I could just be a complete derp.