Jump to content

  • Log In with Google      Sign In   
  • Create Account

Interested in a FREE copy of HTML5 game maker Construct 2?

We'll be giving away three Personal Edition licences in next Tuesday's GDNet Direct email newsletter!

Sign up from the right-hand sidebar on our homepage and read Tuesday's newsletter for details!


We're also offering banner ads on our site from just $5! 1. Details HERE. 2. GDNet+ Subscriptions HERE. 3. Ad upload HERE.


Critique my 3D Vector Class


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
12 replies to this topic

#1 Ravyne   GDNet+   -  Reputation: 7744

Like
0Likes
Like

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.
/******************************************************************************/
/*!
\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;
};


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]

Sponsor:

#2 scgames   Members   -  Reputation: 1977

Like
1Likes
Like

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'.

#3 Matt Aufderheide   Members   -  Reputation: 111

Like
0Likes
Like

Posted 04 June 2006 - 04:26 AM

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.

#4 scgames   Members   -  Reputation: 1977

Like
0Likes
Like

Posted 04 June 2006 - 04:51 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.
I can think of a few reasons, but in any case, do you have a particular alternative in mind? (Just curious...)

#5 skow   Members   -  Reputation: 248

Like
0Likes
Like

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 Nick Gravelyn   Members   -  Reputation: 851

Like
0Likes
Like

Posted 04 June 2006 - 06:21 AM

Quote:
Original post by jyk
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.
I can think of a few reasons, but in any case, do you have a particular alternative in mind? (Just curious...)


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 Basiror   Members   -  Reputation: 241

Like
0Likes
Like

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

#8 JohnBolton   Members   -  Reputation: 1372

Like
0Likes
Like

Posted 04 June 2006 - 04:45 PM

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);
The more common name for this function is "normalize". It might also return a reference to *this.
	inline void		unitize(); 
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);
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
Since all your member data is public, there is no need for non-member functions to be friends.

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

#9 Brian Lawson   Members   -  Reputation: 147

Like
0Likes
Like

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.

#10 Ravyne   GDNet+   -  Reputation: 7744

Like
0Likes
Like

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.



#11 JohnBolton   Members   -  Reputation: 1372

Like
0Likes
Like

Posted 06 June 2006 - 11:23 AM

Quote:
Original post by Ravyne
Quote:
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 )
{
// use v ...
}
And since returning a reference to *this is the convention, your class would be considered broken by those that might depend on it.

Just my opinion, so disregard if you like.


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

#12 Promit   Moderators   -  Reputation: 7336

Like
-1Likes
Like

Posted 06 June 2006 - 11:40 AM

Quote:
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.
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.

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;

#13 etothex   Members   -  Reputation: 728

Like
0Likes
Like

Posted 06 June 2006 - 12:53 PM

I would worry about the compiler (or the programmer) if a + or += operation wasn't inlined. And if it's inlined, the return reference is easily discarded.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS