/******************************************************************************/ /*! \file vec3f.hpp \date 6/14/2005 \brief This is the interface file for the Vec3f class, a 3-element, single-precision(float) vector class. *******************************************************************************/ #ifndef FUEL_VEC3F_H #define FUEL_VEC3F_H #include <math.h> namespace Fuel { namespace Math { class Vec3f; inline bool operator==(const Vec3f &lhs, const Vec3f &rhs); inline bool operator!=(const Vec3f &lhs, const Vec3f &rhs); inline Vec3f operator *(const Vec3f &lhs, const Vec3f &rhs); inline Vec3f operator /(const Vec3f &lhs, const Vec3f &rhs); inline Vec3f operator +(const Vec3f &lhs, const Vec3f &rhs); inline Vec3f operator -(const Vec3f &lhs, const Vec3f &rhs); inline Vec3f operator *(const Vec3f &lhs, const float rhs); inline Vec3f operator /(const Vec3f &lhs, const float rhs); inline Vec3f operator -(const Vec3f &src); /******************************************************************************/ /*! \class Vec3f \brief The Vec3f class impliments a 3-element, single-precision(float) vector class. Operations include: - Default and Copy constructors - Parameterized constructor (X, Y, Z) - Parameterized constructor (from v1 to v2) - Equality and inequality operators - Assignment operator - Multiplication, Division, Addition and subtraction of vectors. - Multiplication and Division by scalars. - Cross product - Dot product - Length^2 and length functions - unitization functions *******************************************************************************/ class Vec3f { public: Vec3f() { } Vec3f(float x, float y, float z) : X(x), Y(y), Z(z) { } Vec3f(const Vec3f &src); Vec3f(const Vec3f &from, const Vec3f &to); // equality operators friend bool operator==(const Vec3f &lhs, const Vec3f &rhs); friend bool operator!=(const Vec3f &lhs, const Vec3f &rhs); // self-modifying operators inline void operator =(const Vec3f &rhs); inline void operator*=(const Vec3f &rhs); inline void operator/=(const Vec3f &rhs); inline void operator+=(const Vec3f &rhs); inline void operator-=(const Vec3f &rhs); inline void operator*=(const float rhs); inline void operator/=(const float rhs); inline void normalize(); // three-opperand methods static inline void mul(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs); static inline void div(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs); static inline void add(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs); static inline void sub(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs); static inline void add(Vec3f &res, const Vec3f &lhs, const float rhs); static inline void sub(Vec3f &res, const Vec3f &lhs, const float rhs); static inline void cross(Vec3f &res, const Vec3f &lhs, const float rhs); static inline Vec3f cross(const Vec3f &lhs, const Vec3f &rhs); static inline float dot(const Vec3f &lhs, const Vec3f &rhs); static inline float rad(const Vec3f &src); // Length^2 static inline float abs(const Vec3f &src); // Length static inline Vec3f norm() const; // temp-modifying operators friend Vec3f operator*(const Vec3f &lhs, const Vec3f &rhs); friend Vec3f operator/(const Vec3f &lhs, const Vec3f &rhs); friend Vec3f operator+(const Vec3f &lhs, const Vec3f &rhs); friend Vec3f operator-(const Vec3f &lhs, const Vec3f &rhs); friend Vec3f operator*(const Vec3f &lhs, const float rhs); friend Vec3f operator/(const Vec3f &lhs, const float rhs); friend Vec3f operator-(const Vec3f &src); // usefull constants static const Vec3f ZEROVECTOR; static const Vec3f UNIT_POS_X; static const Vec3f UNIT_POS_Y; static const Vec3f UNIT_POS_Z; static const Vec3f UNIT_NEG_X; static const Vec3f UNIT_NEG_Y; static const Vec3f UNIT_NEG_Z; float X, Y, Z; };

**0**

# Critique my 3D Vector Class

Started by Ravyne, Jun 03 2006 10:39 AM

12 replies to this topic

###
#1
GDNet+ - Reputation: **13486**

Posted 03 June 2006 - 10:39 AM

Hello, I'm looking to get some feedback on my vector class, which is part of a larger library of math, graphics, input and platform code. I'm confident of the math and my implimentation is well optimized so I'm not posting the function definitions. What I'd like to know is if I've missed anything, have too much, better choices for function types (external friend vs. static method vs. standard method) and other critiques centered more around design.
A few non-standard details of note:
1)The "mul" method and standard '*' operation represent compoment-wise multiplication of elements, that is, for two vectors X is multiplied by X and returned in X. This is repeated for Y and Z and is usefull for non-uniform scaling of the components.
2)The "div" method and standard '/' operation represents component-wise division of elements. It is equivilant to multiplication by a vector comprised of the inverse of each component of one of the multiplication factors.
3)The "rad" method represents the squared length of the vector. The name "rad" comes from the fact that this is the radicand in the vector length computation -- sqrt(X^2 + Y^2 + Z^2)
4)The "abs" method represents the length of the vector.
5)The "unitize" method turns the calling vector into a unit vector, while the "unit" method returns a new vector that is the unit vector of the calling vector.
One thing I've considered adding is the box-product. How useful is it for graphics and other game calculations? Are there any more useful calculations that I've missed?
I also have math.h included in the header file because it is needed for the inline implimentation of the abs (length) function. Aside from the fact that I should rather include cmath, is it a terrible practice to include the math header directly in my own header? If so, is it terrible enough to warrant making the length function non-inlined to avoid the issue alltogether?
[Edited by - Ravyne on June 6, 2006 4:10:56 PM]

throw table_exception("(ノ ゜Д゜)ノ ︵ ┻━┻");

###
#2
Members - Reputation: **2076**

Posted 03 June 2006 - 11:04 AM

I'm not going to dig it up now, but if you look through the last 10-15 pages of the 'game programming' forum, you'll find a similar thread; many of the common issues regarding vector and other math classes were discussed in that thread.

As you might guess, these sorts of discussions often generate more heat than light. There are countless issues involved in writing a math class, many of which elicit strong opinions, so you're unlikely to find concensus. You've also chosen to go against convention in a couple of areas, which may throw people off a bit.

There's only one thing I'm going to comment on, as it's a bit of a pet issue for me, and that is non-obvious operator overloading. If you're the only person who will ever use this class, that's one thing, but otherwise I'd recommend not using operator*() or operator/() for component-wise multiplication and division; it's not obvious or standard, and makes code that uses the class hard to read for others. Perhaps a modulate() function instead?

As for the box product, yes, it's used in a few common algorithms. I'd never heard it called the 'box product' until now though, so you might go with the more common name 'triple product'.

As you might guess, these sorts of discussions often generate more heat than light. There are countless issues involved in writing a math class, many of which elicit strong opinions, so you're unlikely to find concensus. You've also chosen to go against convention in a couple of areas, which may throw people off a bit.

There's only one thing I'm going to comment on, as it's a bit of a pet issue for me, and that is non-obvious operator overloading. If you're the only person who will ever use this class, that's one thing, but otherwise I'd recommend not using operator*() or operator/() for component-wise multiplication and division; it's not obvious or standard, and makes code that uses the class hard to read for others. Perhaps a modulate() function instead?

As for the box product, yes, it's used in a few common algorithms. I'd never heard it called the 'box product' until now though, so you might go with the more common name 'triple product'.

###
#4
Members - Reputation: **2076**

Posted 04 June 2006 - 04:51 AM

Quote:I can think of a few reasons, but in any case, do you have a particular alternative in mind? (Just curious...)

Original post by Matt Aufderheide

I dont have comments on the code, but I do wonder why the heck someone would write their own math classes, like vectors.. the only reason i can see is for learning puposes.

###
#5
Members - Reputation: **248**

Posted 04 June 2006 - 05:46 AM

Quote:

Original post by Matt Aufderheide

I dont have comments on the code, but I do wonder why the heck someone would write their own math classes, like vectors.. the only reason i can see is for learning puposes.

Vector classes are used so much with in graphics programming I find it hard to believe why people wouldn’t write their own custom classes. I myself like an entirely OO driver form of vectors, and I have many custom functions that aren’t in traditional vector classes that I find useful in the code I write such as nearest tangents, closest points on a line, projections etc.

###
#6
Members - Reputation: **855**

Posted 04 June 2006 - 06:21 AM

Quote:

Original post by jykQuote:I can think of a few reasons, but in any case, do you have a particular alternative in mind? (Just curious...)

Original post by Matt Aufderheide

I dont have comments on the code, but I do wonder why the heck someone would write their own math classes, like vectors.. the only reason i can see is for learning puposes.

He probably would recommend the DirectX math classes (assuming you're using DirectX). Otherwise I bet there are open source math libraries out there. Personally I agree with custom writing math classes and functions.

###
#7
Members - Reputation: **241**

Posted 04 June 2006 - 11:24 AM

float X,Y,Z

1. I would use custom typedefs for all datatypes

e.g.: typedef float vector_float

Why? once you probably want to switch to 64 bit platforms and suddenly you get confronted with the use of standard float everywhere. This is not an issue as long as you only work on one and the same platform though.

Imagine you switch to a 64bit platform and your binary files(levels,models ...) store 32 bit floats.

Now you wish you had used custom typedefs.

In the math library I usually use a seperate typedef to make use of the standard datatypes but for IO I use special typedefs to make sure I won t run into trouble later on

I dunne why you call your top namespace Fuel, it rather use a more generic name for the basecode (I assume this vector class shall be part of your growing basecode)

It would be interesting to see you implementation and not only the declarations

1. I would use custom typedefs for all datatypes

e.g.: typedef float vector_float

Why? once you probably want to switch to 64 bit platforms and suddenly you get confronted with the use of standard float everywhere. This is not an issue as long as you only work on one and the same platform though.

Imagine you switch to a 64bit platform and your binary files(levels,models ...) store 32 bit floats.

Now you wish you had used custom typedefs.

In the math library I usually use a seperate typedef to make use of the standard datatypes but for IO I use special typedefs to make sure I won t run into trouble later on

I dunne why you call your top namespace Fuel, it rather use a more generic name for the basecode (I assume this vector class shall be part of your growing basecode)

It would be interesting to see you implementation and not only the declarations

###
#8
Members - Reputation: **1372**

Posted 04 June 2006 - 04:45 PM

My comments and suggestions:

These functions should return a reference to *this, not void.

These functions should return a reference to *this, not void.

// self-modifying operatorsThe more common name for this function is "normalize". It might also return a reference to *this.

inline void operator =(const Vec3f &rhs);

inline void operator*=(const Vec3f &rhs);

inline void operator/=(const Vec3f &rhs);

inline void operator+=(const Vec3f &rhs);

inline void operator-=(const Vec3f &rhs);

inline void operator*=(const float rhs);

inline void operator/=(const float rhs);

inline void unitize();These functions don't need to be members of the class.

// three-operand methodsWhy not name these what they are: length_squared() and length()?

static void mul(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void div(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void add(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void sub(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void add(Vec3f &res, const Vec3f &lhs, const float rhs);

static void sub(Vec3f &res, const Vec3f &lhs, const float rhs);

static void cross(Vec3f &res, const Vec3f &lhs, const float rhs);

friend float rad(const Vec3f &src); // Length^2Since all your member data is public, there is no need for non-member functions to be friends.

friend float abs(const Vec3f &src); // Length

John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!

###
#9
Members - Reputation: **147**

Posted 05 June 2006 - 05:33 AM

Quote:

Original post by NickGravelyn

He probably would recommend the DirectX math classes (assuming you're using DirectX). Otherwise I bet there are open source math libraries out there. Personally I agree with custom writing math classes and functions.

The DirectX math library is a great place to start for basic and even some more advanced functionality. The only drawback to it, is that it's in a lib and the functions can't be inlined. Some of the basic functions are inlined...but some of the very frequently called functions like D3DXMatrixMultiply, D3DXVec3Transform (and all of the vector transform funcs in general), D3DXMatrixInverse, etc, are all implemented in the lib which prevents them from being inlined.

We ran a test once that timed something like 100,000 matrix multiplies -- first round using the simple standard c-style loop over the indicies and the multiply and add; second round using the D3DXMatrixMultiply func.

At first, I was astounded by the results because as you may or may not know, some of the D3DX math functions are SSE and SSE2 optimized -- anyway, the simple c-style home brew implementation blew the D3DX verion out of the water.

The consensus here was that it wasn't the actual implementation that was the problem, but rather the fact that the c-style home brew was able to be inlined by the compiler whereas the D3DX version was not. 100,000 function calls apparently add up significantly and begin to outweigh the benefits of having SSE or SSE2 optimized code.

Pretty interesting -- and food for thought.

Brian L.Video Game Programming

###
#10
GDNet+ - Reputation: **13486**

Posted 06 June 2006 - 10:03 AM

Quote:

Original post by JohnBolton

My comments and suggestions:

These functions should return a reference to *this, not void.// self-modifying operators

inline void operator =(const Vec3f &rhs);

inline void operator*=(const Vec3f &rhs);

inline void operator/=(const Vec3f &rhs);

inline void operator+=(const Vec3f &rhs);

inline void operator-=(const Vec3f &rhs);

inline void operator*=(const float rhs);

inline void operator/=(const float rhs);

This is actually a concious decision on my part for the sake of optimization. The returned reference is rarely used and, although technically 'correct' (or at least adhering to common practice,) promotes ugly code like a = b += c + d. So, to my mind, I've gained some optimization and prevented ugly code, and the fact that a direct reference to the variable in the next statement should be no less efficient seals the deal. Admittedly, it does go against the norm though.

Quote:

The more common name for this function is "normalize". It might also return a reference to *this.inline void unitize();

Agreed and changed. I think my initial theory behind "unitize" was to prevent confusion between it and a normal vector, which probably made sense to me at the time, but doesn't hold water anymore.

Quote:

These functions don't need to be members of the class.// three-operand methods

static void mul(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void div(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void add(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void sub(Vec3f &res, const Vec3f &lhs, const Vec3f &rhs);

static void add(Vec3f &res, const Vec3f &lhs, const float rhs);

static void sub(Vec3f &res, const Vec3f &lhs, const float rhs);

static void cross(Vec3f &res, const Vec3f &lhs, const float rhs);

They don't, its simply to help keep namespace polution to a minimum.

Quote:

Why not name these what they are: length_squared() and length()?friend float rad(const Vec3f &src); // Length^2

friend float abs(const Vec3f &src); // Length

Its just my personal preference that function names be kept to a minimum. abs() is a fairly common representation of the length, as it models after the absolute-value/vector magnitude operator relationship in Linear algebra (as I'm sure you're aware, just laying out my logic) rad is just something conveniantly short that made sense to me.

Quote:

Since all your member data is public, there is no need for non-member functions to be friends.

True enough, but I may someday change that, and for now it isn't hurting anything. It also expresses the close relationship between the functions and class, marking them as part of the official API and not something added on by a third party.

In fact, I've left the non-member opporators as friends, and changed the non-member methods to static methods (again, to avoid polluting the namespace.

throw table_exception("(ノ ゜Д゜)ノ ︵ ┻━┻");

###
#11
Members - Reputation: **1372**

Posted 06 June 2006 - 11:23 AM

Quote:

Original post by RavyneQuote:

Original post by JohnBolton

These functions should return a reference to *this, not void.inline void operator =(const Vec3f &rhs);

...

This is actually a concious decision on my part for the sake of optimization. The returned reference is rarely used and, although technically 'correct' (or at least adhering to common practice,) promotes ugly code like a = b += c + d. So, to my mind, I've gained some optimization and prevented ugly code, and the fact that a direct reference to the variable in the next statement should be no less efficient seals the deal. Admittedly, it does go against the norm though.

hmmm... Returning void instead of a reference to Vec3f will not result in meaningful optimization (if any at all), and you are not preventing ugly code, you are reducing the usability of your class. You don't like using the result of an assignment, but others might find it valuable. Code using the result of assignment is very common (something like this perhaps):

while ( Vec3f::rad( v = GetVector() ) > 0 )And since returning a reference to *this is the convention, your class would be considered broken by those that might depend on it.

{

// use v ...

}

Just my opinion, so disregard if you like.

John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!

###
#12
Moderators - Reputation: **11899**

Posted 06 June 2006 - 11:40 AM

Quote:What incredibly naive and ignorant logic. First, it isn't an optimization. If the returned reference is not used, it will never be returned by the function. The optimizer is more than capable of handling such trivial cases. Second, going against the norm should generally be considered a grievous offense, unless there is a compelling reason. Since you lack a compelling reason here, you need to revise your functions to behave the way they're supposed to.

Original post by Ravyne

This is actually a concious decision on my part for the sake of optimization. [...] Admittedly, it does go against the norm though.

Oh, and because most people tend to screw this up: Implement + in terms of +=, not the other way around. The correct way to write operator+ is:

`return Vec3f( *lhs ) += rhs;`