BUG: Taking screenshot in OpenGL and externalWindow: Crash!

mehdix · Post by **mehdix** » Fri Aug 10, 2007 8:36 pm

Hi,

The title is obvious: Win32Window::writeContentsToFile() calls glReadPixels() that causes:

0xC0000005: Access violation writing location 0x0e073000.

There is no problem if either Direct3D used or parentWindowHandle with OpenGl are used together.

Post by **DWORD** » Sat Aug 11, 2007 11:03 am

Well, did you try to debug it to find the cause? I think that would help a lot.

mehdix · Post by **mehdix** » Sat Aug 11, 2007 1:53 pm

Yes, I debug it to find the cause (and function names

) but the problem is take place in glReadPixels() and anything else seems fine. My base system have an ATI card and I tried it on another PC with nVIDIA GPU and again a CRT debug error was brought out that:

....
HEAP CORRUPTION DETECTED: After Normal block (#..) at 0x0... .
CRT detected that the app wrote to memory after end of heap buffer.
...

Post by **DWORD** » Sat Aug 11, 2007 3:39 pm

Yep. But I doubt there's a bug in glReadPixels(), so maybe the problem lies with the pointer that is fed into that function or some of the function parameters. What about buffer size and image dimensions? Are they fine?

mehdix · Post by **mehdix** » Sat Aug 11, 2007 4:37 pm

All is Ok. I checked the mWidth and mHeight with GetClientSize(mHWnd, &rect), and they have correct values. As you, I think this problem is related to glReadPixels() because as the call stack shows, the problem happens in the graphic driver (atioglxx.dll); but it might be related to some where else too.

Code: Select all

void Win32Window::writeContentsToFile(const String& filename) {
...
// Allocate buffer 
uchar* pBuffer = new uchar[mWidth * mHeight * 3];

// Read pixels
// I love GL: it does all the locking & colour conversion for us
if (mIsFullScreen)
glReadBuffer(GL_FRONT);
glReadPixels(0,0, mWidth, mHeight, GL_RGB, GL_UNSIGNED_BYTE, pBuffer);
...

JamesKilton · Post by **JamesKilton** » Sat Aug 11, 2007 8:18 pm

Ah, ATI and OpenGL. Does this problem exist when using DirectX? If not, it could be the ATI drivers.

mehdix · Post by **mehdix** » Sun Aug 12, 2007 3:31 pm

As I wrote, the problem is only with OpenGL but not specified to ATI products.

Post by **Praetor** » Sun Aug 12, 2007 5:04 pm

Does that mean you've tried it with nvidia hardware? If someone could, even better if you could, that was help out. I seriously would not be shocked if this was an ATI OGL problem. Wouldn't be the first.

Post by **sinbad** » Sun Aug 12, 2007 6:18 pm

Yep, if everything going into glReadPixels is ok, but glReadPixels fails, there's most likely a driver bug. Would definitely not be the first on ATI.

Here's a related topic which suggests in their case it was specific to the FBO implementation: http://www.gamedev.net/community/forums ... _id=421594

You could try changing to PBO just to see if that resolves it.

mehdix · Post by **mehdix** » Sun Aug 12, 2007 8:34 pm

I tried with FBO, PBuffer and Copy, on both ATI and nVIDIA, but it crashed in all cases, like before! In addition I should mention that on nVIDIA card, the screen shot is taken but in this form:

mehdix · Post by **mehdix** » Sun Aug 12, 2007 9:33 pm

I remotely debug my program on the nVIDIA PC; glReadPixels() exceeds the pBuffer isze but there is no access violation like ATI driver. At the end of Win32Window::writeContentsToFile(), delete operator finds that memory locations after end of the buffer are written, and then brings out:
"HEAP CORRUPTION DETECTED: ... "

Post by **sinbad** » Mon Aug 13, 2007 5:10 pm

Had you perchance resized the external window in some way without telling the OGRE window? That 'slanted' view looks exactly like what happens when you try to access a buffer with the wrong horizontal row size.

mehdix · Post by **mehdix** » Tue Aug 14, 2007 8:27 pm

No. On any size changes, and before rendering a new frame, RenderWindow::windowMovedOrResized() will be called. As I wrote I checked the extents with GetClientSize(mHWnd, &rect), before calling glReadPixels(), and they were Ok.

Post by **sinbad** » Wed Aug 15, 2007 1:09 pm

Well, this was a bitch, but I found it.

It was a buffer overrun problem. The default behaviour of glReadPixels is to pack every row to a multiple of 4 bytes, even though the spec doesn't say that (you have to go look at glPixelStore for that). So when you're writing a window which is an odd size rather than the more usual power-of-2 numbers you got a buffer overrun which resulted in random behaviour (I ended up with a crash deep in ntdll.dll).

I've fixed this now, see the patch below. I've also fixed the fact that when you're using multiple windows in GL you weren't guaranteed to write the correct one previously, if another one was updated last. I've tested this in my own embedded app (wxWidgets based) and it works.

Eihort:

Code: Select all

Index: RenderSystems/GL/src/OgreWin32Window.cpp
===================================================================
RCS file: /cvsroot/ogre/ogrenew/RenderSystems/GL/src/OgreWin32Window.cpp,v
retrieving revision 1.45.2.5
diff -u -r1.45.2.5 OgreWin32Window.cpp
--- RenderSystems/GL/src/OgreWin32Window.cpp	10 Aug 2007 10:35:27 -0000	1.45.2.5
+++ RenderSystems/GL/src/OgreWin32Window.cpp	15 Aug 2007 12:06:48 -0000
@@ -511,12 +511,22 @@
 		// Allocate buffer 
 		uchar* pBuffer = new uchar[mWidth * mHeight * 3];
 
+		// Switch context if different from current one
+		RenderSystem* rsys = Root::getSingleton().getRenderSystem();
+		rsys->_setViewport(this->getViewport(0));
+
+		// Must change the packing to ensure no overruns!
+		glPixelStorei(GL_PACK_ALIGNMENT, 1);
+
 		// Read pixels
 		// I love GL: it does all the locking & colour conversion for us
 		if (mIsFullScreen)
 			glReadBuffer(GL_FRONT);
 		glReadPixels(0,0, mWidth, mHeight, GL_RGB, GL_UNSIGNED_BYTE, pBuffer);
 
+		// restore default alignment
+		glPixelStorei(GL_PACK_ALIGNMENT, 4);
+
 		// Wrap buffer in a memory stream
 		DataStreamPtr stream(new MemoryDataStream(pBuffer, mWidth * mHeight * 3, false));

Shoggoth:

Code: Select all

Index: RenderSystems/GL/src/OgreWin32Window.cpp
===================================================================
RCS file: /cvsroot/ogre/ogrenew/RenderSystems/GL/src/OgreWin32Window.cpp,v
retrieving revision 1.50
diff -u -r1.50 OgreWin32Window.cpp
--- RenderSystems/GL/src/OgreWin32Window.cpp	26 Jun 2007 17:56:24 -0000	1.50
+++ RenderSystems/GL/src/OgreWin32Window.cpp	15 Aug 2007 12:00:34 -0000
@@ -521,11 +521,22 @@
 						"Win32Window::copyContentsToMemory" );
 		}
 
+
+		// Switch context if different from current one
+		RenderSystem* rsys = Root::getSingleton().getRenderSystem();
+		rsys->_setViewport(this->getViewport(0));
+
+		// Must change the packing to ensure no overruns!
+		glPixelStorei(GL_PACK_ALIGNMENT, 1);
+
 		glReadBuffer((buffer == FB_FRONT)? GL_FRONT : GL_BACK);
 		glReadPixels((GLint)dst.left, (GLint)dst.top,
 					 (GLsizei)dst.getWidth(), (GLsizei)dst.getHeight(),
 					 format, type, dst.data);
 
+		// restore default alignment
+		glPixelStorei(GL_PACK_ALIGNMENT, 4);
+
 		//vertical flip
 		{
 			size_t rowSpan = dst.getWidth() * PixelUtil::getNumElemBytes(dst.format);