Depth buffer not being shared (sort of...)

[dark_sylinc]

Hi all!

It's around here in this thread (for the lazy, the one about improving the Compositor in the last GSoC 2009) where it popped the "problem" about Z depth buffers being shared across compositors (if they match msaa, resolution, etc; of course).

But the problem I'm facing is that the current code (at least Direct3D's) creates a new depth buffer for the compositors (different from the backbuffer's) and then it starts sharing that new buffer with the other compositor's rendertargets. However, the final rendering to the backbuffer has it's own depth buffer and can't be changed.

I'm implementing my own deferred shader where it is crucial (performance-wise) to share the same depth buffer across all RTs, including the backbuffer. Besides, there wouldn't be any need to create a new one in most cases.

As a workaround, I've added the following code in OgreD3D9RenderSystem.cpp, line 708 after attachRenderTarget(...), in D3D9RenderSystem::_createRenderWindow:

Code: Select all

IDirect3DSurface9 *pDepth = NULL;
renderWindow->getCustomAttribute( "D3DZBUFFER", &pDepth );
if( pDepth )
{
	const ZBufferIdentifier &zBufferIdentifier = getZBufferIdentifier( renderWindow );
	ZBufferRefQueue& zBuffers = mZBufferHash[zBufferIdentifier];
	ZBufferRef zb;
	zb.surface = pDepth;
	zb.width = width;
	zb.height = height;
	zBuffers.push_front(zb);
}

But this has the problem that Ogre will try to release it twice on exit (releasing a dangling pointer).
Does anyone think of a better solution to this?

I'm not very familiar with the rendersystem's code, furthermore I don't want to mess with Noman's changes and break the pause-resume again (actually those two are the main reasons I'm restraining from making an elaborate patch).
In theory, in _setRenderTarget() (line ~2713) it should check the depth buffer from the window's render target, not just from the pool it has for the compositors.

Another alternative I haven't tried yet is, in _setRenderTarget, to access mRenderWindows.front() to get to the main RT (i.e. the backbuffer) and use it's depth buffer, but I'm not sure if it's valid to assume mRenderWindows[0] always holds the main window's RT. And also what happens when there are multiple devices associated with different RTs being hold in that vector.

I have no idea what OpenGL is doing about the Z Buffer sharing, since I've run into this problem and I'm not targeting OpenGL at the moment.

Oh, and I thought this fits better in "Developer Talk" since I'm talking about a behavior in the Direct3D9 render system (it shouldn't create a 2nd depth buffer if it's not necessary)

Thanks
Dark Sylinc

[Noman]

Hi

Part of my decision to solve the depth buffer issue like I did was to not change the way ogre works, and to not pop things to userland. Even before my project, Ogre did not share depth buffers between RTTs and renderwindows. If you are talking about a temporary solution, there might be a minor change that can be done to get ogre to start sharing, I'll have a look at the code sometime soon.

However, the real solution (as discussed in the thread) is to give the user some control over depth buffer creation. Currently the term 'depth/stencil buffer' does not exist in Ogre API land, but is maintained internally in the render systems. Changing this is probably the only real solution, as any uncontrollable one would probably be incorrect for some or inefficient for others...

[Nir Hasson]

Hi -

The current implementation of the D3D9 Render System tries to grab shared depth buffer when a new render target is being set.
This is done in the _getDepthStencilFor method.
However from fast glimpse there it seems to be a problem with the code there that looks for z buffer to share.
Regarding the first render window – you can not assume anything about it.
Working with multiple devices will happen in a seamless way since one of the parameters that used to identify a z buffer is the device itself.
Anyway – I think this is reasnable demand that should be fixed within the render system and I'll try to spare some time to look at it and fix it.
Just to be sure - the test I should do in order to check it is to run some compositor sample that uses the same dimensions and defs as the back buffer and make sure the z buffer is shared across all the RTT ?

[sinbad]

Part of my decision to solve the depth buffer issue like I did was to not change the way ogre works, and to not pop things to userland. Even before my project, Ogre did not share depth buffers between RTTs and renderwindows.

Hmm, what makes you think that? In both v1.6 and trunk D3D9RenderSystem::_getDepthStencilFor operated purely based on format, multisample type and dimensions (actually in trunk the device is also a factor now, to deal with multi-head machines), which therefore does not rule out a window and RTT of the same size (or smaller, actually), format & AA using the same depth buffer. This obviously caused a problem with your interrupted renders, so you wanted to be able to 'check out' and 'check in' depth buffers.

My understanding was that a depth buffer would be 'checked out' (ie made unavailable for sharing) at beginScene, and checked back in at endScene, meaning that the depth/stencil would only be unavailable for sharing in the case of interrupted renders. Otherwise, for targets that update purely sequentially, the same sharing behaviour as before would occur. Is this not the case?

[Noman]

OK, did a re-check and this is what happens :
sinbad was right, RTT<->RenderWindow do not trigger creation of different depth/stencil buffers. In fact, if you turn off the shadows in the demo (comment out the lines in DeferredShadingSystem::DeferredShadingSystem, I'll probably make this runtime-changable soon) only one depth/stencil buffer will be created in the D3D rendersystem.

Only if you do _pauseScene/_resumeScene you will get another one created, which will be used in order to preserve the original scene's depth/stencil buffer when shadow textures are being generated. Are you sure that this is not what you are experiencing?

[dark_sylinc]

Ok, first to clarify, I don't use shadow textures at all (yet..)

When my compositor renders first to an MRT texture, and then it renders to target_output and some materials use that MRT as input.

Internally, D3D9RenderSystem::_setRenderTarget is called for the MRT, where it first tries to grab an existing suitable Depth Buffer. Here, it should get the Depth Buffer used in target_output and use it for the MRT, so that both targets use the same buffer.

However this is where it fails.
First it tries:

Code: Select all

savedTexture = mCheckedOutTextures.find(target);

But is mCheckedOutTextures is empty

Then it tries the next one:

Code: Select all

target->getCustomAttribute( "D3DZBUFFER", &pDepth );

Note that "target" contains the MRT (see D3D9MultiRenderTarget::getCustomAttribute) so pDepth is still null.

After that one, it calls _getDepthStencilFor(), but mZBufferHash is empty.
This was the last attempt to find an existing z-buffer, so inside _getDepthStencilFor, CreateDepthStencilSurface() is called and the new depth buffer is pushed into mZBufferHash.

But target_output uses it's own depth buffer (created with the "OGRE Render Window" RT) which is different from the one in mZBufferHash. Therefore, 2 different depth buffers.

Note though, that if I add to my compositor a second RT and draw to it, ie. "target MySecondRT", the code will have the same path I just explained for the MRT, except that when _getDepthStencilFor gets called, mZBufferHash isn't empty (because it contains the Depth Buffer created for the MRT) and uses that one.

As a result, MRT & MySecondRT (and any subsequent RTT) share the same depth buffer, but target_output (aka "the backbuffer") uses IT'S OWN.

To check this behavior, you can place a breakpoint inside setRenderTarget to single step and ultimately see that CreateDepthStencilSurface is called inside _getDepthStencilFor (don't forget the backbuffer creates it's own depth buffer in OgreD3D9Device.cpp in acquireRenderWindowResources).

Another way to check this is to draw the scene geometry into a RTT and then draw the same geometry with a slight Z positive offset (assuming the camera is pointing towards positive Z) into target_output with depth func greater.
If the depth buffers are shared, you should see the geometry in target_output. If the depth buffers aren't shared, you won't see anything.
Don't forget to disable depth clearing in the code (current viewport->setClearEveryFrame( true, FBT_COLOUR )) or else even with shared depth buffer you won't see anything either since the depth buffer will be cleared between the RTT pass and the target_output pass.

With my workaround, the backbuffer's depth buffer gets placed in mZBufferHash, and so it is used instead of creating a new one. But this comes with the problem that the Z-Buffer is then released twice on exit, when resizing, or device lost.

I'll try to upload a simple test showing the problem.
Cheers
Dark Sylinc

[dark_sylinc]

OK.

Here's the executable with sources included

This is how it is supposed to look:

Notice there's some flickering due to Z Fighting (just ignore it)

In the zipped file the executables are already compiled in case you don't want to compile it.

This sample is very very very simple. It just draws a cube with colour_write off to an RTT, and then a second red-coloured cube with Z greater func and a slight offset in Z in the opposite direction of the camera.
If they share the same Depth Buffer, the red cube will be shown since it passes the test thanks to the first invisible cube drawn. If the Depth Buffers aren't shared, you'll get a blank screen because the depth buffer is filled with 1s, and a 'greater' z func will always fail the test.

"DepthSharingTest.exe" draws directly to the target_output. You get a blank screen, proving the depth buffer isn't shared.
"DepthSharingTestWithTrick.exe" draws to an intermediate RTT defined in the compositor script and then copies that RTT to the target_output. You will see the red cube, which proves the depth buffers are (at least) shared between RTTs, but not with target_output


Just in case someone is having problems with RapidShare, I'm posting the source:
DepthSharingTest.cpp

Code: Select all

#include "DepthSharingTest.h"

#include <OgreCompositorManager.h>

using namespace Ogre;


DepthSharingTestListener::DepthSharingTestListener( RenderWindow* win, Camera* cam,
											SceneManager *sceneMgr )
        : ExampleFrameListener(win, cam)
{
	this->m_sceneManager = sceneMgr;
}


DepthSharingTestApplication::DepthSharingTestApplication()
{
}

// Just override the mandatory create scene method
void DepthSharingTestApplication::createScene(void)
{
    // Set ambient light
    mSceneMgr->setAmbientLight( ColourValue(1, 1, 1) );
	mCamera->getViewport()->setBackgroundColour( Ogre::ColourValue( 0.0f, 0.0f, 0.0f ) );
	mCamera->getViewport()->setClearEveryFrame( false ); //NECESSARY!

	mCamera->setPosition( 0, 0, -10 );
	mCamera->lookAt( 0, 0, 10 );

	{
		//Create invisible cube and put in RenderQueue 50 (see Compoitor.compositor)
		Entity *ent = mSceneMgr->createEntity( "invisiblecube", "Cube_d.mesh" );
		ent->setMaterialName( "Invisible" );
		ent->setRenderQueueGroup( 50 );
		SceneNode *sceneNode = mSceneMgr->getRootSceneNode()->createChildSceneNode();
		sceneNode->attachObject( ent );
		sceneNode->setPosition( 0, 0, 0 );
		sceneNode->scale( 3.0f, 3.0f, 3.0f );
	}

	{
		//Create red cube in RenderQueue 51 and with a slight Z offset
		Entity *ent = mSceneMgr->createEntity( "redcube", "Cube_d.mesh" );
		ent->setMaterialName( "Red" );
		ent->setRenderQueueGroup( 51 );
		SceneNode *sceneNode = mSceneMgr->getRootSceneNode()->createChildSceneNode();
		sceneNode->attachObject( ent );
		sceneNode->setPosition( 0, 0, 3 );
		sceneNode->scale( 3.0f, 3.0f, 3.0f );
	}

	CompositorManager::getSingleton().addCompositor( mCamera->getViewport(), "NotWorkingWithoutZSharing" );
	CompositorManager::getSingleton().setCompositorEnabled( mCamera->getViewport(), "NotWorkingWithoutZSharing", true );

	/*CompositorManager::getSingleton().addCompositor( mCamera->getViewport(), "WorksGreatWithTrick" );
	CompositorManager::getSingleton().setCompositorEnabled( mCamera->getViewport(), "WorksGreatWithTrick", true );*/
}

void DepthSharingTestApplication::destroyScene()
{
	CompositorManager::getSingleton().setCompositorEnabled( mCamera->getViewport(), "NotWorkingWithoutZSharing", false );
	CompositorManager::getSingleton().removeCompositor( mCamera->getViewport(), "NotWorkingWithoutZSharing" );

	/*CompositorManager::getSingleton().setCompositorEnabled( mCamera->getViewport(), "WorksGreatWithTrick", false );
	CompositorManager::getSingleton().removeCompositor( mCamera->getViewport(), "WorksGreatWithTrick" );*/
}

void DepthSharingTestApplication::createFrameListener(void)
{
    mFrameListener = new DepthSharingTestListener( mWindow, mCamera, mSceneMgr );
    mRoot->addFrameListener( mFrameListener );
}

DepthSharingTest.h

Code: Select all

#include "ExampleApplication.h"

class DepthSharingTestListener : public ExampleFrameListener
{
private:
	Ogre::SceneManager			*m_sceneManager;

public:
    DepthSharingTestListener( RenderWindow* win, Camera* cam, SceneManager *sceneMgr );
};

class DepthSharingTestApplication : public ExampleApplication
{
public:
    DepthSharingTestApplication();

	void createScene();
	void destroyScene();
	void createFrameListener();
};

main.cpp

Code: Select all

#include "DepthSharingTest.h"

#if OGRE_PLATFORM == OGRE_PLATFORM_WIN32
#define WIN32_LEAN_AND_MEAN
#include "windows.h"
#endif

#ifdef __cplusplus
extern "C" {
#endif

#if OGRE_PLATFORM == OGRE_PLATFORM_WIN32
INT WINAPI WinMain( HINSTANCE hInst, HINSTANCE, LPSTR strCmdLine, INT )
#else
int main(int argc, char *argv[])
#endif
{
    // Create application object
    DepthSharingTestApplication app;

    try {
        app.go();
    } catch( Ogre::Exception& e ) {
#if OGRE_PLATFORM == OGRE_PLATFORM_WIN32
        MessageBox( NULL, e.getFullDescription().c_str(), "An exception has occured!", MB_OK | MB_ICONERROR | MB_TASKMODAL);
#else
        std::cerr << "An exception has occured: " <<
            e.getFullDescription().c_str() << std::endl;
#endif
    }

    return 0;
}

#ifdef __cplusplus
}
#endif

Compositor.compositor:

Code: Select all

//We draw an invisible cube with Z-func less or equal into a RTT "out1" using
//colour_write off
//We just want the Z information writen to the depth buffer, and use it later in a
//different RTT to prove the z buffers are shared
	
compositor WorksGreatWithTrick
{
	//The trick consists in rendering everything to a tmp RTT (as it shares the same depth
	//buffer) and then copying it as a quad into target_output
	
	technique
	{
		// temporary textures
		texture out1 target_width target_height PF_R8G8B8A8
		texture out2 target_width target_height PF_R8G8B8A8
		
		target out1
		{
			input none

			shadows off //No shadows
			
			pass clear
			{
				clear
				{
					//Clear Colour & depth explicitly
					buffers colour depth
				}
			}

			//Draw invisible cube Z: Less or equal
			pass render_scene
			{
				//These values are synchronized with the code
				first_render_queue 50
				last_render_queue  50
			}
		}
		
		//target_output
		target out2
        {
            // Start with clear output
            input none
			
			pass clear
			{
				//Clear colours BUT NOT depth. If we do, the geometry won't be displayed
				clear
				{
					buffers colour
					colour_value 0 0 0 1
				}
			}

            //Draw Red cube Z: Greater
			//If Z Buffer is shared, we should see a red cube, otherwise blank
            pass render_scene
            {
                first_render_queue 51
				last_render_queue  51
            }
        }

		target_output
		{
			input none

			pass render_quad
			{
				material Copy
				input 0 out2
			}
		}
	}
}

compositor NotWorkingWithoutZSharing
{
	technique
	{
		//This is the same as WorksGreatWithTrick, only this time we render
		//the red cube directly into the backbuffer. If Z buffers are shared,
		//we'll see 
		texture out1 target_width target_height PF_R8G8B8A8
		
		target out1
		{
			input none

			shadows off //No shadows
			
			pass clear
			{
				clear
				{
					//Clear Colour & depth explicitly
					buffers colour depth
				}
			}

			//Draw invisible cube Z: Less or equal
			pass render_scene
			{
				//These values are synchronized with the code
				first_render_queue 50
				last_render_queue  50
			}
		}

		target_output
        {
            input none

			pass clear
			{
				//Clear colours BUT NOT depth. If we do, the geometry won't be displayed
				clear
				{
					buffers colour
					colour_value 0 0 0 1
				}
			}

            //Draw Red cube Z: Greater
			//If Z Buffer is shared, we should see a red cube, otherwise blank
            pass render_scene
            {
                first_render_queue 51
				last_render_queue  51
            }
        }
	}
}

Cube.material

Code: Select all

material Invisible
{
	technique
	{
		pass
		{
			depth_check on
			depth_write on

			colour_write off
			depth_func less_equal
		}
	}
}

material Red
{
	technique
	{
		pass
		{
			depth_check on
			depth_write off //Don't write to the depth buffer makes it simplier

			//Make it bright red
			ambient 0 0 0 0
			diffuse 0 0 0 0
			emissive 1 0 0 1

			colour_write on
			depth_func greater
		}
	}
}

[Noman]

OK, I'll have a look at it soon..

[Noman]

Ok, I see what you're talking about.
When a direct3d renderwindow is created, a depth/stencil surface is sometimes automatically created by the call. I was only looking for the CreateDepthStencilSurface call, but that is not the case. The solution is to add the depth buffer to the pool after a window is created (if it is created with the depth buffer) and during cleanup phase, release the buffer only if its not the automatically created one (there is enough exposed information to determine this in the current rendersystem).

I have to go now, I'll hopefully implement this today, worst case tomorrow.
Thanks!

[Noman]

Here is the patch for your temporary use:

Code: Select all

Index: RenderSystems/Direct3D9/src/OgreD3D9RenderSystem.cpp
===================================================================
--- RenderSystems/Direct3D9/src/OgreD3D9RenderSystem.cpp	(revision 9238)
+++ RenderSystems/Direct3D9/src/OgreD3D9RenderSystem.cpp	(working copy)
@@ -706,8 +706,20 @@
 		updateRenderSystemCapabilities(renderWindow);
 
 		attachRenderTarget( *renderWindow );
+		
+		if (renderWindow->isDepthBuffered())
+		{
+			//Insert the (automatically created) depth buffer to the pool
+			IDirect3DSurface9* depthBuffer = renderWindow->getDevice()->getDepthBuffer(renderWindow);
+			ZBufferIdentifier zBufferIdentifier = getZBufferIdentifier(renderWindow);
+			ZBufferRefQueue& zBuffers = mZBufferHash[zBufferIdentifier];
+			ZBufferRef newRef;
+			newRef.surface = depthBuffer;
+			newRef.height = height;
+			newRef.width = width;
+			zBuffers.push_back(newRef);
+		}
 
-
 		return renderWindow;
 	}	
 	//---------------------------------------------------------------------
@@ -3794,6 +3806,8 @@
 	//---------------------------------------------------------------------
 	void D3D9RenderSystem::_cleanupDepthStencils(IDirect3DDevice9* d3d9Device)
 	{
+		IDirect3DSurface9* deviceSurface;
+		d3d9Device->GetDepthStencilSurface(&deviceSurface);
 		for(ZBufferHash::iterator i = mZBufferHash.begin(); i != mZBufferHash.end();)
 		{
 			/// Release buffer
@@ -3801,7 +3815,12 @@
 			{
 				while (!i->second.empty())
 				{
-					i->second.front().surface->Release();
+					IDirect3DSurface9* surface = i->second.front().surface;
+					if (surface != deviceSurface)
+					{
+						//Only free the manually created surfaces
+						surface->Release();
+					}
 					i->second.pop_front();
 				}
 				ZBufferHash::iterator deadi = i++;

I didn't submit it as a patch, because once someone else sees this and thinks thats its ok, I'll commit it myself.

The only problem for me is, that D3D/GL behavior is different, which is something that we want to avoid.
It already was, as GL does much less for depth buffer sharing, but we really ought to sync em...
The best would be to give the user some sort of control over depth/stencil buffer management, but that needs to be thought over...

After this patch, the NotWorkingWithoutZSharing compositor works cleanly and as expected.

[Noman]

Committed the change

[Nir Hasson]

Fixed a related bug there -
When you call GetDepthStencilSurface or any other method that increase the ref count of the D3D9 Resource that you grabbed, you should make sure you release it when finish using the resource, otherwise IDirect3DDevice9::Reset() will fail, which means that we won't recover from lost state - and thats not good for us

[dark_sylinc]

Found a bug with the new patch while working with it.

When an RTT with a bigger screen resolution than the screen's dimensions, _getDepthStencilFor will attempt to release the main depth buffer (ouch!)

I changed the code so that it looks like this:

Code: Select all

if(zBuffer.width >= width && zBuffer.height >= height)
{
	surface = zBuffer.surface;
} 
else
{
	IDirect3DSurface9* deviceSurface;
	getActiveDevice()->GetDepthStencilSurface(&deviceSurface);

	// Release immediately -> each get increase the ref count.
	if (deviceSurface != NULL)
		deviceSurface->Release();

	if( zBuffer.surface != deviceSurface )
	{
		/// If not, destroy current buffer
		zBuffer.surface->Release();
		zBuffers.pop_front();
	}
}

This avoids destroying our precious depth buffer.
However, while this prevents it, _getDepthStencilFor() will create a new depth buffer and place it in front of the deque. Afterwards, the main depth buffer will never get used again (ouch!) because the other one is preferred.

[s]For the sake of solving my own problem, I just put the new depth buffer on the back of the deque instead of putting it in the front. (zBuffers.push_back(zb) instead of zBuffers.push_front(zb))

Anyway, I noticed the code uses a deque, but it works as if only one depth buffer would be in that deque... (no iteration, always checks the front...)

This is probably, because in theory only the biggest depth buffer (for the same MSAA type) is useful and all the others can be discarded. Though this doesn't hold true for the backbuffer's depth-buffer, which should be preferred always unless the RTT has bigger dimensions.

A caveat of my approach is that if another RTT whose dimensions are bigger than the backbuffer's, it will create a third depth buffer and put it in the back of the deque (because _getDepthStencilFor only checks the first one in the front, instead of iterating)

I didn't even bother about fixing the iteration thing because[/s] I'm waiting for more input so I start writing a permanent solution.

Cheers
Dark Sylinc

Edit: Ahh, I just can't submit a half-made solution. Here's the full patch.
It also fixes a bug when restoring a lost device. (depth buffers weren't resubmitted to the pool)