[BUGFIX] Fix the sampler factory logic. Fixed an intrusive pointer reset that caused leaks

src/anki/gr/vulkan/CommandBufferFactory.cpp

@@ -26,7 +26,6 @@ void MicroCommandBuffer::reset()
 	ANKI_ASSERT(!m_fence.isCreated() || m_fence->done());
 
 	m_objectRefs.destroy(m_fastAlloc);
-	m_objectRefCount = 0;
 
 	m_fastAlloc.getMemoryPool().reset();
 

src/anki/gr/vulkan/CommandBufferFactory.h

@@ -52,13 +52,8 @@ public:
 	template<typename T>
 	void pushObjectRef(T& x)
 	{
-		if(m_objectRefs.getSize() <= m_objectRefCount)
-		{
-			// Grow storage
-			m_objectRefs.resize(m_fastAlloc, max<PtrSize>(10, m_objectRefs.getSize() * 2));
-		}
 		GrObject* grobj = x.get();
-		m_objectRefs[m_objectRefCount++] = IntrusivePtr<GrObject>(grobj);
+		m_objectRefs.emplaceBack(m_fastAlloc, IntrusivePtr<GrObject>(grobj));
 	}
 
 	void setFence(MicroFencePtr& fence)
@@ -75,7 +70,6 @@ private:
 	VkCommandBuffer m_handle = {};
 
 	DynamicArray<IntrusivePtr<GrObject>> m_objectRefs;
-	U32 m_objectRefCount = 0;
 	CommandBufferFlag m_flags = CommandBufferFlag::NONE;
 
 	MicroFencePtr m_fence;

src/anki/gr/vulkan/GrManagerImpl.cpp

@@ -44,7 +44,6 @@ GrManagerImpl::~GrManagerImpl()
 	m_barrierFactory.destroy(); // Destroy before fences
 	m_semaphores.destroy(); // Destroy before fences
 	m_swapchainFactory.destroy(); // Destroy before fences
-	m_samplerFactory.destroy(); // Destroy before fences
 
 	m_pplineLayoutFactory.destroy();
 	m_descrFactory.destroy();
@@ -53,6 +52,8 @@ GrManagerImpl::~GrManagerImpl()
 
 	m_fences.destroy();
 
+	m_samplerFactory.destroy();
+
 	if(m_device)
 	{
 		vkDestroyDevice(m_device, nullptr);
@@ -785,6 +786,28 @@ void GrManagerImpl::trySetVulkanHandleName(CString name, VkDebugReportObjectType
 	}
 }
 
+StringAuto GrManagerImpl::tryGetVulkanHandleName(U64 handle) const
+{
+	StringAuto out(getAllocator());
+
+	LockGuard<SpinLock> lock(m_vkHandleToNameLock);
+
+	auto it = m_vkHandleToName.find(computeHash(&handle, sizeof(handle)));
+	CString objName;
+	if(it != m_vkHandleToName.getEnd())
+	{
+		objName = it->toCString();
+	}
+	else
+	{
+		objName = "Unnamed";
+	}
+
+	out.create(objName);
+
+	return out;
+}
+
 VkBool32 GrManagerImpl::debugReportCallbackEXT(VkDebugReportFlagsEXT flags,
 	VkDebugReportObjectTypeEXT objectType,
 	uint64_t object,
@@ -796,29 +819,19 @@ VkBool32 GrManagerImpl::debugReportCallbackEXT(VkDebugReportFlagsEXT flags,
 {
 	// Get the object name
 	GrManagerImpl* self = static_cast<GrManagerImpl*>(pUserData);
-	LockGuard<SpinLock> lock(self->m_vkHandleToNameLock);
-	auto it = self->m_vkHandleToName.find(computeHash(&object, sizeof(object)));
-	CString objName;
-	if(it != self->m_vkHandleToName.getEnd())
-	{
-		objName = it->toCString();
-	}
-	else
-	{
-		objName = "Unnamed";
-	}
+	StringAuto name = self->tryGetVulkanHandleName(object);
 
 	if(flags & VK_DEBUG_REPORT_ERROR_BIT_EXT)
 	{
-		ANKI_VK_LOGE("%s (handle: %s)", pMessage, objName.cstr());
+		ANKI_VK_LOGE("%s (handle: %s)", pMessage, name.cstr());
 	}
 	else if(flags & (VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT))
 	{
-		ANKI_VK_LOGW("%s (handle: %s)", pMessage, objName.cstr());
+		ANKI_VK_LOGW("%s (handle: %s)", pMessage, name.cstr());
 	}
 	else
 	{
-		ANKI_VK_LOGI("%s (handle: %s)", pMessage, objName.cstr());
+		ANKI_VK_LOGI("%s (handle: %s)", pMessage, name.cstr());
 	}
 
 	return false;

src/anki/gr/vulkan/GrManagerImpl.h

@@ -181,6 +181,13 @@ public:
 		trySetVulkanHandleName(name, type, U64(ptrToNumber(handle)));
 	}
 
+	StringAuto tryGetVulkanHandleName(U64 handle) const;
+
+	StringAuto tryGetVulkanHandleName(void* handle) const
+	{
+		return tryGetVulkanHandleName(U64(ptrToNumber(handle)));
+	}
+
 private:
 	GrManager* m_manager = nullptr;
 

src/anki/gr/vulkan/Pipeline.h

@@ -150,6 +150,8 @@ public:
 
 	void reset()
 	{
+		m_prog.reset(nullptr);
+
 		// Do a special construction. The state will be hashed and the padding may contain garbage. With this trick
 		// zero the padding
 		memset(this, 0, sizeof(*this));
@@ -525,8 +527,13 @@ private:
 class PipelineFactory
 {
 public:
-	PipelineFactory() = default;
-	~PipelineFactory() = default;
+	PipelineFactory()
+	{
+	}
+
+	~PipelineFactory()
+	{
+	}
 
 	void init(GrAllocator<U8> alloc, VkDevice dev, VkPipelineCache pplineCache)
 	{

src/anki/gr/vulkan/SamplerFactory.cpp

@@ -17,11 +17,6 @@ MicroSampler::~MicroSampler()
 	}
 }
 
-GrAllocator<U8> MicroSampler::getAllocator() const
-{
-	return m_factory->m_gr->getAllocator();
-}
-
 Error MicroSampler::init(const SamplerInitInfo& inf)
 {
 	// Fill the create cio
@@ -82,19 +77,31 @@ Error MicroSampler::init(const SamplerInitInfo& inf)
 	return Error::NONE;
 }
 
-Error SamplerFactory::init(GrManagerImpl* gr)
+void SamplerFactory::init(GrManagerImpl* gr)
 {
 	ANKI_ASSERT(gr);
 	ANKI_ASSERT(!m_gr);
 	m_gr = gr;
-	m_recycler.init(gr->getAllocator());
-
-	return Error::NONE;
 }
 
 void SamplerFactory::destroy()
 {
-	m_recycler.destroy();
+	if(!m_gr)
+	{
+		return;
+	}
+
+	GrAllocator<U8> alloc = m_gr->getAllocator();
+	for(auto it : m_map)
+	{
+		MicroSampler* const sampler = it;
+
+		ANKI_ASSERT(sampler->getRefcount().load() == 0 && "Someone still holds a reference to a sampler");
+		alloc.deleteInstance(sampler);
+	}
+
+	m_map.destroy(alloc);
+
 	m_gr = nullptr;
 }
 
@@ -103,9 +110,18 @@ Error SamplerFactory::newInstance(const SamplerInitInfo& inf, MicroSamplerPtr& p
 	ANKI_ASSERT(m_gr);
 
 	Error err = Error::NONE;
-	MicroSampler* out = m_recycler.findToReuse();
+	MicroSampler* out = nullptr;
+	const U64 hash = inf.computeHash();
+
+	LockGuard<Mutex> lock(m_mtx);
+
+	auto it = m_map.find(hash);
 
-	if(out == nullptr)
+	if(it != m_map.getEnd())
+	{
+		out = *it;
+	}
+	else
 	{
 		// Create a new one
 
@@ -119,11 +135,14 @@ Error SamplerFactory::newInstance(const SamplerInitInfo& inf, MicroSamplerPtr& p
 			alloc.deleteInstance(out);
 			out = nullptr;
 		}
+		else
+		{
+			m_map.emplace(alloc, hash, out);
+		}
 	}
 
 	if(!err)
 	{
-		ANKI_ASSERT(out->m_refcount.get() == 0);
 		psampler.reset(out);
 	}
 

src/anki/gr/vulkan/SamplerFactory.h

@@ -6,7 +6,7 @@
 #pragma once
 
 #include <anki/gr/vulkan/FenceFactory.h>
-#include <anki/gr/vulkan/MicroObjectRecycler.h>
+#include <anki/util/HashMap.h>
 
 namespace anki
 {
@@ -37,20 +37,11 @@ public:
 		return m_refcount;
 	}
 
-	GrAllocator<U8> getAllocator() const;
-
-	MicroFencePtr& getFence()
-	{
-		return m_dummyFence;
-	}
-
 private:
 	VkSampler m_handle = VK_NULL_HANDLE;
 	Atomic<U32> m_refcount = {0};
 	SamplerFactory* m_factory = nullptr;
 
-	MicroFencePtr m_dummyFence; ///< This is a dummy fence to satisfy the interface.
-
 	MicroSampler(SamplerFactory* f)
 		: m_factory(f)
 	{
@@ -66,7 +57,11 @@ private:
 class MicroSamplerPtrDeleter
 {
 public:
-	void operator()(MicroSampler* s);
+	void operator()(MicroSampler* s)
+	{
+		ANKI_ASSERT(s);
+		// Do nothing. The samplers will be destroyed at app shutdown
+	}
 };
 
 /// MicroSampler smart pointer.
@@ -76,7 +71,6 @@ using MicroSamplerPtr = IntrusivePtr<MicroSampler, MicroSamplerPtrDeleter>;
 class SamplerFactory
 {
 	friend class MicroSampler;
-	friend class MicroSamplerPtrDeleter;
 
 public:
 	SamplerFactory()
@@ -88,23 +82,18 @@ public:
 		ANKI_ASSERT(m_gr == nullptr && "Forgot to call destroy()");
 	}
 
-	ANKI_USE_RESULT Error init(GrManagerImpl* gr);
+	void init(GrManagerImpl* gr);
 
 	void destroy();
 
-	/// Create a new sampler.
+	/// Create a new sampler. It's thread-safe.
 	ANKI_USE_RESULT Error newInstance(const SamplerInitInfo& inf, MicroSamplerPtr& psampler);
 
 private:
 	GrManagerImpl* m_gr = nullptr;
-	MicroObjectRecycler<MicroSampler> m_recycler;
+	HashMap<U64, MicroSampler*> m_map;
+	Mutex m_mtx;
 };
 /// @}
 
-inline void MicroSamplerPtrDeleter::operator()(MicroSampler* s)
-{
-	ANKI_ASSERT(s);
-	s->m_factory->m_recycler.recycle(s);
-}
-
 } // end namespace anki

src/anki/gr/vulkan/SamplerImpl.cpp

@@ -10,8 +10,14 @@
 namespace anki
 {
 
-Error SamplerImpl::init(const SamplerInitInfo& inf)
+Error SamplerImpl::init(const SamplerInitInfo& inf_)
 {
+	SamplerInitInfo inf = inf_;
+
+	// Set a constant name because the name will take part in hashing. If it's unique every time then there is no point
+	// in having the SamplerFactory
+	inf.setName("SamplerSampler");
+
 	return getGrManagerImpl().getSamplerFactory().newInstance(inf, m_sampler);
 }
 

src/anki/gr/vulkan/TextureImpl.cpp

@@ -51,7 +51,11 @@ TextureImpl::~TextureImpl()
 Error TextureImpl::init(const TextureInitInfo& init_, Texture* tex)
 {
 	TextureInitInfo init = init_;
-	init.m_sampling.setName(init.getName());
+
+	// Set a constant name because the name will take part in hashing. If it's unique every time then there is no point
+	// in having the SamplerFactory
+	init.m_sampling.setName("SamplForTex");
+
 	ANKI_ASSERT(textureInitInfoValid(init));
 
 	ANKI_CHECK(getGrManagerImpl().getSamplerFactory().newInstance(init.m_sampling, m_sampler));

src/anki/renderer/Renderer.cpp

@@ -363,7 +363,7 @@ TexturePtr Renderer::createAndClearRenderTarget(const TextureInitInfo& inf, cons
 			{
 				TextureSurfaceInfo surf(mip, 0, face, layer);
 
-				FramebufferInitInfo fbInit;
+				FramebufferInitInfo fbInit("RendererClearRT");
 				Array<TextureUsageBit, MAX_COLOR_ATTACHMENTS> colUsage = {};
 				TextureUsageBit dsUsage = TextureUsageBit::NONE;
 

src/anki/renderer/RendererObject.h

@@ -32,7 +32,7 @@ anki_internal:
 	{
 	}
 
-	~RendererObject()
+	virtual ~RendererObject()
 	{
 	}
 

src/anki/renderer/Ssao.cpp

@@ -85,6 +85,7 @@ Error Ssao::init(const ConfigSet& config)
 
 	ANKI_R_LOGI("Initializing SSAO. Size %ux%u", m_width, m_height);
 
+	static const Array<const char*, 2> RT_NAMES = {{"SsaoMain #1", "SsaoMain #2"}};
 	for(U i = 0; i < 2; ++i)
 	{
 		// RT
@@ -93,7 +94,7 @@ Error Ssao::init(const ConfigSet& config)
 			Ssao::RT_PIXEL_FORMAT,
 			TextureUsageBit::SAMPLED_FRAGMENT | TextureUsageBit::FRAMEBUFFER_ATTACHMENT_WRITE | TextureUsageBit::CLEAR,
 			SamplingFilter::LINEAR,
-			"ssaomain");
+			&RT_NAMES[i][0]);
 		texinit.m_initialUsage = TextureUsageBit::SAMPLED_FRAGMENT;
 
 		m_rtTextures[i] = m_r->createAndClearRenderTarget(texinit);

src/anki/resource/TextureResource.cpp

@@ -67,7 +67,7 @@ Error TextureResource::load(const ResourceFilename& filename, Bool async)
 	}
 	ImageLoader& loader = ctx->m_loader;
 
-	TextureInitInfo init;
+	TextureInitInfo init("RsrcTex");
 	init.m_usage = TextureUsageBit::SAMPLED_FRAGMENT | TextureUsageBit::SAMPLED_TESSELLATION_EVALUATION
 		| TextureUsageBit::TRANSFER_DESTINATION;
 	init.m_usageWhenEncountered = TextureUsageBit::SAMPLED_FRAGMENT | TextureUsageBit::SAMPLED_TESSELLATION_EVALUATION;