AlloSphere-Research-Group / allolib

Library for interactive multimedia application development
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

Different behavoir for single value constructor in Vec3f and Mesh.vertex(). Unify? #32

Open mantaraya36 opened 4 years ago

mantaraya36 commented 4 years ago

From karl:

Compare to the behavior of the vertex() call on Mesh. vertex(1.0) adds the vertex {1.0, 0.0, .0.0}. vertex(1.0, 2.0) adds the vertex {1.0, 2.0, 0.0}. but Vec3f(1.0) makes {1.0, 1.0, 1.0}?

younkhg commented 4 years ago

Mesh::vertex(x, y) is useful for 2D drawing, and z=0 is desired in that case In that sense Mesh::vertex(x) being (x, 0, 0) makes sense for 1D drawing, but do we do 1D drawing? I think Mesh::vertex(x) can be removed

Vec3f(x) being (x, x, x) can be convenient and intuitive for graphics people since it matches behavior of GLSL vec3 v = vec3(x) = (x, x, x), but in GLSL vec3 does not allow vec3(x, y). One param case is the only special case and other times you have to give all three numbers.

If Vec3f(x) results (x, 0, 0) that can also be reasonable option. I think for this one we can choose one. I'd vote for leaving as it is for compatibility.

Vec3f(x, y) results (x, y, x) and this is the most weird and confusing part in this topic. The Vec::set methods are where this behavior comes from: https://github.com/AlloSphere-Research-Group/allolib/blob/5be6b8312bb5ab8e51620c1df65cd5b823119a13/include/al/math/al_Vec.hpp#L341-L376

These set calls I think need further discussion.

LancePutnam commented 4 years ago

I see no signature Mesh::vertex(float x), so maybe Mesh::vertex(const Vertex& v) is called implicity? If I do mesh.vertex(1), it adds {1,1,1}.

I strongly agree to leave Vec(x) as {x,x,x} for backwards/GLSL compatibility.

You should also pull changes from AlloSystem every so often. The Vec::set functions where fixed several months ago: https://github.com/AlloSphere-Research-Group/AlloSystem/blob/646bac1232d230f61dbdaaef38327de1a244b43f/allocore/allocore/math/al_Vec.hpp#L193

There are probably more fixes that could be pulled in.

I would not make Mesh dependent on OpenGL, especially if just for the primitive types. You are not always needing rendering when working with meshes.

konhyong commented 4 years ago

Yes, it is Mesh::vertex(const Vertex& v) getting called, and what you see with mesh.vertex(1.0) is consistent with what i see.

however Mesh.vertex(1.0, 2.0) goes through Mesh::vertex(float x, float y, float z=0), which would mainly be used for 2D drawing.

we did review the matter 2 days ago, and deemed the Mesh.vertex signatures related to this matter were fine as it is, and Vec set functions are what needed to be adjusted.

we did discuss utilizing static asserts as well with regards to the parameter numbers.

in order to avoid confusing behavior i think the set function for the vector should always have same number of parameters as the vector dimensions.

Vec3f(x, y) doesn't really make any sense to me and should result in a throw.

vec3f(x) resulting in (x, x, x) would probably be the sole exception, although it does mean we live with some of the consequences as this issue.

On Thu, Feb 6, 2020 at 3:34 AM Lance Putnam notifications@github.com wrote:

I see no signature Mesh::vertex(float x), so maybe Mesh::vertex(const Vertex& v) is called implicity? If I do mesh.vertex(1), it adds {1,1,1}.

I strongly agree to leave Vec(x) as {x,x,x} for backwards/GLSL compatibility.

You should also pull changes from AlloSystem every so often. The Vec::set functions where fixed several months ago: https://github.com/AlloSphere-Research-Group/AlloSystem/blob/646bac1232d230f61dbdaaef38327de1a244b43f/allocore/allocore/math/al_Vec.hpp#L193

There are probably more fixes that could be pulled in.

I would not make Mesh dependent on OpenGL, especially if just for the primitive types. The rendering engine details are not being encapsulated very well here and you are not always needing rendering when working with meshes.

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJJEMUZJ6OJVTGN3PTRBPYTLA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK64XCQ#issuecomment-582863754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMJ7TJHBFQA2KXMJMLRI7DRBPYTLANCNFSM4KP3DXXQ .

kybr commented 4 years ago

ok. thanks for all the insight and discussion :)

i'm modifying/refining my complaint. i'm ok with Vec behavior inspired by GLSL. so Vec3f(1.0) is {1,1,1}. fine. as long as we are borrowing from the language of a well-known, well-defined system and we are clear about it.

what i can't have is this:

include "al/math/al_Vec.hpp"

int main() { al::Vec3f v; v += 1; // wtf? v.print(); }

this prints {1,1,1}! which is weird. weird that it is allowed. several students did this. not literally this, but they tried to += scalars to their vectors. the system does not complain.

let's do what GLSL would do in this case and make this a compile error.

-- karl

On Thu, Feb 6, 2020 at 4:48 AM Kon-Hyong Kim notifications@github.com wrote:

Yes, it is Mesh::vertex(const Vertex& v) getting called, and what you see with mesh.vertex(1.0) is consistent with what i see.

however Mesh.vertex(1.0, 2.0) goes through Mesh::vertex(float x, float y, float z=0), which would mainly be used for 2D drawing.

we did review the matter 2 days ago, and deemed the Mesh.vertex signatures related to this matter were fine as it is, and Vec set functions are what needed to be adjusted.

we did discuss utilizing static asserts as well with regards to the parameter numbers.

in order to avoid confusing behavior i think the set function for the vector should always have same number of parameters as the vector dimensions.

Vec3f(x, y) doesn't really make any sense to me and should result in a throw.

vec3f(x) resulting in (x, x, x) would probably be the sole exception, although it does mean we live with some of the consequences as this issue.

On Thu, Feb 6, 2020 at 3:34 AM Lance Putnam notifications@github.com wrote:

I see no signature Mesh::vertex(float x), so maybe Mesh::vertex(const Vertex& v) is called implicity? If I do mesh.vertex(1), it adds {1,1,1}.

I strongly agree to leave Vec(x) as {x,x,x} for backwards/GLSL compatibility.

You should also pull changes from AlloSystem every so often. The Vec::set functions where fixed several months ago:

https://github.com/AlloSphere-Research-Group/AlloSystem/blob/646bac1232d230f61dbdaaef38327de1a244b43f/allocore/allocore/math/al_Vec.hpp#L193

There are probably more fixes that could be pulled in.

I would not make Mesh dependent on OpenGL, especially if just for the primitive types. The rendering engine details are not being encapsulated very well here and you are not always needing rendering when working with meshes.

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJJEMUZJ6OJVTGN3PTRBPYTLA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK64XCQ#issuecomment-582863754 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMJ7TJHBFQA2KXMJMLRI7DRBPYTLANCNFSM4KP3DXXQ

.

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRYZKC32HD72Y3WAELRBQBJLA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7DJGI#issuecomment-582890649, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUS4H352MJMZVQ4RNDRBQBJLANCNFSM4KP3DXXQ .

younkhg commented 4 years ago

We can put explicit in front of the constructor and make it compile error. That will also make mesh.vertex(1.0) a compile error.

konhyong commented 4 years ago

That behavior is actually also defined in GLSL ref: https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.pdf

With a few exceptions, operations are component-wise. Usually, when an operator operates on a vector or matrix, it is operating independently on each component of the vector or matrix, in a component-wise fashion. For example, vec3 v, u; float f; v = u + f; will be equivalent to v.x = u.x + f; v.y = u.y + f; v.z = u.z + f;

We can have a discussion whether we want this behavior or not, but it is surprisingly consistent with GLSL.

On Thu, Feb 6, 2020 at 7:07 PM Keehong Youn notifications@github.com wrote:

We can put explicit in front of the constructor and make it compile error. That will also make mesh.vertex(1.0) a compile error.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJZWYN6CYDCYZZEUBDRBTF6RA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELBSAXA#issuecomment-583213148, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMJ7TPEK2CW7IWJCX3NHATRBTF6RANCNFSM4KP3DXXQ .

kybr commented 4 years ago

On Fri, Feb 7, 2020, 13:24 Kon-Hyong Kim notifications@github.com wrote:

That behavior is actually also defined in GLSL ref: https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.pdf

With a few exceptions, operations are component-wise. Usually, when an operator operates on a vector or matrix, it is operating independently on each component of the vector or matrix, in a component-wise fashion. For example, vec3 v, u; float f; v = u + f; will be equivalent to v.x = u.x + f; v.y = u.y + f; v.z = u.z + f;

πŸ˜§πŸ˜©πŸ€¦πŸ€·πŸ™

We can have a discussion whether we want this behavior or not, but it is surprisingly consistent with GLSL.

On Thu, Feb 6, 2020 at 7:07 PM Keehong Youn notifications@github.com wrote:

We can put explicit in front of the constructor and make it compile error. That will also make mesh.vertex(1.0) a compile error.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJZWYN6CYDCYZZEUBDRBTF6RA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELBSAXA#issuecomment-583213148 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMJ7TPEK2CW7IWJCX3NHATRBTF6RANCNFSM4KP3DXXQ

.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJXHPSKDALRRRKLC7BDRBXGSJA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELEVKZA#issuecomment-583619940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUEKUH7T4U3KI3UTY3RBXGSJANCNFSM4KP3DXXQ .

grrrwaaa commented 4 years ago

Outside voice here, but with my GLSL conditioning, the only thing that looks odd is adding an integer type to a float type.

vec3 v; v += 1.0;

looks fine to me and I’d expect it to be {1.0,1.0,1.0}

BTW I can also recommend looking at the GLM math library, a C++ header-only library directly emulating GLSL.

On Feb 6, 2020, at 9:01 PM, karl yerkes notifications@github.com wrote:

ok. thanks for all the insight and discussion :)

i'm modifying/refining my complaint. i'm ok with Vec behavior inspired by GLSL. so Vec3f(1.0) is {1,1,1}. fine. as long as we are borrowing from the language of a well-known, well-defined system and we are clear about it.

what i can't have is this:

include "al/math/al_Vec.hpp"

int main() { al::Vec3f v; v += 1; // wtf? v.print(); }

this prints {1,1,1}! which is weird. weird that it is allowed. several students did this. not literally this, but they tried to += scalars to their vectors. the system does not complain.

let's do what GLSL would do in this case and make this a compile error.

-- karl

On Thu, Feb 6, 2020 at 4:48 AM Kon-Hyong Kim notifications@github.com wrote:

Yes, it is Mesh::vertex(const Vertex& v) getting called, and what you see with mesh.vertex(1.0) is consistent with what i see.

however Mesh.vertex(1.0, 2.0) goes through Mesh::vertex(float x, float y, float z=0), which would mainly be used for 2D drawing.

we did review the matter 2 days ago, and deemed the Mesh.vertex signatures related to this matter were fine as it is, and Vec set functions are what needed to be adjusted.

we did discuss utilizing static asserts as well with regards to the parameter numbers.

in order to avoid confusing behavior i think the set function for the vector should always have same number of parameters as the vector dimensions.

Vec3f(x, y) doesn't really make any sense to me and should result in a throw.

vec3f(x) resulting in (x, x, x) would probably be the sole exception, although it does mean we live with some of the consequences as this issue.

On Thu, Feb 6, 2020 at 3:34 AM Lance Putnam notifications@github.com wrote:

I see no signature Mesh::vertex(float x), so maybe Mesh::vertex(const Vertex& v) is called implicity? If I do mesh.vertex(1), it adds {1,1,1}.

I strongly agree to leave Vec(x) as {x,x,x} for backwards/GLSL compatibility.

You should also pull changes from AlloSystem every so often. The Vec::set functions where fixed several months ago:

https://github.com/AlloSphere-Research-Group/AlloSystem/blob/646bac1232d230f61dbdaaef38327de1a244b43f/allocore/allocore/math/al_Vec.hpp#L193

There are probably more fixes that could be pulled in.

I would not make Mesh dependent on OpenGL, especially if just for the primitive types. The rendering engine details are not being encapsulated very well here and you are not always needing rendering when working with meshes.

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJJEMUZJ6OJVTGN3PTRBPYTLA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK64XCQ#issuecomment-582863754 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMJ7TJHBFQA2KXMJMLRI7DRBPYTLANCNFSM4KP3DXXQ

.

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRYZKC32HD72Y3WAELRBQBJLA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7DJGI#issuecomment-582890649, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUS4H352MJMZVQ4RNDRBQBJLANCNFSM4KP3DXXQ .

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

kybr commented 4 years ago

i'll just pick a few nits more....

the c++ code i posted was this:

#include "al/math/al_Vec.hpp" int main() {

so the translation to GLSL might be

void main() {

which would error with:

ERROR: 0:xx: '+' does not operate on 'vec3' and 'int'

because GLSL is quite strict with int/float conversions as we all know. we break with GLSL-compatability on that point, right? we aren't going so far as to make v += 1.0 work but v += 1 break!

we also allow this

Vec3f v = 1; // could also be 1.0

but in GLSL:

vec3 v = 1; vec3 p = 1.0;

would both error with

ERROR: 0:19: Incompatible types in initialization (and no available implicit conversion)

GLSL requires initialization to be explicit:

vec3 p = vec3(1.0);

i also checked the behavior of glm:

[cling]$ #include <glm/vec3.hpp> [cling]$ using namespace glm; [cling]$ vec3 v; [cling]$ v += 1; [cling]$ v += 1.0; [cling]$ v += 1.0f; [cling]$ vec3 p = 1.0f; // error: no viable conversion from 'float' to 'glm::vec3' (aka 'vec<3, float, defaultp>')

-

one can find good reasons for adding a scalar to a vector in general math contexts.

but Vec3 is not often used as a vector in the general mathy sense. instead, it is used as a position, velocity, force, acceleration, or heading in a 3D animation or simulation. in this more specific, but still pretty general use case, i cannot think of a good reason to ever add a float/double to a Vec3. can you? it's not a rhetorical question. please think about it. let me know if you think of a case where it "makes sense" or it's "meaningful" to say "v += 1.0".

we use c++ in part because it's strict types allow us to build systems where the compiler reflects our intentions back to us. the more we tell the compiler about the rules of the use case, the more it can reason about our code.

i'm not actually proposing this, but imagine a type Phys3f that is exactly Vec3f but it throws an error when you try to add a scalar to it. when a student tries to treat a Phys3f like a float, they get an error that tells them "that's not how physics works."

-- karl

On Fri, Feb 7, 2020 at 1:35 PM Karl Yerkes karl.yerkes@gmail.com wrote:

On Fri, Feb 7, 2020, 13:24 Kon-Hyong Kim notifications@github.com wrote:

That behavior is actually also defined in GLSL ref: https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.pdf

With a few exceptions, operations are component-wise. Usually, when an operator operates on a vector or matrix, it is operating independently on each component of the vector or matrix, in a component-wise fashion. For example, vec3 v, u; float f; v = u + f; will be equivalent to v.x = u.x + f; v.y = u.y + f; v.z = u.z + f;

πŸ˜§πŸ˜©πŸ€¦πŸ€·πŸ™

We can have a discussion whether we want this behavior or not, but it is surprisingly consistent with GLSL.

On Thu, Feb 6, 2020 at 7:07 PM Keehong Youn notifications@github.com wrote:

We can put explicit in front of the constructor and make it compile error. That will also make mesh.vertex(1.0) a compile error.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJZWYN6CYDCYZZEUBDRBTF6RA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELBSAXA#issuecomment-583213148 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMJ7TPEK2CW7IWJCX3NHATRBTF6RANCNFSM4KP3DXXQ

.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJXHPSKDALRRRKLC7BDRBXGSJA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELEVKZA#issuecomment-583619940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUEKUH7T4U3KI3UTY3RBXGSJANCNFSM4KP3DXXQ .

grrrwaaa commented 4 years ago

A vector in GLSL is more like a vector in the arraylike sense than in the mathy sense. That’s partly because GLSL is designed to run on a GPU that naturally is optimized to work on SIMD instructions over 4 floats as one of its most common cases; and partly because this same representation is used for things like colours etc, not just Euclidean geometric coordinates. GLSL is closer to the machine than to an intentionally smart structure.

I agree that a strong rationale for using C++ is to get useful compiler errors (rather than, say, mysterious runtime errors).*

Personally I am in favour of painting simple underlying data layouts with strongly typed interfaces where this is useful. So, if the underlying data of Vec3f is float[3], then it can be safely cast to glm::vec3 and used that way where the programmer intends it, or even cast to RGBf or something where that intention exists, or a Force3f where that intention exists, etc., and thus meaningful semantics (what adding means, what reasonable ranges are, whether it can be meaningfully negative, etc.) can be in force as needed. Trying to box every intention into one Vec3f is a fool’s game.

Ditto glm::vec4/Vec4f/glm::quat/Quaternion/RGBAf/some other arbitrary application of float[4] …

Graham

On Feb 7, 2020, at 7:57 PM, karl yerkes notifications@github.com wrote:

i'll just pick a few nits more....

the c++ code i posted was this:

#include "al/math/al_Vec.hpp" int main() {

  • al::Vec3f v;*
  • v += 1; // wtf?*
  • v.print(); }*

so the translation to GLSL might be

void main() {

  • vec3 v;*
  • v += 1; // would work if 1 was 1.0*
  • gl_FragColor = vec4(v, 1.0); }*

which would error with:

ERROR: 0:xx: '+' does not operate on 'vec3' and 'int'

because GLSL is quite strict with int/float conversions as we all know. we break with GLSL-compatability on that point, right? we aren't going so far as to make v += 1.0 work but v += 1 break!

we also allow this

Vec3f v = 1; // could also be 1.0

but in GLSL:

vec3 v = 1; vec3 p = 1.0;

would both error with

ERROR: 0:19: Incompatible types in initialization (and no available implicit conversion)

GLSL requires initialization to be explicit:

vec3 p = vec3(1.0);

i also checked the behavior of glm:

[cling]$ #include <glm/vec3.hpp> [cling]$ using namespace glm; [cling]$ vec3 v; [cling]$ v += 1; [cling]$ v += 1.0; [cling]$ v += 1.0f; [cling]$ vec3 p = 1.0f; // error: no viable conversion from 'float' to 'glm::vec3' (aka 'vec<3, float, defaultp>')

-

one can find good reasons for adding a scalar to a vector in general math contexts.

but Vec3 is not often used as a vector in the general mathy sense. instead, it is used as a position, velocity, force, acceleration, or heading in a 3D animation or simulation. in this more specific, but still pretty general use case, i cannot think of a good reason to ever add a float/double to a Vec3. can you? it's not a rhetorical question. please think about it. let me know if you think of a case where it "makes sense" or it's "meaningful" to say "v += 1.0".

we use c++ in part because it's strict types allow us to build systems where the compiler reflects our intentions back to us. the more we tell the compiler about the rules of the use case, the more it can reason about our code.

i'm not actually proposing this, but imagine a type Phys3f that is exactly Vec3f but it throws an error when you try to add a scalar to it. when a student tries to treat a Phys3f like a float, they get an error that tells them "that's not how physics works."

-- karl

On Fri, Feb 7, 2020 at 1:35 PM Karl Yerkes karl.yerkes@gmail.com wrote:

On Fri, Feb 7, 2020, 13:24 Kon-Hyong Kim notifications@github.com wrote:

That behavior is actually also defined in GLSL ref: https://www.khronos.org/registry/OpenGL/specs/gl/GLSLangSpec.4.60.pdf

With a few exceptions, operations are component-wise. Usually, when an operator operates on a vector or matrix, it is operating independently on each component of the vector or matrix, in a component-wise fashion. For example, vec3 v, u; float f; v = u + f; will be equivalent to v.x = u.x + f; v.y = u.y + f; v.z = u.z + f;

πŸ˜§πŸ˜©πŸ€¦πŸ€·πŸ™

We can have a discussion whether we want this behavior or not, but it is surprisingly consistent with GLSL.

On Thu, Feb 6, 2020 at 7:07 PM Keehong Youn notifications@github.com wrote:

We can put explicit in front of the constructor and make it compile error. That will also make mesh.vertex(1.0) a compile error.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMJ7TJZWYN6CYDCYZZEUBDRBTF6RA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELBSAXA#issuecomment-583213148 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMJ7TPEK2CW7IWJCX3NHATRBTF6RANCNFSM4KP3DXXQ

.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJXHPSKDALRRRKLC7BDRBXGSJA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELEVKZA#issuecomment-583619940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUEKUH7T4U3KI3UTY3RBXGSJANCNFSM4KP3DXXQ .

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

younkhg commented 4 years ago

v += 1 could look weird but sometimes v *= 0.5 is quite useful

kybr commented 4 years ago

Of course, multiplication by a scalar is meaningful for 3d physical vectors. That is not in dispute. *= And += are very different.

On Sat, Feb 8, 2020, 15:18 Keehong Youn notifications@github.com wrote:

v += 1 could look weird but sometimes v *= 0.5 is quite useful

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRQGCTLY77F3IEDCDDRB44STA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF6ADQ#issuecomment-583786510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJQZJTMFJZAKZIPL2HDRB44STANCNFSM4KP3DXXQ .

younkhg commented 4 years ago

One can think of them as similar thing under the category of per-component operations, which can useful for when programming. I don't think think every functionality needs to have one to one mapping to physical/mathematical concept.

Well, +=1 can be "make the magnitude 1 larger". Then it is closer to what *= does and also have some physical meaning.

---- On Sat, 08 Feb 2020 15:30:39 -0800 karl yerkesnotifications@github.com wrote ----Of course, multiplication by a scalar is meaningful for 3d physical
vectors. That is not in dispute. *= And += are very different.

On Sat, Feb 8, 2020, 15:18 Keehong Youn notifications@github.com wrote:

v += 1 could look weird but sometimes v *= 0.5 is quite useful

β€”
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRQGCTLY77F3IEDCDDRB44STA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF6ADQ#issuecomment-583786510,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAMILJQZJTMFJZAKZIPL2HDRB44STANCNFSM4KP3DXXQ
.

β€”You are receiving this because you commented.Reply to this email directly, view it on GitHub, or unsubscribe.

kybr commented 4 years ago

One can think of them as similar thing under the category of per-component

operations, which can useful for when programming.

Agree. But can we do better? Who actually uses vecs that way? How many people would immediately understand from looking at the code what vec3f += float? Compare that number to those who might be helped by a compiler error?

I don't think think every functionality needs to have one to one mapping to

physical/mathematical concept.

Agree. But sometimes, if it is not too much trouble, we can make things a little better by moving in that direction.

+=1 can be "make the magnitude 1 larger".

Is that actually what it does though? I think your confusion about what it means illustrates my point.

Vec3f v = randomVector(); v += 1; // how does this make the magnitude 1 larger? v.normalize(v.mag() + 1); // make magnitude 1 larger

Vec3f += float only means "make the magnitude 1 larger" in the pathologically specific case where the vector is {0,0,0} or on the ray {1,1,1}.

Then it is closer to what *= does and also have some physical meaning.

the physical meaning of +=1 is "move thing in the arbitrary direction along {1,1,1}" but that does not making +=1 meaningful.

-- karl

---- On Sat, 08 Feb 2020 15:30:39 -0800 karl yerkesnotifications@github.com

wrote ----Of course, multiplication by a scalar is meaningful for 3d physical vectors. That is not in dispute. *= And += are very different.

On Sat, Feb 8, 2020, 15:18 Keehong Youn notifications@github.com wrote:

v += 1 could look weird but sometimes v *= 0.5 is quite useful

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRQGCTLY77F3IEDCDDRB44STA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF6ADQ#issuecomment-583786510>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAMILJQZJTMFJZAKZIPL2HDRB44STANCNFSM4KP3DXXQ>

.

β€”You are receiving this because you commented.Reply to this email directly, view it on GitHub, or unsubscribe.

β€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJRVERV5P6ELGEMP473RB5A5LA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF6R4Q#issuecomment-583788786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJV4CRNGIBYYARJVAM3RB5A5LANCNFSM4KP3DXXQ .

younkhg commented 4 years ago

I did not have confusion about what the current += does. I was saying if we want some other possibly more meaningful thing there's one option ("can be") of making it work on the magnitude. (which I don't think is better than current functionality)

kybr commented 4 years ago

On Sat, Feb 8, 2020, 16:28 Keehong Youn notifications@github.com wrote:

I did not have confusion about what the current += does.

Oh. Ok. My bad. Sorry. I misunderstood.

I was saying if we want some other possibly more meaningful thing there's

one option ("can be") of making it work on the magnitude. (which I don't think is better than current functionality)

Yeah. I think making +=1 mean "increment the magnitude" is an improvement.

β€”

You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJTDI7KRXHP5RAQVSG3RB5E2DA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF7D5Y#issuecomment-583791095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUC3RGECN35UVSARKLRB5E2DANCNFSM4KP3DXXQ .

kybr commented 4 years ago

Correction. Inline.

On Sat, Feb 8, 2020, 16:41 Karl Yerkes karl.yerkes@gmail.com wrote:

On Sat, Feb 8, 2020, 16:28 Keehong Youn notifications@github.com wrote:

I did not have confusion about what the current += does.

Oh. Ok. My bad. Sorry. I misunderstood.

I was saying if we want some other possibly more meaningful thing there's

one option ("can be") of making it work on the magnitude. (which I don't think is better than current functionality)

Yeah. I think making +=1 mean "increment the magnitude" is an NOT improvement.

β€”

You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/AlloSphere-Research-Group/allolib/issues/32?email_source=notifications&email_token=AAMILJTDI7KRXHP5RAQVSG3RB5E2DA5CNFSM4KP3DXX2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELF7D5Y#issuecomment-583791095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMILJUC3RGECN35UVSARKLRB5E2DANCNFSM4KP3DXXQ .

younkhg commented 4 years ago

I think it is interesting. We learn that a vector has magnitude and direction, and the magnitude approach will operate on the one of the component directly.

Then we can document the code like "operations with scalar work as if the scalar is operated on the magnitude"

matthewjameswright commented 4 years ago

It seems to me that you would need to be using polar coordinates for this to make sense. If vec3f V has a certain azimuth, elevation, and distance, then V+1 should have the same azimuth and elevation but with one greater distance.

That’s very different from point wise addition, like v-mean(v), which also makes sense in a completely different context

Sent from a handheld typo device with autocorrect

On Feb 8, 2020, at 4:50 PM, Keehong Youn notifications@github.com wrote:

I think it is interesting. We learn that a vector has magnitude and direction, and the magnitude approach will operate on the one of the component directly.

Then we can document the code like "operations with scalar work as if the scalar is operated on the magnitude"

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

LancePutnam commented 4 years ago

v + x was implemented as v + Vec3(x, x, x) to be compatible with GLSL. This often comes in handy, for instance applying a smooth step to a vector vv(3.-2.*v). Graham is right that Vec ops are more array-oriented. I have never seen anywhere, in code or math, where v + x means v (|v|+x)/|v|.