Clipspace implementations for left/righted-handed coordinate systems and [-1,1] and [0,1] clipspace #198

raedwulf · 2021-05-13T21:34:31Z

No description provided.

This commit adds the function `glm_perspective_lh_zo`, modelled on the implementation of glm_perspective, but amended to provide a left-hand coordinate system expected by DirectX, Metal and Vulkan (per the GLM project's `glm/detail/setup.hpp`). It uses a clip-space of zero-to-one. The function is tested against a longhand version of the algorithm it seeks to implement as well as against the output of the GLM project's `glm::perspectiveLH_ZO` function. This commit adds a new subdirectory `test/glm_cmp` which contains a basic CMake file and `main.cpp`. An interested user should link or copy or clone the GLM project into this directory. The `main` function can be used to print the reference data used so others can verify behaviour in the future, or add new literal reference values.

This commit adds the RH/ZO perspective function. It does so in the new file `cam_rh_zo.h` and further refactors the LH variant into new file `cam_lh_zo.h`. This creates some churn in the tests and configuration files as new test files were added as well, and all these changes found their way into the build files. Tests passing on Linux.

This commit adds functions `glm_perspective_lh_no` and `glm_perspective_rh_no` to the code. Unit tests are added and this commit follows the new pattern of adding the a new file per coordinate-system and clip-space tuple. . Makefile.am updated . removed test/glm_cmp project stub . unit tests include naive implementations to as well as magic number ref-data generated by the corresponding GLM functions. No tests run yet on Windows or Mac.

Add the initial implementations of the left-handed and right-handed coordinate systems as well as clipspace depth values of [-1, 0] and [0, 1].

raedwulf · 2021-05-13T21:37:10Z

@michael-g @recp The tests are still failing but it's a start for some early reviewing

CMakeLists.txt

include/cglm/call/clipspace/ortho_lh_no.h

Co-authored-by: Michael <michael.guyver@gmail.com>

include/cglm/call/clipspace/ortho_lh_zo.h

include/cglm/call/clipspace/ortho_rh_no.h

include/cglm/call/clipspace/ortho_rh_zo.h

include/cglm/call/clipspace/persp_lh_no.h

include/cglm/call/clipspace/persp_lh_zo.h

include/cglm/call/clipspace/persp_rh_no.h

include/cglm/call/clipspace/persp_rh_zo.h

include/cglm/cam.h

Co-authored-by: Michael <michael.guyver@gmail.com>

michael-g · 2021-05-14T16:21:06Z

Will hopefully do some more later on, but this is a huge amount of work you've done, @raedwulf. Awesome stuff.

recp · 2021-05-14T18:46:04Z

Hi @raedwulf, @michael-g,

Sorry for the late response :/

Many thanks for this work, as @michael-g said this is a huge work.

Also there is a struct api, we should add wrappers fo this change into include/cglm/stuct/clipspace. Struct is just wrappers to existing array api.

raedwulf · 2021-05-15T07:38:47Z

I'll look into the struct API. Currently we do have an extra separation for view too. Would proj just be the combination of ortho and persp? @recp

recp · 2021-05-15T11:22:06Z

Would proj just be the combination of ortho and persp?

Yes, but I'm not sure which way is better, separating them or joining them. There are lot of persp functions and maybe come more for both ortho and persp, in this case separating them may be good idea.

recp · 2021-05-15T13:21:38Z

include/cglm/common.h

+#define CGLM_CLIP_CONTROL_LH_NO (CGLM_CLIP_CONTROL_LH_BIT | CGLM_CLIP_CONTROL_NO_BIT)
+#define CGLM_CLIP_CONTROL_RH_ZO (CGLM_CLIP_CONTROL_RH_BIT | CGLM_CLIP_CONTROL_ZO_BIT)
+#define CGLM_CLIP_CONTROL_RH_NO (CGLM_CLIP_CONTROL_RH_BIT | CGLM_CLIP_CONTROL_NO_BIT)
+


Can we add these options to docs/source/opt.rst? People can recognize the options quickly when reading docs.

recp · 2021-05-15T16:03:28Z

Maybe this is not one of goal of the PR and out of scope, but I'm wonder that should cglm need anything else to support LH. For instance transforms (matrix rotations, quaternions, vector rotations...).

Also there are frustum helpers in frustum.h e.g extracts planes, corners... Unproject functions in project.h... I'm not sure what else should be done to fully support LH.

Actually users should achieve same rotation around X and Y by negating angle but Z should remain same I think. Maybe we should discuss these in another issue[s].

recp · 2021-05-17T17:39:52Z

@raedwulf ping.

I think we need to add tests for other functions too, after that we can merge this PR if there are no any issues

michael-g · 2021-05-17T18:27:55Z

Do you have a sense of how you'd like to approach the testing? We could continue with the small amount of work I've done and paste a load of magic numbers into the tests, or go ahead with the new repository which compares cglm implementations again the glm library? The former is much more of a unit test, obviously, but the latter has its own benefits.

raedwulf · 2021-05-17T18:31:54Z

I looked into the struct API and I can get it done soonish :) thanks @michael-g that would be great!

recp · 2021-05-17T20:28:40Z

Do you have a sense of how you'd like to approach the testing? We could continue with the small amount of work I've done and paste a load of magic numbers into the tests, or go ahead with the new repository which compares cglm implementations again the glm library? The former is much more of a unit test, obviously, but the latter has its own benefits.

@michael-g I think we should add unit tests first then library-to-library test can be written later. The magic numbers could come from the math instead of glm directly, or both maybe, glm also needs tests... I have just created a repo called cglm-libtests, the name can be changed with better one...

I looked into the struct API and I can get it done soonish :)

Awesome!

recp · 2021-05-20T17:23:55Z

It would be nice to finish this work asap, I can merge this PR and we can add tests with new PRs maybe?

@raedwulf @michael-g any progress?

raedwulf · 2021-05-20T17:31:13Z

@recp Just a bit busy last few days, but I'll spend some time this weekend to finalise things with the struct api.

recp · 2021-05-20T17:34:00Z

@raedwulf great 🤗, thanks

michael-g · 2021-05-20T18:01:15Z

It would be nice to finish this work asap, I can merge this PR and we can add tests with new PRs maybe?

@raedwulf @michael-g any progress?

Sorry, I've been busy with work and family stuff.

If only we could auto-gen a bunch of tests using results from GLM...

raedwulf · 2021-05-22T23:16:18Z

I pushed the struct api stuff @recp @michael-g

recp · 2021-05-23T09:06:11Z

@raedwulf awesome work,

There was no need to add clip control to default strruct api like this:

CGLM_INLINE
mat4s
glms_frustum(float left,   float right,
             float bottom, float top,
             float nearZ,  float farZ) {
#if CGLM_CONFIG_CLIP_CONTROL == CGLM_CLIP_CONTROL_LH_ZO
  return glms_frustum_lh_zo(left, right, bottom, top, nearZ, farZ);
#elif CGLM_CONFIG_CLIP_CONTROL == CGLM_CLIP_CONTROL_LH_NO
  return glms_frustum_lh_no(left, right, bottom, top, nearZ, farZ);
#elif CGLM_CONFIG_CLIP_CONTROL == CGLM_CLIP_CONTROL_RH_ZO
  return glms_frustum_rh_zo(left, right, bottom, top, nearZ, farZ);
#elif CGLM_CONFIG_CLIP_CONTROL == CGLM_CLIP_CONTROL_RH_NO
  return glms_frustum_rh_no(left, right, bottom, top, nearZ, farZ);
#endif
}

clip control already done in glm_frustum() this could be just a wrapper as before. But since you did it, let's keep it.

recp · 2021-05-23T09:06:28Z

Now we need to add tests ;)

raedwulf · 2021-05-23T09:08:38Z

Ahh, yeah that's true. That file could be reverted with no problems though.

recp · 2021-05-23T15:31:11Z

@raedwulf @michael-g I'm going to merge this PR, but let's improve this by adding tests, bug fixes with new PRs...

This is still valid question for me: #198 (comment)

Feel free to open issues and bring tests, bugfixes

@raedwulf @michael-g many thanks for this work 🤗

recp · 2021-05-23T15:59:42Z

@raedwulf @michael-g the PR is merged, many thanks 🤗 🎉

Let's write tests ;)

michael-g · 2021-05-23T16:10:17Z

@raedwulf, kudos, amazing work. Will try to contribute this week to the testing code. Again: is there a source of reference implementations?

raedwulf · 2021-05-23T16:25:40Z

Hey @michael-g, they are based on the cglm source code and the glm code for the differences.

warmwaffles · 2021-05-24T16:09:06Z

@raedwulf Love this! Thank you so much for pushing this through.

raedwulf · 2021-05-26T12:18:14Z

@warmwaffles thanks! We're still in the process of testing this and any feedback will be great!

recp · 2021-05-26T15:31:21Z

We should announce this feature after the tests are done e.g. reddit, gamedev... I'll try what I can... After used by community then more improvements can be made.

Great to have great community 🎉

quadroli · 2021-05-27T15:29:32Z

Wow, been following along this project for a while, glad to see it come to fruition, excellent work all at @michael-g @raedwulf and @recp for the insights, kudos 🥳

quadroli · 2021-05-27T15:30:33Z

Quick question, are there noticeable to changes on how to use the library now?

recp · 2021-05-28T11:55:37Z

@quadroli thanks,

are there noticeable to changes on how to use the library now

Actually not, just optionally it is possible to use other clipspaces rather than OpenGL's one now, if there are no any bugs

quadroli · 2021-05-30T16:30:16Z

Awesome 👍
Love the new look to, iss clean 🧼

michael-g and others added 5 commits May 13, 2021 23:18

Updated README.md to remove ref to glm_cmp stub

db46ea1

clipspace: add initial implementations LH, RH and NO, ZO

7065011

Add the initial implementations of the left-handed and right-handed coordinate systems as well as clipspace depth values of [-1, 0] and [0, 1].