From 72a57fbf4c1f6972dc8f35171a9801f7034fd5cb Mon Sep 17 00:00:00 2001 From: Lynix Date: Mon, 26 Aug 2013 00:40:45 +0200 Subject: [PATCH] Fixed crash when resources in use by the Renderer are released Former-commit-id: 98eedb556f0387f0a5c1cafde2fc74645d1d0457 --- include/Nazara/Core/Resource.hpp | 6 +- include/Nazara/Renderer/Context.hpp | 8 +- include/Nazara/Renderer/OpenGL.hpp | 4 +- include/Nazara/Renderer/Renderer.hpp | 6 + src/Nazara/Core/Resource.cpp | 33 ++++- src/Nazara/Renderer/Context.cpp | 12 +- src/Nazara/Renderer/OpenGL.cpp | 6 +- src/Nazara/Renderer/RenderWindow.cpp | 4 +- src/Nazara/Renderer/Renderer.cpp | 189 +++++++++++++++++++++++--- src/Nazara/Renderer/ShaderProgram.cpp | 1 + src/Nazara/Renderer/Texture.cpp | 1 + 11 files changed, 228 insertions(+), 42 deletions(-) diff --git a/include/Nazara/Core/Resource.hpp b/include/Nazara/Core/Resource.hpp index b8ab9700c..cc9a5d9ab 100644 --- a/include/Nazara/Core/Resource.hpp +++ b/include/Nazara/Core/Resource.hpp @@ -42,10 +42,14 @@ class NAZARA_API NzResource void NotifyDestroy(); private: + typedef std::unordered_map> ResourceListenerMap; + + void RemoveResourceListenerIterator(ResourceListenerMap::iterator iterator) const; + NazaraMutexAttrib(m_mutex, mutable) // Je fais précéder le nom par 'resource' pour éviter les éventuels conflits de noms - mutable std::unordered_map m_resourceListeners; + mutable ResourceListenerMap m_resourceListeners; std::atomic_bool m_resourcePersistent; mutable std::atomic_uint m_resourceReferenceCount; bool m_resourceListenersLocked; diff --git a/include/Nazara/Renderer/Context.hpp b/include/Nazara/Renderer/Context.hpp index 836a1f59e..aee424a35 100644 --- a/include/Nazara/Renderer/Context.hpp +++ b/include/Nazara/Renderer/Context.hpp @@ -32,13 +32,13 @@ class NAZARA_API NzContext : public NzResource const NzContextParameters& GetParameters() const; bool IsActive() const; - bool SetActive(bool active); + bool SetActive(bool active) const; void SwapBuffers(); static bool EnsureContext(); - static NzContext* GetCurrent(); - static NzContext* GetReference(); - static NzContext* GetThreadContext(); + static const NzContext* GetCurrent(); + static const NzContext* GetReference(); + static const NzContext* GetThreadContext(); static bool Initialize(); static void Uninitialize(); diff --git a/include/Nazara/Renderer/OpenGL.hpp b/include/Nazara/Renderer/OpenGL.hpp index 253fa57f9..c4e27f75f 100644 --- a/include/Nazara/Renderer/OpenGL.hpp +++ b/include/Nazara/Renderer/OpenGL.hpp @@ -133,8 +133,8 @@ class NAZARA_API NzOpenGL static GLenum TextureTargetProxy[nzImageType_Max+1]; private: - static void OnContextDestruction(NzContext* context); - static void OnContextChange(NzContext* newContext); + static void OnContextDestruction(const NzContext* context); + static void OnContextChange(const NzContext* newContext); }; NAZARA_API extern PFNGLACTIVETEXTUREPROC glActiveTexture; diff --git a/include/Nazara/Renderer/Renderer.hpp b/include/Nazara/Renderer/Renderer.hpp index 76024e34f..813f5c30f 100644 --- a/include/Nazara/Renderer/Renderer.hpp +++ b/include/Nazara/Renderer/Renderer.hpp @@ -23,10 +23,14 @@ class NzIndexBuffer; class NzMaterial; class NzRenderTarget; class NzShaderProgram; +class NzTexture; class NzVertexBuffer; class NAZARA_API NzRenderer { + friend NzShaderProgram; + friend NzTexture; + public: NzRenderer() = delete; ~NzRenderer() = delete; @@ -96,6 +100,8 @@ class NAZARA_API NzRenderer private: static void EnableInstancing(bool instancing); static bool EnsureStateUpdate(); + static void OnProgramReleased(const NzShaderProgram* program); + static void OnTextureReleased(const NzTexture* texture); static void UpdateMatrix(nzMatrixType type); static unsigned int s_moduleReferenceCounter; diff --git a/src/Nazara/Core/Resource.cpp b/src/Nazara/Core/Resource.cpp index 5670630f2..60e94ba65 100644 --- a/src/Nazara/Core/Resource.cpp +++ b/src/Nazara/Core/Resource.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,7 @@ NzResource::~NzResource() { m_resourceListenersLocked = true; for (auto& pair : m_resourceListeners) - pair.first->OnResourceReleased(this, pair.second); + pair.first->OnResourceReleased(this, pair.second.first); #if NAZARA_CORE_SAFE if (m_resourceReferenceCount > 0) @@ -35,7 +36,11 @@ void NzResource::AddResourceListener(NzResourceListener* listener, int index) co NazaraLock(m_mutex) if (!m_resourceListenersLocked) - m_resourceListeners.insert(std::make_pair(listener, index)); + { + auto pair = m_resourceListeners.insert(std::make_pair(listener, std::make_pair(index, 1U))); + if (!pair.second) + pair.first->second.second++; + } } void NzResource::AddResourceReference() const @@ -59,7 +64,11 @@ void NzResource::RemoveResourceListener(NzResourceListener* listener) const NazaraLock(m_mutex); if (!m_resourceListenersLocked) - m_resourceListeners.erase(listener); + { + ResourceListenerMap::iterator it = m_resourceListeners.find(listener); + if (it != m_resourceListeners.end()) + RemoveResourceListenerIterator(it); + } } bool NzResource::RemoveResourceReference() const @@ -105,8 +114,9 @@ void NzResource::NotifyCreated() auto it = m_resourceListeners.begin(); while (it != m_resourceListeners.end()) { - if (!it->first->OnResourceCreated(this, it->second)) - m_resourceListeners.erase(it++); + ResourceListenerMap::iterator iterator = it++; + if (!it->first->OnResourceCreated(this, it->second.first)) + RemoveResourceListenerIterator(it++); else ++it; } @@ -123,11 +133,20 @@ void NzResource::NotifyDestroy() auto it = m_resourceListeners.begin(); while (it != m_resourceListeners.end()) { - if (!it->first->OnResourceDestroy(this, it->second)) - m_resourceListeners.erase(it++); + if (!it->first->OnResourceDestroy(this, it->second.first)) + RemoveResourceListenerIterator(it++); else ++it; } m_resourceListenersLocked = false; } + +void NzResource::RemoveResourceListenerIterator(ResourceListenerMap::iterator iterator) const +{ + unsigned int& referenceCount = iterator->second.second; + if (referenceCount == 1) + m_resourceListeners.erase(iterator); + else + referenceCount--; +} diff --git a/src/Nazara/Renderer/Context.cpp b/src/Nazara/Renderer/Context.cpp index deb73b829..f15bc1b85 100644 --- a/src/Nazara/Renderer/Context.cpp +++ b/src/Nazara/Renderer/Context.cpp @@ -22,8 +22,8 @@ namespace { - thread_local NzContext* currentContext = nullptr; - thread_local NzContext* threadContext = nullptr; + thread_local const NzContext* currentContext = nullptr; + thread_local const NzContext* threadContext = nullptr; std::vector contexts; @@ -219,7 +219,7 @@ bool NzContext::IsActive() const return currentContext == this; } -bool NzContext::SetActive(bool active) +bool NzContext::SetActive(bool active) const { #ifdef NAZARA_RENDERER_SAFE if (!m_impl) @@ -302,17 +302,17 @@ bool NzContext::EnsureContext() return true; } -NzContext* NzContext::GetCurrent() +const NzContext* NzContext::GetCurrent() { return currentContext; } -NzContext* NzContext::GetReference() +const NzContext* NzContext::GetReference() { return s_reference; } -NzContext* NzContext::GetThreadContext() +const NzContext* NzContext::GetThreadContext() { EnsureContext(); diff --git a/src/Nazara/Renderer/OpenGL.cpp b/src/Nazara/Renderer/OpenGL.cpp index 5d6f2078d..ad5879ab5 100644 --- a/src/Nazara/Renderer/OpenGL.cpp +++ b/src/Nazara/Renderer/OpenGL.cpp @@ -71,7 +71,7 @@ namespace }; std::set s_openGLextensionSet; - std::unordered_map s_contexts; + std::unordered_map s_contexts; thread_local ContextStates* s_contextStates = nullptr; NzString s_rendererName; NzString s_vendorName; @@ -1305,12 +1305,12 @@ void NzOpenGL::Uninitialize() } } -void NzOpenGL::OnContextDestruction(NzContext* context) +void NzOpenGL::OnContextDestruction(const NzContext* context) { s_contexts.erase(context); } -void NzOpenGL::OnContextChange(NzContext* newContext) +void NzOpenGL::OnContextChange(const NzContext* newContext) { s_contextStates = (newContext) ? &s_contexts[newContext] : nullptr; } diff --git a/src/Nazara/Renderer/RenderWindow.cpp b/src/Nazara/Renderer/RenderWindow.cpp index 00e9f7fac..85a8cd882 100644 --- a/src/Nazara/Renderer/RenderWindow.cpp +++ b/src/Nazara/Renderer/RenderWindow.cpp @@ -59,7 +59,7 @@ bool NzRenderWindow::CopyToImage(NzImage* image) const } #endif - NzContext* currentContext = NzContext::GetCurrent(); + const NzContext* currentContext = NzContext::GetCurrent(); if (m_context != currentContext) { if (!m_context->SetActive(true)) @@ -112,7 +112,7 @@ bool NzRenderWindow::CopyToTexture(NzTexture* texture) const } #endif - NzContext* currentContext = NzContext::GetCurrent(); + const NzContext* currentContext = NzContext::GetCurrent(); if (m_context != currentContext) { if (!m_context->SetActive(true)) diff --git a/src/Nazara/Renderer/Renderer.cpp b/src/Nazara/Renderer/Renderer.cpp index 2a450c7f5..c49abd2c3 100644 --- a/src/Nazara/Renderer/Renderer.cpp +++ b/src/Nazara/Renderer/Renderer.cpp @@ -33,6 +33,14 @@ namespace { + enum ResourceType + { + ResourceType_Context, + ResourceType_IndexBuffer, + ResourceType_VertexBuffer, + ResourceType_VertexDeclaration + }; + enum UpdateFlags { Update_None = 0, @@ -64,8 +72,9 @@ namespace } using VAO_Key = std::tuple; + using VAO_Map = std::unordered_map>; - std::unordered_map> s_vaos; + VAO_Map s_vaos; std::set s_dirtyTextureUnits; std::vector s_textureUnits; GLuint s_currentVAO = 0; @@ -89,6 +98,90 @@ namespace unsigned int s_maxRenderTarget; unsigned int s_maxTextureUnit; unsigned int s_maxVertexAttribs; + + class ResourceListener : public NzResourceListener + { + public: + void OnResourceReleased(const NzResource* resource, int index) override + { + switch (index) + { + case ResourceType_Context: + { + const NzContext* context = static_cast(resource); + s_vaos.erase(context); + break; + } + + case ResourceType_IndexBuffer: + { + const NzIndexBuffer* indexBuffer = static_cast(resource); + for (auto& pair : s_vaos) + { + auto it = pair.second.begin(); + while (it != pair.second.end()) + { + const VAO_Key& key = it->first; + const NzIndexBuffer* vaoIndexBuffer = std::get<0>(key); + + if (vaoIndexBuffer == indexBuffer) + pair.second.erase(it++); + else + ++it; + } + } + break; + } + + case ResourceType_VertexBuffer: + { + const NzVertexBuffer* vertexBuffer = static_cast(resource); + for (auto& pair : s_vaos) + { + auto it = pair.second.begin(); + while (it != pair.second.end()) + { + const VAO_Key& key = it->first; + const NzVertexBuffer* vaoVertexBuffer = std::get<1>(key); + + if (vaoVertexBuffer == vertexBuffer) + pair.second.erase(it++); + else + ++it; + } + } + break; + } + + case ResourceType_VertexDeclaration: + { + const NzVertexDeclaration* vertexDeclaration = static_cast(resource); + for (auto& pair : s_vaos) + { + auto it = pair.second.begin(); + while (it != pair.second.end()) + { + const VAO_Key& key = it->first; + const NzVertexDeclaration* vaoVertexDeclaration = std::get<2>(key); + const NzVertexDeclaration* vaoInstancingDeclaration = std::get<3>(key); + + if (vaoVertexDeclaration == vertexDeclaration || vaoInstancingDeclaration == vertexDeclaration) + pair.second.erase(it++); + else + ++it; + } + } + break; + } + + default: + NazaraInternalError("Unknown resource type"); + break; + } + } + }; + + ResourceListener s_listener; } void NzRenderer::Clear(nzUInt32 flags) @@ -1164,12 +1257,33 @@ void NzRenderer::Uninitialize() // Libération des VAOs for (auto& pair : s_vaos) { + const NzContext* context = pair.first; + context->SetActive(true); + for (auto& pair2 : pair.second) { + const VAO_Key& key = pair2.first; + const NzIndexBuffer* indexBuffer = std::get<0>(key); + const NzVertexBuffer* vertexBuffer = std::get<1>(key); + const NzVertexDeclaration* vertexDeclaration = std::get<2>(key); + const NzVertexDeclaration* instancingDeclaration = std::get<3>(key); + + if (indexBuffer) + indexBuffer->RemoveResourceListener(&s_listener); + + vertexBuffer->RemoveResourceListener(&s_listener); + vertexDeclaration->RemoveResourceListener(&s_listener); + + if (instancingDeclaration) + instancingDeclaration->RemoveResourceListener(&s_listener); + GLuint vao = static_cast(pair2.second); glDeleteVertexArrays(1, &vao); } + + context->SetActive(false); } + s_vaos.clear(); NzOpenGL::Uninitialize(); @@ -1256,18 +1370,21 @@ bool NzRenderer::EnsureStateUpdate() { TextureUnit& unit = s_textureUnits[i]; - if (!unit.textureUpdated) + if (unit.texture) { - NzOpenGL::BindTextureUnit(i); - unit.texture->Bind(); + if (!unit.textureUpdated) + { + NzOpenGL::BindTextureUnit(i); + unit.texture->Bind(); - unit.textureUpdated = true; - } + unit.textureUpdated = true; + } - if (!unit.samplerUpdated) - { - unit.sampler.Bind(i); - unit.samplerUpdated = true; + if (!unit.samplerUpdated) + { + unit.sampler.Bind(i); + unit.samplerUpdated = true; + } } } } @@ -1277,13 +1394,16 @@ bool NzRenderer::EnsureStateUpdate() { TextureUnit& unit = s_textureUnits[i]; - NzOpenGL::BindTextureUnit(i); + if (unit.texture) + { + NzOpenGL::BindTextureUnit(i); - unit.texture->Bind(); - unit.textureUpdated = true; + unit.texture->Bind(); + unit.textureUpdated = true; - unit.sampler.Apply(unit.texture); - unit.samplerUpdated = true; + unit.sampler.Apply(unit.texture); + unit.samplerUpdated = true; + } } } @@ -1324,10 +1444,18 @@ bool NzRenderer::EnsureStateUpdate() if (s_useVertexArrayObjects) { // Note: Les VAOs ne sont pas partagés entre les contextes, nous avons donc un tableau de VAOs par contexte - auto& vaos = s_vaos[NzContext::GetCurrent()]; + const NzContext* context = NzContext::GetCurrent(); + + auto pair = s_vaos.insert(std::make_pair(context, VAO_Map::mapped_type())); + if (pair.second) + context->AddResourceListener(&s_listener, ResourceType_Context); + + auto& vaos = pair.first->second; // Notre clé est composée de ce qui définit un VAO - VAO_Key key(s_indexBuffer, s_vertexBuffer, s_vertexBuffer->GetVertexDeclaration(), (s_instancing) ? s_instancingDeclaration : nullptr); + const NzVertexDeclaration* vertexDeclaration = s_vertexBuffer->GetVertexDeclaration(); + const NzVertexDeclaration* instancingDeclaration = (s_instancing) ? s_instancingDeclaration : nullptr; + VAO_Key key(s_indexBuffer, s_vertexBuffer, vertexDeclaration, instancingDeclaration); // On recherche un VAO existant avec notre configuration auto it = vaos.find(key); @@ -1339,6 +1467,14 @@ bool NzRenderer::EnsureStateUpdate() // On l'ajoute à notre liste vaos.insert(std::make_pair(key, static_cast(s_currentVAO))); + if (s_indexBuffer) + s_indexBuffer->AddResourceListener(&s_listener, ResourceType_IndexBuffer); + + s_vertexBuffer->AddResourceListener(&s_listener, ResourceType_VertexBuffer); + vertexDeclaration->AddResourceListener(&s_listener, ResourceType_VertexDeclaration); + + if (instancingDeclaration) + instancingDeclaration->AddResourceListener(&s_listener, ResourceType_VertexDeclaration); // Et on indique qu'on veut le programmer update = true; @@ -1474,6 +1610,25 @@ bool NzRenderer::EnsureStateUpdate() return true; } +void NzRenderer::OnProgramReleased(const NzShaderProgram* program) +{ + if (s_program == program) + { + s_program = nullptr; + s_updateFlags |= Update_Program; + } +} + +void NzRenderer::OnTextureReleased(const NzTexture* texture) +{ + for (TextureUnit& unit : s_textureUnits) + { + if (unit.texture == texture) + unit.texture = nullptr; + // Inutile de changer le flag pour une texture désactivée + } +} + void NzRenderer::UpdateMatrix(nzMatrixType type) { #ifdef NAZARA_DEBUG diff --git a/src/Nazara/Renderer/ShaderProgram.cpp b/src/Nazara/Renderer/ShaderProgram.cpp index ba244a2f8..0c1711146 100644 --- a/src/Nazara/Renderer/ShaderProgram.cpp +++ b/src/Nazara/Renderer/ShaderProgram.cpp @@ -40,6 +40,7 @@ m_compiled(program.m_compiled) NzShaderProgram::~NzShaderProgram() { Destroy(); + NzRenderer::OnProgramReleased(this); } bool NzShaderProgram::Create(nzShaderLanguage language) diff --git a/src/Nazara/Renderer/Texture.cpp b/src/Nazara/Renderer/Texture.cpp index bde6e5e46..37f416bb2 100644 --- a/src/Nazara/Renderer/Texture.cpp +++ b/src/Nazara/Renderer/Texture.cpp @@ -164,6 +164,7 @@ NzTexture::NzTexture(const NzImage& image) NzTexture::~NzTexture() { Destroy(); + NzRenderer::OnTextureReleased(this); } bool NzTexture::Create(nzImageType type, nzPixelFormat format, unsigned int width, unsigned int height, unsigned int depth, nzUInt8 levelCount)