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

Improve documentation for the Mat4 projection functions #570

Merged
merged 4 commits into from
Oct 27, 2024

Conversation

kovaxis
Copy link
Contributor

@kovaxis kovaxis commented Oct 16, 2024

Like I mentioned in #569, this PR adds more detailed documentation to the Mat4::perspective_* and Mat4::orthographic_* family of functions, as well as adding Mat4::perspective_rh_vk and Mat4::orthographic_rh_vk.

I initially wrote a very detailed doc explaining what world-view coordinates get mapped to what normalized-device-coordinates, but they look very out-of-place and are probably too much. Instead, I added a small subtext among the lines of Useful to map the standard right-handed coordinate system into what OpenGL expects..

I haven't implemented the *_rh_vk functions or their tests yet. I would like to receive confirmation that this PR is ok before doing so.

@bitshifter
Copy link
Owner

Hi, because there are 8 different projection methods currently adding a vk suffix will double that. At some point I'd like to revisit how this is done in glam, probably separating these out into separate modules for handedness and graphics api so users only get one in their project, although realistically I'm not sure when I will get around to that.

Happy to accept the documentation improvements.

@kovaxis
Copy link
Contributor Author

kovaxis commented Oct 26, 2024

Hi, because there are 8 different projection methods currently adding a vk suffix will double that.

There would only be 2 new _vk functions (perspective_rh_vk and orthographic_rh_vk, not double the amount) just like there are only 2 _gl functions and not double the amount.

But sure, I've removed the functions. This is ready for review then.

At some point I'd like to revisit how this is done in glam, probably separating these out into separate modules for handedness and graphics api so users only get one in their project, although realistically I'm not sure when I will get around to that.

ultraviolet has 4 modules (rh_yup, lh_yup, rh_ydown, lh_ydown), each with their own set of (perspective,orthographic)_(wgpu_dx,gl,vk) functions, if that's good reference. They also export by default the rh_yup to provide a good default for people who don't care (although to be honest I prefer blender-style rh_zup).

EDIT: Another good thing about this approach is that the modules themselves are a very good spot for lengthy documentation without taking up space in the Mat4 doc page.

EDIT 2: Another interesting approach would be to just bless a particular convention (eg. just orthographic ,perspective, perspective_infinite, perspective_infinite_reverse that map rh_yup to WGPU/DX/Metal) and then provide auxiliary constant matrices like TO_GL, TO_VK, FROM_LH_YUP, FROM_RH_YDOWN, FROM_RH_ZUP, FROM_RH_ZDOWN, etc... This reduces the combinatorics and potential bloat at some minimal runtime cost (nobody is bottlenecked on building projection matrices, I hope...)

@kovaxis kovaxis marked this pull request as ready for review October 26, 2024 23:50
@bitshifter
Copy link
Owner

There would only be 2 new _vk functions (perspective_rh_vk and orthographic_rh_vk, not double the amount) just like there are only 2 _gl functions and not double the amount.

So far I've only provided the gl options as kind of compatibility with common gl functions that are in use, I kind of consider gl deprecated and therefore don't provide all of the possibly projection functions.

ultraviolet has 4 modules (rh_yup, lh_yup, rh_ydown, lh_ydown), each with their own set of (perspective,orthographic)_(wgpu_dx,gl,vk) functions, if that's good reference. They also export by default the rh_yup to provide a good default for people who don't care (although to be honest I prefer blender-style rh_zup).

Yes I think ultraviolet might be the only library I've come across that actually tries to solve this. It might be a good approach for glam, I did want to put some thought into it myself but I've never managed to make the time.

@bitshifter bitshifter merged commit e2f4be9 into bitshifter:main Oct 27, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants