Graphics/RenderQueue: Remake it with a naive implementation

The idea is to improve it in the future, after profiling

include/Nazara/Core/Algorithm.hpp

@@ -71,6 +71,9 @@ namespace Nz
 		using type = void; //< FIXME: I can't make SFINAE work
 	};
 
+	template<typename T>
+	using Pointer = T*;
+
 	template<typename T>
 	bool Serialize(SerializationContext& context, T&& value);
 

include/Nazara/Graphics/ElementRenderer.hpp

@@ -8,6 +8,7 @@
 #define NAZARA_ELEMENTRENDERER_HPP
 
 #include <Nazara/Prerequisites.hpp>
+#include <Nazara/Core/Algorithm.hpp>
 #include <Nazara/Graphics/Enums.hpp>
 #include <memory>
 #include <vector>
@@ -23,7 +24,7 @@ namespace Nz
 			ElementRenderer() = default;
 			virtual ~ElementRenderer();
 
-			virtual void Render(CommandBufferBuilder& commandBuffer, const RenderElement** elements, std::size_t elementCount) = 0;
+			virtual void Render(CommandBufferBuilder& commandBuffer, const Pointer<const RenderElement>* elements, std::size_t elementCount) = 0;
 	};
 }
 

include/Nazara/Graphics/RenderQueue.hpp

@@ -8,38 +8,15 @@
 #define NAZARA_RENDERQUEUE_HPP
 
 #include <Nazara/Prerequisites.hpp>
-#include <memory>
 #include <vector>
 
 namespace Nz
 {
-	class RenderQueueInternal
-	{
-		public:
-			using Index = UInt64;
-
-			RenderQueueInternal() = default;
-			RenderQueueInternal(const RenderQueueInternal&) = default;
-			RenderQueueInternal(RenderQueueInternal&&) = default;
-			~RenderQueueInternal() = default;
-
-			RenderQueueInternal& operator=(const RenderQueueInternal&) = default;
-			RenderQueueInternal& operator=(RenderQueueInternal&&) = default;
-
-		protected:
-			using RenderDataPair = std::pair<Index, std::size_t>;
-
-			void Sort();
-
-			std::vector<RenderDataPair> m_orderedRenderQueue;
-	};
-
 	template<typename RenderData>
-	class RenderQueue : public RenderQueueInternal
+	class RenderQueue
 	{
 		public:
-			class const_iterator;
+			using const_iterator = const RenderData*;
-			friend const_iterator;
 			using size_type = std::size_t;
 
 			RenderQueue() = default;
@@ -63,37 +40,8 @@ namespace Nz
 			RenderQueue& operator=(RenderQueue&&) noexcept = default;
 
 		private:
-			const RenderData& GetData(std::size_t i) const;
-
 			std::vector<RenderData> m_data;
 	};
-	
-	template<typename RenderData>
-	class RenderQueue<RenderData>::const_iterator : public std::iterator<std::forward_iterator_tag, const RenderData>
-	{
-		friend RenderQueue;
-
-		public:
-			const_iterator(const const_iterator& it);
-
-			const RenderData& operator*() const;
-
-			const_iterator& operator=(const const_iterator& it);
-			const_iterator& operator++();
-			const_iterator operator++(int);
-
-			bool operator==(const const_iterator& rhs) const;
-			bool operator!=(const const_iterator& rhs) const;
-
-			void swap(const_iterator& rhs);
-
-		private:
-			const_iterator(const RenderQueue* queue, std::size_t nextId);
-
-			std::size_t m_nextDataId;
-			const RenderQueue* m_queue;
-	};
-
 }
 
 #include <Nazara/Graphics/RenderQueue.inl>

include/Nazara/Graphics/RenderQueue.inl

@@ -3,14 +3,13 @@
 // For conditions of distribution and use, see copyright notice in Config.hpp
 
 #include <Nazara/Graphics/RenderQueue.hpp>
-#include <Nazara/Core/Error.hpp>
+#include <algorithm>
 
 namespace Nz
 {
 	template<typename RenderData>
 	void RenderQueue<RenderData>::Clear()
 	{
-		m_orderedRenderQueue.clear();
 		m_data.clear();
 	}
 
@@ -24,113 +23,33 @@ namespace Nz
 	template<typename IndexFunc>
 	void RenderQueue<RenderData>::Sort(IndexFunc&& func)
 	{
-		m_orderedRenderQueue.clear();
+		std::sort(m_data.begin(), m_data.end(), [&](const RenderData& lhs, const RenderData& rhs)
-		m_orderedRenderQueue.reserve(m_data.size());
+		{
-
+			return func(lhs) < func(rhs);
-		std::size_t dataIndex = 0;
+		});
-		for (const RenderData& renderData : m_data)
-			m_orderedRenderQueue.emplace_back(func(renderData), dataIndex++);
-
-		RenderQueueInternal::Sort();
 	}
 
 	template<typename RenderData>
-	typename RenderQueue<RenderData>::const_iterator RenderQueue<RenderData>::begin() const
+	auto RenderQueue<RenderData>::begin() const -> const_iterator
 	{
-		return const_iterator(this, 0);
+		return &m_data[0];
 	}
 
 	template<typename RenderData>
 	bool RenderQueue<RenderData>::empty() const
 	{
-		return m_orderedRenderQueue.empty();
+		return m_data.empty();
 	}
 
 	template<typename RenderData>
-	typename RenderQueue<RenderData>::const_iterator RenderQueue<RenderData>::end() const
+	auto RenderQueue<RenderData>::end() const -> const_iterator
 	{
-		return const_iterator(this, m_orderedRenderQueue.size());
+		return begin() + m_data.size();
 	}
 
 	template<typename RenderData>
-	typename RenderQueue<RenderData>::size_type RenderQueue<RenderData>::size() const
+	auto RenderQueue<RenderData>::size() const -> size_type
 	{
-		return m_orderedRenderQueue.size();
+		return m_data.size();
-	}
-
-	template<typename RenderData>
-	const RenderData& RenderQueue<RenderData>::GetData(std::size_t i) const
-	{
-		NazaraAssert(i < m_orderedRenderQueue.size(), "Cannot dereference post-end iterator");
-
-		return m_data[m_orderedRenderQueue[i].second];
-	}
-
-	
-	template<typename RenderData>
-	RenderQueue<RenderData>::const_iterator::const_iterator(const RenderQueue* queue, std::size_t nextId) :
-	m_nextDataId(nextId),
-	m_queue(queue)
-	{
-	}
-
-	template<typename RenderData>
-	RenderQueue<RenderData>::const_iterator::const_iterator(const const_iterator& it) :
-	m_nextDataId(it.m_nextDataId),
-	m_queue(it.m_queue)
-	{
-	}
-
-	template<typename RenderData>
-	const RenderData& RenderQueue<RenderData>::const_iterator::operator*() const
-	{
-		return m_queue->GetData(m_nextDataId);
-	}
-
-	template<typename RenderData>
-	typename RenderQueue<RenderData>::const_iterator& RenderQueue<RenderData>::const_iterator::operator=(const const_iterator& it)
-	{
-		m_nextDataId = it.m_nextDataId;
-		m_queue = it.m_queue;
-
-		return *this;
-	}
-
-	template<typename RenderData>
-	typename RenderQueue<RenderData>::const_iterator& RenderQueue<RenderData>::const_iterator::operator++()
-	{
-		++m_nextDataId;
-
-		return *this;
-	}
-
-	template<typename RenderData>
-	typename RenderQueue<RenderData>::const_iterator RenderQueue<RenderData>::const_iterator::operator++(int)
-	{
-		return iterator(m_queue, m_nextDataId++);
-	}
-
-	template<typename RenderData>
-	bool RenderQueue<RenderData>::const_iterator::operator==(const typename RenderQueue<RenderData>::const_iterator& rhs) const
-	{
-		NazaraAssert(m_queue == rhs.m_queue, "Cannot compare iterator coming from different queues");
-
-		return m_nextDataId == rhs.m_nextDataId;
-	}
-
-	template<typename RenderData>
-	bool RenderQueue<RenderData>::const_iterator::operator!=(const typename RenderQueue<RenderData>::const_iterator& rhs) const
-	{
-		return !operator==(rhs);
-	}
-
-	template<typename RenderData>
-	void RenderQueue<RenderData>::const_iterator::swap(typename RenderQueue<RenderData>::const_iterator& rhs)
-	{
-		NazaraAssert(m_queue == rhs.m_queue, "Cannot swap iterator coming from different queues");
-
-		using std::swap;
-
-		swap(m_nextDataId, rhs.m_nextDataId);
 	}
 }

include/Nazara/Graphics/SubmeshRenderer.hpp

@@ -18,7 +18,7 @@ namespace Nz
 			SubmeshRenderer() = default;
 			~SubmeshRenderer() = default;
 
-			void Render(CommandBufferBuilder& commandBuffer, const RenderElement** elements, std::size_t elementCount) override;
+			void Render(CommandBufferBuilder& commandBuffer, const Pointer<const RenderElement>* elements, std::size_t elementCount) override;
 	};
 }
 

src/Nazara/Graphics/ForwardFramePipeline.cpp

@@ -325,29 +325,7 @@ namespace Nz
 
 				builder.BindShaderBinding(Graphics::ViewerBindingSet, viewer->GetViewerInstance().GetShaderBinding());
 
-				auto it = m_depthPrepassRenderQueue.begin();
+				ProcessRenderQueue(builder, m_depthPrepassRenderQueue);
-				while (it != m_depthPrepassRenderQueue.end())
-				{
-					const RenderElement* element = *it;
-					UInt8 elementType = element->GetElementType();
-
-					m_temporaryElementList.push_back(element);
-
-					++it;
-					while (it != m_depthPrepassRenderQueue.end() && (*it)->GetElementType() == elementType)
-					{
-						m_temporaryElementList.push_back(*it);
-						++it;
-					}
-
-					if (elementType >= m_elementRenderers.size() || !m_elementRenderers[elementType])
-						continue;
-
-					ElementRenderer& elementRenderer = *m_elementRenderers[elementType];
-					elementRenderer.Render(builder, m_temporaryElementList.data(), m_temporaryElementList.size());
-
-					m_temporaryElementList.clear();
-				}
 			});
 
 			FramePass& forwardPass = frameGraph.AddPass("Forward pass");
@@ -375,29 +353,7 @@ namespace Nz
 
 				builder.BindShaderBinding(Graphics::ViewerBindingSet, viewer->GetViewerInstance().GetShaderBinding());
 
-				auto it = m_forwardRenderQueue.begin();
+				ProcessRenderQueue(builder, m_forwardRenderQueue);
-				while (it != m_forwardRenderQueue.end())
-				{
-					const RenderElement* element = *it;
-					UInt8 elementType = element->GetElementType();
-
-					m_temporaryElementList.push_back(element);
-
-					++it;
-					while (it != m_forwardRenderQueue.end() && (*it)->GetElementType() == elementType)
-					{
-						m_temporaryElementList.push_back(*it);
-						++it;
-					}
-
-					if (elementType >= m_elementRenderers.size() || !m_elementRenderers[elementType])
-						continue;
-
-					ElementRenderer& elementRenderer = *m_elementRenderers[elementType];
-					elementRenderer.Render(builder, m_temporaryElementList.data(), m_temporaryElementList.size());
-
-					m_temporaryElementList.clear();
-				}
 			});
 		}
 
@@ -425,6 +381,31 @@ namespace Nz
 		it->second.usedCount++;
 	}
 
+	void ForwardFramePipeline::ProcessRenderQueue(CommandBufferBuilder& builder, const RenderQueue<RenderElement*>& renderQueue)
+	{
+		auto it = renderQueue.begin();
+		auto itEnd = renderQueue.end();
+		while (it != itEnd)
+		{
+			const RenderElement* element = *it;
+			UInt8 elementType = element->GetElementType();
+
+			const Pointer<const RenderElement>* first = it;
+
+			++it;
+			while (it != itEnd && (*it)->GetElementType() == elementType)
+				++it;
+
+			std::size_t count = it - first;
+			
+			if (elementType >= m_elementRenderers.size() || !m_elementRenderers[elementType])
+				continue;
+
+			ElementRenderer& elementRenderer = *m_elementRenderers[elementType];
+			elementRenderer.Render(builder, first, count);
+		}
+	}
+
 	void ForwardFramePipeline::UnregisterMaterialPass(MaterialPass* material)
 	{
 		auto it = m_materials.find(material);

src/Nazara/Graphics/RenderQueue.cpp

@@ -1,32 +0,0 @@
-// Copyright (C) 2017 Jérôme Leclercq
-// This file is part of the "Nazara Engine - Graphics module"
-// For conditions of distribution and use, see copyright notice in Config.hpp
-
-#include <Nazara/Graphics/RenderQueue.hpp>
-#include <algorithm>
-#include <Nazara/Graphics/Debug.hpp>
-
-namespace Nz
-{
-	void RenderQueueInternal::Sort()
-	{
-		std::sort(m_orderedRenderQueue.begin(), m_orderedRenderQueue.end(), [](const RenderDataPair& lhs, const RenderDataPair& rhs)
-		{
-			/*
-			Original code:
-			if (lhs.first == rhs.first)
-				return lhs.second < rhs.second;
-			else
-				return lhs.first < rhs.first;
-
-			Clang seems to the the only one to prevent branching with the original code (using cmov)
-			Rewriting the code with bit ops seems to prevent branching with Clang/GCC/MSVC
-			*/
-			bool equal = lhs.first == rhs.first;
-			bool compareFirst = lhs.first < rhs.first;
-			bool compareSecond = lhs.second < rhs.second;
-
-			return (equal & compareSecond) | (!equal & compareFirst);
-		});
-	}
-}

src/Nazara/Graphics/SubmeshRenderer.cpp

@@ -10,7 +10,7 @@
 
 namespace Nz
 {
-	void SubmeshRenderer::Render(CommandBufferBuilder& commandBuffer, const RenderElement** elements, std::size_t elementCount)
+	void SubmeshRenderer::Render(CommandBufferBuilder& commandBuffer, const Pointer<const RenderElement>* elements, std::size_t elementCount)
 	{
 		const AbstractBuffer* currentIndexBuffer = nullptr;
 		const AbstractBuffer* currentVertexBuffer = nullptr;