Fix bugs and remove createMeshBufferDuplicate because of its semantics that don't fit any one single function that uses it

Author: devshgraphicsprogramming

include/nbl/asset/utils/IMeshManipulator.h

@@ -202,10 +202,6 @@ class IMeshManipulator : public virtual core::IReferenceCounted
         @param _outIndexType Type of output index buffer data (32bit or 16bit).
         */
         static core::smart_refctd_ptr<ICPUBuffer> idxBufferFromTrianglesFanToTriangles(const void* _input, uint32_t& _idxCount, E_INDEX_TYPE _inIndexType, E_INDEX_TYPE _outIndexType);
-		
-
-		//! Created duplicate of meshbuffer
-		static core::smart_refctd_ptr<ICPUMeshBuffer> createMeshBufferDuplicate(const ICPUMeshBuffer* _src);
 
 		//! Get amount of polygons in mesh buffer.
 		/** \param meshbuffer Input mesh buffer
@@ -613,21 +609,6 @@ class IMeshManipulator : public virtual core::IReferenceCounted
 			}
 		}
 
-
-		//! Creates Mesh Duplicate
-		static inline core::smart_refctd_ptr<ICPUMesh> createMeshDuplicate(const ICPUMesh* _src)
-		{
-			if (!_src)
-				return nullptr;
-	
-			core::smart_refctd_ptr<ICPUMesh> dst = core::make_smart_refctd_ptr<ICPUMesh>();
-			auto& dstMeshBuffers = dst->getMeshBufferVector();
-			for (auto meshbuffer : _src->getMeshBuffers())
-				dstMeshBuffers.emplace_back(createMeshBufferDuplicate(meshbuffer));
-
-			return dst;
-		}
-
 		//! Get amount of polygons in mesh.
 		/** \param meshbuffer Input mesh
 		\param Outputted Number of polygons in mesh, if successful.

src/nbl/asset/bawformat/CBAWMeshFileLoader.cpp

@@ -253,7 +253,7 @@ SAssetBundle CBAWMeshFileLoader::loadAsset(io::IReadFile* _file, const asset::IA
 			auto normalBuffer = core::smart_refctd_ptr<ICPUBuffer>(const_cast<ICPUBuffer*>(originalMeshFormat->getMappedBuffer(normalAttribute)));
 			const bool hasNormal = normalBuffer && normalBuffer->getPointer();
 
-			auto copy = m_manager->getMeshManipulator()->createMeshBufferDuplicate(mb);
+			auto copy = m_manager->getMeshManipulator()->createMeshBufferDuplicate(mb); // TODO: createMeshBufferDuplicate is now non-public
 			auto meshFormat = copy->getMeshDataAndFormat();
 
 			meshFormat->setIndexBuffer(core::smart_refctd_ptr<ICPUBuffer>(const_cast<ICPUBuffer*>(originalMeshFormat->getIndexBuffer())));

src/nbl/asset/utils/CMeshManipulator.cpp

@@ -49,7 +49,7 @@ class CMeshManipulator : public IMeshManipulator
 		static void _filterInvalidTriangles(ICPUMeshBuffer* _input);
 
 		//! Meant to create 32bit index buffer from subrange of index buffer containing 16bit indices. Remember to set to index buffer offset to 0 after mapping buffer resulting from this function.
-		static inline core::smart_refctd_ptr<ICPUBuffer> create32BitFrom16BitIdxBufferSubrange(const uint16_t* _in, size_t _idxCount)
+		static inline core::smart_refctd_ptr<ICPUBuffer> create32BitFrom16BitIdxBufferSubrange(const uint16_t* _in, uint32_t _idxCount)
 		{
 			if (!_in)
 				return nullptr;
@@ -58,7 +58,7 @@ class CMeshManipulator : public IMeshManipulator
 
 			auto* outPtr = reinterpret_cast<uint32_t*>(out->getPointer());
 
-			for (size_t i = 0; i < _idxCount; ++i)
+			for (uint32_t i=0u; i<_idxCount; ++i)
 				outPtr[i] = _in[i];
 
 			return out;
@@ -135,8 +135,8 @@ class CMeshManipulator : public IMeshManipulator
 			return output;
 		}
 
-	private:
-			CQuantNormalCache quantNormalCache;
+	private:	
+		CQuantNormalCache quantNormalCache;
 };
 
 } // end namespace scene

src/nbl/asset/utils/CMeshManipulator.h

@@ -12,96 +12,103 @@
 #include "CMeshManipulator.h"
 #include "os.h"
 
-namespace nbl { namespace asset
+namespace nbl
+{
+namespace asset
 {
 
-core::smart_refctd_ptr<asset::ICPUMeshBuffer> COverdrawMeshOptimizer::createOptimized(asset::ICPUMeshBuffer* _inbuffer, bool _createNew, float _threshold)
+void COverdrawMeshOptimizer::createOptimized(asset::ICPUMeshBuffer* _outbuffer, const asset::ICPUMeshBuffer* _inbuffer, float _threshold)
 {
-	if (!_inbuffer)
-		return nullptr;
+	if (!_outbuffer || !_inbuffer)
+		return;
 
 	const asset::E_INDEX_TYPE indexType = _inbuffer->getIndexType();
-	if (indexType == asset::EIT_UNKNOWN)
-		return nullptr;
-
-	const size_t indexSize = indexType == asset::EIT_16BIT ? 2u : 4u;
-
-	auto outbuffer = _createNew ? IMeshManipulator::createMeshBufferDuplicate(_inbuffer) : core::smart_refctd_ptr<asset::ICPUMeshBuffer>(_inbuffer);
+	const size_t indexSize = indexType == asset::EIT_16BIT ? sizeof(uint16_t):sizeof(uint32_t);
 
-	void* const indices = outbuffer->getIndices();
-	if (!indices)
+	const uint32_t idxCount = _inbuffer->getIndexCount();
+	const void* const inIndices = _inbuffer->getIndices();
+	if (idxCount==0u || indexType==asset::EIT_UNKNOWN || !inIndices)
 	{
 #ifdef _NBL_DEBUG
 		os::Printer::log("Overdraw optimization: no index buffer -- mesh buffer left unchanged.");
 #endif
-		return outbuffer;
+		return;
 	}
 
-	const size_t idxCount = outbuffer->getIndexCount();
-	void* indicesCopy = _NBL_ALIGNED_MALLOC(indexSize*idxCount,_NBL_SIMD_ALIGNMENT);
-	memcpy(indicesCopy, indices, indexSize*idxCount);
-	const size_t vertexCount = IMeshManipulator::upperBoundVertexID(outbuffer.get());
-	core::vector<core::vectorSIMDf> vertexPositions;
+	void* const outIndices = _outbuffer->getIndices();
+	if (_outbuffer->getIndexCount()!=idxCount || _outbuffer->getIndexType()!=indexType || !outIndices)
 	{
-		core::vectorSIMDf pos;
-		for (size_t i = 0; i < vertexCount; ++i)
-		{
-			outbuffer->getAttribute(pos, outbuffer->getPositionAttributeIx(), i);
-			vertexPositions.push_back(pos);
-		}
+#ifdef _NBL_DEBUG
+		os::Printer::log("Overdraw optimization: output meshbuffer's index buffer does not match input -- mesh buffer left unchanged.");
+#endif
+		return;
 	}
 
-	uint32_t* const hardClusters = (uint32_t*)_NBL_ALIGNED_MALLOC((idxCount/3) * 4,_NBL_SIMD_ALIGNMENT);
+	void* indexCopy = nullptr;
+	const uint16_t* inIndices16 = reinterpret_cast<const uint16_t*>(inIndices);
+	const uint32_t* inIndices32 = reinterpret_cast<const uint32_t*>(inIndices);
+	// TODO: more accure check for overlap
+	if (_outbuffer->getIndexBufferBinding().buffer==_inbuffer->getIndexBufferBinding().buffer)
+	{
+		const size_t dataSize = indexSize*idxCount;
+		indexCopy = _NBL_ALIGNED_MALLOC(dataSize,indexSize);
+		memcpy(outIndices,inIndices,dataSize);
+		inIndices16 = reinterpret_cast<const uint16_t*>(dataSize);
+		inIndices32 = reinterpret_cast<const uint32_t*>(dataSize);
+	}
+	uint16_t* outIndices16 = reinterpret_cast<uint16_t*>(outIndices);
+	uint32_t* outIndices32 = reinterpret_cast<uint32_t*>(outIndices);
+
+	const uint32_t vertexCount = IMeshManipulator::upperBoundVertexID(_inbuffer);
+	core::vector<core::vectorSIMDf> vertexPositions(vertexCount);
+	for (uint32_t i=0u; i<vertexCount; ++i)
+		_inbuffer->getAttribute(vertexPositions[i],_inbuffer->getPositionAttributeIx(),i);
+
+	uint32_t* const hardClusters = reinterpret_cast<uint32_t*>(_NBL_ALIGNED_MALLOC((idxCount/3)*sizeof(uint32_t),_NBL_SIMD_ALIGNMENT));
 	const size_t hardClusterCount = indexType == asset::EIT_16BIT ?
-		genHardBoundaries(hardClusters, (uint16_t*)indices, idxCount, vertexCount) :
-		genHardBoundaries(hardClusters, (uint32_t*)indices, idxCount, vertexCount);
+		genHardBoundaries(hardClusters, inIndices16, idxCount, vertexCount) :
+		genHardBoundaries(hardClusters, inIndices32, idxCount, vertexCount);
 
-	uint32_t* const softClusters = (uint32_t*)_NBL_ALIGNED_MALLOC((idxCount/3 + 1) * 4,_NBL_SIMD_ALIGNMENT);
+	uint32_t* const softClusters = reinterpret_cast<uint32_t*>(_NBL_ALIGNED_MALLOC((idxCount/3+1)*sizeof(uint32_t),_NBL_SIMD_ALIGNMENT));
 	const size_t softClusterCount = indexType == asset::EIT_16BIT ?
-		genSoftBoundaries(softClusters, (uint16_t*)indices, idxCount, vertexCount, hardClusters, hardClusterCount, _threshold) :
-		genSoftBoundaries(softClusters, (uint32_t*)indices, idxCount, vertexCount, hardClusters, hardClusterCount, _threshold);
+		genSoftBoundaries(softClusters, inIndices16, idxCount, vertexCount, hardClusters, hardClusterCount, _threshold) :
+		genSoftBoundaries(softClusters, inIndices32, idxCount, vertexCount, hardClusters, hardClusterCount, _threshold);
 
 	ClusterSortData* sortedData = (ClusterSortData*)_NBL_ALIGNED_MALLOC(softClusterCount*sizeof(ClusterSortData),_NBL_SIMD_ALIGNMENT);
 	if (indexType == asset::EIT_16BIT)
-		calcSortData(sortedData, (uint16_t*)indices, idxCount, vertexPositions, softClusters, softClusterCount);
+		calcSortData(sortedData, inIndices16, idxCount, vertexPositions, softClusters, softClusterCount);
 	else
-		calcSortData(sortedData, (uint32_t*)indices, idxCount, vertexPositions, softClusters, softClusterCount);
+		calcSortData(sortedData, inIndices32, idxCount, vertexPositions, softClusters, softClusterCount);
 
-	std::stable_sort(sortedData, sortedData + softClusterCount, std::greater<ClusterSortData>());
+	std::stable_sort(sortedData, sortedData+softClusterCount, std::greater<ClusterSortData>());
 
-	for (size_t it = 0, jt = 0; it < softClusterCount; ++it)
+	auto reorderIndices = [&](auto* out, const auto* in)
 	{
-		const uint32_t cluster = sortedData[it].cluster;
+		for (size_t it = 0, jt = 0; it < softClusterCount; ++it)
+		{
+			const uint32_t cluster = sortedData[it].cluster;
 
-		size_t start = softClusters[cluster];
-		size_t end = (cluster+1 < softClusterCount) ? softClusters[cluster+1] : idxCount/3;
+			size_t start = softClusters[cluster];
+			size_t end = (cluster+1<softClusterCount) ? softClusters[cluster+1]:idxCount/3;
 
-		if (indexType == asset::EIT_16BIT)
-		{
-			for (size_t i = start; i < end; ++i)
-			{
-				((uint16_t*)indices)[jt++] = ((uint16_t*)indicesCopy)[3*i + 0];
-				((uint16_t*)indices)[jt++] = ((uint16_t*)indicesCopy)[3*i + 1];
-				((uint16_t*)indices)[jt++] = ((uint16_t*)indicesCopy)[3*i + 2];
-			}
-		}
-		else
-		{
 			for (size_t i = start; i < end; ++i)
 			{
-				((uint32_t*)indices)[jt++] = ((uint32_t*)indicesCopy)[3*i + 0];
-				((uint32_t*)indices)[jt++] = ((uint32_t*)indicesCopy)[3*i + 1];
-				((uint32_t*)indices)[jt++] = ((uint32_t*)indicesCopy)[3*i + 2];
+				out[jt++] = in[3 * i + 0];
+				out[jt++] = in[3 * i + 1];
+				out[jt++] = in[3 * i + 2];
 			}
 		}
-	}
+	};
+	if (indexType==asset::EIT_16BIT)
+		reorderIndices(outIndices16,inIndices16);
+	else
+		reorderIndices(outIndices32,inIndices32);
 
-	_NBL_ALIGNED_FREE(indicesCopy);
+	if (indexCopy)
+		_NBL_ALIGNED_FREE(indexCopy);
 	_NBL_ALIGNED_FREE(hardClusters);
 	_NBL_ALIGNED_FREE(softClusters);
 	_NBL_ALIGNED_FREE(sortedData);
-
-	return outbuffer;
 }
 
 template<typename IdxT>

src/nbl/asset/utils/COverdrawMeshOptimizer.cpp

@@ -9,52 +9,52 @@
 
 // Based on zeux's meshoptimizer (https://github.com/zeux/meshoptimizer) available under MIT license
 
-namespace nbl { namespace asset
+namespace nbl
+{
+namespace asset
 {
 
 class COverdrawMeshOptimizer
 {
-	enum
-	{
-		CACHE_SIZE = 16
-	};
-
-	struct ClusterSortData
-	{
-		uint32_t cluster;
-		float dot;
+		_NBL_STATIC_INLINE_CONSTEXPR size_t CACHE_SIZE = 16;
 
-		bool operator>(const ClusterSortData& other) const
+		struct ClusterSortData
 		{
-			// high product = possible occluder, render early
-			return dot > other.dot;
-		}
-	};
-
-	// private, undefined constructor
-	COverdrawMeshOptimizer() = delete;
-
-public:
-	//! Creates new or modifies given mesh reordering indices to reduce pixel overdraw and vertex shader invocations.
-	/**
-	@param _inbuffer Input mesh buffer.
-	@param _createNew Flag deciding whether to create new mesh (not modifying given one) or just optimize given mesh. Defaulted to true (i.e. create new).
-	@param _threshold Indicates how much the overdraw optimizer can degrade vertex cache efficiency (1.05 = up to 5%) to reduce overdraw more efficiently. Defaulted to 1.05 (i.e. 5%).
-	*/
-	static core::smart_refctd_ptr<asset::ICPUMeshBuffer> createOptimized(asset::ICPUMeshBuffer* _inbuffer, bool _createNew = true, float _threshold = 1.05f);
-
-private:
-	template<typename IdxT>
-	static size_t genHardBoundaries(uint32_t* _dst, const IdxT* _indices, size_t _idxCount, size_t _vtxCount);
-	template<typename IdxT>
-	static size_t genSoftBoundaries(uint32_t* _dst, const IdxT* _indices, size_t _idxCount, size_t _vtxCount, const uint32_t* _clusters, size_t _clusterCount, float _threshold);
-
-	template<typename IdxT>
-	static void calcSortData(ClusterSortData* _dst, const IdxT* _indices, size_t _idxCount, const core::vector<core::vectorSIMDf>& _positions, const uint32_t* _clusters, size_t _clusterCount);
-
-	static size_t updateCache(uint32_t _a, uint32_t _b, uint32_t _c, size_t* _cacheTimestamps, size_t& _timestamp);
+			uint32_t cluster;
+			float dot;
+
+			bool operator>(const ClusterSortData& other) const
+			{
+				// high product = possible occluder, render early
+				return dot > other.dot;
+			}
+		};
+
+		// private, undefined constructor
+		COverdrawMeshOptimizer() = delete;
+
+	public:
+		//! Creates new or modifies given mesh reordering indices to reduce pixel overdraw and vertex shader invocations.
+		/**
+		@param _inbuffer Input mesh buffer.
+		@param _createNew Flag deciding whether to create new mesh (not modifying given one) or just optimize given mesh. Defaulted to true (i.e. create new).
+		@param _threshold Indicates how much the overdraw optimizer can degrade vertex cache efficiency (1.05 = up to 5%) to reduce overdraw more efficiently. Defaulted to 1.05 (i.e. 5%).
+		*/
+		static void createOptimized(asset::ICPUMeshBuffer* _outbuffer, const asset::ICPUMeshBuffer* _inbuffer, float _threshold = 1.05f);
+
+	private:
+		template<typename IdxT>
+		static size_t genHardBoundaries(uint32_t* _dst, const IdxT* _indices, size_t _idxCount, size_t _vtxCount);
+		template<typename IdxT>
+		static size_t genSoftBoundaries(uint32_t* _dst, const IdxT* _indices, size_t _idxCount, size_t _vtxCount, const uint32_t* _clusters, size_t _clusterCount, float _threshold);
+
+		template<typename IdxT>
+		static void calcSortData(ClusterSortData* _dst, const IdxT* _indices, size_t _idxCount, const core::vector<core::vectorSIMDf>& _positions, const uint32_t* _clusters, size_t _clusterCount);
+
+		static size_t updateCache(uint32_t _a, uint32_t _b, uint32_t _c, size_t* _cacheTimestamps, size_t& _timestamp);
 };
 
-}}
+}
+}
 
 #endif

src/nbl/asset/utils/COverdrawMeshOptimizer.h

@@ -138,7 +138,7 @@ namespace nbl
 			}
 		}
 
-		CSmoothNormalGenerator::VertexHashMap CSmoothNormalGenerator::setupData(asset::ICPUMeshBuffer * buffer, float epsilon)
+		CSmoothNormalGenerator::VertexHashMap CSmoothNormalGenerator::setupData(const asset::ICPUMeshBuffer* buffer, float epsilon)
 		{
 			const size_t idxCount = buffer->getIndexCount();
 			_NBL_DEBUG_BREAK_IF((idxCount % 3));

src/nbl/asset/utils/CSmoothNormalGenerator.cpp

@@ -71,7 +71,7 @@ class CSmoothNormalGenerator
 	};
 
 private:
-	static VertexHashMap setupData(asset::ICPUMeshBuffer* buffer, float epsilon);
+	static VertexHashMap setupData(const asset::ICPUMeshBuffer* buffer, float epsilon);
 	static void processConnectedVertices(asset::ICPUMeshBuffer* buffer, VertexHashMap& vertices, float epsilon, uint32_t normalAttrID, IMeshManipulator::VxCmpFunction vxcmp);
 
 };