Skip to content

Commit

Permalink
Fix shared depth buffers leaking (#103)
Browse files Browse the repository at this point in the history
Use a reference count system to track which shared depth buffers are in
use, and release those that reach 0 ref count

Additionally, added TextureGpu::getSourceType for useful stats tracking

Patch originally by SolarPortal.
See https://forums.ogre3d.org/viewtopic.php?p=548396#p548396

Modifications by Matias N. Goldberg:
- Original patch exposed several functions as public which shouldn't be
- Renamed "DepthBuffer" to "SharedDepthBuffer", since RenderSystem
doesn't manage all depth buffers. It only manages automatically
generated ones
- Moved mSourceType so that it takes advantage of preexisting padding
- _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
- Due to these zombie entries, negative ref count was possible. Got rid
of this and ref count is now unsigned. Underflow is considered an Ogre
bug and will assert
- Assert if user tries to call _setDepthBufferDefaults on a shared
depth buffer
- Original version did not release Stencil-only buffers (at the time
only iOS supports this)
- Added mSharedDepthBufferZeroRefCandidates so that the cost of
_cleanupDepthBuffers (which gets called every frame) is almost always
O(1), rather than being O(N) where N is the number of live shared depth
buffers.
- Ported changes to Metal
  • Loading branch information
darksylinc committed Jul 6, 2020
1 parent ae48695 commit 33bb1b3
Show file tree
Hide file tree
Showing 18 changed files with 201 additions and 19 deletions.
14 changes: 13 additions & 1 deletion OgreMain/include/OgreRenderSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ namespace Ogre

typedef vector<TextureGpu*>::type TextureGpuVec;
typedef map< uint16, TextureGpuVec >::type DepthBufferMap2;
typedef map<TextureGpu *, uint16>::type DepthBufferRefMap;
typedef set<TextureGpu *>::type TextureGpuSet;

/// Enum describing the ways to generate texture coordinates
enum TexCoordCalcMethod
Expand Down Expand Up @@ -755,9 +757,17 @@ namespace Ogre

protected:
virtual TextureGpu* createDepthBufferFor( TextureGpu *colourTexture, bool preferDepthTexture,
PixelFormatGpu depthBufferFormat );
PixelFormatGpu depthBufferFormat, uint16 poolId );

/// Detroys a depth buffer associated in the pool. If no texture is found the it skips.
void destroySharedDepthBuffer( TextureGpu *depthTexture );
void referenceSharedDepthBuffer( TextureGpu *depthBuffer );
public:
void _cleanupDepthBuffers( void );
/// Releases the reference count on a shared depth buffer.
/// Does nothing if input is not a shared depth buffer.
void _dereferenceSharedDepthBuffer( TextureGpu *depthBuffer );

virtual TextureGpu* getDepthBufferFor( TextureGpu *colourTexture, uint16 poolId,
bool preferDepthTexture,
PixelFormatGpu depthBufferFormat );
Expand Down Expand Up @@ -1394,6 +1404,8 @@ namespace Ogre
void destroyAllRenderPassDescriptors(void);

DepthBufferMap2 mDepthBufferPool2;
DepthBufferRefMap mSharedDepthBufferRefs;
TextureGpuSet mSharedDepthBufferZeroRefCandidates;

typedef set<RenderPassDescriptor*>::type RenderPassDescriptorSet;
RenderPassDescriptorSet mRenderPassDescs;
Expand Down
27 changes: 27 additions & 0 deletions OgreMain/include/OgreTextureGpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ namespace Ogre
PoolOwner = 1u << 13u
};
}

namespace TextureSourceType
{
enum TextureSourceType
{
Standard, /// Regular texture
Shadow, /// Created by compositor, for shadow mapping
Compositor, /// Created by compositor
PoolOwner, /// TextureFlags::PoolOwner is set
SharedDepthBuffer, /// Created automatically, may be shared and reused by multiple colour
/// targets
NumTextureSourceTypes
};
}

/**
@remarks
Internal layout of data in memory:
Expand Down Expand Up @@ -208,6 +223,14 @@ namespace Ogre
/// our actual data is, inside a texture array which we do not own.
uint16 mInternalSliceStart;

/// This setting is for where the texture is created, e.g. its a compositor texture, a shadow
/// texture or standard texture loaded for a mesh etc...
///
/// This value is merely for statistical tracking purposes
///
/// @see TextureSourceType::TextureSourceType
uint8 mSourceType;

/// This setting can only be altered if mResidencyStatus == OnStorage).
TextureTypes::TextureTypes mTextureType;
PixelFormatGpu mPixelFormat;
Expand Down Expand Up @@ -305,6 +328,10 @@ namespace Ogre
TextureTypes::TextureTypes getTextureType(void) const;
TextureTypes::TextureTypes getInternalTextureType(void) const;

void _setSourceType( uint8 type );
/// @copydoc TextureGpu::mSourceType
uint8 getSourceType( void ) const;

/** Sets the pixel format.
@remarks
If prefersLoadingFromFileAsSRGB() returns true, the format may not be fully honoured
Expand Down
13 changes: 13 additions & 0 deletions OgreMain/src/Compositor/OgreCompositorShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,19 @@ namespace Ogre
mShadowMapCameras.reserve( definition->mShadowMapTexDefinitions.size() );
mLocalTextures.reserve( mLocalTextures.size() + definition->mShadowMapTexDefinitions.size() );

// Set all textures to shadow tex source
{
CompositorChannelVec::iterator tempIt = mLocalTextures.begin();
CompositorChannelVec::iterator tempEn = mLocalTextures.end();
while( tempIt != tempEn )
{
TextureGpu *refTex = *tempIt;
if( refTex )
refTex->_setSourceType( TextureSourceType::Shadow );
++tempIt;
}
}

SceneManager *sceneManager = workspace->getSceneManager();
SceneNode *pseudoRootNode = 0;

Expand Down
1 change: 1 addition & 0 deletions OgreMain/src/Compositor/OgreTextureDefinition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ namespace Ogre
(textureDef.depthOrSlices == 1u || textureDef.textureType > TextureTypes::Type2D ) &&
(textureDef.depthOrSlices == 6 || textureDef.textureType != TextureTypes::TypeCube) );

tex->_setSourceType( TextureSourceType::Compositor );
tex->setResolution( width, height, textureDef.depthOrSlices );
if( textureDef.format != PFG_UNKNOWN )
tex->setPixelFormat( textureDef.format );
Expand Down
20 changes: 18 additions & 2 deletions OgreMain/src/Compositor/Pass/OgreCompositorPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,24 @@ namespace Ogre
mNumPassesLeft = mDefinition->mNumInitialPasses;

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

if( mRenderPassDesc->mStencil.texture &&
mRenderPassDesc->mStencil.texture == mRenderPassDesc->mDepth.texture )
{
renderSystem->_dereferenceSharedDepthBuffer( mRenderPassDesc->mStencil.texture );
mRenderPassDesc->mStencil.texture = 0;
}
}
if( mRenderPassDesc->mStencil.texture )
{
renderSystem->_dereferenceSharedDepthBuffer( mRenderPassDesc->mStencil.texture );
mRenderPassDesc->mStencil.texture = 0;
}

for( int i=0; i<OGRE_MAX_MULTIPLE_RENDER_TARGETS; ++i )
{
Expand Down
99 changes: 95 additions & 4 deletions OgreMain/src/OgreRenderSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,24 @@ namespace Ogre {
assert( itor != mRenderPassDescs.end() && "Already destroyed?" );
if( itor != mRenderPassDescs.end() )
mRenderPassDescs.erase( itor );

if( renderPassDesc->mDepth.texture )
{
_dereferenceSharedDepthBuffer( renderPassDesc->mDepth.texture );

if( renderPassDesc->mStencil.texture &&
renderPassDesc->mStencil.texture == renderPassDesc->mDepth.texture )
{
_dereferenceSharedDepthBuffer( renderPassDesc->mStencil.texture );
renderPassDesc->mStencil.texture = 0;
}
}
if( renderPassDesc->mStencil.texture )
{
_dereferenceSharedDepthBuffer( renderPassDesc->mStencil.texture );
renderPassDesc->mStencil.texture = 0;
}

delete renderPassDesc;
}
//---------------------------------------------------------------------
Expand Down Expand Up @@ -502,8 +520,71 @@ namespace Ogre {
mUavRenderingDirty = true;
}
//---------------------------------------------------------------------
void RenderSystem::destroySharedDepthBuffer( TextureGpu *depthBuffer )
{
TextureGpuVec &bufferVec = mDepthBufferPool2[depthBuffer->getDepthBufferPoolId()];
TextureGpuVec::iterator itor = std::find( bufferVec.begin(), bufferVec.end(), depthBuffer );
if( itor != bufferVec.end() )
{
efficientVectorRemove( bufferVec, itor );
mTextureGpuManager->destroyTexture( depthBuffer );
}
}
//---------------------------------------------------------------------
void RenderSystem::_cleanupDepthBuffers( void )
{
TextureGpuSet::const_iterator itor = mSharedDepthBufferZeroRefCandidates.begin();
TextureGpuSet::const_iterator endt = mSharedDepthBufferZeroRefCandidates.end();

while( itor != endt )
{
// When a shared depth buffer ends up in mSharedDepthBufferZeroRefCandidates,
// it's because its ref. count reached 0. However it may have been reacquired.
// We need to check its reference count is still 0 before deleting it.
DepthBufferRefMap::iterator itMap = mSharedDepthBufferRefs.find( *itor );
if( itMap != mSharedDepthBufferRefs.end() && itMap->second == 0u )
{
destroySharedDepthBuffer( *itor );
mSharedDepthBufferRefs.erase( itMap );
}
++itor;
}

mSharedDepthBufferZeroRefCandidates.clear();
}
//---------------------------------------------------------------------
void RenderSystem::referenceSharedDepthBuffer( TextureGpu *depthBuffer )
{
OGRE_ASSERT_MEDIUM( depthBuffer->getSourceType() == TextureSourceType::SharedDepthBuffer );
OGRE_ASSERT_MEDIUM( mSharedDepthBufferRefs.find( depthBuffer ) != mSharedDepthBufferRefs.end() );
++mSharedDepthBufferRefs[depthBuffer];
}
//---------------------------------------------------------------------
void RenderSystem::_dereferenceSharedDepthBuffer( TextureGpu *depthBuffer )
{
if( !depthBuffer )
return;

DepthBufferRefMap::iterator itor = mSharedDepthBufferRefs.find( depthBuffer );

if( itor != mSharedDepthBufferRefs.end() )
{
OGRE_ASSERT_MEDIUM( depthBuffer->getSourceType() == TextureSourceType::SharedDepthBuffer );
OGRE_ASSERT_LOW( itor->second > 0u && "Releasing a shared depth buffer too much" );
--itor->second;

if( itor->second == 0u )
mSharedDepthBufferZeroRefCandidates.insert( depthBuffer );
}
else
{
// This is not a shared depth buffer (e.g. one created by the user)
OGRE_ASSERT_MEDIUM( depthBuffer->getSourceType() != TextureSourceType::SharedDepthBuffer );
}
}
//---------------------------------------------------------------------
TextureGpu* RenderSystem::createDepthBufferFor( TextureGpu *colourTexture, bool preferDepthTexture,
PixelFormatGpu depthBufferFormat )
PixelFormatGpu depthBufferFormat, uint16 poolId )
{
uint32 textureFlags = TextureFlags::RenderToTexture;

Expand All @@ -517,13 +598,16 @@ namespace Ogre {
TextureGpu *retVal = mTextureGpuManager->createTexture( depthBufferName.c_str(),
GpuPageOutStrategy::Discard,
textureFlags, TextureTypes::Type2D );

retVal->setResolution( colourTexture->getWidth(), colourTexture->getHeight() );
retVal->setPixelFormat( depthBufferFormat );
retVal->_setDepthBufferDefaults( poolId, preferDepthTexture, depthBufferFormat );
retVal->_setSourceType( TextureSourceType::SharedDepthBuffer );
retVal->setSampleDescription( colourTexture->getRequestedSampleDescription() );

retVal->_transitionTo( GpuResidency::Resident, (uint8*)0 );

// Start reference count on the depth buffer here
mSharedDepthBufferRefs[retVal] = 1u;
return retVal;
}
//---------------------------------------------------------------------
Expand All @@ -544,7 +628,7 @@ namespace Ogre {
if( poolId == DepthBuffer::POOL_NON_SHAREABLE )
{
TextureGpu *retVal = createDepthBufferFor( colourTexture, preferDepthTexture,
depthBufferFormat );
depthBufferFormat, poolId );
return retVal;
}

Expand All @@ -562,6 +646,7 @@ namespace Ogre {
(*itor)->supportsAsDepthBufferFor( colourTexture ) )
{
retVal = *itor;
referenceSharedDepthBuffer( retVal );
}
else
{
Expand All @@ -573,7 +658,8 @@ namespace Ogre {
//Not found yet? Create a new one!
if( !retVal )
{
retVal = createDepthBufferFor( colourTexture, preferDepthTexture, depthBufferFormat );
retVal =
createDepthBufferFor( colourTexture, preferDepthTexture, depthBufferFormat, poolId );
mDepthBufferPool2[poolId].push_back( retVal );

if( !retVal )
Expand Down Expand Up @@ -640,6 +726,11 @@ namespace Ogre {
mHwOcclusionQueries.clear();

destroyAllRenderPassDescriptors();
_cleanupDepthBuffers();
OGRE_ASSERT_LOW( mSharedDepthBufferRefs.empty() &&
"destroyAllRenderPassDescriptors followed by _cleanupDepthBuffers should've "
"emptied mSharedDepthBufferRefs. Please report this bug to "
"https://github.com/OGRECave/ogre-next/issues/" );

OGRE_DELETE mTextureGpuManager;
mTextureGpuManager = 0;
Expand Down
8 changes: 7 additions & 1 deletion OgreMain/src/OgreRoot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1545,8 +1545,14 @@ namespace Ogre {
// This belongs here, as all render targets must be updated before events are
// triggered, otherwise targets could be mismatched. This could produce artifacts,
// for instance, with shadows.
for (SceneManagerEnumerator::SceneManagerIterator it = getSceneManagerIterator(); it.hasMoreElements(); it.moveNext())
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;
}
Expand Down
5 changes: 5 additions & 0 deletions OgreMain/src/OgreTextureGpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace Ogre
mDepthOrSlices( 0 ),
mNumMipmaps( 1 ),
mInternalSliceStart( 0 ),
mSourceType( TextureSourceType::Standard ),
mTextureType( initialType ),
mPixelFormat( PFG_UNKNOWN ),
mTextureFlags( textureFlags ),
Expand Down Expand Up @@ -261,6 +262,10 @@ namespace Ogre
return mInternalSliceStart;
}
//-----------------------------------------------------------------------------------
void TextureGpu::_setSourceType( uint8 type ) { mSourceType = type; }
//-----------------------------------------------------------------------------------
uint8 TextureGpu::getSourceType( void ) const { return mSourceType; }
//-----------------------------------------------------------------------------------
void TextureGpu::setSampleDescription( SampleDescription desc )
{
assert( mResidencyStatus == GpuResidency::OnStorage );
Expand Down
3 changes: 3 additions & 0 deletions OgreMain/src/OgreTextureGpuManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ namespace Ogre
newPool.usedMemory = 0;
newPool.usedSlots.reserve( numSlices );

newPool.masterTexture->_setSourceType( TextureSourceType::PoolOwner );
newPool.masterTexture->setResolution( width, height, numSlices );
newPool.masterTexture->setPixelFormat( pixelFormat );
newPool.masterTexture->setNumMipmaps( numMipmaps );
Expand Down Expand Up @@ -347,6 +348,7 @@ namespace Ogre

TextureGpu *retVal = createTextureImpl( pageOutStrategy, idName, textureFlags, initialType );
retVal->setTexturePoolId( poolId );
retVal->_setSourceType( TextureSourceType::Standard );

mEntriesMutex.lock();
mEntries[idName] = ResourceEntry( name, aliasName, resourceGroup, retVal, filters );
Expand Down Expand Up @@ -1884,6 +1886,7 @@ namespace Ogre
TextureFlags::PoolOwner,
TextureTypes::Type2DArray );
const uint16 numSlices = mTextureGpuManagerListener->getNumSlicesFor( texture, this );
newPool.masterTexture->_setSourceType( TextureSourceType::PoolOwner );

newPool.manuallyReserved = false;
newPool.usedMemory = 0;
Expand Down
2 changes: 1 addition & 1 deletion RenderSystems/Direct3D11/include/OgreD3D11RenderSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ namespace Ogre
virtual void endRenderPassDescriptor(void);

TextureGpu* createDepthBufferFor( TextureGpu *colourTexture, bool preferDepthTexture,
PixelFormatGpu depthBufferFormat );
PixelFormatGpu depthBufferFormat, uint16 poolId );

const String& getName(void) const;

Expand Down
4 changes: 2 additions & 2 deletions RenderSystems/Direct3D11/src/OgreD3D11RenderSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,7 @@ namespace Ogre
}
//-----------------------------------------------------------------------------------
TextureGpu* D3D11RenderSystem::createDepthBufferFor( TextureGpu *colourTexture, bool preferDepthTexture,
PixelFormatGpu depthBufferFormat )
PixelFormatGpu depthBufferFormat, uint16 poolId )
{
if( depthBufferFormat == PFG_UNKNOWN )
{
Expand All @@ -1592,7 +1592,7 @@ namespace Ogre
}

return RenderSystem::createDepthBufferFor( colourTexture, preferDepthTexture,
depthBufferFormat );
depthBufferFormat, poolId );
}
//---------------------------------------------------------------------
void D3D11RenderSystem::_notifyWindowDestroyed( Window *window )
Expand Down
2 changes: 2 additions & 0 deletions RenderSystems/Direct3D11/src/OgreD3D11TextureGpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,8 @@ namespace Ogre
uint16 depthBufferPoolId, bool preferDepthTexture, PixelFormatGpu desiredDepthBufferFormat )
{
assert( isRenderToTexture() );
OGRE_ASSERT_MEDIUM( mSourceType != TextureSourceType::SharedDepthBuffer &&
"Cannot call _setDepthBufferDefaults on a shared depth buffer!" );
mDepthBufferPoolId = depthBufferPoolId;
mPreferDepthTexture = preferDepthTexture;
mDesiredDepthBufferFormat = desiredDepthBufferFormat;
Expand Down
2 changes: 1 addition & 1 deletion RenderSystems/GL3Plus/include/OgreGL3PlusRenderSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ namespace Ogre {
virtual void endRenderPassDescriptor(void);

TextureGpu* createDepthBufferFor( TextureGpu *colourTexture, bool preferDepthTexture,
PixelFormatGpu depthBufferFormat );
PixelFormatGpu depthBufferFormat, uint16 poolId );

/** See
RenderSystem
Expand Down
Loading

0 comments on commit 33bb1b3

Please sign in to comment.