BUG: Taking screenshot in OpenGL and externalWindow: Crash!

[mehdix]

Hi,

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

Code: Select all

0xC0000005: Access violation writing location 0x0e073000.

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

[DWORD]

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

[mehdix]

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

[DWORD]

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]

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]

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

[mehdix]

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

[Praetor]

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.

[sinbad]

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]

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]

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

[sinbad]

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]

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.

[sinbad]

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);