Skip to content

Commit

Permalink
Add TextureGpu::scheduleReupload
Browse files Browse the repository at this point in the history 
Behavior of TextureGpu::notifyDataIsReady changed. Calling it
excessively is now considered incorrect.

Documented in #101

Added mDataPreparationsPending which is required. Otherwise calling
scheduleReupload (>=1x times) then destroying the texture will
immediately destroy the pointer while it may actually be still used in
the worker thread.

Previously this was not needed because if a texture was in the worker
thread, then it meant notifyDataIsReady had never been called yet since
the last time we transitioned to Resident; thus destroyTexture would
automatically delay the destruction.
  • Loading branch information
@darksylinc
darksylinc committed May 15, 2021
1 parent c6d080b commit 829d228
Show file tree
Hide file tree
Showing 18 changed files with 195 additions and 49 deletions.
2 changes: 0 additions & 2 deletions Components/Overlay/src/OgreFont.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,6 @@ namespace Ogre
OGRE_FREE_SIMD( imageData, MEMCATEGORY_RESOURCE );
imageData = 0;

mTexture->notifyDataIsReady();

FT_Done_FreeType(ftLibrary);
}
//---------------------------------------------------------------------
Expand Down
2 changes: 0 additions & 2 deletions OgreMain/include/OgreStagingTexture.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ namespace Ogre
stagingTexture->upload( box0, texture0 );
stagingTexture->upload( box1, texture1 );
textureManager.removeStagingTexture( stagingTexture );
texture0->notifyDataIsReady();
texture1->notifyDataIsReady();
@par
There are other possibilities, as you can request a StagingTexture with twice the
width & height and then start calling mapRegion with smaller textures and we'll
Expand Down
43 changes: 43 additions & 0 deletions OgreMain/include/OgreTextureGpu.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,15 @@ namespace Ogre
/// @see TextureSourceType::TextureSourceType
uint8 mSourceType;

/// See TextureGpu::_isDataReadyImpl
///
/// It is increased with every scheduleReupload/scheduleTransitionTo (to resident)
/// It is decreased every time the loading is done
///
/// _isDataReadyImpl can NOT return true if mDataReady != 0
/// _isDataReadyImpl CAN return false if mDataReady == 0
uint8 mDataPreparationsPending;

/// This setting can only be altered if mResidencyStatus == OnStorage).
TextureTypes::TextureTypes mTextureType;
PixelFormatGpu mPixelFormat;
Expand Down Expand Up @@ -387,6 +396,40 @@ namespace Ogre
void scheduleTransitionTo( GpuResidency::GpuResidency nextResidency,
Image2 *image=0, bool autoDeleteImage=true );

/** There are times where you want to reload a texture again (e.g. file on
disk changed, uploading a new Image2, etc) without visual disruption.
e.g. if you were to call:
@code
tex->scheduleTransitionTo( GpuResidency::OnStorage );
tex->scheduleTransitionTo( GpuResidency::Resident, ... );
@endcode
you'll achieve the same result, however the texture becomes immediately
unavailable causing a few frames were all the user sees is a blank texture
until it is fully reloaded.
This routine allows for an in-place hot-reload, where the old texture
is swapped for the new one once it's done loading.
This is also faster because DescriptorTextureSets don't change
@remarks
1. Assumes the last queued transition to perform is into
Resident or OnSystemRam
2. Visual hitches are unavoidable if metadata changes (e.g. new
texture is of different pixel format, different number of
mipmaps, resolution, etc)
If that's the case, it is faster to transition to OnStorage,
remove the metadata entry from cache, then to Resident again
@param image
See TextureGpu::unsafeScheduleTransitionTo
@param autoDeleteImage
Same TextureGpu::unsafeScheduleTransitionTo
*/
void scheduleReupload( Image2 *image = 0, bool autoDeleteImage = true );

// See isMetadataReady for threadsafety on these functions.
void setResolution( uint32 width, uint32 height, uint32 depthOrSlices=1u );
uint32 getWidth(void) const;
Expand Down
28 changes: 17 additions & 11 deletions OgreMain/include/OgreTextureGpuManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,13 +469,15 @@ namespace Ogre
GpuResidency::GpuResidency targetResidency;
Image2 *image;
bool autoDeleteImage;
bool reuploadOnly;

void init( GpuResidency::GpuResidency _targetResidency,
Image2 *_image, bool _autoDeleteImage )
void init( GpuResidency::GpuResidency _targetResidency, Image2 *_image,
bool _autoDeleteImage, bool _reuploadOnly )
{
targetResidency = _targetResidency;
image = _image;
image = _image;
autoDeleteImage = _autoDeleteImage;
reuploadOnly = _reuploadOnly;
}
};

Expand Down Expand Up @@ -742,7 +744,7 @@ namespace Ogre

/// It is not enough to call waitForStreamingCompletion to render
/// single frame with all textures loaded, as new loading requests
/// could be added during frame rendering. In this case waiting
/// could be added during frame rendering. In this case waiting
/// and rendering could be repeated to avoid the problem.
bool hasNewLoadRequests() const { return mAddedNewLoadRequestsSinceWaitingForStreamingCompletion; }

Expand Down Expand Up @@ -1159,8 +1161,8 @@ namespace Ogre
VaoManager* getVaoManager(void) const;

protected:
void scheduleLoadRequest( TextureGpu *texture, Image2 *image,
bool autoDeleteImage, bool toSysRam );
void scheduleLoadRequest( TextureGpu *texture, Image2 *image, bool autoDeleteImage,
bool toSysRam, bool reuploadOnly );

/// Transitions a texture from OnSystemRam to Resident; and asks the worker thread
/// to transfer the data in the background.
Expand All @@ -1169,14 +1171,18 @@ namespace Ogre
/// disk and loading listeners.
void scheduleLoadFromRam( TextureGpu *texture );

void scheduleLoadRequest( TextureGpu *texture,
const String &name, const String &resourceGroup,
void scheduleLoadRequest( TextureGpu *texture, const String &name, const String &resourceGroup,
uint32 filters, Image2 *image, bool autoDeleteImage, bool toSysRam,
bool skipMetadataCache=false,
uint32 sliceOrDepth=std::numeric_limits<uint32>::max() );
bool reuploadOnly, bool skipMetadataCache = false,
uint32 sliceOrDepth = std::numeric_limits<uint32>::max() );

public:
void _scheduleUpdate( TextureGpu *texture, uint32 filters, Image2 *image, bool autoDeleteImage,
bool skipMetadataCache = false,
uint32 sliceOrDepth = std::numeric_limits<uint32>::max() );

void _scheduleTransitionTo( TextureGpu *texture, GpuResidency::GpuResidency targetResidency,
Image2 *image, bool autoDeleteImage );
Image2 *image, bool autoDeleteImage, bool reuploadOnly );
void _queueDownloadToRam( TextureGpu *texture, bool resyncOnly );
/// When true we will ignore all tasks in mScheduledTasks and execute transitions immediately
/// Caller is responsible for ensuring this is safe to do.
Expand Down
45 changes: 44 additions & 1 deletion OgreMain/src/OgreTextureGpu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ namespace Ogre
mNumMipmaps( 1 ),
mInternalSliceStart( 0 ),
mSourceType( TextureSourceType::Standard ),
mDataPreparationsPending( 0u ),
mTextureType( initialType ),
mPixelFormat( PFG_UNKNOWN ),
mTextureFlags( textureFlags ),
Expand Down Expand Up @@ -161,7 +162,7 @@ namespace Ogre
else
{
//Schedule transition, we'll be loading from a worker thread.
mTextureManager->_scheduleTransitionTo( this, nextResidency, image, autoDeleteImage );
mTextureManager->_scheduleTransitionTo( this, nextResidency, image, autoDeleteImage, false );
}
}
//-----------------------------------------------------------------------------------
Expand All @@ -172,6 +173,25 @@ namespace Ogre
unsafeScheduleTransitionTo( nextResidency, image, autoDeleteImage );
}
//-----------------------------------------------------------------------------------
void TextureGpu::scheduleReupload( Image2 *image, bool autoDeleteImage )
{
OGRE_ASSERT_LOW( mNextResidencyStatus != GpuResidency::OnStorage );
OGRE_ASSERT_LOW( mDataPreparationsPending < std::numeric_limits<uint8>::max() &&
"Overflow. Too many transitions queued up" );

if( mNextResidencyStatus == GpuResidency::OnSystemRam )
{
scheduleTransitionTo( Ogre::GpuResidency::OnStorage );
scheduleTransitionTo( Ogre::GpuResidency::OnSystemRam, image, autoDeleteImage );
}
else
{
++mDataPreparationsPending;
mTextureManager->_scheduleTransitionTo( this, GpuResidency::Resident, image, autoDeleteImage,
true );
}
}
//-----------------------------------------------------------------------------------
void TextureGpu::setResolution( uint32 width, uint32 height, uint32 depthOrSlices )
{
assert( mResidencyStatus == GpuResidency::OnStorage || isRenderWindowSpecific() );
Expand Down Expand Up @@ -479,6 +499,25 @@ namespace Ogre
bool allowResidencyChange = true;
TextureGpuListener::Reason listenerReason = TextureGpuListener::Unknown;

if( mResidencyStatus == GpuResidency::Resident )
{
const uint8 savedValue = mDataPreparationsPending;
mDataPreparationsPending = 0u;
const bool bWasDataReady = _isDataReadyImpl();
mDataPreparationsPending = savedValue;

if( !bWasDataReady )
{
OGRE_ASSERT_LOW( mDataPreparationsPending > 0u && "This should never happen" );
// If we reach here, then we're aborting a load. e.g.
// user went to Resident then to OnStorage without ever
// calling notifyDataIsReady()
// This is perfectly valid, and can happen if the texture
// metadata cache got out of date for example.
--mDataPreparationsPending;
}
}

if( newResidency == GpuResidency::Resident )
{
if( mPageOutStrategy != GpuPageOutStrategy::AlwaysKeepSystemRamCopy )
Expand All @@ -496,6 +535,10 @@ namespace Ogre
mSysRamCopy = sysRamCopy;
}

OGRE_ASSERT_LOW( mDataPreparationsPending < std::numeric_limits<uint8>::max() &&
"Overflow. Too many transitions queued up" );
++mDataPreparationsPending;

transitionToResident();
listenerReason = TextureGpuListener::GainedResidency;
}
Expand Down
68 changes: 46 additions & 22 deletions OgreMain/src/OgreTextureGpuManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1440,15 +1440,19 @@ namespace Ogre

const GpuResidency::GpuResidency targetResidency = task.residencyTransitionTask.targetResidency;

if( texture->getResidencyStatus() == GpuResidency::OnStorage )
if( texture->getResidencyStatus() == GpuResidency::OnStorage ||
task.residencyTransitionTask.reuploadOnly )
{
OGRE_ASSERT_MEDIUM( targetResidency == GpuResidency::Resident ||
targetResidency == GpuResidency::OnSystemRam );
OGRE_ASSERT_MEDIUM( !task.residencyTransitionTask.reuploadOnly ||
texture->getResidencyStatus() == GpuResidency::Resident );

scheduleLoadRequest( texture,
task.residencyTransitionTask.image,
task.residencyTransitionTask.autoDeleteImage,
targetResidency == GpuResidency::OnSystemRam );
targetResidency == GpuResidency::OnSystemRam,
task.residencyTransitionTask.reuploadOnly );
}
else
{
Expand Down Expand Up @@ -1631,16 +1635,15 @@ namespace Ogre
return mVaoManager;
}
//-----------------------------------------------------------------------------------
void TextureGpuManager::scheduleLoadRequest( TextureGpu *texture,
const String &name,
const String &resourceGroup,
uint32 filters,
Image2 *image,
bool autoDeleteImage,
bool toSysRam,
bool skipMetadataCache,
void TextureGpuManager::scheduleLoadRequest( TextureGpu *texture, const String &name,
const String &resourceGroup, uint32 filters,
Image2 *image, bool autoDeleteImage, bool toSysRam,
bool reuploadOnly, bool skipMetadataCache,
uint32 sliceOrDepth )
{
// These two can't be true at the same time
OGRE_ASSERT_MEDIUM( !( toSysRam && reuploadOnly ) );

Archive *archive = 0;
ResourceLoadingListener *loadingListener = 0;
if( resourceGroup != BLANKSTRING )
Expand All @@ -1658,7 +1661,7 @@ namespace Ogre
archive = resourceGroupManager._getArchiveToResource( name, resourceGroup );
}

if( !skipMetadataCache && !toSysRam &&
if( !skipMetadataCache && !toSysRam && !reuploadOnly &&
texture->getGpuPageOutStrategy() != GpuPageOutStrategy::AlwaysKeepSystemRamCopy )
{
bool metadataSuccess = applyMetadataCacheTo( texture );
Expand All @@ -1677,10 +1680,29 @@ namespace Ogre
mWorkerWaitableEvent.wake();
}
//-----------------------------------------------------------------------------------
void TextureGpuManager::_scheduleUpdate( TextureGpu *texture, uint32 filters, Image2 *image, bool autoDeleteImage,
bool skipMetadataCache,
uint32 sliceOrDepth )
{
Archive *archive = 0;
ResourceLoadingListener *loadingListener = 0;

mAddedNewLoadRequests = true;
mAddedNewLoadRequestsSinceWaitingForStreamingCompletion = true;
ThreadData &mainData = mThreadData[c_mainThread];
mLoadRequestsMutex.lock();
mainData.loadRequests.push_back( LoadRequest( "", archive, loadingListener, image,
texture, sliceOrDepth, filters,
autoDeleteImage, false ) );
mLoadRequestsMutex.unlock();
mWorkerWaitableEvent.wake();
}
//-----------------------------------------------------------------------------------
void TextureGpuManager::scheduleLoadRequest( TextureGpu *texture, Image2 *image,
bool autoDeleteImage, bool toSysRam )
bool autoDeleteImage, bool toSysRam, bool reuploadOnly )
{
OGRE_ASSERT_LOW( texture->getResidencyStatus() == GpuResidency::OnStorage );
OGRE_ASSERT_LOW( ( texture->getResidencyStatus() == GpuResidency::OnStorage && !reuploadOnly ) ||
( texture->getResidencyStatus() == GpuResidency::Resident && reuploadOnly ) );

String name, resourceGroup;
uint32 filters = 0;
Expand All @@ -1695,7 +1717,7 @@ namespace Ogre
if( texture->getTextureType() != TextureTypes::TypeCube )
{
scheduleLoadRequest( texture, name, resourceGroup, filters,
image, autoDeleteImage, toSysRam );
image, autoDeleteImage, toSysRam, reuploadOnly );
}
else
{
Expand All @@ -1720,7 +1742,7 @@ namespace Ogre
// XX HACK there should be a better way to specify whether
// all faces are in the same file or not
scheduleLoadRequest( texture, name, resourceGroup, filters,
image, autoDeleteImage, toSysRam );
image, autoDeleteImage, toSysRam, reuploadOnly );
}
else
{
Expand All @@ -1729,9 +1751,9 @@ namespace Ogre
for( uint32 i=0; i<6u; ++i )
{
const bool skipMetadataCache = i != 0;
scheduleLoadRequest( texture, baseName + suffixes[i] + ext,
resourceGroup, filters, i == 0 ? image : 0,
autoDeleteImage, toSysRam, skipMetadataCache, i );
scheduleLoadRequest( texture, baseName + suffixes[i] + ext, resourceGroup, filters,
i == 0 ? image : 0, autoDeleteImage, toSysRam, reuploadOnly,
skipMetadataCache, i );
}
}
}
Expand Down Expand Up @@ -1796,16 +1818,18 @@ namespace Ogre
//-----------------------------------------------------------------------------------
void TextureGpuManager::_scheduleTransitionTo( TextureGpu *texture,
GpuResidency::GpuResidency targetResidency,
Image2 *image, bool autoDeleteImage )
Image2 *image, bool autoDeleteImage,
bool reuploadOnly )
{
ScheduledTasks task;
task.tasksType = TaskTypeResidencyTransition;
task.residencyTransitionTask.init( targetResidency, image, autoDeleteImage );
task.residencyTransitionTask.init( targetResidency, image, autoDeleteImage, reuploadOnly );

//getPendingResidencyChanges should be > 1 because it gets incremented by caller
OGRE_ASSERT_MEDIUM( texture->getPendingResidencyChanges() != 0u );
OGRE_ASSERT_MEDIUM( texture->getPendingResidencyChanges() != 0u || reuploadOnly );

if( texture->getPendingResidencyChanges() == 1u || mIgnoreScheduledTasks )
if( texture->getPendingResidencyChanges() == 1u ||
( reuploadOnly && texture->getPendingResidencyChanges() == 0u ) || mIgnoreScheduledTasks )
{
//If we're here, there are no pending tasks that will perform further work
//on the texture (with one exception: if _isDataReadyImpl does not return true; which
Expand Down
7 changes: 6 additions & 1 deletion RenderSystems/Direct3D11/src/OgreD3D11TextureGpu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,11 @@ namespace Ogre
assert( mResidencyStatus == GpuResidency::Resident );
assert( mFinalTextureName || mPixelFormat == PFG_NULL );

OGRE_ASSERT_LOW( mDataPreparationsPending > 0u &&
"Calling notifyDataIsReady too often! Remove this call"
"See https://github.com/OGRECave/ogre-next/issues/101" );
--mDataPreparationsPending;

mDefaultDisplaySrv.Reset();

mDisplayTextureName = mFinalTextureName.Get();
Expand All @@ -379,7 +384,7 @@ namespace Ogre
//-----------------------------------------------------------------------------------
bool D3D11TextureGpu::_isDataReadyImpl(void) const
{
return mDisplayTextureName == mFinalTextureName.Get();
return mDisplayTextureName == mFinalTextureName.Get() && mDataPreparationsPending == 0u;
}
//-----------------------------------------------------------------------------------
void D3D11TextureGpu::_setToDisplayDummyTexture(void)
Expand Down
4 changes: 4 additions & 0 deletions RenderSystems/Direct3D11/src/OgreD3D11TextureGpuWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ namespace Ogre
void D3D11TextureGpuWindow::notifyDataIsReady(void)
{
assert( mResidencyStatus == GpuResidency::Resident );
OGRE_ASSERT_LOW( mDataPreparationsPending > 0u &&
"Calling notifyDataIsReady too often! Remove this call"
"See https://github.com/OGRECave/ogre-next/issues/101" );
--mDataPreparationsPending;
notifyAllListenersTextureChanged( TextureGpuListener::ReadyForRendering );
}
//-----------------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 829d228

Please sign in to comment.