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

tvec2 allows implicit conversion from another tvec2 type. #241

Closed
Gachapen opened this issue Sep 21, 2014 · 2 comments
Closed

tvec2 allows implicit conversion from another tvec2 type. #241

Gachapen opened this issue Sep 21, 2014 · 2 comments
Assignees
Labels
Milestone

Comments

@Gachapen
Copy link

I just discovered a lot of errors in our code when including an older glm version (0.9.5.1). We had accidentally implicitly converted vec2 to ivec2 and vice versa when calling some of our functions. With the current glm version (0.9.5.4), no errors or warnings were produced, which in my opinion is bad.

The reason for this problem is the fix for issue #159 (commit). I don't think the construction of a tvec2 from another type of tvec2 should be implicit. It is more inconvenient than convenient, since it can lead to many errors in the user's code. I suggest that this constructor is made explicit again. I also see that the only type that has this constructor not marked as explicit is the tvec2, which is inconsistent.

@Groovounet
Copy link
Member

Yes this is inconsistent with other types. This is now fixed for GLM 0.9.6 in master branch,

Thanks for reporting,
Christophe

@JaapSuter
Copy link

JaapSuter commented Aug 17, 2017

Hello,

Sorry to bring up this old issue, but it talks about something I found surprising, hence it seemed relevant.

I'm confused with regards to the following code:

     // vector of 64-bit precision doubles
    glm::dvec4 d;

    // to vector of 32-bit floats, compiles ok
    glm::fvec4 f1{ d };

    // whereas this won't compile.
    glm::fvec4 f2 = d;

    // but this compiles ok again
    glm::fvec4 f3;
    f3 = d;

I'm compiling with Visual Studio 2015, using GLM version 0.9.8.4, and GLM_FORCE_EXPLICIT_CTOR defined.

The second case results in a compile error, as expected. But the first and third case compile fine (and end up static_casting 64-bit doubles to 32-bit floats. I would've expected these cases to not compile, similar to the second case.

Am I overlooking something? In which case, is the rationale for the implicitness/explicitness of various conversions documented anywhere? Alternatively, is it an actual issue (still/again)? In which case, I may take a stab at resolving it, and cook up a pull request for review.

I briefly looked through the GLM manual as well as the Github issue list, but couldn't find a definitive "here's how GLM thinks conversions should work.". Then again, given there's already at least one #define that lets the user decide some of behavior (GLM_FORCE_EXPLICIT_CTOR), maybe that implies that GLM doesn't have strong opinions on the matter?

We limit the amount of code where double and single precision interact, so the risk potential of these implicit conversions is somewhat mitigated for us. So please consider this question to be very low priority. I'm mostly just sharing what I felt was surprising behavior (and it'll probably still turn out the surprising behavior was in my head, not in your code).

Thanks for all the work on such a great library! Cheers,

Jaap Suter

(note; I only just noticed that version 0.9.8.5 has come out, I'll report back if that makes a difference.)

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

No branches or pull requests

3 participants