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 projection matrix constructors #569

Open
kovaxis opened this issue Oct 16, 2024 · 3 comments
Open

Improve projection matrix constructors #569

kovaxis opened this issue Oct 16, 2024 · 3 comments

Comments

@kovaxis
Copy link
Contributor

kovaxis commented Oct 16, 2024

Hello! I got very confused choosing the proper Mat4::perspective_* function. I think they could be documented a lot better. In particular, listing from what coordinate system they are mapping from, and into what coordinate system they are mapping into.

I've now done the research, and the transformations are:

perspective_rh_gl: X+ right, Y+ up, Z+ out of screen -> X+ right [-1, 1], Y+ up [-1, 1], Z+ into screen [-1, 1]
perspective_rh:    X+ right, Y+ up, Z+ out of screen -> X+ right [-1, 1], Y+ up [-1, 1], Z+ into screen [0, 1]
perspective_lh:    X+ right, Y+ up, Z+ into screen   -> X+ right [-1, 1], Y+ up [-1, 1], Z+ into screen [0, 1]

Where OpenGL uses Z+ into screen [-1, 1], Metal/DirectX/WebGPU use Z+ into screen [0, 1] and Vulkan uses Y+ down, Z+ into screen [0, 1].

The same issue applies to orthographic projections.

I propose the following to avoid breaking changes:

  • Add perspective_rh_vk and orthographic_rh_vk functions, that map from X+ right, Y+ up, Z+ out of screen to Y+ down, Z+ into screen [0, 1] (what Vulkan expects).
  • Document all of the projection functions with their corresponding modelview-coordinate-system -> graphics-api pairs.
    • perspective_rh_gl: Z+ out of screen -> GL
    • perspective_rh_vk: Z+ out of screen -> VK
    • perspective_rh: Z+ out of screen -> Metal/DX/WGPU
    • perspective_lh: Z+ into screen -> Metal/DX/WGPU
    • Corresponding orthographic functions.
  • Further document the perspective_infinite and perspective_infinite_reverse.

There would be no Z+ into screen -> VK, just like currently there is no Z+ into screen -> GL. Maybe those two could be added? (perspective_lh_gl, perspective_lh_vk).

Actually, I collected my thoughts while writing this issue and I think I'll write up a PR.

@bitshifter bitshifter changed the title Improve documentation for projection matrix constructors Improve projection matrix constructors Oct 30, 2024
@bitshifter
Copy link
Owner

Leaving this open as while the existing docs have been improved I want to consider moving the projection matrix methods out of Mat4 and DMat4 entirely. There are a lot of combinations, most project will only use one variant of these, e.g. rh_vk or lh_dx or whatever naming gets used, so ideally a project would use the variant they need at never see the rest.

Separate to projection matrix creation, there are a bunch of other methods that have lh and rh variants (e..g. look_at_lh) which could perhaps also be grouped with other lh or rh methods so only relevant methods are imported into a project, although for those it's probably best if they are still methods on types which would mean using traits, which I'm a bit less keen on.

@aloucks
Copy link
Contributor

aloucks commented Nov 9, 2024

although for those it's probably best if they are still methods on types which would mean using traits, which I'm a bit less keen on.

Please, no traits.

@kovaxis
Copy link
Contributor Author

kovaxis commented Nov 10, 2024

Although there's precedent in the Swizzle traits.

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

No branches or pull requests

3 participants