diff --git a/meshroom/core/attribute.py b/meshroom/core/attribute.py index 7f26e27d2c..78875d9f8a 100644 --- a/meshroom/core/attribute.py +++ b/meshroom/core/attribute.py @@ -92,6 +92,14 @@ def __init__(self, node, attributeDesc: desc.Attribute, isOutput: bool, root=Non self._linkExpression: Optional[str] = None self._initValue() + self._isInsideList: bool = False + current = self._root() if self._root else None + while current is not None: + if isinstance(current, ListAttribute): + self._isInsideList = True + break + current = current._root() if current._root else None + def _getFullName(self) -> str: """ Get the attribute name following the path from the node to the attribute. @@ -105,11 +113,18 @@ def _getRootName(self) -> str: Return: groupName.subGroupName.name """ if isinstance(self.root, ListAttribute): - return f'{self.root.rootName}[{self.root.index(self)}]' + try: + return f'{self.root.rootName}[{self.root.index(self)}]' + except ValueError: + return f'{self.root.rootName}' elif isinstance(self.root, GroupAttribute): return f'{self.root.rootName}.{self._desc.name}' return self._desc.name + @property + def isInsideList(self) -> bool: + return self._isInsideList + def asLinkExpr(self) -> str: """ Return the link expression for this Attribute. diff --git a/meshroom/core/node.py b/meshroom/core/node.py index 465598d00a..eb94f03a82 100644 --- a/meshroom/core/node.py +++ b/meshroom/core/node.py @@ -1555,11 +1555,13 @@ def _updateNodeSize(self): def _getAttributeChangedCallback(self, attr: Attribute) -> Optional[Callable]: """ Get the node descriptor-defined value changed callback associated to `attr` if any. """ - # Callbacks cannot be defined on nested attributes. - if attr.root is not None: + # Callbacks cannot be defined on ListAttributes, but may be defined on nested attributes in GroupAttributes + if attr.root is not None and attr.isInsideList: return None - attrCapitalizedName = attr.name[:1].upper() + attr.name[1:] + attrCapitalizedName = "" + for sub in attr.rootName.split("."): + attrCapitalizedName += sub[:1].upper() + sub[1:] callbackName = f"on{attrCapitalizedName}Changed" callback = getattr(self.nodeDesc, callbackName, None) diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 482961b8c0..5e760b804f 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -1,10 +1,11 @@ +import logging +import pytest + +from meshroom.core.attribute import Attribute from meshroom.core.graph import Graph from tests.utils import registerNodeDesc from tests.nodes.test.nodeValidators import NodeWithValidators -import pytest - -import logging logger = logging.getLogger('test') valid3DExtensionFiles = [(f'test.{ext}', True) for ext in ('obj', 'stl', 'fbx', 'gltf', 'abc', 'ply', 'usda', 'usdc')] @@ -193,3 +194,42 @@ def test_attribute_isText_by_description_semantic(): # Then assert n0.input.isTextDisplayable + +def test_attribute_isInsideList(): + """ + Check whether an attribute is, either directly or indirectly, contained within a ListAttribute. + """ + graph = Graph("") + node = graph.addNewNode("GroupAttributes") + + # IntParam within a GroupAttribute: it has a parent, but it is not inside a list + attr1 = node.attribute("firstGroup.firstGroupIntA") + assert attr1.root + assert attr1.depth == 1 + assert not attr1.isInsideList + + # FloatParam within a nested GroupAttribute: it has parents, but it is not inside a list + attr2 = node.attribute("firstGroup.nestedGroup.nestedGroupFloat") + assert attr2.root + assert attr2.depth == 2 + assert not attr2.isInsideList + + # ListAttribute within a GroupAttribute: it has a parent but is not inside a list + attr3 = node.attribute("firstGroup.singleGroupedList") + assert attr3.root + assert attr3.depth == 1 + assert not attr3.isInsideList + + # Insert an IntParam into the ListAttribute: the list attribute is not inside a list, but the IntParam is inside a list + attr3.insert(0, 1) + + # Check that the value was correctly inserted + assert isinstance(attr3.value[0], Attribute) + assert attr3.value[0].value == 1 + + assert not attr3.isInsideList # Check that the ListAttribute itself is still not inside a list + + # Check that the IntParam inside the ListAttribute is considered to be inside a list + assert attr3.value[0].root + assert attr3.value[0].depth == 2 + assert attr3.value[0].isInsideList diff --git a/tests/test_nodeAttributeChangedCallback.py b/tests/test_nodeAttributeChangedCallback.py index 8812b21b2d..d45ac6ed28 100644 --- a/tests/test_nodeAttributeChangedCallback.py +++ b/tests/test_nodeAttributeChangedCallback.py @@ -183,6 +183,160 @@ def test_loadingGraphDoesNotTriggerCallbackForConnectedAttributes( assert loadedNodeB.affectedInput.value == 2 +class NodeWithGroupListAttributeChangedCallback(desc.BaseNode): + """ + A Node containing a GroupAttribute with a nested IntParam that has an 'on{GroupName}{ChildName}Changed' callback, + called whenever the nested attribute value is changed explicitly. + It also contains a ListAttribute with an 'on{ListName}Changed' callback to verify that callbacks are not triggered + for attributes nested inside a list (i.e. when ``isInsideList`` is True). + """ + + inputs = [ + desc.GroupAttribute( + name="groupInput", + label="Group Input", + description="GroupAttribute with a nested IntParam that has a value changed callback.", + items=[ + desc.IntParam( + name="int", + label="Int", + description="Attribute with a value changed callback (onGroupInputIntChanged).", + value=0, + range=None, + ) + ], + ), + desc.ListAttribute( + name="listInput", + label="List Input", + description="ListAttribute of FloatParams whose elements have isInsideList=True.", + elementDesc=desc.FloatParam( + name="float", + label="Float", + description="", + value=0, + range=None, + ), + ), + desc.IntParam( + name="affectedInt", + label="Affected Int Input", + description="Updated to groupInput.int * 2 whenever 'groupInput.int' is explicitly modified.", + value=0, + range=None, + ), + desc.FloatParam( + name="affectedFloat", + label="Affected Float Input", + description="Updated to listInput.float * 2 whenever 'listInput.float' is explicitly modified", + value=0.0, + range=None, + ) + ] + + def onGroupInputIntChanged(self, node: Node): + node.affectedInt.value = node.groupInput.int.value * 2 + + def onListInputChanged(self, node: Node): + """ + This callback's name matches 'listInput' but not any indexed element. + It can be triggered when the list content changes (e.g. append, remove). + """ + node.affectedFloat.value = 999.0 + + def onListInputFloatChanged(self, node: Node): + """ + This callback's name matches the list elements' name, but since the elements have isInsideList=True, + this callback should NOT be triggered when an element's value changes. + """ + node.affectedFloat.value = 400.0 + + +class TestAttributeCallbackForGroupListAttribute: + + @classmethod + def setup_class(cls): + registerNodeDesc(NodeWithGroupListAttributeChangedCallback) + + @classmethod + def teardown_class(cls): + unregisterNodeDesc(NodeWithGroupListAttributeChangedCallback) + + def test_assignValueToNestedGroupAttributeTriggersCallback(self): + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + assert node.affectedInt.value == 0 + + node.groupInput.int.value = 5 + assert node.affectedInt.value == 10 + + def test_assignDefaultValueDoesNotTriggerCallback(self): + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + # Callback fires on the first assignment (3 → 6) + node.groupInput.int.value = 3 + assert node.affectedInt.value == 6 + + # Manually move affectedInt to a sentinel value so we can detect + # whether the callback fires again when the same value is re-assigned. + node.affectedInt.value = 100 + node.groupInput.int.value = 3 # Same value — same-value guard skips the callback + assert node.affectedInt.value == 100 + + node.groupInput.int.value = 4 # Different value — callback should fire + assert node.affectedInt.value == 8 + + def test_listElementIsInsideList(self): + """ Elements appended to a ListAttribute must have isInsideList evaluate to True. """ + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + assert node.affectedFloat.value == 0.0 + node.listInput.append(0.0) + element = node.listInput.at(0) + assert element.isInsideList + assert not node.listInput.isInsideList + + def test_listStructureChangeTriggersListLevelCallback(self): + """ + `onListInputChanged` should fire when the list structure itself changes (append/remove). + """ + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + assert node.affectedFloat.value == 0.0 + + node.listInput.append(0.0) + assert node.affectedFloat.value == 999.0 + + def test_changingListElementValueDoesNotTriggerCallback(self): + """ + Changing an element's value does NOT trigger ``onListInputChanged`` because + the element has ``isInsideList=True``, which causes + ``_getAttributeChangedCallback`` to return ``None`` and skip the dispatch. + """ + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + node.listInput.append(0.0) + + element = node.listInput.at(0) + assert element.isInsideList + assert node.affectedFloat.value == 999.0 + + # Reset affectedFloat to isolate from the list-structure change above. + node.affectedFloat.value = 0.0 + + # Change the list element's value and verify that the list-level callback is NOT triggered. + element.value = 42.0 + assert node.affectedFloat.value == 0.0 + + def test_changingMultipleListElementValuesDoesNotTriggerCallback(self): + """ Changing many elements' values never triggers the callback. """ + node = Node(NodeWithGroupListAttributeChangedCallback.__name__) + node.listInput.extend([1.0, 2.0, 3.0]) + + # Reset affectedFloat after the list-structure changes from extend. + node.affectedFloat.value = 0.0 + + for i in range(len(node.listInput)): + node.listInput.at(i).value = i * 10.0 + + assert node.affectedFloat.value == 0.0 + + class NodeWithCompoundAttributes(desc.BaseNode): """ A Node containing a variation of compound attributes (List/Groups), @@ -343,8 +497,6 @@ def processChunk(self, chunk): class TestAttributeCallbackBehaviorWithUpstreamDynamicOutputs: - # nodePluginAttributeChangedCallback = NodePlugin(NodeWithAttributeChangedCallback) - # nodePluginDynamicOutputValue = NodePlugin(NodeWithDynamicOutputValue) @classmethod def setup_class(cls):