Add a primitive reveral mode to IIndexingCallback::SContext

Make hash recompute disable-able in smooth normal calculation

fix some bugs like bad ListIndexingCallbacks and missing return values

Author: devshgraphicsprogramming

include/nbl/asset/IGeometryCollection.h

@@ -34,8 +34,7 @@ class NBL_API2 IGeometryCollection : public virtual core::IReferenceCounted
                     if (jointRedirectView.getElementCount()<geometry->getJointCount())
                         return false;
                 }
-                else
-                    return true;
+                return true;
             }
 
             inline bool hasTransform() const {return !core::isnan(transform[0][0]);}

include/nbl/asset/IPolygonGeometry.h

@@ -10,6 +10,7 @@
 #include "nbl/asset/IAccelerationStructure.h"
 
 #include <span>
+#include <ranges>
 
 namespace nbl::asset
 {
@@ -39,41 +40,53 @@ class IPolygonGeometryBase : public virtual core::IReferenceCounted
                 template<typename OutT> requires (sizeof(OutT)<8 && hlsl::concepts::UnsignedIntegralScalar<OutT>)
                 struct SContext final
                 {
-                    // `indexOfIndex` is somewhat of a baseIndex
-                    template<typename Range>
-                    inline void streamOut(const uint32_t indexOfIndex, const Range& permutation)
-                    {
-                        auto& typedOut = reinterpret_cast<OutT*&>(out);
-                        if (indexBuffer)                  
-                        switch (indexSize)
+                    private:
+                        // `indexOfIndex` is somewhat of a baseIndex
+                        template<typename Range>
+                        inline void impl_streamOut(const uint32_t indexOfIndex, const Range permutation)
                         {
-                            case 1:
-                                for (const auto relIx : permutation)
-                                    *(typedOut++) = reinterpret_cast<const uint8_t*>(indexBuffer)[indexOfIndex+relIx];
-                                break;
-                            case 2:
-                                for (const auto relIx : permutation)
-                                    *(typedOut++) = reinterpret_cast<const uint16_t*>(indexBuffer)[indexOfIndex+relIx];
-                                break;
-                            case 4:
-                                for (const auto relIx : permutation)
-                                    *(typedOut++) = reinterpret_cast<const uint32_t*>(indexBuffer)[indexOfIndex+relIx];
-                                break;
-                            default:
-                                assert(false);
-                                break;
+                            auto& typedOut = reinterpret_cast<OutT*&>(out);
+                            if (indexBuffer)                  
+                            switch (indexSize)
+                            {
+                                case 1:
+                                    for (const auto relIx : permutation)
+                                        *(typedOut++) = reinterpret_cast<const uint8_t*>(indexBuffer)[indexOfIndex+relIx];
+                                    break;
+                                case 2:
+                                    for (const auto relIx : permutation)
+                                        *(typedOut++) = reinterpret_cast<const uint16_t*>(indexBuffer)[indexOfIndex+relIx];
+                                    break;
+                                case 4:
+                                    for (const auto relIx : permutation)
+                                        *(typedOut++) = reinterpret_cast<const uint32_t*>(indexBuffer)[indexOfIndex+relIx];
+                                    break;
+                                default:
+                                    assert(false);
+                                    break;
+                            }
+                            else
+                            for (const auto relIx : permutation)
+                                *(typedOut++) = indexOfIndex+relIx;
                         }
-                        else
-                        for (const auto relIx : permutation)
-                            *(typedOut++) = indexOfIndex+relIx;
-                    }
-
-                    // always the base pointer, doesn't get advanced
-                    const void* const indexBuffer;
-                    const uint64_t indexSize : 3;
-                    const uint64_t beginPrimitive : 30;
-                    const uint64_t endPrimitive : 31;
-                    void* out;
+
+                    public:
+                        template<std::ranges::view V> requires std::ranges::bidirectional_range<V>
+                        inline void streamOut(const uint32_t indexOfIndex, const V& permutation)
+                        {
+                            if (reversePrims)
+                                impl_streamOut(indexOfIndex,permutation|std::views::reverse);
+                            else
+                                impl_streamOut(indexOfIndex,permutation);
+                        }
+
+                        // always the base pointer, doesn't get advanced
+                        const void* const indexBuffer;
+                        const uint64_t indexSize : 3;
+                        const uint64_t beginPrimitive : 30;
+                        const uint64_t endPrimitive : 30;
+                        const uint64_t reversePrims : 1 = false;
+                        void* out;
                 };
                 // could have been a static if not virtual
                 virtual void operator()(SContext<uint8_t>& ctx) const = 0;

src/nbl/asset/ICPUPolygonGeometry.cpp

@@ -12,8 +12,8 @@ class CListIndexingCB final : public IPolygonGeometryBase::IIndexingCallback
         template<typename OutT>
         static void operator_impl(SContext<OutT>& ctx)
         {
-            auto indexOfIndex = ctx.beginPrimitive*3;
-            for (const auto end=ctx.endPrimitive*3; indexOfIndex!=end; indexOfIndex+=3)
+            auto indexOfIndex = ctx.beginPrimitive*Order;
+            for (const auto end=ctx.endPrimitive*Order; indexOfIndex!=end; indexOfIndex+=Order)
                 ctx.streamOut(indexOfIndex,std::ranges::iota_view{0,int(Order)});
         }
 
@@ -70,7 +70,7 @@ class CTriangleStripIndexingCB final : public IPolygonGeometryBase::IIndexingCal
                 indexOfIndex = ctx.beginPrimitive+2;
             const int32_t perm[] = {-1,-2,0};
             for (const auto end=ctx.endPrimitive+2; indexOfIndex!=end; indexOfIndex++)
-                ctx.streamOut(indexOfIndex,perm);
+                ctx.streamOut<std::span<const int32_t>>(indexOfIndex,perm);
         }
 
     public:
@@ -106,7 +106,7 @@ class CTriangleFanIndexingCB final : public IPolygonGeometryBase::IIndexingCallb
             {
                 // first index is always global 0
                 perm[0] = -indexOfIndex;
-                ctx.streamOut(indexOfIndex,perm);
+                ctx.streamOut<std::span<const int32_t>>(indexOfIndex,perm);
             }
         }
 

src/nbl/asset/utils/CSmoothNormalGenerator.cpp

@@ -29,13 +29,18 @@ static bool compareVertexPosition(const hlsl::float32_t3& a, const hlsl::float32
 	return (difference.x <= epsilon && difference.y <= epsilon && difference.z <= epsilon);
 }
 
-CSmoothNormalGenerator::Result CSmoothNormalGenerator::calculateNormals(const asset::ICPUPolygonGeometry* polygon, float epsilon, VxCmpFunction vxcmp)
+CSmoothNormalGenerator::Result CSmoothNormalGenerator::calculateNormals(const asset::ICPUPolygonGeometry* polygon, float epsilon, VxCmpFunction vxcmp, const bool recomputeHash)
 {
 	assert(polygon->getIndexingCallback()->degree() == 3);
+
 	static constexpr auto MinEpsilon = 0.00001f;
 	const auto patchedEpsilon = epsilon < MinEpsilon ? MinEpsilon : epsilon;
 	VertexHashMap vertexHashMap = setupData(polygon, patchedEpsilon);
+	
 	const auto smoothPolygon = processConnectedVertices(polygon, vertexHashMap, patchedEpsilon,vxcmp);
+	if (recomputeHash)
+		CPolygonGeometryManipulator::recomputeContentHashes(smoothPolygon.get());
+
 	return { vertexHashMap, smoothPolygon };
 }
 
@@ -51,33 +56,42 @@ CSmoothNormalGenerator::VertexHashMap CSmoothNormalGenerator::setupData(const as
 	{
 		//calculate face normal of parent triangle
 		hlsl::float32_t3 v0, v1, v2;
+		// TODO: could iterate over an index buffer properly
 		polygon->getPositionView().decodeElement<hlsl::float32_t3>(i, v0);
 		polygon->getPositionView().decodeElement<hlsl::float32_t3>(i + 1, v1);
 		polygon->getPositionView().decodeElement<hlsl::float32_t3>(i + 2, v2);
 
-		const auto faceNormal = normalize(cross(v1 - v0, v2 - v0));
+		auto faceNormal = cross(v1 - v0, v2 - v0);
+		// if any triangle edge is 0 length, the cross product will be 0 length too
+		const float normLen2 = dot(faceNormal,faceNormal);
+		// need to filter invalid triangles while we're at it
+		if (normLen2<hlsl::numeric_limits<float>::min)
+			continue;
+		faceNormal *= hlsl::rsqrt(normLen2);
 
 		//set data for m_vertices
-		const auto angleWages = hlsl::shapes::util::anglesFromTriangleEdges(v2 - v1, v0 - v2, v1 - v2);
+		const auto angleWeights = hlsl::shapes::util::anglesFromTriangleEdges(v2 - v1, v0 - v2, v1 - v2);
 
-		vertices.add({ i,	0,	faceNormal * angleWages.x, v0});
-		vertices.add({ i + 1,	0,	faceNormal * angleWages.y,v1});
-		vertices.add({ i + 2,	0,	faceNormal * angleWages.z, v2});
+		vertices.add({ i,	0,	faceNormal * angleWeights.x, v0});
+		vertices.add({ i + 1,	0,	faceNormal * angleWeights.y,v1});
+		vertices.add({ i + 2,	0,	faceNormal * angleWeights.z, v2});
 	}
 
 	vertices.bake();
 
 	return vertices;
 }
 
-core::smart_refctd_ptr<ICPUPolygonGeometry> CSmoothNormalGenerator::processConnectedVertices(const asset::ICPUPolygonGeometry* polygon, VertexHashMap& vertexHashMap, float epsilon, VxCmpFunction vxcmp)
+core::smart_refctd_ptr<ICPUPolygonGeometry> CSmoothNormalGenerator::processConnectedVertices(const asset::ICPUPolygonGeometry* polygon, VertexHashMap& vertexHashMap, const float epsilon, VxCmpFunction vxcmp)
 {
+	// TODO: its semi doable to defer unwelding/rewelding until later an just work on a duplicated normal buffer only
 	auto outPolygon = core::move_and_static_cast<ICPUPolygonGeometry>(polygon->clone(0u));
 	static constexpr auto NormalFormat = EF_R32G32B32_SFLOAT;
 	const auto normalFormatBytesize = asset::getTexelOrBlockBytesize(NormalFormat);
 	auto normalBuf = ICPUBuffer::create({ normalFormatBytesize * outPolygon->getPositionView().getElementCount()});
 	auto normalView = polygon->getNormalView();
 
+	// TODO: compute actual range
 	hlsl::shapes::AABB<4,hlsl::float32_t> aabb;
 	aabb.maxVx = hlsl::float32_t4(1, 1, 1, 0.f);
 	aabb.minVx = -aabb.maxVx;
@@ -118,8 +132,6 @@ core::smart_refctd_ptr<ICPUPolygonGeometry> CSmoothNormalGenerator::processConne
 		memcpy(normalPtr + (normalStride * processedVertex.index), &normal, sizeof(normal));
 	}
 
-	CPolygonGeometryManipulator::recomputeContentHashes(outPolygon.get());
-
 	return outPolygon;
 }
 

src/nbl/asset/utils/CSmoothNormalGenerator.h

@@ -11,18 +11,20 @@ namespace nbl::asset
 {
 
 // TODO: implement a class template that take position type(either float32_t3 or float64_t3 as template argument
-class CSmoothNormalGenerator
+class CSmoothNormalGenerator final
 {
 	public:
 		CSmoothNormalGenerator() = delete;
 		~CSmoothNormalGenerator() = delete;
 
-    struct VertexData
-    {
-      uint32_t index;									     //offset of the vertex into index buffer
+		struct VertexData
+		{
+			//offset of the vertex into index buffer
+			uint32_t index;
 			uint32_t hash;
-      hlsl::float32_t3 weightedNormal;
-      hlsl::float32_t3 position;							   //position of the vertex in 3D space
+			hlsl::float32_t3 weightedNormal;
+			//position of the vertex in 3D space
+			hlsl::float32_t3 position;
 
 			hlsl::float32_t3 getPosition() const
 			{
@@ -39,21 +41,20 @@ class CSmoothNormalGenerator
 				return hash;
 			};
 
-    };
+		};
 
 		using VxCmpFunction = std::function<bool(const VertexData&, const VertexData&, const ICPUPolygonGeometry*)>;
 
 		using VertexHashMap = CVertexHashGrid<VertexData>;
 
-	  struct Result
-	  {
+		struct Result
+		{
 			VertexHashMap vertexHashGrid;
 			core::smart_refctd_ptr<ICPUPolygonGeometry> geom;
-	  };
-		static Result calculateNormals(const ICPUPolygonGeometry* polygon, float epsilon, VxCmpFunction function);
+		};
+		static Result calculateNormals(const ICPUPolygonGeometry* polygon, float epsilon, VxCmpFunction function, const bool recomputeHash=true);
 
 	private:
-
 		static VertexHashMap setupData(const ICPUPolygonGeometry* polygon, float epsilon);
 		static core::smart_refctd_ptr<ICPUPolygonGeometry> processConnectedVertices(const ICPUPolygonGeometry* polygon, VertexHashMap& vertices, float epsilon, VxCmpFunction vxcmp);
 };