recp / cglm

📽 Highly Optimized 2D / 3D Graphics Math (glm) for C
MIT License
2.34k stars 231 forks source link

Having trouble whit the library + a lot of warns caused by nameless struture #307

Closed Nonesence999 closed 1 year ago

Nonesence999 commented 1 year ago

Hello I am having a bad time whit the library I need to do :

camera->Position += camera->speed * camera->Orientation;

This is the closest I have gotten :

vec3 temp;
temp = glm_vec3_mul(camera->speed, camera->Orientation, temp);
 glm_vec3_add(camera->Position, temp, camera->Position);

Its realy unoptimized and not working since you cannot glm_vec3_mul(**float**,vect3). I had to use multiple opperations separatly and a temporary variable.

I am also having trouble doing things like **-**glm_normalize(out) because it normalize the source and returns nothing or **-**camera->Orientation(vect3) because - is invalid I can maybe use inverse but it seems overkill.

To conclude I am looking for: -a glm_normalize() that returns and don't normalize the input

I have fund warnings because there are a ton of unnamed strutures in the strucutre header.

recp commented 1 year ago

Hello @Nonesence999,

Here some usage to achieve:

*camera->Position += camera->speed camera->Orientation;**

assuming camera->Position is vec3 ( could be vec4 )

Array API (use glmc prefix to use compiled version instead of inline e.g. glm ):

  1. Verbose, use temp as you did:

    vec3 temp;
    temp = glm_vec3_mul(camera->speed, camera->Orientation, temp);
    glm_vec3_add(camera->Position, temp, camera->Position);
  2. use _muladd version (_muladds for scalar) which don't require temp function:

    glm_vec3_muladd(camera->Position, camera->speed, camera->Orientation);
  3. Struct API

    
    camera->Position = glms_vec3_muladd(camera->Position, camera->speed, camera->Orientation);

// or camera->Position camera->Position = glms_vec3_add(camera->Position, glms_vec3_mul(camera->speed, camera->Orientation));

// Omit Struct Namespace ( see documentation ) camera->Position = vec3_add(camera->Position, vec3_mul(camera->speed, camera->Orientation));


To normalize vec3/vec4...

`glm_normalize()` normalizes vector in-place. But `glm_normalize_to()` normalizes and stores it in `dest` as you specified.

```C
vec3 b;
glm_normalize_to(a, b);

Struct API could be more easy for you if you find simplicity:

vec3s b = glms_normalize(a)
vec3s b = glms_vec3_normalize(a)

// or if namespace is omitted:
vec3s b = vec3_normalize(a)

To multiply vector with scalar use glm_vec3_scale(): multiply/scale vec3 vector with scalar: v = v * s glm_vec3_scale_as(): make vec3 vector scale as specified: v = unit(v) * s

To multiply matrix .e.g with scalar use glm_mat4_scale().

Hope it helps. Feel free to bring any ideas, feedbacks pls

recp commented 1 year ago

Can you put some warning here and compiler version may I ask please to fix or suppress them.

Nonesence999 commented 1 year ago

Thank you for the tips. Here is a sample of the issue in C99:

cglm/types-struct.h:154:6: warning: ISO C99 doesn't support unnamed structs/unions [-Wpedantic]
  154 |     };
      |      ^
cglm/types-struct.h:161:6: warning: ISO C99 doesn't support unnamed structs/unions [-Wpedantic]
  161 |     };
      |      ^
cglm/types-struct.h:173:6: warning: ISO C99 doesn't support unnamed structs/unions [-Wpedantic]
  173 |     };
      |      ^

I only included 3 of them but the thing is that structures should always be named even if its meaningless for C99 compatibility. Best regard.

Nonesence999 commented 1 year ago

I also have an other issue :

if (glfwGetKey(window, GLFW_KEY_SPACE) == GLFW_PRESS) {
        printf("before %f %f %f speed : %f up : %f %f %f \n", camera->Position[0], camera->Position[1],
               camera->Position[2],
               camera->speed, camera->Up[0], camera->Up[1], camera->Up[2]);
        glm_vec3_muladd(camera->Up, &camera->speed, camera->Position);
        printf("after %f %f %f \n", camera->Position[0], camera->Position[1], camera->Position[2]);
    }

before 0.000000 0.000000 2.000000 speed : 0.100000 up : 0.000000 1.000000 0.000000 after 0.000000 100.000000 2.000000

Its wrong. 0.0+=0.1*0.1 != 100

gottfriedleibniz commented 1 year ago

This check seems wrong: 201112L. Also, __STDC_VERSION__ may not be defined if the header is compiled as C++ and _MSVC_VER is not a predefined macro (_MSC_VER).

In your second case, would it be possible to print all three values of camera->speed? Otherwise, if speed is only a scalar, you may want to be using glm_vec3_muladds.

Nonesence999 commented 1 year ago

Thank you I wouldn't have guessed.

recp commented 1 year ago

This check seems wrong: 201112L. Also, __STDC_VERSION__ may not be defined if the header is compiled as C++ and _MSVC_VER is not a predefined macro (_MSC_VER).

@gottfriedleibniz CharGPT suggests this update :)

#  elif  (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) || \
         (defined(__cplusplus)      && __cplusplus >= 201103L)      || \
          defined(_MSC_VER)
#    define CGLM_USE_ANONYMOUS_STRUCT 1
#  elif ...

any better suggestions or is it enough to go?

gottfriedleibniz commented 1 year ago

The _MSC_VER check could probably be dropped (see subsequent logic).

The C++ bits are a little trickier due to C and C++ deviating in this regard (see https://godbolt.org/z/7fKMWbYWo or -Wgnu-anonymous-struct with clang++). Although, all C++ compilers I've tested support this extension so it is probably fine.

Nonesence999 commented 1 year ago

what is the equivalent of glm::quat ?

Nonesence999 commented 1 year ago

Also why does it need libwinpthread-1.dll ??? Is there a none mingw64 version ? I want to be able tu use it nativaly.

recp commented 1 year ago

The _MSC_VER check could probably be dropped (see subsequent logic).

@gottfriedleibniz Wow yes, thanks for the catch, I created a PR for this: https://github.com/recp/cglm/pull/309 any review would be awesome

recp commented 1 year ago

what is the equivalent of glm::quat ?

See https://github.com/recp/cglm/blob/master/include/cglm/quat.h and the docs please. Feel free to bring any ideas, missing features...

Also why does it need libwinpthread-1.dll ???

cglm don't need that, if there are some bugs ob build files it would be nice to fix it.


EDIT:

Is there a none mingw64 version ? I want to be able tu use it nativaly.

Yes you can use it, cglm provides a few build configurations VS Solution, AutoTools, CMake, Meson...

Nonesence999 commented 1 year ago

@recp I am getting Unknown type name 'quat' whit #include <cglm/struct.h> and #include <cglm/quat.h>

recp commented 1 year ago

@Nonesence999 cglm uses versor as quat type ( there could be quat type alias, or still can be maybe if it wont conflict with system types or other types )

I'm going to close this and feel free to bring ant bugs, ideas, issues ...

recp commented 1 year ago

warning: ISO C99 doesn't support unnamed structs/unions [-Wpedantic]

@Nonesence999 #309 is merged can you validate that warnings are gone?