Public Group

# Help me with my application?

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

## Recommended Posts

Hello. I'm finally completed with my first application in Direct X. However I get the feeling that it's really messy and inefficent. Would anybody like to help me out? I've got the actual application and source code posted at this location. http://www.guilddnr.net/dunescape/d3d_app.rar (this code was compiled with Visual C++ .NET 2003 edition, with the Direct X 9.0c SDK) Additionally, here is the code plainly:
//theJ89's Direct X Test and window creation program
#include <windows.h>	//Includes windows,
#include <d3d9.h>		//D3D,
#include <time.h>		//D3D,
#include <d3dx9.h>		//D3D,
#include <dxerr9.h>		//and the D3D error handler?

//Removes rarely used things in windows.
#define WIN32_LEAN_AND_MEAN
//Describes the format of the vertex...
//Even though I make my own custom vertex structure I think it has to conform to these specifications.
#define POINT_FLAGS (D3DFVF_XYZRHW | D3DFVF_TEX1)

//Width and height of the menu along with the name of my program.
#define APP_NAME "theJ89's DirectX Test Program"
#define SCREEN_WIDTH 1024
#define SCREEN_HEIGHT 768
#define MAX_TEXTURES 16

//Window and instance pointers
HWND window;
HINSTANCE hInst;

//Program stops running when bRun is set to false
bool bRun=true;

//Alert displays a message box with the name of the application, an "OK" button, an exclamation mark and the intended message.
{
ShowCursor(true);
if(window!=NULL){
MessageBox(window,lpMessage,APP_NAME,MB_OK | MB_ICONEXCLAMATION);
} else {
MessageBox(NULL,lpMessage,APP_NAME,MB_OK | MB_ICONEXCLAMATION);
}
ShowCursor(false);
}

//Confirm is like alert, but instead it has a question icon, yes/no buttons, and it returns the user's choice - true for yes and false for no.
bool Confirm(LPCTSTR lpMessage)
{
ShowCursor(true);
int returnedValue=0;
if(window!=NULL){
returnedValue=MessageBox(window,lpMessage,APP_NAME,MB_YESNO | MB_ICONQUESTION);
} else {
returnedValue=MessageBox(NULL,lpMessage,APP_NAME,MB_YESNO | MB_ICONQUESTION);
}
ShowCursor(false);
if(returnedValue==IDYES){
return true;
} else {
return false;
}

}

//This is my vertex format, according to the second DX3D tutorial. It's got X, Y, and Z along with a reciprocol homogeneous w component and the color.
struct Vertex2D
{
float x,y,z,rhw,u,v;
D3DCOLOR color;
};
//This is my array of vertices, arranged like so:
/*
1----2
|  / |
| /  |
0----3
*/

class Image
{
public:
Image();
Image(float x, float y, float width, float height, IDirect3DTexture9* cTex);
~Image();
void Move(float x, float y);
void UpdateVerts();

bool show;
float x,y,width,height,rotation;
short unsigned int index;
D3DCOLOR color;
IDirect3DTexture9* cSource;
};

Image::Image()
{
//Init index to -1; find open slot.
int oIndex=-1;
{
if(cImages==NULL)
{
oIndex=i;
}
}
if(oIndex<0)
{
Alert("Attempted to create image but no open slots.");
return;
}
else
{
index=oIndex;
}
cImages[index]=this;

this->show=false;
this->cSource=NULL;
this->x=0;
this->y=0;
this->width=0;
this->height=0;
this->rotation=0.0f;

//Set U, V coordinates and RHW, Z, and Color to defaults
vertices[(index*4)].u=0.0f;
vertices[(index*4)].v=1.0f;
vertices[(index*4)+1].u=0.0f;
vertices[(index*4)+1].v=0.0f;
vertices[(index*4)+2].u=1.0f;
vertices[(index*4)+2].v=0.0f;
vertices[(index*4)+3].u=1.0f;
vertices[(index*4)+3].v=1.0f;
for(int i=0; i<4; i++)
{
vertices[(index*4)+i].rhw=1.0f;
vertices[(index*4)+i].z=1.0f;
vertices[(index*4)+i].color=0xFFFFFFFF;
}
//Set X/Y positions
UpdateVerts();
}

Image::Image(float x, float y, float width, float height, IDirect3DTexture9* cTex)
{
//Init index to -1; find open slot.
int oIndex=-1;
{
if(cImages==NULL)
{
oIndex=i;
}
}
if(oIndex<0)
{
Alert("Attempted to create image but no open slots.");
return;
}
else
{
index=oIndex;
}
cImages[index]=this;

this->show=true;
this->cSource=cTex;
this->x=x;
this->y=y;
this->width=width;
this->height=height;
this->rotation=0.0f;

//Set U, V coordinates and RHW, Z, and Color to defaults
vertices[(index*4)].u=0.0f;
vertices[(index*4)].v=1.0f;
vertices[(index*4)+1].u=0.0f;
vertices[(index*4)+1].v=0.0f;
vertices[(index*4)+2].u=1.0f;
vertices[(index*4)+2].v=0.0f;
vertices[(index*4)+3].u=1.0f;
vertices[(index*4)+3].v=1.0f;
for(int i=0; i<4; i++)
{
vertices[(index*4)+i].rhw=1.0f;
vertices[(index*4)+i].z=1.0f;
vertices[(index*4)+i].color=0xFFFFFFFF;
}
//Set X/Y positions
UpdateVerts();
}

Image::~Image()
{
cImages[index]=NULL;
}

void Image::Move(float x, float y)
{
this->x=x;
this->y=y;
UpdateVerts();
}
void Image::UpdateVerts()
{
vertices[(index*4)].x=x;
vertices[(index*4)].y=y+height;
vertices[(index*4)+1].x=x;
vertices[(index*4)+1].y=y;
vertices[(index*4)+2].x=x+width;
vertices[(index*4)+2].y=y;
vertices[(index*4)+3].x=x+width;
vertices[(index*4)+3].y=y+height;
}

//DXGraphics is my singleton class to handle the Direct 3D interface, the device, the vertex buffer, rendering, and manage creation and cleanup.
//Currently I have the Render function set to draw a triangle fan that forms a rect as you can tell by the vertices.
class DXGraphics
{
public:
IDirect3DTexture9* d3dTextures[MAX_TEXTURES];

//Constructor, makes sure all pointers are initilized to NULL
DXGraphics()
{
d3d=NULL;
d3dDevice=NULL;
d3dVertexBuffer=NULL;
for(int i=0; i<MAX_TEXTURES; i++)
{
d3dTextures=NULL;
}
}
//Destructor, releases the vertex buffers, device, and d3d interface
~DXGraphics()
{
for(int i=0; i<MAX_TEXTURES; i++)
{
if(d3dTextures!=NULL)
d3dTextures->Release();
}
if(d3dVertexBuffer!=NULL)
d3dVertexBuffer->Release();
if(d3dDevice!=NULL)
d3dDevice->Release();
if(d3d!=NULL)
d3d->Release();
}
//Initilizes the interface and allows me to create the device...
HRESULT createD3D()
{
if(NULL==(d3d=Direct3DCreate9(D3D_SDK_VERSION)))
{
return E_FAIL;
}
return S_OK;
}
//Creates the device used to handle... um, pretty much everything.
HRESULT createDevice()
{
D3DPRESENT_PARAMETERS d3dPresent;
ZeroMemory(&d3dPresent,sizeof(d3dPresent));
d3dPresent.Windowed=TRUE;
d3dPresent.BackBufferFormat=D3DFMT_UNKNOWN;

if( FAILED( d3d->CreateDevice(D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, window, D3DCREATE_HARDWARE_VERTEXPROCESSING, &d3dPresent, &d3dDevice)))
{
return E_FAIL;
}
d3dDevice->SetRenderState(D3DRS_ALPHABLENDENABLE,TRUE);
d3dDevice->SetRenderState(D3DRS_SRCBLEND,D3DBLEND_SRCALPHA);
d3dDevice->SetRenderState(D3DRS_DESTBLEND,D3DBLEND_INVSRCALPHA);
d3dDevice->SetFVF(POINT_FLAGS);
return S_OK;
}
{
int iEmpty=-1;
char message[255];
char cBuffer[33];
ZeroMemory(&message,sizeof(message));
for(int i=0; i<MAX_TEXTURES&&iEmpty<0; i++)
{
if(d3dTextures==NULL)
iEmpty=i;
}
if(iEmpty<0)
{
strcat(message, pszImgPath);
strcat(message, " - No available slots.");
return E_FAIL;
}

if(FAILED(D3DXCreateTextureFromFile(d3dDevice, pszImgPath, &d3dTextures[iEmpty]))){
strcat(message, "Attempting to load texture ");
strcat(message, pszImgPath);
strcat(message, " has failed.");
return E_FAIL;
}
strcat(message, pszImgPath);
itoa(iEmpty,cBuffer,10);
strcat(message, "(slot ");
strcat(message, cBuffer);
strcat(message, ")");
strcat(message, " has succeeded!");
return S_OK;
}
//Creates a vertex buffer for my polygons.
HRESULT createVertexBuffer()
{
if( FAILED( d3dDevice->CreateVertexBuffer(MAX_QUADS*4*sizeof(Vertex2D),0, POINT_FLAGS, D3DPOOL_DEFAULT, &d3dVertexBuffer, NULL ) ) )
{
return E_FAIL;
}
return S_OK;
}
//Copies the vertexes from the array in physical memory to the buffer on video memory, if I understood that correctly.
HRESULT fillVertexBuffer()
{
VOID* pVertices;
if(FAILED(d3dVertexBuffer->Lock(0,sizeof(vertices),(void**)&pVertices,0)))
{
Alert("Attempting to fill the vertex buffer has failed!");
return E_FAIL;
}
memcpy(pVertices,vertices,sizeof(vertices));
d3dVertexBuffer->Unlock();
//Alert("The vertex buffer has been filled!");
return S_OK;
}
//Clears the back buffer to black, starts drawing the scene,
//sets the stream to the buffer, sets the vertex format (I don't think
//it should be here but whatever works), and lastly draws the
//primative - a triangle fan comprised of the four vertices.
//From there it just ends the scene and displays it.
void Render()
{
if(d3dDevice!=NULL)
{
d3dDevice->Clear(0,NULL,D3DCLEAR_TARGET,D3DCOLOR_XRGB(0,0,0),1.0f,0);
if(SUCCEEDED(d3dDevice->BeginScene()))
{
d3dDevice->SetStreamSource(0,d3dVertexBuffer,0,sizeof(Vertex2D));
{
if(cImages!=NULL&&cImages->show)
{
d3dDevice->SetTexture(0,cImages->cSource);
d3dDevice->SetTextureStageState(0,D3DTSS_COLOROP,D3DTOP_SELECTARG1);
d3dDevice->SetTextureStageState(0,D3DTSS_COLORARG1,D3DTA_TEXTURE);
d3dDevice->SetTextureStageState(0,D3DTSS_COLORARG2,D3DTA_DIFFUSE);
d3dDevice->SetTextureStageState(0,D3DTSS_ALPHAOP,D3DTOP_SELECTARG1);
d3dDevice->SetTextureStageState(0,D3DTSS_ALPHAARG1,D3DTA_TEXTURE);
d3dDevice->SetTextureStageState(0,D3DTSS_ALPHAARG2,D3DTA_DIFFUSE);
d3dDevice->DrawPrimitive(D3DPT_TRIANGLEFAN,i*4,2);
}
}
d3dDevice->EndScene();
}
d3dDevice->Present(NULL,NULL,NULL,NULL);
}
}
private:
IDirect3D9* d3d;
IDirect3DDevice9* d3dDevice;
IDirect3DVertexBuffer9* d3dVertexBuffer;
};

//Creates the singleton object that handles Direct3D
DXGraphics dx3d;

Image* pMyImage=NULL;
//Message Handler for windows
LRESULT CALLBACK WindowProcedure(HWND hWnd, UINT uMessage, WPARAM wParam, LPARAM lParam)
{
POINTS coords;
switch(uMessage)
{
case WM_DESTROY:
bRun=false;
break;
case WM_MOUSEMOVE:
coords = MAKEPOINTS (lParam);
if(pMyImage!=NULL) {
pMyImage->Move(coords.x-16,coords.y-16);
}
break;
case WM_LBUTTONDOWN:
bRun=false;
break;
/*
case WM_PAINT:
dx3d.Render();
ValidateRect( hWnd, NULL );
break;
*/
default:
break;
}
return DefWindowProc(hWnd,uMessage,wParam,lParam);
}
//I made a function to create a window because I like having everything for this in one function.
int MakeWindow(HINSTANCE hInstance)
{
WNDCLASSEX wc;
wc.cbSize = sizeof(WNDCLASSEX);
wc.style = CS_HREDRAW | CS_VREDRAW;
wc.lpfnWndProc = WindowProcedure;
wc.cbClsExtra = 0;
wc.cbWndExtra = 0;
wc.hInstance = hInstance;
wc.hIcon = NULL;
wc.hCursor = NULL;
wc.hbrBackground = (HBRUSH) GetStockObject (BLACK_BRUSH);
wc.lpszClassName = "DXTest";
wc.hIconSm = NULL;

if(!RegisterClassEx(&wc))
return false;
window=CreateWindowEx(NULL,"DXTest",APP_NAME,WS_CAPTION | WS_VISIBLE,0,0,SCREEN_WIDTH,SCREEN_HEIGHT,NULL,NULL,hInstance,NULL);

if(!window)
return false;

return true;
}
//Main windows function. Message loop, initilizes Direct X and renders the scene.
int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCommandLine, int nCommandShow)
{
MSG msg;
hInst=hInstance;

int iTime=time(NULL);
int framecount=0;
float fps=1;

ZeroMemory(&vertices,sizeof(vertices));
{
cImages=NULL;
}

MakeWindow(hInst);
ShowCursor(false);
if(FAILED(dx3d.createD3D()))
return 0;
if(FAILED(dx3d.createDevice()))
return 0;
if(FAILED(dx3d.createVertexBuffer()))
return 0;
return 0;
return 0;

Image MyImages[100]={
Image(0,0,32,32,dx3d.d3dTextures[0]),
Image(32,0,32,32,dx3d.d3dTextures[0]),
Image(64,0,32,32,dx3d.d3dTextures[0]),
Image(96,0,32,32,dx3d.d3dTextures[0]),
Image(128,0,32,32,dx3d.d3dTextures[0]),
Image(160,0,32,32,dx3d.d3dTextures[0]),
Image(192,0,32,32,dx3d.d3dTextures[0]),
Image(224,0,32,32,dx3d.d3dTextures[0]),
Image(256,0,32,32,dx3d.d3dTextures[0]),
Image(288,0,32,32,dx3d.d3dTextures[0]),

Image(0,32,32,32,dx3d.d3dTextures[0]),
Image(32,32,32,32,dx3d.d3dTextures[0]),
Image(64,32,32,32,dx3d.d3dTextures[0]),
Image(96,32,32,32,dx3d.d3dTextures[0]),
Image(128,32,32,32,dx3d.d3dTextures[0]),
Image(160,32,32,32,dx3d.d3dTextures[0]),
Image(192,32,32,32,dx3d.d3dTextures[0]),
Image(224,32,32,32,dx3d.d3dTextures[0]),
Image(256,32,32,32,dx3d.d3dTextures[0]),
Image(288,32,32,32,dx3d.d3dTextures[0]),

Image(0,64,32,32,dx3d.d3dTextures[0]),
Image(32,64,32,32,dx3d.d3dTextures[0]),
Image(64,64,32,32,dx3d.d3dTextures[0]),
Image(96,64,32,32,dx3d.d3dTextures[0]),
Image(128,64,32,32,dx3d.d3dTextures[0]),
Image(160,64,32,32,dx3d.d3dTextures[0]),
Image(192,64,32,32,dx3d.d3dTextures[0]),
Image(224,64,32,32,dx3d.d3dTextures[0]),
Image(256,64,32,32,dx3d.d3dTextures[0]),
Image(288,64,32,32,dx3d.d3dTextures[0]),

Image(0,96,32,32,dx3d.d3dTextures[0]),
Image(32,96,32,32,dx3d.d3dTextures[0]),
Image(64,96,32,32,dx3d.d3dTextures[0]),
Image(96,96,32,32,dx3d.d3dTextures[0]),
Image(128,96,32,32,dx3d.d3dTextures[0]),
Image(160,96,32,32,dx3d.d3dTextures[0]),
Image(192,96,32,32,dx3d.d3dTextures[0]),
Image(224,96,32,32,dx3d.d3dTextures[0]),
Image(256,96,32,32,dx3d.d3dTextures[0]),
Image(288,96,32,32,dx3d.d3dTextures[0]),

Image(0,128,32,32,dx3d.d3dTextures[0]),
Image(32,128,32,32,dx3d.d3dTextures[0]),
Image(64,128,32,32,dx3d.d3dTextures[0]),
Image(96,128,32,32,dx3d.d3dTextures[0]),
Image(128,128,32,32,dx3d.d3dTextures[0]),
Image(160,128,32,32,dx3d.d3dTextures[0]),
Image(192,128,32,32,dx3d.d3dTextures[0]),
Image(224,128,32,32,dx3d.d3dTextures[0]),
Image(256,128,32,32,dx3d.d3dTextures[0]),
Image(288,128,32,32,dx3d.d3dTextures[0]),

Image(0,160,32,32,dx3d.d3dTextures[0]),
Image(32,160,32,32,dx3d.d3dTextures[0]),
Image(64,160,32,32,dx3d.d3dTextures[0]),
Image(96,160,32,32,dx3d.d3dTextures[0]),
Image(128,160,32,32,dx3d.d3dTextures[0]),
Image(160,160,32,32,dx3d.d3dTextures[0]),
Image(192,160,32,32,dx3d.d3dTextures[0]),
Image(224,160,32,32,dx3d.d3dTextures[0]),
Image(256,160,32,32,dx3d.d3dTextures[0]),
Image(288,160,32,32,dx3d.d3dTextures[0]),

Image(0,192,32,32,dx3d.d3dTextures[0]),
Image(32,192,32,32,dx3d.d3dTextures[0]),
Image(64,192,32,32,dx3d.d3dTextures[0]),
Image(96,192,32,32,dx3d.d3dTextures[0]),
Image(128,192,32,32,dx3d.d3dTextures[0]),
Image(160,192,32,32,dx3d.d3dTextures[0]),
Image(192,192,32,32,dx3d.d3dTextures[0]),
Image(224,192,32,32,dx3d.d3dTextures[0]),
Image(256,192,32,32,dx3d.d3dTextures[0]),
Image(288,192,32,32,dx3d.d3dTextures[0]),

Image(0,224,32,32,dx3d.d3dTextures[0]),
Image(32,224,32,32,dx3d.d3dTextures[0]),
Image(64,224,32,32,dx3d.d3dTextures[0]),
Image(96,224,32,32,dx3d.d3dTextures[0]),
Image(128,224,32,32,dx3d.d3dTextures[0]),
Image(160,224,32,32,dx3d.d3dTextures[0]),
Image(192,224,32,32,dx3d.d3dTextures[0]),
Image(224,224,32,32,dx3d.d3dTextures[0]),
Image(256,224,32,32,dx3d.d3dTextures[0]),
Image(288,224,32,32,dx3d.d3dTextures[0]),

Image(0,256,32,32,dx3d.d3dTextures[0]),
Image(32,256,32,32,dx3d.d3dTextures[0]),
Image(64,256,32,32,dx3d.d3dTextures[0]),
Image(96,256,32,32,dx3d.d3dTextures[0]),
Image(128,256,32,32,dx3d.d3dTextures[0]),
Image(160,256,32,32,dx3d.d3dTextures[0]),
Image(192,256,32,32,dx3d.d3dTextures[0]),
Image(224,256,32,32,dx3d.d3dTextures[0]),
Image(256,256,32,32,dx3d.d3dTextures[0]),
Image(288,256,32,32,dx3d.d3dTextures[0]),

Image(0,288,32,32,dx3d.d3dTextures[0]),
Image(32,288,32,32,dx3d.d3dTextures[0]),
Image(64,288,32,32,dx3d.d3dTextures[0]),
Image(96,288,32,32,dx3d.d3dTextures[0]),
Image(128,288,32,32,dx3d.d3dTextures[0]),
Image(160,288,32,32,dx3d.d3dTextures[0]),
Image(192,288,32,32,dx3d.d3dTextures[0]),
Image(224,288,32,32,dx3d.d3dTextures[0]),
Image(256,288,32,32,dx3d.d3dTextures[0]),
Image(288,288,32,32,dx3d.d3dTextures[0])
};

Image Cursor=Image(288,288,32,32,dx3d.d3dTextures[1]);
pMyImage=&Cursor;
for(int i=0; i<100; i++)
{
MyImages;
}

while (bRun)
{
framecount++;
if(time(NULL)>iTime) {
fps=framecount;
framecount=0;
}
//Exactly what is the difference between PeekMessage and GetMessage?
if (PeekMessage(&msg,NULL,0,0,PM_REMOVE))
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
//The vertex buffer is filled each frame because I used to have something that moved the vertexes one pixel each frame, and needed to refresh the buffer.
dx3d.fillVertexBuffer();
//Last step, render the scene with the singleton's functions.
dx3d.Render();
}

return 0;
}


I've created two classes to handle Direct 3D, for the most part - DXGraphics and Image - and of course a custom struct for my vertex which contains x,y,z,rhw,u,v, and a D3DCOLOR. As you can tell by the RHW parameter, I am making a 2D application with Direct 3D. I have defined the max number of quads (256 last I checked) and the max number of textures (16). These limits are set when the application is compiled. The size of the vertex buffer, and the size of my global vertex array is equal to MAX_QUADS*4. My class, DXGraphics, contains the array of textures (however it is not a static variable, instead it is simply an array of IDirect3DTexture9 the size of MAX_TEXTURES (16)). Only one object is of the DXGraphics class, and I call it dx3d. Whenever my DXGraphics object constructs, it sets all of it's interface pointers to NULL. Likewise it releases all of the interfaces not equal to NULL whenever it's destructed. It contains a D3D interface pointer, Device interface pointer, Vertex Buffer pointer, and of course 16 texture interface pointers for loaded textures (however at the time only two are used). I have functions created for initilizing each of these interfaces. Additionally in DXGraphics there are also functions for filling the vertex buffer and rendering. My image class, Image, is used to handle and display quads. Image contains the float variables X, Y, Width, Height, and Rotation. Additionally there is the integer value index, a boolean value show which determines if the Image is drawn, and a D3DCOLOR (also integer I believe but whatever) variable Color. The image class contains functions for constructing and destructing the Image class, Moving the image onscreen, and updating the vertices. Every Image when constructed is automatically placed in the first available slot in a global array of Image pointers. Likewise when the Image is destructed the pointer is set to NULL. The vertices associated with the Image are determined by the Image's index in this array - so, if it's index was 5, then the vertices associated with it would be 20,21,22, and 23, because 5*4=20. This keeps track of the Images so that the renderer can know what vertices in the array to draw. So far this setup has worked wonderfully for me, but I am very unexperienced and this is only a simple setup. Could you possibly give me some pointers? [Edited by - theJ89 on September 26, 2006 4:32:07 PM]

##### Share on other sites
I've updated the first post to be more organized and detailed.

##### Share on other sites
You will probrably get alot more responses if you post the code in with your posts instead of getting people to download it. Just paste it into [ source ] and [ / source] blocks in your post ( loose the spaces though ).

From your description, is there a reason you are using fixed sized arrays instead of Vectors? If not, without having read your code that would be my first suggestion. I will have more if you post the code in [smile]

##### Share on other sites
Quote:
 Original post by SerapthYou will probrably get alot more responses if you post the code in with your posts instead of getting people to download it. Just paste it into [ source ] and [ / source] blocks in your post ( loose the spaces though ).From your description, is there a reason you are using fixed sized arrays instead of Vectors? If not, without having read your code that would be my first suggestion. I will have more if you post the code in [smile]

Thank you for the reply! I'll post the code in source tags now.

I have been told Vectors are slow for fast access purposes, so I have been using arrays.

##### Share on other sites
Quote:
 Original post by theJ89I have been told Vectors are slow for fast access purposes, so I have been using arrays.

That is for the most part, untrue.

You may want to check this thread:
http://www.gamedev.net/community/forums/topic.asp?topic_id=268669

From my own experience, vectors can be slightly slower in debug builds then arrays, build then... its debug. In 99.9% of cases, who cares. In release builds, Vectors and arrays perform almost identically.

The lose in flexability generally isnt worth the performance trade off, even if one exists. Not to mention, if you have a vector of 2 objects, vs a static array, pre-allocated to 16, you have major waste, and probrably slower performance.

##### Share on other sites
Keep in mind, alot of what im about to say is nitpicking. Just the little stuff you might not see or care about. First of, you really should consider using STL classes instead of fixed sized containers, you would make your code a whole lot easier.

That said, for the most part it all looks good. This is going to sound like an odd compliment, but your commenting style is very good. You actually write comments that might help you, as opposed to doing it "because your supposed to" or *shudder* not doing it at all.

#defines are nasty evil. Switch them to const strings. This makes a night and day difference when debugging, and for other people working with your code.

for(int i=0; i<MAX_QUADS&&oIndex<0; i++)	{		if(cImages==NULL)		{			oIndex=i;		}	}

Is a bug waiting to happen. In debug mode, the compiler zeros all memory, however in release it doesnt. If you dont implicity ZeroMemory() or memset() to 0 cImages array, this code could lead to a hard to find bug down the road. ( Another reason to use vectors [smile] )

Also, this is a matter of opinion, but move oIndex out of the for loop and replace it with a break statement. Its much easier to read and maintain:
for(int i=0; i<MAX_QUADS; i++)	{		if(cImages==NULL)		{			oIndex=i;			break;		}	}

I havent gotten to where cImages[index] was called ( Aka, the call to new Image() ), but if you newed the Image() this is a memory leak.

Image::~Image()
{
cImages[index]=NULL;
}

EDIT::: NOPE, not a memory leak, you statically declared every Image. Still leaving this comment in as an object lesson to others that might read this, or for yourself if you decide to go more dynamic in the future.

Potential gotcha bug or exploit:
strcat(message, pszImgPath);
strcat(message, " - No available slots.");

If pszImgPath ends up being really long, you can have a buffer overflow here. Probrably not a big deal, but still worth checking the size of pszImgPath before copying to an memory buffer with a fixed sized limit ( 255 ). Youu repeat this behavour a few times.

Call me Mr Anti compound logic in an if statement guy, but I would break this into two tests. Keep in mind, the compiler compiles it down to the same code in the end:

if(cImages!=NULL&&cImages->show)

Finally:

Image MyImages[100]={
Image(0,0,32,32,dx3d.d3dTextures[0]),
Image(32,0,32,32,dx3d.d3dTextures[0]),
Image(64,0,32,32,dx3d.d3dTextures[0]),
Image(96,0,32,32,dx3d.d3dTextures[0]),
<snip><snip>

My god man, put that in a loop! ;) You could drop about 70+ lines of code easily there. Also, ill use this time once again to recommend vectors :)

You gotta admit, this is a bit simpler:
	for(int y = 0; y < 10; y ++)	{		for(int x = 0; x< 10; x++)		{		Image(x*32,y*32,32,32,dx3d.d3dTextures[0])					}	}

Btw... if the results turn out reverse, that was done in my head :) just flip x and y :)

[Edited by - Serapth on September 26, 2006 5:09:08 PM]

##### Share on other sites
Quote:
 Original post by SerapthKeep in mind, alot of what im about to say is nitpicking. Just the little stuff you might not see or care about. First of, you really should consider using STL classes instead of fixed sized containers, you would make your code a whole lot easier.

Thanks for the reply. I am considering using Linked lists or vectors, which ever is faster.

Quote:
 Original post by SerapthThat said, for the most part it all looks good. This is going to sound like an odd compliment, but your commenting style is very good. You actually write comments that might help you, as opposed to doing it "because your supposed to" or *shudder* not doing it at all.

Thank you, when I was working on a game project written in javascript (it was a scripted game created with HTML) I made sure that I commented each function specifically so that if someone else were to read it, it would help them out. I'm kind of being slack on this because I'm not taking it as seriously as I should be.

Quote:
 Original post by Serapth#defines are nasty evil. Switch them to const strings. This makes a night and day difference when debugging, and for other people working with your code.

I've been wondering why this is. From what I can tell #define just replaces all instances of the keyword with the given code when it's building the source files. I can see how this would considerably increase the size of the file if, say for example you defined a string and then used the keyword all over the file, thus creating the same string in several places in the file which would be un-necessary. From what I understand consts are just read-only memory. The reason I was using defines is that I didn't want to take up unnecessary memory for things that were not called very often.

Quote:
 Original post by Serapth*** Source Snippet Removed ***Is a bug waiting to happen. In debug mode, the compiler zeros all memory, however in release it doesnt. If you dont implicity ZeroMemory() or memset() to 0 cImages array, this code could lead to a hard to find bug down the road. ( Another reason to use vectors [smile] )Also, this is a matter of opinion, but move oIndex out of the for loop and replace it with a break statement. Its much easier to read and maintain:*** Source Snippet Removed ***I havent gotten to where cImages[index] was called ( Aka, the call to new Image() ), but if you newed the Image() this is a memory leak.Image::~Image(){ cImages[index]=NULL;}EDIT::: NOPE, not a memory leak, you statically declared every Image. Still leaving this comment in as an object lesson to others that might read this, or for yourself if you decide to go more dynamic in the future.

Actually, cImages is not used for dynamically allocating images in. It's used for keeping track of images in the program. Now that you mention it, I should use break. I don't use break very often. Plus, that's one less condition check I need to do in that loop.

Quote:
 Original post by SerapthPotential gotcha bug or exploit: strcat(message, "Cannot load texture "); strcat(message, pszImgPath); strcat(message, " - No available slots."); Alert(message);If pszImgPath ends up being really long, you can have a buffer overflow here. Probrably not a big deal, but still worth checking the size of pszImgPath before copying to an memory buffer with a fixed sized limit ( 255 ). Youu repeat this behavour a few times.

Ahhh, yeah, I noticed this here! I couldn't come up with an acceptable solution so I just left it that way. With other languages I don't have to worry about buffer sizes so I don't really know a solution to it offhand.

Quote:
 Original post by SerapthCall me Mr Anti compound logic in an if statement guy, but I would break this into two tests. Keep in mind, the compiler compiles it down to the same code in the end:if(cImages!=NULL&&cImages->show)

I figured it was just a matter of style. According to a book on C++ I read if the first condition is false none of the other conditions are even considered - but, the way you mentioned is usually the way I do it.

Quote:
 Original post by SerapthFinally: Image MyImages[100]={ Image(0,0,32,32,dx3d.d3dTextures[0]), Image(32,0,32,32,dx3d.d3dTextures[0]), Image(64,0,32,32,dx3d.d3dTextures[0]), Image(96,0,32,32,dx3d.d3dTextures[0]), My god man, put that in a loop! ;) You could drop about 70+ lines of code easily there. Also, ill use this time once again to recommend vectors :)

Oh god I know. I was hoping to use a default constructer and then manually move them in a loop but then C++ threw the whole "you can't a default constructor with arguments" crap at me. Oh well.

Quote:
 Original post by Serapthfor(int y = 0; y < 10; y ++) { for(int x = 0; x< 10; x++) { Image(x*32,y*32,32,32,dx3d.d3dTextures[0]) } }

Only problem I can see here is that "Image" isn't stored in a variable. It needs to be, the only thing that cImages does is point to existing images.

Thanks for all of the help, I'll be working on my code to see if I can improve it.

##### Share on other sites
You are worrying far to much about optomization. Write it correctly first, then make it fast.

For example:
I figured it was just a matter of style. According to a book on C++ I read if the first condition is false none of the other conditions are even considered - but, the way you mentioned is usually the way I do it.

Technically this is true. It will do only the first condition if there is a failure. Yet, broken into two if statements results in the exact same compiled code. If the outer if block fails, the inner if is never evaluated.

As to preventing buffer overflows, the easiest answer to is validate the parameter size before passing it into a method that takes a fixed size buffer. Instead of constructing your string in 3 parts, build it all in advance, then check the size of the resulting string before passing it to strcat. Microsoft just spent a ton of money and time doing this to all of the Windows source code, so obviously performance isnt a huge issue.

Finally, the reason why #defines are evil is one, they are easily buried in code, and two, they dont evaluate in the debugger. When debugging, the const string is a thousand times easier to comprehend and the performance tradeoff is all but none existant.

##### Share on other sites
Oh! Yes, one last thing if you will. I am having trouble using diffusion to color my quads. For some reason, they become a jumble of red and black lines. For the time being, D3DFVF_DIFFUSE has been removed from the FVF. Do you know a possible solution for this?

1. 1
2. 2
Rutin
21
3. 3
JoeJ
18
4. 4
5. 5
gaxio
12

• 14
• 40
• 23
• 13
• 13
• ### Forum Statistics

• Total Topics
631724
• Total Posts
3001897
×