[2.2] Depth Buffer Textures are never deleted

Discussion area about developing with Ogre2 branches (2.1, 2.2 and beyond)
Post Reply
User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

[2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

Hi, me again :P So working through the texture memory management and i came across a bug or what i feel is a bug that when you resize the window, it recreates the depth buffers to keep correct resolution, however the older depth buffers never get cleared off, which ive seen through the dumpMemoryUsage(), if i do it enough ive ended up with 30+ depth buffers that are never removed..

Is this a bug, or just not done yet? Or is it something happening with my version only?

I know there are ground rules with changing it as some depth buffers are shared between textures and wouldn't want to knack that up.
Thoughts?

Image

Just to note that the editor is an engine window that grabs data in a similar fashion to your dumpMemoryUsage() function but far more readable :P

Edit: The HDR Demo would be closest to mine for a test

Edit: I have managed to reproduce in the HDR sample, add an f5 key to output log, run demo > f5(baseline) / check log > resize window a few times > f5 - and look for DepthBuffer_ in the log list. And it will have the old depth buffers that are not running. When depthbuffers are 1080p, they hold quite a bit of space as you resize a window or an editor like ourselves.
Im looking at the code now trying to find the best way of handling it as the depth buffers have no association with the RTT that it was made from. Thus it would need some sort of storage variable to know that it could be deleted because it was linked with that RTT or windowTexture at the time, as its unlikely to be used or shared again. I know the depth buffers are stored in:

Code: Select all

mDepthBufferPool2[poolId]
Edit 3: I cant test but from looking at the code, the stencil textures when notifyRecreated on the compositor pass is called also wont be deleted i think.
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

posting seperatly as its a potential solution:

I think i have a solution however how good it is im uncertain. but here is my patch and it clears the depth&stencil(if shared) buffer and cleans up the old one and doesnt touch any of the other types of depth buffer:

REMOVED CODE AS DEPRECIATED

If you could let me know how to do it correctly, that would be good.. thanks :)

Edit: Worked in the ogre demo, but not for my engine, it deletes one of the textures but there are others being made in its place. :P Still looking and working on this one as other textures apart of the compositor chain also change resolution based on the final target so need to clear their old textures too :)
Last edited by SolarPortal on Sun Jun 21, 2020 2:52 pm, edited 1 time in total.
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

Ok slight tweaks and possibly a hack but it works and im clearing off my depth buffers now.
New code:

OgreRenderSystem.cpp - line 505

Code: Select all

	
line 505
	void RenderSystem::destroyDepthBuffer(TextureGpu *depthTexture)
	{
		if (!depthTexture) return;
		TextureGpuVec& bufferVec = mDepthBufferPool2[depthTexture->getDepthBufferPoolID()];
		TextureGpuVec::iterator itor = std::find(bufferVec.begin(), bufferVec.end(), depthTexture);
		if (itor != bufferVec.end()) {
			bufferVec.erase(itor);
			mTextureGpuManager->destroyTexture(depthTexture);
		}
	}
	
// Head to getDepthBufferFor()
// line 557
	if( poolId == DepthBuffer::POOL_NON_SHAREABLE )
        {
            TextureGpu *retVal = createDepthBufferFor( colourTexture, preferDepthTexture,
                                                       depthBufferFormat );
++           retVal->setDepthBufferPoolID(poolId); // Unused but at least the texture knows where it belongs
            return retVal;
        }
        
// line 587
        //Not found yet? Create a new one!
        if( !retVal )
        {
            retVal = createDepthBufferFor( colourTexture, preferDepthTexture, depthBufferFormat );
++            retVal->setDepthBufferPoolID(poolId); // Unused but at least the texture knows where it belongs
            mDepthBufferPool2[poolId].push_back( retVal );

            if( !retVal )
            {
                LogManager::getSingleton().logMessage( "WARNING: Couldn't create a suited "
                                                       "DepthBuffer for RTT: " +
                                                       colourTexture->getNameStr(), LML_CRITICAL );
            }
        }

	
OgreRenderSystem.h - line 764

Code: Select all

		
		/** Internal use only, detroys a depth buffer associated in the pool. If no texture is found
			the it skips. Useful for clearing the renderwindow depth and stencil buffer.
		*/
		void destroyDepthBuffer( TextureGpu *depthTexture );

OgreCompositorPass.cpp

Code: Select all


...
//     
    CompositorPass::~CompositorPass()
    {
        if( mRenderPassDesc )
        {
            RenderSystem *renderSystem = mParentNode->getRenderSystem();

            // Clear off the render pass depth buffer
++            if(mRenderPassDesc->mDepth.texture)
++                        renderSystem->destroyDepthBuffer(mRenderPassDesc->mDepth.texture);

            renderSystem->destroyRenderPassDescriptor( mRenderPassDesc );
            mRenderPassDesc = 0;
        }

        _removeAllBarriers();
    }


...
// notifyRecreated()
        if( usedByUs )
        {
            mNumPassesLeft = mDefinition->mNumInitialPasses;

            //Reset texture pointers and setup RenderPassDescriptor again
++            RenderSystem *renderSystem = mParentNode->getRenderSystem();
++            renderSystem->destroyDepthBuffer(mRenderPassDesc->mDepth.texture);
            mRenderPassDesc->mDepth.texture = 0;
            mRenderPassDesc->mStencil.texture = 0;

            for( int i=0; i<OGRE_MAX_MULTIPLE_RENDER_TARGETS; ++i )
            {
                mRenderPassDesc->mColour[i].texture = 0;
                mRenderPassDesc->mColour[i].resolveTexture = 0;
            }

            const CompositorNodeDef *nodeDef = mParentNode->getDefinition();
            const CompositorTargetDef *targetDef = mDefinition->getParentTargetDef();
            const RenderTargetViewDef *rtvDef =
                    nodeDef->getRenderTargetViewDef( targetDef->getRenderTargetName() );
			
            // Destroy the previous depth buffer
            setupRenderPassDesc( rtvDef );
        }
OgreTextureGPU.cpp

Code: Select all

    
    ...
    // line 68
    		mDepthBufferPoolId( 0 )

    
    ...
   // line 278 ish
    //-----------------------------------------------------------------------------------
	void TextureGpu::setDepthBufferPoolID(uint16 id) { mDepthBufferPoolId = id; }
    //-----------------------------------------------------------------------------------
	uint16 TextureGpu::getDepthBufferPoolID(void) const { return mDepthBufferPoolId; }
OgreTextureGPU.h

Code: Select all

//line 235
        /// Not used for anything but the depth buffers to identify the depth buffer pool they belong too. As its quicker than searching all depth buffers.
        uint32		mDepthBufferPoolId;

...

// line 329

		// setter and getter for a depth buffer texture to retrieve its slot in the depth buffer map
		void setDepthBufferPoolID(uint16 id);
		uint16 getDepthBufferPoolID(void) const;
I used a seperate pool id storage as didnt want to affect anything else, but perhaps you have a better placement of all these changes.

This works on my engine also and i do not get an ever increasing list of depth buffers which is better on memory now.

Edit: Updated because i missed a couple bits.

Edit 2: Updated the compositor pass deconstructor to remove the depth buffer which means when compositor effects are turned on and off, it cleans up the buffers :)

I think i will leave it at this as it seems to be working now :)
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

Created a ticket.

Your solution is reading from a dangling pointer when you call depthTexture->getDepthBufferPoolID() if the depth buffer has already been destroyed.

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

ok, will take a look but if i dont find something i will run with what i have until a complete fix is in place as ive tested fairly extensively and not come across any crashes, thats with resizing, loading different scenes and changing post effects on / off.
However, looking at 2.1 source, it only ever seemed to delete a depth buffer if the pool was non shareable, or when the render system was shut down.
I agree, dangling pointers are never a good thing :P

The rate at which depth buffers can be recreated at times can skyrocket memory very quickly so i personally would say this is quite a big leak. Roughly 10 depth buffers to 100mb at around 720p, scaling up and it can get quite memory intensive very quickly.

Thanks for responding :)
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

Patch Notes:
Updated to using a reference based system instead that keeps track of all active and inactive depth buffers.
When the reference is 0, the texture is destroyed as are entries
Because stencil buffers are most of the time the same as the depth buffer, this also increments, but we control all the decrementing
in the destroyRenderPassDescriptor() and the notifyRecreate()
As mentioned in the ticket i have created a function called _cleanupDepthBuffers which is called in the root _updateAllRenderTargets().
I have tested on many different nodes through post processors, workspace refreshes and many scenes and it seems to work.
There may be other areas in code that you think might need to be included but i have done the most obvious.
Anyway, patch notes here for you to test.

++ marks the spot :P

OgreRenderSystem.cpp

Code: Select all


...

// function - Shutdown()
    void RenderSystem::shutdown(void)
    {

		...

        destroyAllRenderPassDescriptors();
++		_dereferenceAllDepthBuffer();
++		_cleanupDepthBuffers();

    }

...

// Line 512
++   //---------------------------------------------------------------------
++	void RenderSystem::destroyDepthBuffer(TextureGpu *depthBuffer)
++	{
++		TextureGpuVec& bufferVec = mDepthBufferPool2[depthBuffer->getDepthBufferPoolID()];
++		TextureGpuVec::iterator itor = std::find(bufferVec.begin(), bufferVec.end(), depthBuffer);
++		if (itor != bufferVec.end()) {
++			bufferVec.erase(itor);
++			mTextureGpuManager->destroyTexture(depthBuffer);
++		}
++	}
++    //---------------------------------------------------------------------
++	void RenderSystem::_cleanupDepthBuffers(void)
++	{
++		DepthBufferRefMap2::iterator itor = mDepthBufferReferences.begin();
++		DepthBufferRefMap2::iterator end = mDepthBufferReferences.end();
++		while (itor != end){
++			if (itor->second <= 0) {
++				destroyDepthBuffer(itor->first);
++				itor = mDepthBufferReferences.erase(itor);
++				end = mDepthBufferReferences.end();
++			}else {
++				itor++;
++			}
++		}
++	}
++	//---------------------------------------------------------------------
++	void RenderSystem::_dereferenceDepthBuffer( TextureGpu* depthBuffer)
++	{
++		if (!depthBuffer) return;
++		mDepthBufferReferences[depthBuffer]--;
++		mDepthBufferReferences[depthBuffer] = mDepthBufferReferences[depthBuffer] <= 0 ? 0 : mDepthBufferReferences[depthBuffer];
++	}
++	//---------------------------------------------------------------------
++	void RenderSystem::_dereferenceAllDepthBuffer()
++	{
++		DepthBufferRefMap2::iterator itor = mDepthBufferReferences.begin();
++		DepthBufferRefMap2::iterator end = mDepthBufferReferences.end();
++		while (itor != end) {
++			itor->second = 0;
++			itor++;
++		}
++	}
++	//---------------------------------------------------------------------
++	void RenderSystem::_referenceDepthBuffer( TextureGpu* depthBuffer )
++	{
++		if (!depthBuffer) return;
++		mDepthBufferReferences[depthBuffer]++;
++
++	}

...

// GetDepthBuffersFor() -- i have added the full function as it will be easier to see change.

    TextureGpu* RenderSystem::getDepthBufferFor( TextureGpu *colourTexture, uint16 poolId,
                                                 bool preferDepthTexture,
                                                 PixelFormatGpu depthBufferFormat )
    {
        if( poolId == DepthBuffer::POOL_NO_DEPTH || depthBufferFormat == PFG_NULL )
            return 0; //RenderTarget explicitly requested no depth buffer

        if( colourTexture->isRenderWindowSpecific() )
        {
            Window *window;
            colourTexture->getCustomAttribute( "Window", &window );
            return window->getDepthBuffer();
        }

        if( poolId == DepthBuffer::POOL_NON_SHAREABLE )
        {
            TextureGpu *retVal = createDepthBufferFor( colourTexture, preferDepthTexture,
                                                       depthBufferFormat );
++			retVal->setDepthBufferPoolID(poolId); // Unused but at least the texture knows where it belongs
++			_referenceDepthBuffer(retVal);
			return retVal;
        }

        //Find a depth buffer in the pool
        TextureGpuVec::const_iterator itor = mDepthBufferPool2[poolId].begin();
        TextureGpuVec::const_iterator end  = mDepthBufferPool2[poolId].end();

        TextureGpu *retVal = 0;

        while( itor != end && !retVal )
        {
            if( preferDepthTexture == (*itor)->isTexture() &&
                (depthBufferFormat == PFG_UNKNOWN ||
                 depthBufferFormat == (*itor)->getPixelFormat()) &&
                (*itor)->supportsAsDepthBufferFor( colourTexture ) )
            {
                retVal = *itor;
++				_referenceDepthBuffer(retVal);
            }
            else
            {
                retVal = 0;
            }
            ++itor;
        }

        //Not found yet? Create a new one!
        if( !retVal )
        {
            retVal = createDepthBufferFor( colourTexture, preferDepthTexture, depthBufferFormat );
++			retVal->setDepthBufferPoolID(poolId); // Unused but at least the texture knows where it belongs
++			_referenceDepthBuffer(retVal);
            mDepthBufferPool2[poolId].push_back( retVal );

            if( !retVal )
            {
                LogManager::getSingleton().logMessage( "WARNING: Couldn't create a suited "
                                                       "DepthBuffer for RTT: " +
                                                       colourTexture->getNameStr(), LML_CRITICAL );
            }
        }

        return retVal;
    }
	
...

// destoryRenderPassDescriptor() - same again, full function
    void RenderSystem::destroyRenderPassDescriptor( RenderPassDescriptor *renderPassDesc )
    {
        RenderPassDescriptorSet::iterator itor = mRenderPassDescs.find( renderPassDesc );
        assert( itor != mRenderPassDescs.end() && "Already destroyed?" );
        if( itor != mRenderPassDescs.end() )
            mRenderPassDescs.erase( itor );
++
++		if (renderPassDesc->mDepth.texture) {
++			_dereferenceDepthBuffer(renderPassDesc->mDepth.texture);
++			
++			if (renderPassDesc->mStencil.texture && renderPassDesc->mStencil.texture == renderPassDesc->mDepth.texture) {
++				_dereferenceDepthBuffer(renderPassDesc->mDepth.texture);
++			}
++		}
        delete renderPassDesc;
    }
OgreRenderSystem.h

Code: Select all


...
// line 56

    typedef vector<TextureGpu*>::type TextureGpuVec;
    typedef map< uint16, TextureGpuVec >::type DepthBufferMap2;
++    typedef map< TextureGpu*, int16 >::type DepthBufferRefMap2;


...

// line 762
++		/** Internal use only, detroys a depth buffer associated in the pool. If no texture is found
++			the it skips. Useful for clearing the renderwindow depth and stencil buffer.
++		*/
++		void destroyDepthBuffer( TextureGpu *depthTexture );
++		void _cleanupDepthBuffers(void);
++		void _dereferenceDepthBuffer( TextureGpu* depthBuffer );
++		void _dereferenceAllDepthBuffer();
++		void _referenceDepthBuffer( TextureGpu* depthBuffer );

//line 1404
		DepthBufferMap2 mDepthBufferPool2;
++		DepthBufferRefMap2 mDepthBufferReferences;



OgreCompositorPass.cpp

Code: Select all

...

// notifyRecreated()
    bool CompositorPass::notifyRecreated( const TextureGpu *channel )
    {

	...
		if( usedByUs )
		{
			mNumPassesLeft = mDefinition->mNumInitialPasses;
	
			//Reset texture pointers and setup RenderPassDescriptor again
++			RenderSystem *renderSystem = mParentNode->getRenderSystem();
++			//renderSystem->_dereferenceDepthBuffer(mRenderPassDesc->mDepth.texture);
++			if (mRenderPassDesc->mDepth.texture) {
++				renderSystem->_dereferenceDepthBuffer(mRenderPassDesc->mDepth.texture);
++	
++				if (mRenderPassDesc->mStencil.texture && mRenderPassDesc->mStencil.texture == mRenderPassDesc->mDepth.texture) {
++					renderSystem->_dereferenceDepthBuffer(mRenderPassDesc->mDepth.texture);
++				}
++			}
	
			mRenderPassDesc->mDepth.texture = 0;
			mRenderPassDesc->mStencil.texture = 0;
	
			...
		}
		
		...
}
...

OgreRoot.cpp -- full function

Code: Select all

    //-----------------------------------------------------------------------
    bool Root::_updateAllRenderTargets(void)
    {
       ...
	   
        for (SceneManagerEnumerator::SceneManagerIterator it = getSceneManagerIterator(); it.hasMoreElements(); it.moveNext())
            it.peekNextValue()->_handleLodEvents();
++		
++		// Release all the depth buffers which are no longer in use
++		mActiveRenderer->_cleanupDepthBuffers();

        return ret;
    }



OgreTextureGpu.cpp

Code: Select all

// Line 68 - constructor

--        mTexturePool( 0 )

++        mTexturePool( 0 ),
++		mDepthBufferPoolId( 0 )

...


// With the other 
++  //-----------------------------------------------------------------------------------
++	// Skyline tweak - setter and getter for a depth buffer texture to retrieve its slot in the depth buffer map
++	void TextureGpu::setDepthBufferPoolID(uint16 id) { mDepthBufferPoolId = id; }
++  //-----------------------------------------------------------------------------------
++	uint16 TextureGpu::getDepthBufferPoolID(void) const { return mDepthBufferPoolId; }
++	//-----------------------------------------------------------------------------------
OgreTextureGpu.h

Code: Select all

//Line 330: 

++		// Skyline tweak - setter and getter for a depth buffer texture to retrieve its slot in the depth buffer map
++		void setDepthBufferPoolID(uint16 id);
++		uint16 getDepthBufferPoolID(void) const;

// Line 230


        /// See TextureFlags::TextureFlags
        uint32      mTextureFlags;
        /// Used if hasAutomaticBatching() == true
        uint32		mPoolId;
++        /// Not used for anything but the depth buffers to identify the depth buffer pool they belong too. As its quicker than searching all depth buffers.
++        uint32		mDepthBufferPoolId;

If there is something missing, it will be obvious enough for you to write up :)

Hope this is accepatable and avoids the dangling pointer.

Edit: Just removed some log bits. refresh post :)
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

Mmmm this looks more robust than what I was thinking, but I have one complaint:

Code: Select all

/// Not used for anything but the depth buffers to identify the depth buffer pool they belong too. As its quicker than searching all depth buffers.
++        uint32		mDepthBufferPoolId;
I'm not a fan of penalizing every texture in existence because a couple TextureGpu that act as shared depth buffers need it.

However this is fixable.
TextureGpu::getDepthBufferPoolId is unused by depth buffers (it's only used by colour targets), which means we could use _setDepthBufferDefaults in RenderSystem::createDepthBufferFor to save the needed data there, avoiding those 4 extra bytes.

_setDepthBufferDefaults only works on TextureGpu with TextureFlags::RenderToTexture flag, but fortunately all Depth buffers have this flag set :)

Could you provide this as a git diff?
e.g. run:

Code: Select all

git diff > patch.diff
and post it here?

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

Sure I have done the recommended changes and have a patch file but just testing it in our engine first.its working in the ogre demo but they dont use a lot of depth buffers :P
Should be ready later

Edit: Come across a bug when using the depth buffer defaults in that some of the render pass descriptors are not depth buffers and do not hold a render target. Namely issues with the shadow textures. So once I fix this, I'll put the patch online for you. Just haven't had a lot of time sat at the pc today lol
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

Here is the patch file zipped so it could be uploaded to forum:
patch.zip
(4.3 KiB) Downloaded 19 times
Few Notes:
  • Moved the reference incrementer into the createDepthBufferFor function as it could also be initialised properly.
  • There is a new variable in TextureGpu which is only a byte large but it allows the user to know where the texture has been created, for instance is it a compositor texture, shadow texture, pool texture, standard texture or is it the depth buffer. I already had this variable in my version of Ogre(ours is far from vanilla ) and i have used it in the editor to know what the intention for the texture was. Its also a useful command for other information return or as i found out doing this depth buffer job; a very fast way of checking the texture source or origin, and if it was depth buffer allow it, otherwise deny as
  • The renderpassdesc.mDepth.texture was not always a depth buffer... very strange but i did get some wierd calls into my functions from the shadows which are self managed and deleted as standard textures in the compositor node causing fails with the destroytexture after the _setDepthBufferDefault changes. Also does not remove the "RenderWindow DepthBuffer"
  • Fully tested and working between Ogre and Skyline. i had to merge the files back into the ogre-next branch which is where i pull the latest changes too and then merge to my version. Perhaps not the most efficient but been doing it the same way for 10 years now and it feels normal to me lol
  • Changed the system over to your _setDepthBufferDefaults function(which is where i got the new issues because some mDepth.texture were not rendertargets) and added all the necessary changes to DX11 and OpenGL. I dont have the Metal rendersystem to add that change
  • Using the preexisting getDepthBufferPoolId() you mentioned
Hope this helps and that the 1 byte extra per texture is ok with you as it does serve a purpose and on my version of ogre, i have to have it regardless for one of the editors.

enjoy and stay safe!
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

Thanks!

Pushed your fix. Thanks for the patch! It saved me a lot of time!

I did not like the use of mSourceType for logic control, but I like the notion of stats keeping. So now mSourceType is still present for a few asserts, but it's not used by Ogre outside of that.

I made a couple changes, the commit describes them.
SolarPortal wrote:
Mon Jun 22, 2020 8:25 pm
[*] The renderpassdesc.mDepth.texture was not always a depth buffer... very strange but i did get some wierd calls into my functions from the shadows which are self managed and deleted as standard textures in the compositor node causing fails with the destroytexture after the _setDepthBufferDefault changes. Also does not remove the "RenderWindow DepthBuffer"
That happened because of this:
- _dereferenceSharedDepthBuffer original version accepted any input
(even non-shared depth buffers) thus adding zombie entries to
mSharedDepthBufferRefs. It later relied on destroySharedDepthBuffer's
mSourceType to avoid destroying these. Fixed by only accepting shared
depth buffer as input
renderpassdesc.mDepth.texture always contains a depth buffer. What it did not always contain was a shared one. I renamed your changes to make it explicit the code is handling shared depth buffers, not any depth buffer. Shared depth buffers despite being so common and frequent are actually the exception path in Ogre 2.2 (it's just that most people don't care or the concept of just using a pool_id for uniqueness and letting Ogre handle the rest is very convenient! Also originally from Ogre 1.x & 2.1 non-shared depth buffers were the exception and used the same path with a special pool id POOL_NON_SHAREABLE).

Once this patch is tested a bit more, I will backport this to Ogre 2.2 (it was pushed to 2.3); and in Ogre 2.3 will remove POOL_NON_SHAREABLE which has no use anymore.

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

brilliant, thanks for doing that :) Will test the master branch tomorrow as it seems the shadow biias branch has been merged. All changes look good to me and thanks for adding the source type in :)
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
SolarPortal
OGRE Contributor
OGRE Contributor
Posts: 203
Joined: Sat Jul 16, 2011 8:29 pm
Location: UK
x 50
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by SolarPortal »

seems to work from master branch
Lead developer of the Skyline Game Engine: https://aurasoft-skyline.co.uk

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 241
Joined: Wed Apr 23, 2014 3:49 pm
x 45

Re: [2.2] Depth Buffer Textures are never deleted

Post by TaaTT4 »

dark_sylinc wrote:
Wed Jun 24, 2020 8:03 pm
Once this patch is tested a bit more, I will backport this to Ogre 2.2 (it was pushed to 2.3); and in Ogre 2.3 will remove POOL_NON_SHAREABLE which has no use anymore.
I've just updated my repo to the latest master branch commit and I have an issue with POOL_NON_SHAREABLE depth buffers. I don't know if I'm on an edge case or simply I have to adapt something.

This is the situation:

Code: Select all

m_renderPass->mColour[0].texture = m_texture;
m_renderPass->mColour[0].loadAction = LoadAction::Clear;
m_renderPass->mColour[0].storeAction = StoreAction::Store;
m_renderPass->mColour[0].clearColour.a = 0.0f;

m_renderPass->mDepth.texture = Root::getSingleton().getRenderSystem()->getDepthBufferFor(m_texture, DepthBuffer::POOL_NON_SHAREABLE, false,
		PFG_D32_FLOAT_S8X24_UINT);

m_renderPass->mDepth.loadAction = LoadAction::Clear;
m_renderPass->mDepth.storeAction = StoreAction::DontCare;
m_renderPass->mStencil.texture = m_renderPass->mDepth.texture;
m_renderPass->mStencil.loadAction = m_renderPass->mDepth.loadAction;
m_renderPass->mStencil.storeAction = m_renderPass->mDepth.storeAction;
m_renderPass->entriesModified(RenderPassDescriptor::All);

auto renderTargetNative = NativeRenderTarget{};
m_renderPass->getCustomAttribute("ID3D11RenderTargetView", &renderTargetNative.Texture, 0);
m_renderPass->getCustomAttribute("ID3D11DepthStencilView", &renderTargetNative.DepthStencilTexture, 0);

viewRenderer = WebPageManager::getSingleton().renderer()->CreateViewRenderer(m_view, renderTargetNative, m_texture->getWidth(),
		m_texture->getHeight(), 1);
I have to create a bunch of RenderPassDescriptor to use with a 3rd party library (Coherent GT) which I use to render the UI. Until today, I used POOL_NON_SHAREABLE to be sure that every RenderPassDescriptor had its own depth buffer. Problem is that using POOL_NON_SHAREABLE causes depth buffer to be created with SharedDepthBuffer type and zero refcount. And when I destroy the RenderPassDescriptor the following assert is raised in RenderSystem::_dereferenceSharedDepthBuffer:

Code: Select all

            OGRE_ASSERT_LOW( itor->second > 0u && "Releasing a shared depth buffer too much" );
I know that I could have used an unique pool id for every RenderPassDescriptor that I create (and probably doing so wouldn't have caused the issue), but since I don't have the need to read the depth buffer content, POOL_NON_SHAREABLE was an easier and ready approach than managing an unique pool id set.

Thoughts?
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 241
Joined: Wed Apr 23, 2014 3:49 pm
x 45

Re: [2.2] Depth Buffer Textures are never deleted

Post by TaaTT4 »

Took a further look at it.
TaaTT4 wrote:
Tue Jun 30, 2020 9:54 am
Problem is that using POOL_NON_SHAREABLE causes depth buffer to be created with SharedDepthBuffer type and zero refcount.
This is not really true: the refcount is set to one.
TaaTT4 wrote:
Tue Jun 30, 2020 9:54 am
And when I destroy the RenderPassDescriptor the following assert is raised in RenderSystem::_dereferenceSharedDepthBuffer:

Code: Select all

            OGRE_ASSERT_LOW( itor->second > 0u && "Releasing a shared depth buffer too much" );
This happens because the depth buffer is dereferenced twice: once for itself and once for the stencil buffer (depth and stencil share the same texture).

I've temporarily solved replacing POOL_NON_SHAREABLE with a fixed pool id and

Code: Select all

m_renderPass->mStencil.texture = m_renderPass->mDepth.texture;
with

Code: Select all

m_renderPass->mStencil.texture = Root::getSingleton().getRenderSystem()->getDepthBufferFor(m_texture, /*DepthBuffer::POOL_NON_SHAREABLE*/ 23, false,
		PFG_D32_FLOAT_S8X24_UINT);
Anyway, my considerations remain valid: I find POOL_NON_SHAREABLE useful for advanced stuff. What is missing is let RenderSystem::referenceSharedDepthBuffer being public.
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

TaaTT4 wrote:
Tue Jun 30, 2020 1:57 pm
Anyway, my considerations remain valid: I find POOL_NON_SHAREABLE useful for advanced stuff. What is missing is let RenderSystem::referenceSharedDepthBuffer being public.
The plan is to remove POOL_NON_SHAREABLE. If you want a depth buffer unique for yourself, you can create a depth buffer:

Code: Select all

mTexture = textureManager->createTexture( texName, GpuPageOutStrategy::Discard,
                                                      TextureFlags::RenderToTexture, TextureTypes::Type2D );
mTexture->setPixelFormat( PFG_D32_FLOAT_S8X24_UINT );
// mTexture-> set to resident and resolution and stuff
m_renderPass->mDepth.texture = mTexture;
m_renderPass->mStencil.texture = mTexture;
In script code:

Code: Select all

compositor_node ScreenSpaceReflectionsRenderingNode
{
	texture gBufferNormals			target_width target_height PFG_R10G10B10A2_UNORM	msaa_auto explicit_resolve
	texture gBufferShadowRoughness	target_width target_height PFG_RG16_UNORM			msaa_auto explicit_resolve
	texture gBufferDepthBuffer		target_width target_height PFG_D32_FLOAT			msaa_auto

	rtv mrtGBuffer
	{
		colour	gBufferNormals gBufferShadowRoughness
		depth	gBufferDepthBuffer
	}

	target mrtGBuffer
	{
	}
}
Note that you can also alter auto generated RTVs (the compositor auto generates an RTV with the same name as declared textures):

Code: Select all

compositor_node MyNode
{
	in 0 myInput
	texture customDepthBuffer		target_width target_height PFG_D32_FLOAT			msaa_auto

	// myInput was already auto generated. We're just modifying it
	rtv myInput
	{
		depth	customDepthBuffer
	}

	target myInput
	{
	}
}
If you want to use the depth pool IDs due to convenience, you can just make sure the pool IDs are not reused?
Does this sound user hostile or is it ok?

In Ogre 2.1 and earlier non shareable depth pools were a hack, particularly used for shadow mapping. But in Ogre 2.2 the RenderSystem only manages shareable depth buffers. Non-shareable depth buffers are just regular TextureGpu.

Cheers
Matias

User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 241
Joined: Wed Apr 23, 2014 3:49 pm
x 45

Re: [2.2] Depth Buffer Textures are never deleted

Post by TaaTT4 »

dark_sylinc wrote:
Tue Jun 30, 2020 3:58 pm
If you want a depth buffer unique for yourself, you can create a depth buffer:

Code: Select all

mTexture = textureManager->createTexture( texName, GpuPageOutStrategy::Discard,
                                                      TextureFlags::RenderToTexture, TextureTypes::Type2D );
mTexture->setPixelFormat( PFG_D32_FLOAT_S8X24_UINT );
// mTexture-> set to resident and resolution and stuff
m_renderPass->mDepth.texture = mTexture;
m_renderPass->mStencil.texture = mTexture;
I hadn't thought at this approach. As you said, in OGRE 2.1 depth buffers were treated in an hacky way so I had sticked to RenderSystem::getDepthBufferFor as a reminiscence.
dark_sylinc wrote:
Tue Jun 30, 2020 3:58 pm
If you want to use the depth pool IDs due to convenience, you can just make sure the pool IDs are not reused?
Does this sound user hostile or is it ok?
I just want to create some RenderPassDescriptor being sure that their depth buffers are not shared anywhere else. I have no need to read their content back so I have no need to go through pool IDs. The method you suggested me above should be good enough for me. I'll try it, thanks!
Senior game programmer at Vae Victis
Working on Racecraft

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

I'm working on Unit Testing because lately coverage has been outpacing me (this is a good sign because users are contributing more).

This unit testing has already uncovered a bug in Normal Offset Mapping but it also uncovered a bug in LocalCubemaps sample.

If you press enough keys (F2/F3/F7/F8) eventually the reflections start to flicker. This is hard to reproduce by hand but Unit Testing records the keystrokes and lets you playback to the error.

Investigation revealed that sometimes the sample recreates the workspace; and for some reason late at some point shared depth buffers get destroyed but they are later attempted to be used.

I haven't yet located what's exactly wrong but it seems to be related to shared depth buffers.

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 4501
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 936
Contact:

Re: [2.2] Depth Buffer Textures are never deleted

Post by dark_sylinc »

OH MY GOD.

I'm already loving this unit test suite. I've found a bug that would usually be hard or tedious to repro and was able to analyze its trigger conditions in 10 minutes, what could otherwise have taken hours.

There was no bug in depth buffer sharing, it just exposed a dormant bug that only affected GL3+.

Post Reply