set function for Vector2, Vector3, Vector4 and Quaternion

What it says on the tin: a place to discuss proposed new features.
Post Reply
User avatar
betajaen
OGRE Moderator
OGRE Moderator
Posts: 3447
Joined: Mon Jul 18, 2005 4:15 pm
Location: Wales, UK
Contact:

set function for Vector2, Vector3, Vector4 and Quaternion

Post by betajaen » Fri Aug 13, 2010 12:31 pm

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.
0 x

User avatar
Zonder
Ogre Magi
Posts: 1133
Joined: Mon Aug 04, 2008 7:51 pm
Location: Manchester - England
x 22

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by Zonder » Fri Aug 13, 2010 1:01 pm

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 :)
0 x
There are 10 types of people in the world: Those who understand binary, and those who don't...

User avatar
lingfors
Hobgoblin
Posts: 505
Joined: Mon Apr 02, 2007 12:18 am
Location: Sweden

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by lingfors » Fri Aug 13, 2010 2:17 pm

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.
0 x

User avatar
betajaen
OGRE Moderator
OGRE Moderator
Posts: 3447
Joined: Mon Jul 18, 2005 4:15 pm
Location: Wales, UK
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by betajaen » Fri Aug 13, 2010 2:34 pm

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.
0 x

User avatar
xavier
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 9481
Joined: Fri Feb 18, 2005 2:03 am
Location: Dublin, CA, US

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by xavier » Fri Aug 13, 2010 10:30 pm

Zonder wrote: And I think your right it may execute faster . prob not by much though :)
Not if the compiler is using even naive optimization algorithms...
betajaen wrote: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...
0 x
Do you need help? What have you tried?

Image

Angels can fly because they take themselves lightly.

User avatar
lingfors
Hobgoblin
Posts: 505
Joined: Mon Apr 02, 2007 12:18 am
Location: Sweden

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by lingfors » Sat Aug 14, 2010 1:36 am

betajaen wrote: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;
0 x

User avatar
_tommo_
Gnoll
Posts: 677
Joined: Tue Sep 19, 2006 6:09 pm
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by _tommo_ » Sat Aug 14, 2010 2:30 am

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 :twisted:

Anyway i think you can afford two more lines of code, at least once :D
0 x
OverMindGames Blog
IndieVault.it: Il nuovo portale italiano su Game Dev & Indie Games

User avatar
Kojack
OGRE Moderator
OGRE Moderator
Posts: 7152
Joined: Sun Jan 25, 2004 7:35 am
Location: Brisbane, Australia
x 19

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by Kojack » Sat Aug 14, 2010 5:04 am

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.
0 x

User avatar
betajaen
OGRE Moderator
OGRE Moderator
Posts: 3447
Joined: Mon Jul 18, 2005 4:15 pm
Location: Wales, UK
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by betajaen » Sat Aug 14, 2010 9:42 am

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.
0 x

User avatar
madmarx
OGRE Expert User
OGRE Expert User
Posts: 1669
Joined: Mon Jan 21, 2008 10:26 pm

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by madmarx » Sat Aug 14, 2010 11:20 am

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?
0 x
Tutorials + Ogre searchable API + more for Ogre1.7 : http://sourceforge.net/projects/so3dtools/
Corresponding thread : http://www.ogre3d.org/forums/viewtopic. ... 93&start=0

reptor
Ogre Magi
Posts: 1120
Joined: Wed Nov 15, 2006 7:41 pm
Location: Finland

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by reptor » Sat Aug 14, 2010 3:19 pm

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

SamJ
Halfling
Posts: 40
Joined: Tue Jan 25, 2011 9:12 pm

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by SamJ » Wed Feb 02, 2011 3:29 pm

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.
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by jacmoe » Wed Feb 02, 2011 3:36 pm

It's obvious that the OP prefers other languages over C++. :wink:
To me 'set' doesn't look like C++.
But why not?
I can see that it's useful in some situations. :)
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

User avatar
betajaen
OGRE Moderator
OGRE Moderator
Posts: 3447
Joined: Mon Jul 18, 2005 4:15 pm
Location: Wales, UK
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by betajaen » Wed Feb 02, 2011 3:38 pm

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. ;)
0 x

User avatar
so0os
Bugbear
Posts: 833
Joined: Thu Apr 15, 2010 7:42 am
Location: Poznan, Poland

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by so0os » Wed Feb 02, 2011 3:54 pm

You can do it the forgotten way too.

Code: Select all

Vector3 v = {500, 600, 700};
but that just for initialising.
0 x
Sos Sosowski :)
http://www.sos.gd

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by Klaim » Wed Feb 02, 2011 11:27 pm

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 :P)


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

User avatar
so0os
Bugbear
Posts: 833
Joined: Thu Apr 15, 2010 7:42 am
Location: Poznan, Poland

Re: set function for Vector2, Vector3, Vector4 and Quaternio

Post by so0os » Thu Feb 03, 2011 9:01 am

I wonder if C1x will allow such things
0 x
Sos Sosowski :)
http://www.sos.gd

Post Reply