Fixed particular voxel graph not compiling properly.

In the following case:
- A depends on B
- A depends on C
- B depends on C
Dependency search was finding C twice.

generators/graph/program_graph.cpp

@@ -1,5 +1,7 @@
 #include "program_graph.h"
+#include "../../util/container_funcs.h"
 #include "../../util/errors.h"
+
 #include <core/io/file_access.h>
 #include <core/io/resource.h>
 #include <core/variant/variant.h>
@@ -274,33 +276,30 @@ void ProgramGraph::find_dependencies(std::vector<uint32_t> nodes_to_process, std
 	std::unordered_set<uint32_t> visited_nodes;
 
 	while (nodes_to_process.size() > 0) {
+	found:
+		// The loop can come back multiple times to the same node, until all its dependencies have been processed.
 		const Node &node = get_node(nodes_to_process.back());
-		const uint32_t nodes_to_process_begin = nodes_to_process.size();
 		ZN_ASSERT_MSG(nodes_to_process.size() <= get_nodes_count(), "Invalid graph?");
 
-		// Find ancestors
-		for (uint32_t ii = 0; ii < node.inputs.size(); ++ii) {
-			const Port &p = node.inputs[ii];
-			for (auto cit = p.connections.begin(); cit != p.connections.end(); ++cit) {
-				const PortLocation src = *cit;
-				// A node can have two connections to the same destination node
-				if (range_contains(nodes_to_process, src.node_id, nodes_to_process_begin, nodes_to_process.size())) {
-					continue;
+		// Pick first non-visited dependency
+		for (const Port &port : node.inputs) {
+			for (const PortLocation src : port.connections) {
+				if (visited_nodes.find(src.node_id) == visited_nodes.end()) {
+					nodes_to_process.push_back(src.node_id);
+					goto found;
 				}
-				if (visited_nodes.find(src.node_id) != visited_nodes.end()) {
-					continue;
-				}
-				nodes_to_process.push_back(src.node_id);
 			}
 		}
 
-		if (nodes_to_process_begin == nodes_to_process.size()) {
-			// No ancestor to visit, process the node
-			out_order.push_back(node.id);
-			visited_nodes.insert(node.id);
-			nodes_to_process.pop_back();
-		}
+		// No dependencies left to visit, process node
+		out_order.push_back(node.id);
+		visited_nodes.insert(node.id);
+		nodes_to_process.pop_back();
 	}
+
+#if DEBUG_ENABLED
+	ZN_ASSERT(!has_duplicate(to_span_const(out_order)));
+#endif
 }
 
 void ProgramGraph::find_immediate_dependencies(uint32_t node_id, std::vector<uint32_t> &deps) const {
@@ -323,37 +322,6 @@ void ProgramGraph::find_immediate_dependencies(uint32_t node_id, std::vector<uin
 	}
 }
 
-void ProgramGraph::find_depth_first(uint32_t start_node_id, std::vector<uint32_t> &order) const {
-	// Finds each descendant from the given node, and returns them in the order they were found, depth-first.
-
-	std::vector<uint32_t> nodes_to_process;
-	std::unordered_set<uint32_t> visited_nodes;
-
-	nodes_to_process.push_back(start_node_id);
-
-	while (nodes_to_process.size() > 0) {
-		const Node &node = get_node(nodes_to_process.back());
-		nodes_to_process.pop_back();
-		uint32_t nodes_to_process_begin = nodes_to_process.size();
-		order.push_back(node.id);
-		visited_nodes.insert(node.id);
-
-		for (uint32_t oi = 0; oi < node.outputs.size(); ++oi) {
-			const Port &p = node.outputs[oi];
-			for (auto cit = p.connections.begin(); cit != p.connections.end(); ++cit) {
-				PortLocation dst = *cit;
-				if (range_contains(nodes_to_process, dst.node_id, nodes_to_process_begin, nodes_to_process.size())) {
-					continue;
-				}
-				if (visited_nodes.find(dst.node_id) != visited_nodes.end()) {
-					continue;
-				}
-				nodes_to_process.push_back(dst.node_id);
-			}
-		}
-	}
-}
-
 void ProgramGraph::debug_print_dot_file(String file_path) const {
 	// https://www.graphviz.org/pdf/dotguide.pdf
 

generators/graph/program_graph.h

@@ -88,7 +88,6 @@ public:
 	void find_dependencies(uint32_t node_id, std::vector<uint32_t> &out_order) const;
 	void find_dependencies(std::vector<uint32_t> nodes_to_process, std::vector<uint32_t> &out_order) const;
 	void find_immediate_dependencies(uint32_t node_id, std::vector<uint32_t> &deps) const;
-	void find_depth_first(uint32_t start_node_id, std::vector<uint32_t> &order) const;
 	void find_terminal_nodes(std::vector<uint32_t> &node_ids) const;
 
 	template <typename F>

tests/tests.cpp

@@ -722,6 +722,29 @@ void test_voxel_graph_equivalence_merging() {
 	}
 }
 
+// https://github.com/Zylann/godot_voxel/issues/427
+void test_voxel_graph_issue427() {
+	Ref<VoxelGeneratorGraph> graph;
+	graph.instantiate();
+
+	const uint32_t n_in_y = graph->create_node(VoxelGeneratorGraph::NODE_INPUT_Y, Vector2()); // 1
+	const uint32_t n_sub = graph->create_node(VoxelGeneratorGraph::NODE_SUBTRACT, Vector2()); // 2
+	const uint32_t n_out_sdf = graph->create_node(VoxelGeneratorGraph::NODE_OUTPUT_SDF, Vector2()); // 3
+	const uint32_t n_mul = graph->create_node(VoxelGeneratorGraph::NODE_MULTIPLY, Vector2()); // 4
+	const uint32_t n_fn2_2d = graph->create_node(VoxelGeneratorGraph::NODE_FAST_NOISE_2_2D, Vector2()); // 5
+	const uint32_t n_distance_3d = graph->create_node(VoxelGeneratorGraph::NODE_DISTANCE_3D, Vector2()); // 6
+
+	graph->add_connection(n_in_y, 0, n_sub, 0);
+	graph->add_connection(n_sub, 0, n_out_sdf, 0);
+	graph->add_connection(n_fn2_2d, 0, n_mul, 0);
+	graph->add_connection(n_distance_3d, 0, n_mul, 1);
+	// Was crashing after adding this connection
+	graph->add_connection(n_mul, 0, n_sub, 1);
+
+	VoxelGraphRuntime::CompilationResult result = graph->compile(true);
+	ZYLANN_TEST_ASSERT(result.success);
+}
+
 void test_island_finder() {
 	const char *cdata = "X X X - X "
 						"X X X - - "
@@ -2327,6 +2350,7 @@ void run_voxel_tests() {
 	VOXEL_TEST(test_voxel_graph_generator_expressions);
 	VOXEL_TEST(test_voxel_graph_generator_texturing);
 	VOXEL_TEST(test_voxel_graph_equivalence_merging);
+	VOXEL_TEST(test_voxel_graph_issue427);
 	VOXEL_TEST(test_island_finder);
 	VOXEL_TEST(test_unordered_remove_if);
 	VOXEL_TEST(test_instance_data_serialization);

util/container_funcs.h

@@ -97,6 +97,11 @@ size_t find_duplicate(Span<const T> items) {
 	return items.size();
 }
 
+template <typename T>
+bool has_duplicate(Span<const T> items) {
+	return find_duplicate(items) != items.size();
+}
+
 // Tests if POD items in an array are all the same.
 // Better tailored for more than hundred items that have power-of-two size.
 template <typename Item_T>