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

Make coordinate system more clear in function naming (LH, RH, NO, ZO) #75

Closed
kavika13 opened this issue Feb 10, 2018 · 19 comments
Closed
Assignees

Comments

@kavika13
Copy link

kavika13 commented Feb 10, 2018

When porting old games from D3D -> OpenGL, it's very useful to understand what type of coordinate system you're using. I think pushing this information into the function names could be useful.

GLM supports all these variations. However, even if you decided to support "the one true way" (whatever way that is hehe), then at least being clear about what that one way (is in the function name) might be good.

Note, this is not a feature request to support all these alternate coordinate systems, if you don't already (browsing the source, I think not?). If that were interesting, I think it should probably be a separate ticket.

@bvisness
Copy link
Member

This is a good idea. I was just considering this myself, so I’m glad we have an issue for it now!

At the very least we will clearly document which coordinate system we use. I would love to support more coordinate systems as well.

@bvisness bvisness self-assigned this Feb 10, 2018
@StrangeZak
Copy link
Member

@bvisness You wanna do this sometime soon? Not sure what your schedule looks like right now. Kinda wanna smash out a bunch of these issues before the end of the year. Maybe a solid 2.0 RC

@bvisness
Copy link
Member

bvisness commented Dec 2, 2018

Would be a good idea and probably big enough to break all the backwards compatibility. We should maybe make a GitHub milestone for 2.0 so we can tag some issues to it.

@StrangeZak
Copy link
Member

That sounds like an awesome idea, ill get that going.

@StrangeZak StrangeZak added this to the Handmade Math 2.0 milestone Dec 2, 2018
@bvisness
Copy link
Member

bvisness commented Aug 3, 2019

Let's discuss how we want to go about this. Do we want to keep a single set of functions like HMM_Perspective that change their behavior based on a #define? Or do we want to make multiple functions, like HMM_Perspective_LH and HMM_Perspective_RH?

I think multiple functions would be a good idea just to keep things flexible - both implementations will be there, so we might as well make both implementations callable from the same project. But maybe we can make one the default according to some other #define.

I think the following functions care about handedness:

  • HMM_Orthographic
  • HMM_Perspective
  • HMM_Rotate? We specify an axis angle, and the direction of rotation around that axis would care about handedness, right?
  • HMM_LookAt?
  • HMM_QuaternionFromAxisAngle? (for reasons similar to HMM_Rotate)
  • HMM_Mat4ToQuaternion?

In case you couldn't tell, I'm not sure how handedness would affect anything to do with rotation.

@StrangeZak
Copy link
Member

Okay, so i think we should keep functions like HMM_Othographic around. Simply to not break compiles for people using the library. By default HMM_Orthographic and all other functions will call their *_LH versions unless a flag is specified to use the RH versions. People should also be able to explicitly call LH and RH versions if they want.

But usually i just want to #define USE_RIGHT_HANDED for my D3D builds and #define USE_LEFT_HANDED for my OpenGL builds. Then i would just call HMM_Orthorgraphic, etc, and have Handmade Math handle all of the Left/Right Handed-Ness.

@bvisness
Copy link
Member

bvisness commented Aug 3, 2019

I thought OpenGL was conventionally right-handed and D3D was conventionally left-handed. And I think our current stuff is right-handed. But yeah.

@kavika13
Copy link
Author

kavika13 commented Aug 3, 2019

My $0.02:

What @StrangeZak said matches how GLM works, and I think it can be a good way to go, possibly. There were a few hiccups I had when using GLM, doing the same things.

  1. Ambiguous functions and handedness macros:

If you make HMM_Orthographic default to one handedness type, then you can hit subtle bugs when the #define values accidentally are defined differently in different modules in new code, i.e. when you forgot to use #define HMM_USE_XYZ_HANDED in one module and it defaults to the wrong handedness.

If you're willing to slightly break backwards compatibility, forcing either #define HMM_USE_RIGHT_HANDED or #define HMM_USE_LEFT_HANDED might be an option (with a clear error message). You can also skip this forced detection if the user doesn't use ambiguous functions like HMM_Orthographic and opt to use HMM_Orthographic_LH or HMM_Orthographic_RH directly.

If you're trying to "not break compiles for people using the library" - if the user has embedded your code then they aren't auto-updating, so they won't be broken. If they pull the updates for your library, isn't that a manual process? And if so, can't some (easily debuggable and easily fixable) breaking changes be acceptable?

  1. Docs:

In GLM's case, it's tough to find the docs to remember the proper defines for handedness, and the huge spider-web of includes in that library also makes it tough to find. If the docs on these "global defines" is super easy to find, then I think this solution sounds good.

@bvisness
Copy link
Member

bvisness commented Aug 4, 2019

It's true that a manual update process means breaking changes are less of an issue, but I still want to preserve backwards compatibility to the fullest extent we can. And we plan to put this in the next major, breaking release anyway.

I think the idea of forcing a #define might be pretty nice. But I also think that it might be ok to give preference to one system. I think it's nice to be able to just drop it in and start using it, but I also think it might be confusing to have both HMM_Orthographic_RH and HMM_Orthographic both be equivalent out of the box. I'm curious for @StrangeZak's thoughts.

As for docs, I think our doc situation is still pretty manageable, since it's just a page or so of comments at the start of the library. And at some point I think I would like to generate HTML documentation of some kind, which might be a good idea if we keep introducing new options. But regardless of what system we use, HMM will always be a single header, which means you at least won't get lost trying to find what an option means.

@StrangeZak
Copy link
Member

I don’t think that have both a HMM_Orthographic and a HMM_Orthographic_RH would be too big of a deal. As you’d just look at the HMM_Orthographic source and see that it just calls the RH function. The only reason we’d really preserve HMM_Orthographic in the first place is just for backwards compatibility.

I’m not really a huge fan of forcing a #define as it won’t really be a drop in library anymore. I also think it’s safe to assume that most people using our lib at the moment are using OpenGL. Due to the lack of Left hand support. So I think what I purposed in the comment above is the best approach for the time being.

Also while I’m here. We should grab a D3D11 test application and change out the math lib (DXMath) for Handmade Math once we get proper left hand support.

@RandyGaul
Copy link

I'm in favor of explicit HMM_Perspective_LH and HMM_Perspective_RH. Windows uses the whole macro thing for wide vs ansi strings (W and A at the end of function names, and a macro without either), and I always hated it. I don't use the macros, just use the explicit functions. The macro adds not-so-useful of a feature in a tradeoff for more header bloat and complexity.

@StrangeZak
Copy link
Member

Usually I’d agree with you (I also think the A vs W thing in windows is annoying with the #define UNICODE), but I really don’t wanna break compatibility for people who are already using the library. Version upgrades are meant to be sort of “seamless”

@bvisness
Copy link
Member

bvisness commented Aug 7, 2019

Well this is supposed to be for a breaking change, so a little bit is to be expected. Maybe we can have only the explicit versions out of the box, but doing a #define can enable versions without the LH and RH suffixes. Then the extent of “breakage” is just that you have to indicate which convention you’ll be using.

@RandyGaul
Copy link

RandyGaul commented Aug 7, 2019

My personal priority of concerns is something like this.

  1. Small header without complexity.
  2. Clear API requiring minimal docs, obvious how to use at-a-glance.
  3. Avoid build-breaking changes.

Pretty much I don’t value the “generic” nature of any macros at all since they violate 1 and 2. Trying to use macros to avoid a build break overrides higher priority concerns. So to me the best option might be to just make the build breaking change.

Also, we can leave in the old function signatures and call into a handedness-explicit function (maybe LH), and mark it as deprecated. Then remove it later. This addresses the final concern number 3.

@StrangeZak
Copy link
Member

@bvisness If were gonna do a API breaking change we should just hold them all until we release version 2. Then we only break the library once ideally

@bvisness
Copy link
Member

bvisness commented Aug 8, 2019

@StrangeZak Yeah this is a 2.0 change, so breaking things is fine and expected (to a point).

I wasn't really considering the macros as a means of achieving backwards compatibility; I was just thinking of them in terms of ease of use. But it's not particularly hard to tack LH or RH onto a function that probably won't even show up that often.

Since I think we're in agreement that we want explicitly-named functions, we can certainly at least start with only the explicit ones, and assess the situation from there.

I also think we should maybe just confirm that we don't want to require any #defines beyond HANDMADE_MATH_IMPLEMENTATION to get things going. Being able to quickly use HMM in a project is part of the appeal. This would rule out the idea mentioned above where you are forced to define your preferred handedness.

@bvisness
Copy link
Member

bvisness commented Aug 8, 2019

And you know, the magic of C/C++ is that if you really want to use HMM_Orthographic without the suffix, you can just define it yourself. 🙂

@StrangeZak
Copy link
Member

A thought I just had regarding this:

A lot of people who are using our library I assume are using it with OpenGL. If they’re novices in graphics programming they might not understand Left/Handed coordinate systems.

Also, my idea was never to force someone to #define what they want but simply allow a switch keep Hmm-Ortho calling the RH function, and then If the #define is supplied switch it calling the LH function. There would still be explicit *LH and *RH functions

@bvisness
Copy link
Member

Done in HMM 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants