Blender node actions: Soul fixes (cycle walk keyed by pointer, O(links); unknown instance counts)
bpy hands out a fresh wrapper per access, so id() keys made reaches() stop early and accept cycles on chains of six or more nodes; keyed by as_pointer() with one adjacency map per call. Objects the stopped instance scan never reached report instanceCount null. removeModifier validates its name. Live: 158 checks pass on Blender 5.2.3. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Libby
parent
a318851ed2
commit
23a72b2579
6 changed files with 130 additions and 15 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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"])
|
||||
|
|
|
|||
Loading…
Reference in a new issue