Don't store a pointer to some resource, maybe it was deleted and you …

…didn't know
Nemirtingas · Oct 13, 2024 · f85b47e · f85b47e
1 parent 2d78b78
commit f85b47e
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 57 deletions.
diff --git a/src/RendererHookInternal.cpp b/src/RendererHookInternal.cpp
@@ -37,21 +37,10 @@ void RendererHookInternal_t::_LoadResources()
 	if (_ResourcesToLoad.empty())
 		return;
 
-	for (auto& resource : _OldResourcesToUnload)
-		resource->UnloadOldResource();
-
-	_OldResourcesToUnload.clear();
-
 	auto batchSize = _ResourcesToLoad.size() > _BatchSize ? _BatchSize : _ResourcesToLoad.size();
 
 	for (int i = 0; i < batchSize; ++i)
-	{
-		// Remove old resources next frame, it might be currently in use.
-		if (_ResourcesToLoad[i]->AttachementChanged())
-			_OldResourcesToUnload.emplace_back(_ResourcesToLoad[i]);
-
 		_ResourcesToLoad[i]->LoadAttachedResource();
-	}
 
 	_ResourcesToLoad.erase(_ResourcesToLoad.begin(), _ResourcesToLoad.begin() + batchSize);
 }

diff --git a/src/RendererHookInternal.h b/src/RendererHookInternal.h
@@ -35,7 +35,6 @@ class RendererHookInternal_t : public RendererHook_t
     ResourceAutoLoad_t _AutoLoad;
     uint32_t _BatchSize;
     std::vector<RendererResourceInternal_t*> _ResourcesToLoad;
-    std::vector<RendererResourceInternal_t*> _OldResourcesToUnload;
 
 protected:
     RendererHookInternal_t();

diff --git a/src/RendererResourceInternal.cpp b/src/RendererResourceInternal.cpp
@@ -26,9 +26,7 @@ namespace InGameOverlay {
 RendererResourceInternal_t::RendererResourceInternal_t(RendererHookInternal_t* rendererHook, ResourceAutoLoad_t autoLoad) noexcept :
 	_RendererHook(rendererHook),
 	_AutoLoad(autoLoad),
-	_Data(nullptr),
-	_Width(0),
-	_Height(0)
+	_Data(nullptr)
 {
 }
 
@@ -43,14 +41,6 @@ bool RendererResourceInternal_t::_DoBatchLoad()
 	return false;
 }
 
-bool RendererResourceInternal_t::_DoImmediateLoad()
-{
-	Unload(false);
-
-	_RendererResource = _RendererHook->CreateImageResource(_Data, _Width, _Height);
-	return IsLoaded();
-}
-
 bool RendererResourceInternal_t::_DoAutoLoad()
 {
 	if (_AutoLoad == ResourceAutoLoad_t::None || !CanBeLoaded())
@@ -59,7 +49,8 @@ bool RendererResourceInternal_t::_DoAutoLoad()
 	if (_AutoLoad == ResourceAutoLoad_t::Batch)
 		return _DoBatchLoad();
 
-	return _DoImmediateLoad();
+	UnloadOldResource();
+	return LoadAttachedResource();
 }
 
 void RendererResourceInternal_t::Delete()
@@ -69,7 +60,7 @@ void RendererResourceInternal_t::Delete()
 
 bool RendererResourceInternal_t::IsLoaded() const
 {
-	return !_RendererResource.expired();
+	return !_RendererResource.RendererResource.expired();
 }
 
 ResourceAutoLoad_t RendererResourceInternal_t::AutoLoad() const
@@ -100,20 +91,20 @@ bool RendererResourceInternal_t::LoadAttachedResource()
 	if (!CanBeLoaded())
 		return false;
 
-	_RendererResource = _RendererHook->CreateImageResource(_Data, _Width, _Height);
-	return !_RendererResource.expired();
+	_RendererResource.RendererResource = _RendererHook->CreateImageResource(_Data, _RendererResource.Width, _RendererResource.Height);
+	return IsLoaded();
 }
 
 bool RendererResourceInternal_t::Load(const void* data, uint32_t width, uint32_t height)
 {
 	Unload(true);
 
 	_Data = nullptr;
-	_Width = width;
-	_Height = height;
-	_OldRendererResource.reset();
-	_RendererResource = _RendererHook->CreateImageResource(data, width, height);
-	return !_RendererResource.expired();
+	_RendererResource.Width = width;
+	_RendererResource.Height = height;
+	_OldRendererResource.Reset();
+	_RendererResource.RendererResource = _RendererHook->CreateImageResource(data, width, height);
+	return IsLoaded();
 }
 
 uint64_t RendererResourceInternal_t::GetResourceId()
@@ -126,13 +117,17 @@ uint64_t RendererResourceInternal_t::GetResourceId()
 
 	if (IsLoaded())
 	{
-		auto r = _RendererResource.lock();
+		// When in batch autoload, the old resource might still be loaded, unload it now.
+		if (AttachementChanged())
+			UnloadOldResource();
+
+		auto r = _RendererResource.RendererResource.lock();
 		if (r != nullptr)
 			return *r;
 	}
 	if (AttachementChanged())
 	{
-		auto r = _OldRendererResource.lock();
+		auto r = _OldRendererResource.RendererResource.lock();
 		if (r != nullptr)
 			return *r;
 	}
@@ -142,60 +137,58 @@ uint64_t RendererResourceInternal_t::GetResourceId()
 
 uint32_t RendererResourceInternal_t::Width() const
 {
-	return _Width;
+	return IsLoaded()
+		? _RendererResource.Width
+		: _OldRendererResource.Width;
 }
 
 uint32_t RendererResourceInternal_t::Height() const
 {
-	return _Height;
+	return IsLoaded()
+		? _RendererResource.Height
+		: _OldRendererResource.Height;
 }
 
 void RendererResourceInternal_t::AttachResource(const void* data, uint32_t width, uint32_t height)
 {
 	if (IsLoaded())
 		_OldRendererResource = _RendererResource;
 
-	_RendererResource.reset();
+	_RendererResource.RendererResource.reset();
 	_Data = data;
-	_Width = width;
-	_Height = height;
+	_RendererResource.Width = width;
+	_RendererResource.Height = height;
 }
 
 void RendererResourceInternal_t::ClearAttachedResource()
 {
 	_Data = nullptr;
-
-	if (!IsLoaded())
-	{
-		_Width = 0;
-		_Height = 0;
-	}
 }
 
 void RendererResourceInternal_t::Unload(bool clearAttachedResource)
 {
 	UnloadOldResource();
 
-	if (auto r = _RendererResource.lock())
-		_RendererHook->ReleaseImageResource(_RendererResource);
+	if (auto r = _RendererResource.RendererResource.lock())
+		_RendererHook->ReleaseImageResource(_RendererResource.RendererResource);
 
-	_RendererResource.reset();
+	_RendererResource.Reset();
 
 	if (clearAttachedResource)
 		ClearAttachedResource();
 }
 
 bool RendererResourceInternal_t::AttachementChanged()
 {
-	return !_OldRendererResource.expired();
+	return !_OldRendererResource.RendererResource.expired();
 }
 
 void RendererResourceInternal_t::UnloadOldResource()
 {
-	if (auto r = _OldRendererResource.lock())
-		_RendererHook->ReleaseImageResource(_OldRendererResource);
+	if (auto r = _OldRendererResource.RendererResource.lock())
+		_RendererHook->ReleaseImageResource(_OldRendererResource.RendererResource);
 
-	_OldRendererResource.reset();
+	_OldRendererResource.Reset();
 }
 
 }
diff --git a/src/RendererResourceInternal.h b/src/RendererResourceInternal.h
@@ -29,20 +29,31 @@ class RendererHookInternal_t;
 
 class RendererResourceInternal_t : public RendererResource_t
 {
+    struct ResourceState_t
+    {
+        std::weak_ptr<uint64_t> RendererResource;
+        uint32_t Width = 0;
+        uint32_t Height = 0;
+
+        inline void Reset()
+        {
+            RendererResource.reset();
+            Width = 0;
+            Height = 0;
+        }
+    };
+
 protected:
     RendererHookInternal_t* _RendererHook;
 
     bool _DoBatchLoad();
-    bool _DoImmediateLoad();
     bool _DoAutoLoad();
 
 public:
-    std::weak_ptr<uint64_t> _OldRendererResource;
-    std::weak_ptr<uint64_t> _RendererResource;
+    ResourceState_t _OldRendererResource;
+    ResourceState_t _RendererResource;
     ResourceAutoLoad_t _AutoLoad;
     const void* _Data;
-    uint32_t _Width;
-    uint32_t _Height;
 
     RendererResourceInternal_t(RendererHookInternal_t* rendererHook, ResourceAutoLoad_t autoLoad) noexcept;