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

TODO: Make cglm work with Metal, Vulkan and DirectX #152

Closed
recp opened this issue Jul 31, 2020 · 49 comments · Fixed by #198
Closed

TODO: Make cglm work with Metal, Vulkan and DirectX #152

recp opened this issue Jul 31, 2020 · 49 comments · Fixed by #198

Comments

@recp
Copy link
Owner

recp commented Jul 31, 2020

cglm must support Metal and Vulkan (and maybe DirectX). To do this cglm may provide alternative functions for alternative NDC coordinates. Or we could do that with preprocessor macros. But providing extra functions will provide ability to switch between graphics APIs without rebuilding the code.

Resources:

  1. https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf
  2. https://metashapes.com/blog/opengl-metal-projection-matrix-problem/
@warmwaffles
Copy link

I just ran into this issue over the weekend. I think providing secondary methods depending on the backend is a good idea. I am not a huge fan of how glm does it with a macro.

@recp
Copy link
Owner Author

recp commented Sep 14, 2020

@warmwaffles thanks for your feedbacks, sounds good to me ;)

@michael-g
Copy link
Contributor

At risk of resurrecting an old thread -- for which apologies -- does the above relate to the way Vulkan requires its depth buffer, for example, to be zero-to-one and therefore the way that glm_perspective would have to perform its calculation? I've been comparing its impl to this and wondering about the best way to proceed. FWIW, separate functions rather than a macro (like GLM_CUSTOM_CLIPSPACE) would be my preference.

Any elaboration or discussion would be very much appreciated, as I'm learning all this as I go. Thanks.

@recp
Copy link
Owner Author

recp commented Apr 26, 2021

Hi @michael-g

Thanks for your feedbacks,

FWIW, separate functions rather than a macro (like GLM_CUSTOM_CLIPSPACE) would be my preference.

+1

I'm interested to do some work on this, cglm could provide glm_perspective/ortho/frustum_lh() , glm_look[at]_lh()... and _zo vesions...

glm solution:

	template<typename T>
	GLM_FUNC_QUALIFIER mat<4, 4, T, defaultp> perspective(T fovy, T aspect, T zNear, T zFar)
	{
#		if GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_LH_ZO
			return perspectiveLH_ZO(fovy, aspect, zNear, zFar);
#		elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_LH_NO
			return perspectiveLH_NO(fovy, aspect, zNear, zFar);
#		elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_ZO
			return perspectiveRH_ZO(fovy, aspect, zNear, zFar);
#		elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_NO
			return perspectiveRH_NO(fovy, aspect, zNear, zFar);
#		endif
	}

we can follow similar path.

And yes we need update cglm for Vulkan clip space as soon as possible. Any help would be appreciated (Discussions, Feedbacks, PRs/Coding, testing and more testing...)


I'have implemented FMA and implemented some missing NEON parts e.g. mat4_inv(), affine... there are a few more left and then I can work on something new like this maybe. (and yes cglm is Highly Optimizedfor both scalar and SIMD versions 😉)

If you or someone else could help to implement these stuff it would make us faster to add these features to cglm (putting resources here or to discussions maybe, help to coding, testing and more testing...)

@michael-g
Copy link
Contributor

If you or someone else could help to implement these stuff it would make us faster to add these features to cglm (putting resources here or to discussions maybe, help to coding, testing and more testing...)

I would be happy to (try to) help, so long as we understand that any algorithms I implement are likely be a fairly slavish copy of what I see in GLM: I'm too much of a Luddite to extemporise those. I can of course avoid repeating calculations (in a similar way that cglm_perspective does). Do you have a contributor guide about the branching model you prefer?

@recp
Copy link
Owner Author

recp commented Apr 27, 2021

I would be happy to (try to) help ...

Thanks 🤗

Do you have a contributor guide about the branching model you prefer?

See: CONTRIBUTING.md

@michael-g
Copy link
Contributor

Hi Recep, I've got myself a fork, a dev branch and a new function in cam.h which for better or worse I've named glm_perspectiveLH_ZO. I have a couple of questions about testing:

  1. What would be "the right way" to test this implementation? I suspect that implementing the algorithm using primitive functions (mul, etc) in the test function and comparing the result with the output of the new function would be the clearest for those that come after and most useful. A source for that standard algorithm would be useful... I'm working through Real-Time Rendering 4.7.2, but it does also feel like I'm likely to repeat any mistakes I've made in the function under test.
  2. Is there a simple way of capturing the output of glm::perspectiveLH_ZO without creating a C++ project just for that purpose? The contribution docs do stipulate testing against the GLM library, so it must have been done a few times before.
  3. Also ... I don't have access to a Mac and am not setup on Windows for development. Are these tests taken care of by Travis and AppVeyor?

@recp
Copy link
Owner Author

recp commented Apr 28, 2021

I'm not sure about the naming:

glm_perspective_lh_zo();
glm_perspective_zo();
...

vs

glm_perspectiveLH_ZO();
glm_perspective_ZO();
...

What would be "the right way" to test this implementation?

You can check https://github.com/recp/cglm/blob/master/test/src/test_affine.h as test sample. More and complex tests would be awesome.

Is there a simple way of capturing the output of glm::perspectiveLH_ZO without creating a C++ project

You can create a separate C++ project then test GLM and cglm both for compaing results maybe. Or you can add C++ file to cglm then remove it after if you are convinced that the results/implementations are correct.

Also it is better to compare algorithm with original one on the paper, because GLM may have bugs...

Also ... I don't have access to a Mac and am not setup on Windows for development. Are these tests taken care of by Travis and AppVeyor?

Currently the tests cannot bee run on AppVeyor. Tests must be run manually on Windows by running test project, I coundn't rrun prrint the tests on AppVeyor.

I'm using Mac, I can run tests, Travis also can run tests on macOS, Linux... https://travis-ci.org/github/recp/cglm .

@michael-g
Copy link
Contributor

michael-g commented Apr 28, 2021

I have a working function with a unit-test now in this branch. I've also added a discrete/disconnected CMake C++ project stub under test/glm_cmp to make it easier for future contributors to generate reference data values. It's README has more information. This produced the reference values to be found in the test for glm_perspective_lh_zo.

Please let me know your thoughts on this work-in-progress: it would be useful to find out if you're horrified by the changes before I make too many more. ;)

EDIT: having done this I've realised I've been proceeding under a misapprehension, namely that Vulkan requires left hand NDCs (but does in fact require right hand NDCs). I'll correct the doc and amend the commit message. I think I was mislead by GLM (here)...

@recp
Copy link
Owner Author

recp commented Apr 29, 2021

@michael-g good work!

I have some ideas which I want to share and get feedbacks;

we can create separate files for each clip-space, for instance:

cam.h - default
frustum.h - default
...
clipspace/cam.lh.no.h - left hand clipspace + NegativeOne
clipspace/cam.lh.zo.h - left hand clipspace + ZeroToOne
clipspace/cam.rh.no.h - right hand clipspace + NegativeOne
clipspace/cam.rh.zo.h - right hand clipspace + ZeroToOne
clipspace/frustum.rh.zo.h - rigght hand clipspace + ZeroToOne
...

then we import all the headers into cam.h header. We can now make decisions to which api to call:

CGLM_INLINE
void
glm_perspective(float fovy, float aspect, float nearVal, float farVal, mat4 dest) {
/* GLM_CLIP_CONTROL_RH_NO may stay as default  */
#if GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_LH_ZO
  glm_perspective_lh_zo(fovy, aspect, nearVal, farVal, dest);
#elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_LH_NO
  glm_perspective_lh_no(fovy, aspect, nearVal, farVal, dest);
#elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_ZO
  glm_perspective_rh_zo(fovy, aspect, nearVal, farVal, dest);
/* #elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_NO */
#else
  glm_perspective_rh_no(fovy, aspect, nearVal, farVal, dest);
#endif
}

we can rename nearVal/farVal to zNear/zFar. Windows was failed to build cglm, because near and far values are defined in windows.h, then I renamed it. But zNear/zFar are better names

with separate files it would be easy to maintain and it may keep things clean.

user can set GLM_CLIP_CONTROL_RH_ZO... with macros or they can include related header and call related function e.g glm_perspective_rh_zo()


We can provide convenience headers to make things easier for not-experts:

default:

#include <cglm/cglm.h>

cglm/cglm-vulkan.h:

/*
 * Copyright (c), Recep Aslantas.
 *
 * MIT License (MIT), http://opensource.org/licenses/MIT
 * Full license can be found in the LICENSE file
 */

#define GLM_CLIP_CONTROL_RH_ZO

#include <cglm/cglm.h>

...

then if user want to onlly use Vulkan then he/she can do this by including cglm-vulkan.h instead on setting macro manually:

/* use Vulkan clipspace and configuration */
#include <cglm/cglm-vulkan.h>

/* or Metal */
#include <cglm/cglm-metal.h>

/* or OpenGL */
#include <cglm/cglm-opengl.h>

/* or cglm default, user need set clipspace with macro, 
   if they don't want to use the default one exported by cglm */
#include <cglm/cglm.h>

I'm working on a GPU library (https://github.com/recp/gpu) (in-progress) and Universal Shading Language (https://github.com/UniversalShading/spec) (in-progress) to make render layer GPU independent. Separating functions like glm_perspective_lh_zo() will help a lot.

typedef struct camera {
   void (*perspective)(float, float, float, float, mat4);
   /* ... */
} camera;

/* we can change clipspace just by changing pointer: */
camera->perspective = glm[c]_perspective_rh_no;

/* or */
camera->perspective = glm[c]_perspective_rh_zo;

@michael-g
Copy link
Contributor

Excellent, I like the ideas. I'll make a start on implementing some of these and fix the nearVal/farVal issue (and bear that in mind for the future).

It would be useful if you had any links to online resources that describe what these functions should all look like, since at the moment I'm relying on the GLM implementation, and what I can find online, e.g. this gem. It's way more detail than I need but at least it's clear how a perspective_rh_zo should look. Surely these things are all written down, somewhere ... ?

Can I take it that you don't hate the glm_cmp project-stub under test/?

@recp
Copy link
Owner Author

recp commented Apr 29, 2021

Excellent, I like the ideas

Perfect 🤗

One resouce I suggest to look at is DirectXMath, AFAIK, they are implemented LH and RH functions. I'm not sure if they implemented Vulkan clipspace.

but at least it's clear how a perspective_rh_zo should look

Let's implment _rh_zo then we can move step further, small iterations, good tests and better motivation...

Surely these things are all written down, somewhere ... ?

Sure. For Metal, I'll look at Apple docs later. I'll take a look API differences and may bring some resouces here.

You can add anything you need to test/ if you mean this.


EDIT:

e.g. this gem

good resource. Maybe we could keep the resouce links in RESOURCES.md or in Wikis to take a look later if needed. CREDITS file already doing similar job but I'm nut sure, it's goal is different

@michael-g
Copy link
Contributor

A few more changes in the branch. New glm_perspective_rh_zo function and two new files for the LH and RH ZO variants. I need to remember to update the .rst docs in due course. I don't plan to implement the macro switching until after at least all the perspective projection functions are implemented. Shouldn't be long. Tests pass using ref data produced by GLM. Thanks for the feedback ;)

@recp
Copy link
Owner Author

recp commented Apr 29, 2021

Can I take it that you don't hate the glm_cmp project-stub under test/?

Wow, now I undestand what you mean by glm_cmp, I thought that you may write a temp function called glm_cmp() for tests 😲

It is better to avoid external deps. If we need tests to compare cglm with glm then we can create a separate repo for that.


I need to remember to update the .rst docs in due course.

I think postponing docs until coding has done is good idea since we may need to change somethings?

A few more changes in the branch

It looks good! It seems glm_perspective_rh_zo() has less instuctions than previous glm_perspective(), I like that :) What about moving clipspace related stuff to clipspace/ as I mentioned before? Or do you think keeping them in root folder is better?

I didn't check if computations are correct or not, I'll try to do that later

I don't plan to implement the macro switching until after at least all the perspective projection functions are implemented

+1

Shouldn't be long.

🤗

@michael-g
Copy link
Contributor

Can I take it that you don't hate the glm_cmp project-stub under test/?

Wow, now I undestand what you mean by glm_cmp, I thought that you may write a temp function called glm_cmp() for tests astonished

No problem, I can remove the subdirectory, main.cpp and its CMakeLists.txt from Git.

Just to be clear, though: it was never part of the build. If you didn't know it existed you'd never need to link GLM into position.

What I do think is important is that we have a way of regenerating the reference values I've used in the tests -- and to which I will add more functions as I reimplement/transpose/transliterate more of the L/RH Z/NO functions to cglm.

If at some point in the future someone wants to validate where all those magic numbers came from, the content of main.cpp and a way to run it (easily) will be important. Can you suggest another way of capturing the ... "ref data harness" [?] for other developers?

@recp
Copy link
Owner Author

recp commented Apr 30, 2021

Just to be clear, though: it was never part of the build. If you didn't know it existed you'd never need to link GLM into position.

I'm not sure really, let's keep folder clean, since we are not only trying to get GLM's results, separate-repo like cglm-cmp-test would be better, we could coompare cglm with GLM and other libraries? People use this library would not copy-paste each time these libray-library cmp test foldes...

If at some point in the future someone wants to validate where all those magic numbers came from, the content of main.cpp and a way to run it (easily) will be important. Can you suggest another way of capturing the ... "ref data harness" [?] for other developers?

You are right, but what about separate test repo as I mentioned above? If someone want to validate cglm then he/she can clone test repo and run libary-library tests to compae cglm results with others e.g. GLM, Eigen...

@michael-g
Copy link
Contributor

I hear you ;) In that case, the next steps seem something like

  1. You (rather than me) set up a new repo in your Github spaced named something like cglm-cmp (since it could compile against more than just GLM in the future).
  2. Let it be a C++ CMake project
  3. Let GLM be a dependency, perhaps even a submodule
  4. Let cglm be a dependency, or, again, a submodule
  5. Contributors add to the test application which, as you say, does runtime comparisons of equivalent functions' outputs.

... and I'll remove the subdirectory under test!

@michael-g
Copy link
Contributor

I've updated the branch and we should now have a complete set of LH/RH coord. system and ZO/NO clip-space variants. All have test coverage against both naive algo implemenations and magic numbers generated by the GLM equivalents. Happy either to raise a PR new for proper review or implement any other feedback you may have.

@recp
Copy link
Owner Author

recp commented Apr 30, 2021

Great! It would be better to continue with PR 🤗 We could get more reviews from other users maybe

One thing I must say that be aware that _rh_no implementations must be same with cglm's existing implementations, since it is OpenGL's coord sys.

@raedwulf
Copy link
Contributor

raedwulf commented May 1, 2021

Blimey... I missed this thread and started working on this myself! Good work though @michael-g !

Edit: Oh wait, how far have you got in implementation - I'm checking your branch out and perhaps we can work on this together if you're not completed.
My stuff hasn't been pushed yet - but I've implemented almost all of cam.h (untested) with all the different coordinate systems with function names like _rh_no as suggested here.

@raedwulf
Copy link
Contributor

raedwulf commented May 1, 2021

Where should glm_lookat_lh and glm_lookat_rh be located? Maybe something like lookat_lh.h and lookat_rh.h?

@recp
Copy link
Owner Author

recp commented May 1, 2021

perhaps we can work on this together if you're not completed.

@realaltffour thanks for help 🤗

I think working together will boost the process to implement the clip-space differences. What do you say @michael-g ?

Where should glm_lookat_lh and glm_lookat_rh be located? Maybe something like lookat_lh.h and lookat_rh.h?

I think each function must stay where it belongs. See #152 (comment) comment. glm_look() and glm_lookat() are in cam.h we can move glm_look[at]_lh[_no/zo]()... to cam.lh.h or cam_lh_[no/zo].h or clipspace/cam[ _ | - | .]lh.h...

@raedwulf
Copy link
Contributor

raedwulf commented May 1, 2021

Great, I think having them in clipspace/ will make the repository cleaner. I think glm_look[at]_lh_no is identical to glm_look[at]_lh_zo (according to glm).

@recp
Copy link
Owner Author

recp commented May 1, 2021

I think having them in clipspace/ will make the repository cleaner

+1

cam.h can import all the camea headers. Or we can avoid to import all, if user specified clipspace by GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_LH_ZO to help compiler to bypass these unused headers.

I think glm_look[at]_lh_no is identical to glm_look[at]_lh_zo (according to glm)

I didn't check yet, if so one can be another's wapper instead of duplcating implementations.

@recp
Copy link
Owner Author

recp commented May 3, 2021

@michael-g ping.

@michael-g
Copy link
Contributor

Thanks for these, I'll have a look.

* https://vincent-p.github.io/notes/20201216234910-the_projection_matrix_in_vulkan/

Re the above, while I love the explanations and the way it's all laid out for us, I can't reconcile his final C++ code with any of the [RL]H_[NZ]O implementations in GLM. In particular, his value for A, at [2][2] is different ...

@michael-g
Copy link
Contributor

Hi @raedwulf, sorry, been busy!

The branch is up-to-date with my local changes and I was going to do some more work on it tonight. However, I'm happy to be hands-off until you've added your implementations to the branch. Did you really implement all four variants for all six ortho functions? It's quite an explosion of function variants. Excellent work if you did!

One happy aspect is that I don't think we need to add any more files, which is a bit painful because of the number of places you need to remember to add them (both CMakeLists.txt, Makefile.am, tests.h, cglm.h and I'm sure a couple of other places). With that said, @raedwulf & @recp, I'm not quite sure about the clipspace/ comment but I think you mean moving the cam_[lh]h_[zn]o headers into a new directory. Obvs happy to go with the flow here, it's your baby, Recep. Shall we bite that off sooner rather than later? Would you like to refactor while you're adding your new functions, Raedwulf?

@recp, re:

One thing I must say that be aware that _rh_no implementations must be same with cglm's existing implementations, since it is OpenGL's coord sys.

there is one material difference in the glm_perspective_rh_no implementation, in that the existing implementation has

dest[3][2] = 2.0f * nearVal * farVal * fn

while my implementation, which oddly agrees with the GLM implementation has

dest[3][2] = -2.0f * farVal * nearVal * fn

The other difference should -- I think -- be non-material, as the existing impl is b - a while what I've written down is -(a - b).

Any thoughts on the divergence between the existing cglm implementation and the GLM equivalent? Have I just written it down wrong?

@recp
Copy link
Owner Author

recp commented May 3, 2021

...it's your baby, Recep...

But let's grow it together 👶

I'm not quite sure about the clipspace/ comment but I think you mean moving the cam_[lh]h_[zn]o headers into a new directory.

Yes moving these rh/lh/no/zo headers to clipspace/ folder and include them in cam.h/frustum.h/poject.h... if theere is no opposite opinion

cam.h:

...

#ifndef cglm_vcam_h
#define cglm_vcam_h

#include "common.h"
#include "plane.h"

/* or */ 
#include "clipspace/cam.lh.no.h"
#include "clipspace/cam.lh.zo.h"
#include "clipspace/cam.rh.no.h"
#include "clipspace/cam.rh.zo.h"

/* or */ 
#include "clipspace/cam-lh-no.h"
#include "clipspace/cam-lh-zo.h"
#include "clipspace/cam-rh-no.h"
#include "clipspace/cam-rh-zo.h"

/* or */ 
#include "clipspace/cam_lh_no.h"
#include "clipspace/cam_lh_zo.h"
#include "clipspace/cam_rh_no.h"
#include "clipspace/cam_rh_zo.h"
...

we can include related files conditionally if we want:

...

#ifndef cglm_vcam_h
#define cglm_vcam_h

#include "common.h"
#include "plane.h"

#ifndef CGLM_CLIPSPACE_INLUCE_ALL
#  if GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_ZO
#    include "clipspace/cam_rh_zo.h"
#  elif GLM_CONFIG_CLIP_CONTROL == GLM_CLIP_CONTROL_RH_NO
#    include "clipspace/cam_rh_no.h"
#  elif ...
      ...
#  endif
#else
#  include "clipspace/cam_lh_no.h"
#  include "clipspace/cam_lh_zo.h"
#  include "clipspace/cam_rh_no.h"
#  include "clipspace/cam_rh_zo.h"
#endif

Any thoughts on the divergence between the existing cglm implementation and the GLM equivalent? Have I just written it down wrong?

Here is the documentation I were followed:
https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/gluPerspective.xml

it is exactly same as glm_perspective().

The GLM's implementation seems different:
https://github.com/g-truc/glm/blob/master/glm/ext/matrix_clip_space.inl#L249

I'm not sure why it is different maybe it fixes something e.g. improves projection precision maybe? dest[2][2] is also different :/

@raedwulf
Copy link
Contributor

raedwulf commented May 3, 2021

@michael-g Yeah I'll do the move to clipspace. I have a couple of files left to do as I originally implemented it all in cam.h and merged the decomp functions.

With the b-a and -(a-b) differences, i kept the CGLM version intact. The compiler should be able to figure it out and there shouldn't be a precision loss because floating point spec reserves a sign.

@recp
Copy link
Owner Author

recp commented May 3, 2021

I vote for b-a instead of -(a-b) because not all compilers may produce same output. I'm re-writing mat4_inv for both NEON and SSE to eliminate xor's:

/* from (existing impl) */
dest[1][0] =-(e * t[0] - g * t[3] + h * t[4]);

/* to (new impl) */
dest[1][0] = g * t[3] - e * t[0] - h * t[4];

I'll push to a PR after finished. Hope it will worth it. Negating SIMD register requires extra instruction[s] (and exta load on x86)
...


EDIT:

I don't know if there is any difference in FPU/SIMD for b-a and -(a-b) due to floating point [rounding] implementation, seems like stackoverflow question :/

@recp recp changed the title TODO: Make cglm work with Metal and Vulkan (and maybe DirectX) TODO: Make cglm work with Metal, Vulkan and DirectX May 3, 2021
@michael-g
Copy link
Contributor

@michael-g Yeah I'll do the move to clipspace.

@raedwulf you're a gent, that would be great.

FWIW,

/* or */
#include "clipspace/cam_lh_no.h"
#include "clipspace/cam_lh_zo.h"
#include "clipspace/cam_rh_no.h"
#include "clipspace/cam_rh_zo.h"

feels the most "C like" to me.

maybe it fixes something e.g. improves projection precision maybe?

Speaking from a position of ignorance, my guess is that it flips the one of the axes to avoid everyone having to implement some hack or other. But it's a pure guess.

Are we going to implement the coordinate system/clip-space implementation-hiding like GLM does here?

I'm not sufficiently experienced in 3d programming to know whether the provision of "partially hidden" functions has any utility, e.g. providing perspective_rh or perspective_no.

@recp, I have no idea what I was thinking when I wrote

The other difference should -- I think -- be non-material, as the existing impl is b - a while what I've written down is -(a - b).

I must be delirious from lack of sleep. What I wrote down was -(a + b) so the above is nonsense!

@recp
Copy link
Owner Author

recp commented May 3, 2021

Speaking from a position of ignorance, my guess is that it flips the one of the axes to avoid everyone having to implement some hack or other. But it's a pure guess.

I'll investigate this or I may ask to author of GLM later. What kind of hack may I ask?

@raedwulf
Copy link
Contributor

raedwulf commented May 4, 2021

I'll have some time to work on it today (I just started a new job yesterday so was a bit exhausted)!

@raedwulf
Copy link
Contributor

raedwulf commented May 5, 2021

https://github.com/raedwulf/cglm/tree/clipspace-wip

I've not done the code shifting or tested it (I'm sure I've missed some things), but I'll have more time today when I get home!

@recp
Copy link
Owner Author

recp commented May 5, 2021

@raedwulf many thanks 🤗 I'll take a look asap

@michael-g
Copy link
Contributor

@raedwulf you're a machine ;) awesome work. Let me know if there's a discrete area in which I can contribute to the tests etc. I'm not sure how much time I'll have but just in case...

@raedwulf
Copy link
Contributor

raedwulf commented May 5, 2021

Thanks! Yeah tests would be great :) I'll still need to rearrange the files a bit - hopefully will have some time later today.

@raedwulf
Copy link
Contributor

raedwulf commented May 6, 2021

Update - will have a chance this weekend to work more on this, but a bit exhausted last few days. @michael-g no hurry!

@michael-g
Copy link
Contributor

Roger that. Take it easy!

@recp
Copy link
Owner Author

recp commented May 6, 2021

I'm still waiting a PR ;) Let's discuss changes on the PR? I'll check your branch soon,

Let's make cglm work with LH/RH and NO/ZO 🤗

@recp
Copy link
Owner Author

recp commented May 8, 2021

Perspective function from Apple's Metal sample:

matrix_float4x4 matrix_perspective_right_hand(float fovyRadians, float aspect, float nearZ, float farZ)
{
    float ys = 1 / tanf(fovyRadians * 0.5);
    float xs = ys / aspect;
    float zs = farZ / (nearZ - farZ);

    return (matrix_float4x4) {{
        { xs,   0,          0,  0 },
        {  0,  ys,          0,  0 },
        {  0,   0,         zs, -1 },
        {  0,   0, nearZ * zs,  0 }
    }};
}

Also I liked nearZ and farZ param names, it seems cool and better than nearVal / farVal and zNear / zFar... I have renamed nearVal / farVal to nearZ and farZ in a242d83

@raedwulf
Copy link
Contributor

Thanks, I've updated my branch with the new nearZ/farZ names - will push me stuff later after I check it compiles.

@recp
Copy link
Owner Author

recp commented May 10, 2021

@realaltffour awesome 🤗

@recp
Copy link
Owner Author

recp commented May 12, 2021

@raedwulf ping

@raedwulf
Copy link
Contributor

Just working on compilation errors :)

@recp
Copy link
Owner Author

recp commented May 12, 2021

:)

@michael-g
Copy link
Contributor

Awesome.

@recp
Copy link
Owner Author

recp commented May 15, 2021

Follow at #198

Thanks @raedwulf, @michael-g

@recp recp closed this as completed in #198 May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants