set function for Vector2, Vector3, Vector4 and Quaternion

[betajaen]

Hi chaps,

Would it be possible to have a "set" function put into the Vector2, Vector3, Vector4 and Quaternion classes.

Example:

Instead of doing this:

Code: Select all

mVertices[ver_index++] = Ogre::Vector3(left, bottom, depth);

I could do this:

Code: Select all

mVertices[ver_index++].set(left, bottom, depth);

To me it's easier to read, and you don't have to temporarily allocate another Vector3 there (Although I imagine the compiler would do some optimisation on that part).

It's would take two minutes to add to the Ogre source code. I would even do it myself, but I know next to nothing about submitting patches or mercurial.

[Zonder]

this is one of the reasons they switched to the new source control eithen if they don't accept the fix you can patch your local repository and still get latest code from ogre. I belive the patch would be submitting the changeset information for the files? INever done this my self either

And I think your right it may execute faster . prob not by much though

[lingfors]

x, y and z are public member variables, so you could either do

Code: Select all

mVertices[ver_index].x = left;
mVertices[ver_index].y = bottom;
mVertices[ver_index].z = depth;

or define your own function:

Code: Select all

void setVector3(vec, left, bottom, depth) {
vec.x = left;
vec.y = bottom;
vec.z = depth;
}

No need to bloat more than necessary.

[betajaen]

An inline three line function is hardly bloat.

The separate lines for x,y,z in your first example gets confusing if you have many lines like that, and the second one means I have to carry a reference to ver_index every time I call it, it is also function called by one function to do one job. I may as well use a macro.

[xavier]

And I think your right it may execute faster . prob not by much though

Not if the compiler is using even naive optimization algorithms...

An inline three line function is hardly bloat.

Especially since the compiler is going to replace the function call with the same three load instructions anyway.

People really need to stop trying to "help" the compiler with old, useless "optimizations". It's not 1995 anymore...

[lingfors]

An inline three line function is hardly bloat.

The separate lines for x,y,z in your first example gets confusing if you have many lines like that, and the second one means I have to carry a reference to ver_index every time I call it, it is also function called by one function to do one job. I may as well use a macro.

If you only need this feature at one place in your code, why do you need it to be in Ogre?

And you don't need to carry the index, you call it like this:

Code: Select all

setVector3(mVertices[ver_index], left, bottom, depth)

And adding a new method for something as simple as an assignment is bloat. If it was something that would take a long time to come up with or do by yourself, sure. But an assignment is as basic as you get, why would you need a special function for it just because it "reads" better (to you)? It would more likely lead to new users to wonder what's wrong with

Code: Select all

myVector = Ogre::Vector(x, y, z)

or

Code: Select all

myVector.x = x;
myVector.y = y;
myVector.z = z;

[_tommo_]

Code: Select all

float* ptr = (float*)mVertices;
float* end = ptr + vertices_number*3;
while( ptr < end )
{
    *ptr++ = x;
    *ptr++ = y;
    *ptr++ = z;
}

Can't beat this fast

Anyway i think you can afford two more lines of code, at least once

[Kojack]

Return Value Optimisation should kick in and both versions will produce the same code.
(C++ standard section 12.8.15)

when a temporary class object that has not been bound to a reference (12.2) would be copied to a class object with the same cv-unqualified type, the copy operation can be omitted by constructing the temporary object directly into the target of the omitted copy

That most likely won't happen in debug mode, in which case it will probably be faster to do the set. But in release with optimisation on, all good compilers should optimise it away.

[betajaen]

Fair enough on the optimisation. I was just guessing about that, and I didn't know the compiler was that smart.

Anyway, for neatness sake. Can we have a set function.

I have bunches of lines like this in my code right now:

Code: Select all

mRectangleData.mVertices[ver_index++] = Ogre::Vector3(left, bottom, depth);
mRectangleData.mVertices[ver_index++] = Ogre::Vector3(right, bottom, depth);
mRectangleData.mVertices[ver_index++] = Ogre::Vector3(left, top, depth);
mRectangleData.mVertices[ver_index++] = Ogre::Vector3(right, bottom, depth);
mRectangleData.mVertices[ver_index++] = Ogre::Vector3(right, top, depth);
mRectangleData.mVertices[ver_index++] = Ogre::Vector3(left, top, depth);

To me it looks ugly and a little bit confusing if you don't know how a vector and an Vector3 works. PhysX, Opcode and NxOgre all have set function in their vector classes. Honestly I don't know what the big deal is adding a three/four line function to each of the Vector/Quaternion classes.

[madmarx]

To me it looks ugly and a little bit confusing if you don't know how a vector and an Vector3 works.

I disagree with that sentence . It's confusing only if you don't know C++. vector is C++. move semantics is C++. RVO, NRVO and copy elision are important C++ programmer knowledge nowadays. Therefore, the

Code: Select all

= Ogre::Vector3(left, bottom, depth);

immediately tells 'I am good c++ and optimised!' to the eyes of the programmer.

Honestly I don't know what the big deal is adding a three/four line function to each of the Vector/Quaternion classes.

Concerning the 'set' function, I don't see a problem, maybe you can submit a patch?

[reptor]

I think such classes should be kept as lean as possible.

[SamJ]

Agree with OP, it's something that annoys me too, the extra allocation if you want to do it in one line. I don't know why everybody is saying this would be bloat. It's not like Ogre3D is a lean library (6MB dll and that's only ogremain), some more utility functions wouldn't hurt.

[jacmoe]

It's obvious that the OP prefers other languages over C++.
To me 'set' doesn't look like C++.
But why not?
I can see that it's useful in some situations.

[betajaen]

The OP likes C++ as well, the OP just stole the "set" idea from PhysX, which is about "C++ like" as you can get.

I should put it in as a papercut, since my other one was accepted.

[so0os]

You can do it the forgotten way too.

Code: Select all

Vector3 v = {500, 600, 700};

but that just for initialising.

[Klaim]

It's not as forgotten as you might think : C++0x generalize all object construction to this way of initializing them. I mean, read this : http://en.wikipedia.org/wiki/C%2B%2B0x# ... ialization


And by the way, GCC 4.4 already allow this (and I'm using it at work with so much pleasure it makes me feel I'm scripting... - but the compile times )


I think a setter function have no sense because C++ already allow optimization if you use the assignation syntax.

[so0os]

I wonder if C1x will allow such things