detect cycles of BxDF layers, optimize the DOT Graphviz printing to not print in cycles

Author: devsh

examples_tests

@@ -778,21 +778,6 @@ class CFrontendIR : public CNodePool
 inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, system::logger_opt_ptr logger) const
 {
 	constexpr auto ELL_ERROR = system::ILogger::E_LOG_LEVEL::ELL_ERROR;
-
-	core::stack<const CLayer*> layerStack;
-	auto pushLayer = [&](const TypedHandle<const CLayer> layerHandle)->bool
-	{
-		const auto* layer = deref(layerHandle);
-		if (!layer)
-		{
-			logger.log("Layer node %u of type %s not a `CLayer` node!",ELL_ERROR,layerHandle.untyped.value,getTypeName(layerHandle).data());
-			return false;
-		}
-		layerStack.push(layer);
-		return true;
-	};
-	if (!pushLayer(rootHandle))
-		return false;
 
 	enum class SubtreeContributorState : uint8_t
 	{
@@ -807,6 +792,11 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
 		SubtreeContributorState contribState = SubtreeContributorState::Required;
 	};
 	core::stack<StackEntry> exprStack;
+	// unused yet
+	core::unordered_set<TypedHandle<const INode>,HandleHash> visitedNodes;
+	// should probably size it better, if I knew total node count allocated or live
+	visitedNodes.reserve(m_rootNodes.size()<<3);
+	//
 	auto validateExpression = [&](const TypedHandle<const IExprNode> exprRoot, const bool isBTDF) -> bool
 	{
 		if (!exprRoot)
@@ -865,6 +855,8 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
 						logger.log("Expression too complex, more than %d contributors encountered",ELL_ERROR,MaxContributors);
 						return false;
 					}
+					// cannot optimize with `unordered_set visitedNodes` because we need to check contributor slots, if we really wanted to we could do it with an
+					// `unordered_map` telling us the contributor slot range remapping (and presence of contributor) but right now it would be premature optimization.
 					exprStack.push(newEntry);
 				}
 				else if (childHandle)
@@ -893,15 +885,30 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
 		}
 		return true;
 	};
-	while (!layerStack.empty())
+
+	core::vector<const CLayer*> layerStack;
+	auto pushLayer = [&](const TypedHandle<const CLayer> layerHandle)->bool
 	{
-		const auto* layer = layerStack.top();
-		layerStack.pop();
-		if (layer->coated && !pushLayer(layer->coated))
+		const auto* layer = deref(layerHandle);
+		if (!layer)
 		{
-			logger.log("\tcoatee %d was specificed but is of wrong type",ELL_ERROR,layer->coated);
+			logger.log("Layer node %u of type %s not a `CLayer` node!",ELL_ERROR,layerHandle.untyped.value,getTypeName(layerHandle).data());
 			return false;
 		}
+		auto found = std::find(layerStack.begin(),layerStack.end(),layer);
+		if (found!=layerStack.end())
+		{
+			logger.log("Layer node %u is involved in a Cycle!",ELL_ERROR,layerHandle.untyped.value);
+			return false;
+		}
+		layerStack.push_back(layer);
+		return true;
+	};
+	if (!pushLayer(rootHandle))
+		return false;
+	while (true)
+	{
+		const auto* layer = layerStack.back();
 		if (!layer->brdfTop && !layer->btdf && !layer->brdfBottom)
 		{
 			logger.log("At least one BRDF or BTDF in the Layer is required.",ELL_ERROR);
@@ -913,6 +920,13 @@ inline bool CFrontendIR::valid(const TypedHandle<const CLayer> rootHandle, syste
 			return false;
 		if (!validateExpression(layer->brdfBottom,false))
 			return false;
+		if (!layer->coated)
+			break;
+		if (!pushLayer(layer->coated))
+		{
+			logger.log("\tcoatee %d was specificed but is invalid!",ELL_ERROR,layer->coated);
+			return false;
+		}
 	}
 	return true;
 }

include/nbl/asset/material_compiler3/CFrontendIR.h

@@ -25,7 +25,8 @@ class CNodePool : public core::IReferenceCounted
 			using value_t = uint32_t;
 			constexpr static inline value_t Invalid = ~value_t(0);
 
-			inline operator bool() const {return value!=Invalid;}
+			explicit inline operator bool() const {return value!=Invalid;}
+			inline bool operator==(const Handle& other) const {return value==other.value;}
 
 			// also serves as a byte offset into the pool
 			value_t value = Invalid;
@@ -89,7 +90,8 @@ class CNodePool : public core::IReferenceCounted
 		{
 			using node_type = T;
 
-			inline operator bool() const {return untyped;}
+			explicit inline operator bool() const {return bool(untyped);}
+			inline bool operator==(const TypedHandle& other) const {return untyped==other.untyped;}
 
 			inline operator const TypedHandle<const T>&() const
 			{
@@ -117,6 +119,13 @@ class CNodePool : public core::IReferenceCounted
 		}
 
 	protected:
+		struct HandleHash
+		{
+			inline size_t operator()(const TypedHandle<const INode> handle) const
+			{
+				return std::hash<Handle::value_t>()(handle.untyped.value);
+			}
+		};
 		// save myself some typing
 		using refctd_pmr_t = core::smart_refctd_ptr<core::refctd_memory_resource>;
 

include/nbl/asset/material_compiler3/CNodePool.h

@@ -219,7 +219,10 @@ auto CFrontendIR::createNamedFresnel(const std::string_view name) -> TypedHandle
 void CFrontendIR::printDotGraph(std::ostringstream& str) const
 {
 	str << "digraph {\n";
-
+	
+	core::unordered_set<TypedHandle<const INode>,HandleHash> visitedNodes;
+	// should probably size it better, if I knew total node count allocated or live
+	visitedNodes.reserve(m_rootNodes.size()<<3);
 	// TODO: track layering depth and indent accordingly?
 	// assign in reverse because we want materials to print in order
 	core::vector<TypedHandle<const CLayer>> layerStack(m_rootNodes.rbegin(),m_rootNodes.rend());
@@ -228,6 +231,11 @@ void CFrontendIR::printDotGraph(std::ostringstream& str) const
 	{
 		const auto layerHandle = layerStack.back();
 		layerStack.pop_back();
+		// don't print layer nodes multiple times
+		const auto visited = visitedNodes.find(layerHandle);
+		if (visited!=visitedNodes.end())
+			continue;
+		visitedNodes.insert(layerHandle);
 		const auto* layerNode = deref(layerHandle);
 		//
 		const auto layerID = getNodeID(layerHandle);
@@ -242,8 +250,14 @@ void CFrontendIR::printDotGraph(std::ostringstream& str) const
 		{
 			if (!root)
 				return;
+			// print the link from the layer to the expression
 			str << "\n\t" << layerID << " -> " << getNodeID(root) << "[label=\"" << edgeLabel << "\"]";
+			// but not the expression again
+			const auto visited = visitedNodes.find(root);
+			if (visited!=visitedNodes.end())
+				return;
 			exprStack.push(root);
+			visitedNodes.insert(root);
 		};
 		pushExprRoot(layerNode->brdfTop,"Top BRDF");
 		pushExprRoot(layerNode->btdf,"BTDF");
@@ -266,7 +280,11 @@ void CFrontendIR::printDotGraph(std::ostringstream& str) const
 					if (const auto child=deref(childHandle); child)
 					{
 						str << getNodeID(childHandle) << " ";
+						const auto visited = visitedNodes.find(childHandle);
+						if (visited!=visitedNodes.end())
+							continue;
 						exprStack.push(childHandle);
+						visitedNodes.insert(childHandle);
 					}
 				}
 				str << "}\n";