Jump to content
  • Advertisement
Sign in to follow this  
paulecoyote

[C++] Please help debugging this Vector implementation

This topic is 4833 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi there, I'm trying to make a *simple* vector template that I can use as a basis as 2 types of vectors, one with 3 floats in and one with 6, that will be exported from a dll for use with the openal stuff I am trying to develop. I want the vector to be able to be used as a return value from a function, to be used as a parameter, and to clean up it's array of floats when it gets disposed of. I also want it to transparently be able to be used in the openal functions that take a float* and expect an array of 3 or 6 floats. This implementation I'm using is apparently so nasty that it causes the machine to reset after handing it off to openal in the set listener functions, then asking it to play. [rolleyes] Anyway here's the template and the 3 float version. The 6 float version is essentially the same with a different constructor and ResetValues. Thanks for looking it over!
#pragma once

#include "SoundDirectorDll.h"
#include <exception>


namespace pde
{	
	template <class ValueType, unsigned int vectorWidth>
	class  VectorTemplate
	{
	public:
		//As this is the first data member, &instance should be the same address as values[3].  But with the overloaded operators, that should be neither here or there I guess!
		ValueType* value;

	
		VectorTemplate(void);		
		VectorTemplate(const VectorTemplate< ValueType, vectorWidth >& rhs);
		VectorTemplate(const ValueType rhs[]);		

		virtual ~VectorTemplate(void);

		virtual void ResetValues(void) {} ;		

		VectorTemplate< ValueType, vectorWidth >& operator = (const VectorTemplate< ValueType, vectorWidth >& rhs);
		VectorTemplate< ValueType, vectorWidth >& operator = (const ValueType rhs[]);

		const ValueType& operator[] ( int subscript) const;
		ValueType& operator[] ( int subscript);

		operator ValueType*();
		operator const ValueType*() const;	

	protected:
		virtual const char* GenerateRangeErrorString(void) const { return "Subscript out of range for Vector.";} ;
	};


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(void)
	{
		value = new ValueType[vectorWidth];
		ResetValues();
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(const VectorTemplate< ValueType, vectorWidth > & rhs)
	{
		value = new ValueType[vectorWidth];
		
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs.value;
		}
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::VectorTemplate(const ValueType rhs[])
	{
		value = new ValueType[vectorWidth];
		
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs;
		}
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::~VectorTemplate(void)
	{
		delete[] value;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >& VectorTemplate< ValueType, vectorWidth >::operator = (const VectorTemplate< ValueType, vectorWidth >& rhs)	
	{
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs.value;
		}

		return *this;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >& VectorTemplate< ValueType, vectorWidth >::operator = (const ValueType rhs[])
	{
		for (unsigned int i=0; i<vectorWidth; ++i)
		{
			value=rhs;
		}

		return *this;
	}


	template <class ValueType, unsigned int vectorWidth>
	const ValueType& VectorTemplate< ValueType, vectorWidth >::operator [] (int subscript) const
	{
		if ((subscript<0) || (subscript>=vectorWidth))
		{
			throw std::out_of_range(GenerateRangeErrorString());
		}

		return value[subscript];
	}


	template <class ValueType, unsigned int vectorWidth>
	ValueType& VectorTemplate< ValueType, vectorWidth >::operator [] (int subscript)
	{
		if ((subscript<0) || (subscript>=vectorWidth))
		{
			throw std::out_of_range(GenerateRangeErrorString());
		}

		return value[subscript];
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::operator ValueType*()
	{
		return value;
	}


	template <class ValueType, unsigned int vectorWidth>
	VectorTemplate< ValueType, vectorWidth >::operator const ValueType*() const
	{
		return value;
	}
}




#pragma once

#include "SoundDirectorDll.h"
#include <string>
//#include <Vector>
#include "VectorTemplate.h"

namespace pde
{

	class SOUNDDIRECTOR_API Vector3F : public VectorTemplate<float, 3>
	{
	public:
		Vector3F(void);
		Vector3F(const float x, const float y, const float z);
		Vector3F(const Vector3F& rhs);
		Vector3F(const float rhs[]);
	
		virtual ~Vector3F(void);

	protected:
		virtual void ResetValues(void);
	};
}



#include "..\include\vector3f.h"

#include <exception>

using namespace std;

namespace pde
{	
	Vector3F::Vector3F(void):VectorTemplate<float, 3>()
	{		
	}


	Vector3F::Vector3F(const float x, const float y, const float z):VectorTemplate<float, 3>()
	{		
		value[0]=x;
		value[1]=y;
		value[2]=z;
	}


	Vector3F::Vector3F(const Vector3F& rhs):VectorTemplate<float, 3>(rhs)
	{		
	}

	Vector3F::Vector3F(const float rhs[]):VectorTemplate<float, 3>(rhs)
	{
	}


	Vector3F::~Vector3F(void)
	{		
	}


	void Vector3F::ResetValues(void)
	{
		value[0]=value[1]=value[2]=0;
	}
}



[Edited by - paulecoyote on August 24, 2005 3:34:32 AM]

Share this post


Link to post
Share on other sites
Advertisement
I noticed that the vector's members are allocated on the heap. Might I suggest the following:


namespace pde
{
template <class ValueType, unsigned int vectorWidth>
class VectorTemplate
{
public:
ValueType value[vectorWidth];

...




You save 4 bytes per vector, but more importantly you save all those small memory allocations, which can chew through (not to mention fragment) memory at an alarming rate. It also makes it impossible to have a memory leak.

Share this post


Link to post
Share on other sites
foofightr thanks, ratings +++++

I may try that actually, I was using the pointer stuff because I found it easier to do the cast operators. To be honest the cast operators are the bit I'm most unsure of.

But with the way I'm doing things now, can anyone see why it might cause such an almighty crash that there isn't even a bluescreen, just a reset?

May be OpenAL does not copy the values across, but just copies the pointer and that's falling out of scope. I'll try that out tommorow.

Share this post


Link to post
Share on other sites
Alrighty, I gave my vector variables a longer lifetime (beyond that of using alListenerf to set the vector value, and of starting off the sound) - not changing my vector implementation at all and it works fine.

The vector implementation I'm using is not particularly optimal - anyone looking for more optimal solutions might try looking at http://www.gamedev.net/community/forums/topic.asp?topic_id=261920 and http://www.gamedev.net/community/forums/topic.asp?topic_id=230443 for inspiration... thanks again to James Trotter for pointing me towards those threads in a private message [smile].

As for the OpenAL alListenerf behaviour - I cannot be sure. It does indeed seem to copy a pointer across to the float array rather then the contents of the float array.

Share this post


Link to post
Share on other sites
seems like you got rid of the dynamic memory allocation that's otherwise a common source for errors when working with DLL, you should always contain allocation/deallocation on "right" side of the DLL boundry, if your process allocated it your process should deallocate it, if the DLL allocated it the DLL should destroy it.

Also a vector conceptually a very simple thing, I can't really think of more than one sensible implementation and I can see exactly no reason you'd ever want to use it polymorphicly so why the virtual functions?

Yet another thing, usually it's a quite bad idea to default vectors to zero since it's all to easy to botch up and cerate a large array of temporary vectors just to later assign values to them. Put that in a innerloop and you'll spend half the time writing zero's (actually I've seen a case where a whooping 67% of the runtime was spent zeroing out temporary vectors (or in that case vertices) just to a few lines later fill them with computed values). (If you wonder how it could rise above 50% it was because someone had concluded that using a huge temporary array to save reallocations for a std::vector was somehow a sane idea).

Share this post


Link to post
Share on other sites
Hey cheers DigitalDelusion

You got here before I could say that it was not fixed - I had just taken out the test line by mistake. Indeed this time I was treated to a bluescreen on 2 of my 3 screens. LOL. Got stuck trying to dump memory.

Anyhow this was the error

STOP UNEXPECTED_KERNEL_MODE_TRAP 0X0000007F (0x00000000, 0x00000000, 0x00000000, 0x00000000)

MS reckons that this usually means a hardware fault, I reckon that my implementation is so "special" that it is causing it
MSDN

SO I'll take everyone's constructive critism onboard and err code again.

The virtual functions were there so Reset could be different per derivitive, and the error message could be different per derivitive. It's only a work in progress and I was just test ideas out.

Allocation and deallocation should both happen on the one side of the dll, though I will look out for that.

Share this post


Link to post
Share on other sites
Easy way to check for memory handling errors, simply leak.
Try removing your delete [] statement and rerun it. If it doesn't bomb you know that somewhere you're double deleting.

About the error reporint etc might I advice having both checked and unchecked access operations? std::vector has the unchecked operatr[] and performs checking when you use at(). It's a nice convention and by some bizarre reasoning if you don't teammates usually have "creative" ways of skipping the access check anyhow (since they *never* step out of bounds anyhow...).

Share this post


Link to post
Share on other sites
Thats a good idea actually... just letting it leak. Though I probably shouldn't do it that way anyway. I'm getting a feeling it would be easier to just start from a clean slate.

I think my spec for what I want in a vector is pretty simple
  • Transparently use as float* / Vector3F
  • Be able to be used as a return value and parameter
  • copyable
  • clean up after itself
  • exportable and usable across dll as part of the interface (ie used as parameter and return type in abstract exported class)


So I'm going to take on board everything thats been said, armed with a few threads and example implementations and try *again* to make something that doesn't bluescreen.

Share this post


Link to post
Share on other sites
def was the vector classes screwing things up, recoded with all your advice and works fine so far.
When I've throughly tested it and commented it I'll post it up if anyone is interested for future reference.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!