From 152f5dc897b39414157f03d766d1213946f63ea3 Mon Sep 17 00:00:00 2001
From: Jacques Lucke <jacques@blender.org>
Date: Wed, 30 Aug 2023 14:50:53 +0200
Subject: [PATCH] Fix #111633: crash with cyclic node tree zones

The problem was that there was a cyclic nesting dependency that was not detected
correctly, which led to problems further down the line. See the PR for details.

Right now, the zones just disappear when there is an error. This has to be improved
at some point, but it outside of the scope of this patch.

A regression test that covers this case is added as well.

Pull Request: https://projects.blender.org/blender/blender/pulls/111689
---
 .../blenkernel/intern/node_tree_zones.cc      | 59 +++++++++++++------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/source/blender/blenkernel/intern/node_tree_zones.cc b/source/blender/blenkernel/intern/node_tree_zones.cc
index 4986e6d5d9f..ec88a254adb 100644
--- a/source/blender/blenkernel/intern/node_tree_zones.cc
+++ b/source/blender/blenkernel/intern/node_tree_zones.cc
@@ -68,13 +68,23 @@ static Vector<std::unique_ptr<bNodeTreeZone>> find_zone_nodes(
 struct ZoneRelation {
   bNodeTreeZone *parent;
   bNodeTreeZone *child;
+
+  uint64_t hash() const
+  {
+    return get_default_hash_2(this->parent, this->child);
+  }
+
+  friend bool operator==(const ZoneRelation &a, const ZoneRelation &b)
+  {
+    return a.parent == b.parent && a.child == b.child;
+  }
 };
 
-static Vector<ZoneRelation> get_direct_zone_relations(
+static std::optional<Vector<ZoneRelation>> get_direct_zone_relations(
     const Span<std::unique_ptr<bNodeTreeZone>> all_zones,
     const BitGroupVector<> &depend_on_input_flag_array)
 {
-  Vector<ZoneRelation> zone_relations;
+  VectorSet<ZoneRelation> all_zone_relations;
 
   /* Gather all relations, even the transitive once. */
   for (const std::unique_ptr<bNodeTreeZone> &zone : all_zones) {
@@ -86,35 +96,42 @@ static Vector<ZoneRelation> get_direct_zone_relations(
       const BoundedBitSpan depend_on_input_flags = depend_on_input_flag_array[node->index()];
       bits::foreach_1_index(depend_on_input_flags, [&](const int parent_zone_i) {
         if (parent_zone_i != zone_i) {
-          zone_relations.append({all_zones[parent_zone_i].get(), zone.get()});
+          all_zone_relations.add_new({all_zones[parent_zone_i].get(), zone.get()});
         }
       });
     }
   }
 
+  for (const ZoneRelation &relation : all_zone_relations) {
+    const ZoneRelation reverse_relation{relation.child, relation.parent};
+    if (all_zone_relations.contains(reverse_relation)) {
+      /* There is a cyclic zone dependency. */
+      return std::nullopt;
+    }
+  }
+
   /* Remove transitive relations. This is a brute force algorithm currently. */
   Vector<int> transitive_relations;
-  for (const int a : zone_relations.index_range()) {
-    const ZoneRelation &relation_a = zone_relations[a];
-    for (const int b : zone_relations.index_range()) {
+  for (const int a : all_zone_relations.index_range()) {
+    const ZoneRelation &relation_a = all_zone_relations[a];
+    for (const int b : all_zone_relations.index_range()) {
       if (a == b) {
         continue;
       }
-      const ZoneRelation &relation_b = zone_relations[b];
-      for (const int c : zone_relations.index_range()) {
-        if (a == c || b == c) {
-          continue;
-        }
-        const ZoneRelation &relation_c = zone_relations[c];
-        if (relation_a.child == relation_b.parent && relation_a.parent == relation_c.parent &&
-            relation_b.child == relation_c.child)
-        {
-          transitive_relations.append_non_duplicates(c);
-        }
+      const ZoneRelation &relation_b = all_zone_relations[b];
+      if (relation_a.child != relation_b.parent) {
+        continue;
+      }
+      const ZoneRelation transitive_relation{relation_a.parent, relation_b.child};
+      const int transitive_relation_i = all_zone_relations.index_of_try(transitive_relation);
+      if (transitive_relation_i != -1) {
+        transitive_relations.append_non_duplicates(transitive_relation_i);
       }
     }
   }
   std::sort(transitive_relations.begin(), transitive_relations.end(), std::greater<>());
+
+  Vector<ZoneRelation> zone_relations = all_zone_relations.as_span();
   for (const int i : transitive_relations) {
     zone_relations.remove_and_reorder(i);
   }
@@ -266,11 +283,15 @@ static std::unique_ptr<bNodeTreeZones> discover_tree_zones(const bNodeTree &tree
     }
   }
 
-  const Vector<ZoneRelation> zone_relations = get_direct_zone_relations(
+  const std::optional<Vector<ZoneRelation>> zone_relations = get_direct_zone_relations(
       tree_zones->zones, depend_on_input_flag_array);
+  if (!zone_relations) {
+    /* Found cyclic relations. */
+    return {};
+  }
 
   /* Set parent and child pointers in zones. */
-  for (const ZoneRelation &relation : zone_relations) {
+  for (const ZoneRelation &relation : *zone_relations) {
     relation.parent->child_zones.append(relation.child);
     BLI_assert(relation.child->parent_zone == nullptr);
     relation.child->parent_zone = relation.parent;