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

Introduce the ability to allow most explicit conversions to be implicit #481

Closed

Conversation

JesseTG
Copy link

@JesseTG JesseTG commented Feb 19, 2016

  • Use the new GLM_IMPLICIT_CONVERSION_CTOR #define
  • By default GLM_EXPLICIT_CONV is defined as "explicit"
  • #define GLM_IMPLICIT_CONVERSION_CTOR defines it as empty

I feel this is a better way to solve the problem I proposed in #421 ; namely, converting between various GLM types freely in a manner suited to template metaprogramming. Except now instead of registering a function pointer, I can just declare the existence of an implicit conversion, and just tell QVariant to use it. This should greatly simplify the original file that I was concerned about, without introducing any more complexity to GLM.

By the way, my other PR #458 is ready to merge, pending your approval (I'd nearly forgotten about it, actually).

- Use the new GLM_IMPLICIT_CONVERSION_CTOR #define
- By default GLM_EXPLICIT_CONV is defined as "explicit"
- #define GLM_IMPLICIT_CONVERSION_CTOR defines it as empty
@JesseTG
Copy link
Author

JesseTG commented Mar 5, 2016

@Groovounet Any thoughts on this PR?

Groovounet added a commit that referenced this pull request Mar 5, 2016
@Groovounet Groovounet added this to the GLM 0.9.8 milestone Mar 5, 2016
@Groovounet Groovounet self-assigned this Mar 5, 2016
@JesseTG
Copy link
Author

JesseTG commented Mar 5, 2016

Huh, I guess 251ad15 works too. Thank you!

@JesseTG JesseTG closed this Mar 5, 2016
@JesseTG JesseTG deleted the jtg/all-conversions-configurable branch March 5, 2016 22:11
@Groovounet
Copy link
Member

I applied your changes based on the existing GLM_FORCE_EXPLICIT_CTOR instead of adding another #define and made sure the coverage is exhaustive.

Thanks,
Christophe

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

Successfully merging this pull request may close these issues.

2 participants