Access Violation Of Memory Buffer WHY??

Hi in the tutorial I have been reading I have been Taught to write to the video memory like this but when I exit the application I keep getting an Access violation error Which Refers to the line
theBuffer[x+y*thePitch] = Colour;
why?

Here is my Realtime Message Loop

Code:

	while (1)
	{
		if (PeekMessage(&theMsg,NULL,0,0,PM_REMOVE))
		{
			if (theMsg.message == WM_QUIT)
				break;
			TranslateMessage(&theMsg);
			DispatchMessage(&theMsg);
		}
		// First Of All Clear Structure and the Lock
		memset(&theSurface,0,sizeof(theSurface));
		theSurface.dwSize = sizeof(theSurface);
		lpddsPrimary->Lock(NULL,&theSurface, DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR , NULL);

		// Draw at some random spot
		UCHAR * theBuffer = (UCHAR*)theSurface.lpSurface;
		int thePitch = (int) theSurface.lPitch;

		int x = rand()%640;
		int y = rand()%480;
		UCHAR Colour = rand()%256;
		theBuffer[x+y*thePitch] = Colour;
		lpddsPrimary->Unlock(NULL);
	}

>>but when I exit the application I keep getting an Access violation error Which Refers to the line

Can you show the part of you WndProc that handles the WM_CLOSE message, or whatever you're using to tell your program to quit? I'm suspecting that you're doing your cleanup in the message handler, and then doing PostQuitMessage() at the end of the cleanup... If you do that, then once the message is processed, the main loop will continue and access lpddsPrimary before calling PeekMessage() again at the next loop to check for a WM_QUIT message and break out.

If that's the problem, then I think a simple fix would be to put everything after the if(PeekMessage()) block into an else{} block.

Hope this helps! (and I'd still like to see your windows procedure)

No all my clean up is done after the while loop. So when the quit message is posted the while loop breaks and then the clean up code is after that. This means that my WndProc command is kept extremely simple.

Code:

	case WM_DESTROY:
		{
			// The the computer user wants to quit
			PostQuitMessage(0);
			return 0;
			break;
		}

Hmm....your buffer code looks fine. Need more code to diagnose the problem.

I agree with Bubba. At this point, I can only make a wild guess.. but maybe some message in the quit sequence causes the program to lose focus before WM_QUIT, or change resolution, or something like that and the surface gets lost, making the surface memory invalid. Again, I'm just tossing out wild possibilities, but maybe you can do some tests and make sure that's not the case anyways.

> lpddsPrimary->Lock(NULL,&theSurface, DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR , NULL);
Will this ALWAYS initialise theSurface, or are you just assuming it will and totally ignoring any status return?

No it won't always succeed but if it fails it won't do anything at all and you won't be able to write to the surface. It is failsafe even if you fail to check whether it succeeded using FAILED() or SUCEEDED().

In order to write to the back buffer you must specify that it can be locked. If not you either won't see anything on the display or you might get an access violation error.

Given the memset() and a failure, you end up with theSurface.lpSurface == NULL
Classic access violation territory.

Code:

memset(&theSurface,0,sizeof(theSurface));
		theSurface.dwSize = sizeof(theSurface);
		lpddsPrimary->Lock(NULL,&theSurface, DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR , NULL);

Yep, I knew that looked suspicious and almost posted that was the problem. Salem is correct, you are destroying the pointer to your surface therefore it is NULL when you attempt to write to it. Very bad thing.

>>you are destroying the pointer to your surface therefore it is NULL when you attempt to write to it.

Gee, I thought lpddsPrimary->Lock() was the line that fills in the pointer for you. theSurface shouldn't need to contain a valid pointer to the surface memory before calling Lock() should it? Besides, the problem only occurs on application exit; accessing a NULL pointer should cause a crash pretty much on startup.

You first need to get a pointer to the actual Surface interface.
Here is some sample code, maybe this will clear things up.

CBackBuffer.h

Code:

#ifndef BACKBUFFER
#define BACKBUFFER
#include <d3dx9.h>
#include "D3DError.h"
 
class CBackBuffer
{
protected:
	IDirect3DDevice9		*Device;
	IDirect3DSwapChain9	 *SwapChain;
	IDirect3DSurface9	 *Surface;
	D3DSURFACE_DESC		 BufferDesc;
	D3DLOCKED_RECT		 BLRect;
	void				 *Buffer;
	int					 MemPitch;
 
public:
	CBackBuffer(IDirect3DDevice9 *device);
 
	int Pitch(void) {return MemPitch;};
	int Width(void) {return BufferDesc.Width;};
	int Height(void) {return BufferDesc.Height;};
	void *Lock(void);
	HRESULT Unlock();
 
};
#endif

CBackBuffer.cpp

Code:

#include "CBackBuffer.h"
#include <stdio.h>
 
 
CBackBuffer::CBackBuffer(IDirect3DDevice9 *device)
{
Device=device;
 
Device->GetSwapChain(0,&SwapChain);
SwapChain->GetBackBuffer(D3DADAPTER_DEFAULT,D3DBACKBUFFER_TYPE_MONO,&Surface);
if (!Surface) ::MessageBox(0,"GetBackBuffer Failed",0,0);
 
if FAILED(Surface->GetDesc(&BufferDesc)) ::MessageBox(0,"GetDesc Failed",0,0);
}
 
void *CBackBuffer::Lock(void)
{
  D3DLOCKED_RECT lrect;
  HRESULT hr;
  hr=Surface->LockRect(&lrect,0,0);
  if FAILED(hr)
  { 
	
	char *text=D3DError::GetTextFromD3DError(hr);
	::MessageBox(0,text,0,0);
	return NULL;
  }
 
  
  MemPitch=lrect.Pitch>>2;
  
  return  lrect.pBits;
}
HRESULT CBackBuffer::Unlock(void)
{
  HRESULT hr=Surface->UnlockRect();
  return hr;
}

Examine that and see if you can figure out your problem. Something does not look right in your code and I think you are skipping a step. You cannot lock a buffer that you do not have access to via a COM interface pointer.
I use IDirect3DSurface9 as the Surface interface pointer and then use BufferDesc which is a D3DSURFACE_DESC object. Since I want the back buffer in the swap chain I also retrieve a pointer to that interface. Basically I walk down the hierarchy chain from the actual swap chain down to the surface, down to the buffer, down to the locked buffer, down to a pointer to the buffer.

>>You first need to get a pointer to the actual Surface interface.
Isn't that what lpddsPrimary is? Or are you saying that he didn't initialize that?... Remember, this is DirectDraw (I think the latest was 7), not Direct3D 9.

Right all the initializeation is done before the program enters the real time loop. Therefore everything is already defined.
lpddsPrimary being the surface that is defined with the palette lpddp.
I have fixed the problem it was to do with what some of you were saying.
lpddsPrimary->Lock(NULL,&theSurface, DDLOCK_WAIT | DDLOCK_SURFACEMEMORYPTR , NULL);

I simply put this in a if statement with a failed function, and now there is no error. I would be most grateful if something could explain why this results in solving the error.

Code:

		if (FAILED(lpddsPrimary->Lock(NULL,&theSurface, DDLOCK_WAIT | 
			DDLOCK_SURFACEMEMORYPTR , NULL)))
			return 0;

Well, I'm not totally sure what would cause the Lock() to fail, but the reason it stops the program from crashing is because when Lock() fails, the pointer to surface memory returned in theSurface is invalid; since you then use the pointer to modify the "surface memory", you get an access violation. With the addition of the FAILED condition, the program quits (return 0; ) before trying to access the pointer, thus preventing the crash.

You might want to check what's causing Lock() to fail on exit anyways, and fix it; because other problems might cause Lock() to fail at some point, and if it does you should pop up a messagebox or something to let the user know before quitting - but if you don't fix whatever's causing the problem right now, you'd get a messagebox every time you quit, and that would be annoying

Yeah I totally agree with you it needes fixing, but I cant fix a problem that I don't know how is being caused. Well not so much how but why its being caused, because surely if the program wanted to quit a WM_QUIT message should be made, and the if statement right at the beginning of the code should catch it and then the loop should be broken, so the other code which is all the direct draw shouldn't be called at all?