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

Add missing cols/rows constants #491

Merged
merged 2 commits into from Apr 30, 2016

Conversation

JesseTG
Copy link

@JesseTG JesseTG commented Mar 14, 2016

Should I add anything like rows or cols to the vector type traits? Because they can be treated as either column vectors or row vectors, so I dunno...

@regnirpsj
Copy link

i vote yes, then #441 can be unified (instead of separate handling for vector and matrix).

regnirpsj added a commit to regnirpsj/glm that referenced this pull request Mar 14, 2016
@JesseTG
Copy link
Author

JesseTG commented Mar 14, 2016

But as row vectors or column vectors?

@regnirpsj
Copy link

iirc, they should be column vectors by default, i.e. vec4 is a mat1x4.

@JesseTG
Copy link
Author

JesseTG commented Mar 14, 2016

Actually, the spec says that it depends on whether the vector is the left or right operand in a vector/matrix multiplication. Nowhere does it otherwise refer to the notion of a row vector or column vector.

@regnirpsj
Copy link

that is correct in a general sense but given p' = P * V * M * p would be the usual way in opengl to transform a point, vectors would be assumed to be column oriented; this could/should be documented. the alternative is to just stick with the type<>::columns and not assign col/row dimensions for non-matrices.

@JesseTG
Copy link
Author

JesseTG commented Mar 15, 2016

Let's see what @Groovounet has to say.

@JesseTG
Copy link
Author

JesseTG commented Mar 21, 2016

...so what do you have to say, @Groovounet? I'm fine with the metaprogramming stuff being moved to a separate file, but until this PR is merged in I can't pull in the latest GLM changes (because I need this fix for my project).

@JesseTG
Copy link
Author

JesseTG commented Apr 16, 2016

Notice me, senpai~

@JesseTG
Copy link
Author

JesseTG commented Apr 21, 2016

@Groovounet Can you take a look at this bugfix, please?

@JesseTG
Copy link
Author

JesseTG commented Apr 29, 2016

@Groovounet Hi. Do you like this change set?

@Groovounet
Copy link
Member

I made a branch today type_trait where I investigated. Somehow there are build errors with Visual Studio.

@JesseTG
Copy link
Author

JesseTG commented Apr 30, 2016

Seriously? Christ. I'll take a look over the weekend.

@Groovounet
Copy link
Member

For your information, it was already broken before your changes. I came up with a workaround (in the branch) but I don't know whether it is good.

@Groovounet Groovounet merged commit ae15b89 into g-truc:master Apr 30, 2016
@JesseTG JesseTG deleted the jtg/type-traits-size branch April 30, 2016 16:33
@JesseTG
Copy link
Author

JesseTG commented Apr 30, 2016

All right, thank you for taking a look at this pull request.

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.

None yet

3 participants