diff --git a/surfaces/blender/brokkr_bridge/blender_target.py b/surfaces/blender/brokkr_bridge/blender_target.py index dcb1add..73a88b8 100644 --- a/surfaces/blender/brokkr_bridge/blender_target.py +++ b/surfaces/blender/brokkr_bridge/blender_target.py @@ -854,12 +854,15 @@ class BrokkrBlenderTarget: mesh.vertices.foreach_get("co", coords) finally: evaluated.to_mesh_clear() - entry = found.get(obj.name) or {"count": 0, "first": [], "min": [], "max": []} + entry = found.get(obj.name) + reached = entry is not None + entry = entry or {"count": 0, "first": [], "min": [], "max": []} finite = all(math.isfinite(c) for c in coords) if coords is not None else True finite = finite and all(math.isfinite(c) for c in (entry["min"] or []) + (entry["max"] or []) + [c for p in entry["first"] for c in p]) points = [coords[i:i + 3] for i in range(0, 3 * count, 3)] if coords is not None else [] result.update({ - "instanceCount": entry["count"], + # An object the stopped scan never reached has an unknown count, not zero. + "instanceCount": entry["count"] if complete or reached else None, "instancesComplete": complete, "positionsSkipped": count > EVALUATED_VERTEX_LIMIT, "nonFinite": not finite, diff --git a/surfaces/blender/brokkr_bridge/node_actions.py b/surfaces/blender/brokkr_bridge/node_actions.py index 844a6d8..2f46eeb 100644 --- a/surfaces/blender/brokkr_bridge/node_actions.py +++ b/surfaces/blender/brokkr_bridge/node_actions.py @@ -298,7 +298,7 @@ class NodeActions: def _removeModifier(self, command: dict[str, Any]) -> tuple[str, str]: obj = self._object(command) - name = str(command.get("modifierName") or "Brokkr GeoNodes") + name = text(command, "modifierName", KEY_PATTERN, "modifierName", required=False) or "Brokkr GeoNodes" modifier = obj.modifiers.get(name) if modifier is None: return f"Modifier {name!r} was already absent.", obj.name @@ -321,18 +321,29 @@ class NodeActions: return f"Modifier input {item.name} set.", obj.name +def node_key(node: Any) -> Any: + """A stable identity for a node: bpy hands out a fresh wrapper per access, so id() of one is reused by the next.""" + return node.as_pointer() if hasattr(node, "as_pointer") else id(node) + + def reaches(tree: Any, start: Any, goal: Any) -> bool: - """True if following links forward from `start` arrives at `goal` (so a link goal -> start closes a cycle).""" - seen: set[int] = set() + """True if following links forward from `start` arrives at `goal` (so a link goal -> start closes a cycle). + The adjacency is built once per call, so a call costs O(links).""" + forward: dict[Any, list[Any]] = {} + for link in tree.links: + forward.setdefault(node_key(link.from_node), []).append(link.to_node) + target = node_key(goal) + seen: set[Any] = set() stack = [start] while stack: node = stack.pop() - if node == goal: + key = node_key(node) + if key == target: return True - if id(node) in seen: + if key in seen: continue - seen.add(id(node)) - stack.extend(l.to_node for l in tree.links if l.from_node == node) + seen.add(key) + stack.extend(forward.get(key, ())) return False diff --git a/surfaces/blender/docs/node-actions.md b/surfaces/blender/docs/node-actions.md index b9d5928..f98e702 100644 --- a/surfaces/blender/docs/node-actions.md +++ b/surfaces/blender/docs/node-actions.md @@ -25,9 +25,11 @@ polygon counts, `instanceCount`, and up to 256 vertex and instance locations (`t positions and `vertexBounds*` are in the object's local space; instance locations and `instanceBounds*` are world space. Above 1,000,000 vertices only counts are read (`positionsSkipped`); a non-finite coordinate sets `nonFinite` and drops the positions and bounds. The instances come from one depsgraph scan per capture. -`instancesComplete` is false when the depsgraph scan stopped at 100,000 items (`instanceCount` and the instance bounds are then lower bounds); the scan is what costs time (about 5 microseconds per instance in Python), so a 2000 x 2000 grid instanced on points captures in 1.4 s on the test machine where an unbounded scan took 22 s. `instanceCount` counts evaluated instances whose parent is the object, so a curve, point or volume result +`instancesComplete` is false when the depsgraph scan stopped at 100,000 items (`instanceCount` and the instance bounds are then lower bounds); the scan is what costs time (about 5 microseconds per instance in Python), so a 2000 x 2000 grid instanced on points captures in 1.4 s on the test machine where an unbounded scan took 22 s. `instanceCount` is `null` for an object the stopped scan never reached (unknown, not zero); the 100,000 budget is shared across all objects in depsgraph order, so a host after a heavy one can be starved. Otherwise `instanceCount` counts evaluated instances whose parent is the object, so a curve, point or volume result also reads as one instance. `mesh` stays the base mesh. +Known edges: a node duplicated by hand copies its `brokkr_id`, so id lookups act on the first match; `gin`/`gout` are protected by id, and a group whose output node was deleted by hand cannot get a new one through `addNode`; a group left with no users stays in the file until it is saved and reloaded. + Not built: zones, item sockets, multi-input ordering, library groups, group-in-group nodes, batches/rollback across intents, an expectedRevision check, per-node cost estimates. Int values are capped at +-4096 and floats at +-1e9, which bounds a request but not the cost of a graph: Blender evaluates what it is given, and the snapshot evaluates diff --git a/surfaces/blender/tests/live/gn_actions.py b/surfaces/blender/tests/live/gn_actions.py index a3a4016..af669ae 100644 --- a/surfaces/blender/tests/live/gn_actions.py +++ b/surfaces/blender/tests/live/gn_actions.py @@ -215,6 +215,20 @@ check("removeModifier removes it; evaluated block gone", len(bpy.data.objects["R bpy.data.objects["RHost"].modifiers.new("Sub", "SUBSURF") r = act(t4, action="removeModifier", objectName="RHost", modifierName="Sub"); check("non-Geometry-Nodes modifier is not removed", r["status"] == "failed" and len(bpy.data.objects["RHost"].modifiers) == 1, r["message"]) + +# ---- cycle detection on a long chain (bpy hands out a fresh wrapper per access; id() would be reused) +t5 = fresh() +ok(t5, action="createNodeGroup", treeKey="C", interface=GEO_OUT) +ids = ["c%d" % i for i in range(60)] +for i in ids: act(t5, action="addNode", treeKey="C", id=i, blIdname="ShaderNodeMath") +for a_, b_ in zip(ids, ids[1:]): act(t5, action="linkSockets", treeKey="C", fromNode=a_, fromSocket="Value", toNode=b_, toSocket="Value") +for a_, b_ in zip(ids[:-2], ids[2:]): act(t5, action="linkSockets", treeKey="C", fromNode=a_, fromSocket="Value", toNode=b_, toSocket="Value_001") +before = len(bpy.data.node_groups["C"].links) +t0 = time.time() +refused = [act(t5, action="linkSockets", treeKey="C", fromNode=ids[-1], fromSocket="Value", toNode=h, toSocket="Value_002")["status"] == "failed" for h in ids[:-1]] +R["cycle_probe_seconds"] = round(time.time() - t0, 3) +check("every closing link on a 60-node chain with skip links is refused", all(refused) and len(bpy.data.node_groups["C"].links) == before, (sum(refused), len(refused), R["cycle_probe_seconds"])) + # ---- cost: a heavy graph must not stall capture def heavy(n, instanced): tt = fresh() diff --git a/surfaces/blender/tests/live/gn_actions.result.json b/surfaces/blender/tests/live/gn_actions.result.json index 460dcbf..1e4e68e 100644 --- a/surfaces/blender/tests/live/gn_actions.result.json +++ b/surfaces/blender/tests/live/gn_actions.result.json @@ -944,6 +944,20 @@ "ok": true, "detail": "'Sub' is not a Geometry Nodes modifier; only those are removed here." }, + { + "name": "createNodeGroup C", + "ok": true, + "detail": "Created node group; interface identifiers: output Geometry=Socket_0" + }, + { + "name": "every closing link on a 60-node chain with skip links is refused", + "ok": true, + "detail": [ + 59, + 59, + 0.008 + ] + }, { "name": "createNodeGroup Heavy", "ok": true, @@ -993,7 +1007,7 @@ "name": "2000x2000 instanced grid: capture is bounded", "ok": true, "detail": [ - 1.38, + 1.29, 99999, false ] @@ -1032,6 +1046,7 @@ ] } ], - "capture_2000_grid_instanced_seconds": 1.375, - "capture_2000_grid_mesh_seconds": 0.051 + "cycle_probe_seconds": 0.008, + "capture_2000_grid_instanced_seconds": 1.285, + "capture_2000_grid_mesh_seconds": 0.054 } \ No newline at end of file diff --git a/surfaces/blender/tests/test_node_actions.py b/surfaces/blender/tests/test_node_actions.py index 2597eb7..80adb79 100644 --- a/surfaces/blender/tests/test_node_actions.py +++ b/surfaces/blender/tests/test_node_actions.py @@ -31,12 +31,19 @@ class Sock: class Node(dict): """A node; ID properties are the dict part.""" + counter = 1000 + + def as_pointer(self): + return self.pointer + PROPS = {"operation": NS(type="ENUM", is_readonly=False), "flag": NS(type="BOOLEAN", is_readonly=False), "filepath": NS(type="STRING", is_readonly=False), "location": NS(type="FLOAT", is_readonly=False)} def __init__(self, idname): super().__init__() self.bl_idname, self.location, self.label, self.operation, self.flag = idname, [0, 0], "", "ADD", False + Node.counter += 1 + self.pointer = Node.counter self.bl_rna = NS(properties=self.PROPS) self.inputs = [Sock("A", "A"), Sock("Size", "Size", "VECTOR", (1.0, 1.0, 1.0)), Sock("B", "Dup"), Sock("B_VEC", "Dup")] self.outputs = [Sock("Out", "Out")] @@ -61,6 +68,24 @@ class Nodes(list): list.remove(self, node) +class Proxy: + """A fresh wrapper per access, as bpy hands out: equal by pointer, with a different id() each time.""" + + def __init__(self, node): + self._node = node + + def as_pointer(self): + return self._node.pointer + + def __getattr__(self, name): + return getattr(self._node, name) + + def __eq__(self, other): + return getattr(other, "pointer", None) == self._node.pointer or (isinstance(other, Proxy) and other._node is self._node) + + __hash__ = None + + class Links(list): def __init__(self, valid=True): super().__init__() @@ -69,7 +94,7 @@ class Links(list): def new(self, a, b): for old in [l for l in self if l.to_socket is b]: # a single-link input loses its old link, as in Blender list.remove(self, old) - link = NS(from_socket=a, to_socket=b, from_node=a.node, to_node=b.node, is_valid=self.valid(a, b) if callable(self.valid) else self.valid) + link = NS(from_socket=a, to_socket=b, from_node=Proxy(a.node), to_node=Proxy(b.node), is_valid=self.valid(a, b) if callable(self.valid) else self.valid) self.append(link) return link @@ -331,6 +356,44 @@ class NodeActionTests(unittest.TestCase): self.assertEqual(list(tree.links), before) self.accepted(action="linkSockets", treeKey="G", fromNode="a", fromSocket="Out", toNode="c", toSocket="Size") # a forward skip is no cycle + def test_cycle_detection_survives_fresh_node_wrappers_on_long_chains(self): + tree = self.group() + ids = [f"n{i}" for i in range(12)] + for node_id in ids: + self.accepted(action="addNode", treeKey="G", id=node_id, blIdname="ShaderNodeMath") + for a, b in zip(ids, ids[1:]): + self.accepted(action="linkSockets", treeKey="G", fromNode=a, fromSocket="Out", toNode=b, toSocket="A") + # a diamond ladder: every rung reachable two ways + for a, b in zip(ids[:-2], ids[2:]): + self.accepted(action="linkSockets", treeKey="G", fromNode=a, fromSocket="Out", toNode=b, toSocket="Size") + count = len(tree.links) + for head in ids[:-1]: + self.refused(action="linkSockets", treeKey="G", fromNode=ids[-1], fromSocket="Out", toNode=head, toSocket="Size") + self.assertEqual(len(tree.links), count) + + def test_cycle_through_a_later_branch_is_found(self): + tree = self.group() + for node_id in "abcd": + self.accepted(action="addNode", treeKey="G", id=node_id, blIdname="ShaderNodeMath") + for src, dst in (("a", "b"), ("a", "c"), ("c", "d")): + self.accepted(action="linkSockets", treeKey="G", fromNode=src, fromSocket="Out", toNode=dst, toSocket="A") + self.refused(action="linkSockets", treeKey="G", fromNode="d", fromSocket="Out", toNode="a", toSocket="A") + + def test_cycle_walk_visits_each_node_once_on_a_ladder(self): + tree = self.group() + ids = [f"n{i}" for i in range(40)] + for node_id in ids: + self.accepted(action="addNode", treeKey="G", id=node_id, blIdname="ShaderNodeMath") + for a, b in zip(ids, ids[1:]): + tree.links.new(tree.nodes[2 + ids.index(a)].outputs[0], tree.nodes[2 + ids.index(b)].inputs[0]) + for a, b in zip(ids, ids[2:]): + tree.links.new(tree.nodes[2 + ids.index(a)].outputs[0], tree.nodes[2 + ids.index(b)].inputs[1]) + calls = [] + real = na.node_key + with unittest.mock.patch.object(na, "node_key", lambda n: calls.append(1) or real(n)): + self.assertFalse(na.reaches(tree, tree.nodes[2], NS(pointer=-1, as_pointer=lambda: -1))) + self.assertLess(len(calls), 4 * len(tree.links)) + def test_remove_node(self): tree = self.chain() self.accepted(action="removeNode", treeKey="G", id="b") @@ -368,6 +431,8 @@ class NodeActionTests(unittest.TestCase): self.assertEqual(host.tagged, tags + 1) self.assertIn("already absent", self.accepted(action="removeModifier", objectName="Host", modifierName="GN")["message"]) self.refused(action="removeModifier", objectName="Nope", modifierName="GN") + for bad in (["x"], "x" * 64, "a/b"): + self.refused(action="removeModifier", objectName="Host", modifierName=bad) # setObjectCustomProperty on a Geometry Nodes host def test_custom_property_naming_a_modifier_input_identifier_is_refused(self): @@ -492,8 +557,13 @@ class NodeActionTests(unittest.TestCase): host = self.snapshot_fixture([(0.0, 0.0, 0.0)], many()) with unittest.mock.patch.object(bt, "EVALUATED_INSTANCE_SCAN_LIMIT", 10): - data = self.evaluated(host) + scan = self.target._instance_scan() + data = self.target._evaluated_snapshot(host, scan) self.assertEqual((data["instanceCount"], data["instancesComplete"]), (10, False)) + other = Obj("Other") + other.modifiers.new("GN", "NODES") + other.evaluated_get = host.evaluated_get + self.assertIsNone(self.target._evaluated_snapshot(other, scan)["instanceCount"]) # never reached: unknown, not 0 self.assertLessEqual(len(visited), 11) # the generator is abandoned, not drained host = self.snapshot_fixture([(0.0, 0.0, 0.0)], [self.instance(host, (1.0, 0.0, 0.0))]) self.assertTrue(self.evaluated(host)["instancesComplete"])