Merge pull request #3001 from alicevision/dev/outputListAttributes

[core] Enable output list attributes in the graph

Author: cbentejac

meshroom/core/attribute.py

@@ -4,6 +4,7 @@
 import copy
 import os
 import re
+import threading
 import weakref
 import logging
 import inspect
@@ -185,8 +186,17 @@ def _getValue(self):
             raise RuntimeError(f"Cannot get value of {self._getFullName()}, the attribute is keyable.")
         if self.isLink:
             return self._getInputLink().value
+        self._resolveValue()
         return self._value
 
+    def _resolveValue(self):
+        """
+        Hook for subclasses to resolve pending values before returning _value.
+        Called by _getValue before returning self._value.
+        Default implementation is a no-op.
+        """
+        pass
+
     def _setValue(self, value):
         """
         Set the attribute value from a given value, a given function or a given attribute.
@@ -820,6 +830,9 @@ def getSerializedValue(self):
 
 class ListAttribute(Attribute):
 
+    # Sentinel to distinguish 'no pending dynamic value' from 'pending reset to empty'
+    _NO_PENDING_VALUE = object()
+
     def __init__(self, node, attributeDesc: desc.ListAttribute, isOutput: bool,
                  root=None, parent=None):
         super().__init__(node, attributeDesc, isOutput, root, parent)
@@ -861,13 +874,14 @@ def insert(self, index, value):
         self._value.insert(index, attrs)
         self.valueChanged.emit()
         self._applyExpr()
-        self.requestGraphUpdate()
+        if self.isInput:
+            self.requestGraphUpdate()
 
     @raiseIfLink
     def remove(self, index, count=1):
         if self._value is None:
             return
-        if self.node.graph:
+        if self.node.graph and self.isInput:
             from meshroom.core.graph import GraphModification
             with GraphModification(self.node.graph):
                 # remove potential links
@@ -877,15 +891,41 @@ def remove(self, index, count=1):
                         # delete edge if the attribute is linked
                         self.node.graph.removeEdge(attr)
         self._value.removeAt(index, count)
-        self.requestGraphUpdate()
+        if self.isInput:
+            self.requestGraphUpdate()
         self.valueChanged.emit()
 
     # Override
     def _initValue(self):
+        self._dynamicValueLock = threading.Lock()
+        self._dynamicValue = ListAttribute._NO_PENDING_VALUE
         self.resetToDefaultValue()
 
     # Override
     def _setValue(self, value):
+        if self.isOutput:
+            # For output attributes (set during processChunk in a worker thread,
+            # or during loadOutputAttr in the TaskThread), store raw values without
+            # creating QObject children to avoid cross-thread parenting issues.
+            # The raw values are:
+            # - serialized by saveOutputAttr via getPrimitiveValue
+            # - materialized into QObjects lazily by _getValue on the main thread
+            with self._dynamicValueLock:
+                if value is None:
+                    self._dynamicValue = None  # pending reset
+                else:
+                    self._dynamicValue = self._desc.validateValue(value)
+            return
+
+        # Input attribute path: handle None
+        if value is None:
+            if self.node.graph and self._value is not None and len(self._value) > 0:
+                self.remove(0, len(self))
+            if self._value is None:
+                self._value = ListModel(parent=self)
+            self.valueChanged.emit()
+            return
+
         if self.node.graph:
             self.remove(0, len(self))
         if self._handleLinkValue(value):
@@ -897,7 +937,8 @@ def _setValue(self, value):
                 self._value = ListModel(parent=self)
             newValue = self._desc.validateValue(value)
             self.extend(newValue)
-        self.requestGraphUpdate()
+        if self.isInput:
+            self.requestGraphUpdate()
 
     # Override
     def _applyExpr(self):
@@ -907,6 +948,20 @@ def _applyExpr(self):
             for value in self._value:
                 value._applyExpr()
 
+    def _populateFromDynamicValue(self, value):
+        """Store raw dynamic values for lazy materialization.
+
+        Does NOT create QObject children — safe to call from any thread.
+        The actual ListModel population happens lazily in _getValue()
+        when the main thread (e.g. QML) reads the value.
+        """
+        with self._dynamicValueLock:
+            if value is None:
+                self._dynamicValue = None  # pending reset
+            else:
+                self._dynamicValue = self._desc.validateValue(value)
+        self.valueChanged.emit()
+
     # Override
     def resetToDefaultValue(self):
         self._value = ListModel(parent=self)
@@ -922,8 +977,57 @@ def getSerializedValue(self):
             return self._getInputLink().asLinkExpr()
         return [attr.getSerializedValue() for attr in self._value]
 
+    value = Property(Variant, Attribute._getValue, _setValue, notify=Attribute.valueChanged)
+
+    # Override
+    def _resolveValue(self):
+        """
+        Lazily materialize QObject children from pending raw dynamic values.
+        Called by Attribute._getValue (base) before returning self._value.
+        This hook dispatches via normal Python MRO, bypassing PySide Property
+        getter dispatch limitations.
+        Must only create QObjects on the main thread to avoid cross-thread issues.
+        """
+        if self._dynamicValue is not ListAttribute._NO_PENDING_VALUE:
+            if threading.current_thread() is threading.main_thread():
+                self._materializeDynamicValue()
+
+    def _materializeDynamicValue(self):
+        """
+        Create QObject children in the ListModel from pending raw dynamic values.
+        Must only be called on the main thread.
+        """
+
+        # Thread proof reading of dynamic value
+        with self._dynamicValueLock:
+            pendingValue = self._dynamicValue
+            self._dynamicValue = ListAttribute._NO_PENDING_VALUE
+
+        # Create an empty list if the value is None
+        if self._value is None:
+            self._value = ListModel(parent=self)
+        elif len(self._value) > 0:
+            # Erase all items before reassigning
+            self._value.removeAt(0, len(self._value))
+        
+        # Effectively create the objects from the raw data
+        if pendingValue:
+            attrs = [attributeFactory(self._desc.elementDesc, v, self.isOutput, self.node, self)
+                     for v in pendingValue]
+            self._value.insert(0, attrs)
+        
+        self.valueChanged.emit()
+
     # Override
     def getPrimitiveValue(self, exportDefault=True):
+        # If there is a pending dynamic value (set or reset), return it directly
+        # without touching the ListModel (which may be on a different thread).
+        with self._dynamicValueLock:
+            if self._dynamicValue is not ListAttribute._NO_PENDING_VALUE:
+                if self._dynamicValue is None:
+                    return []
+                return list(self._dynamicValue)
+        
         if exportDefault:
             return [attr.getPrimitiveValue(exportDefault=exportDefault) for attr in self._value]
         return [attr.getPrimitiveValue(exportDefault=exportDefault) for attr in self._value

meshroom/core/desc/attribute.py

@@ -181,15 +181,17 @@ class ListAttribute(Attribute):
     """ A list of Attributes """
     @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead")
     def __init__(self, elementDesc, name, label=None, description=None, group="allParams", commandLineGroup=_setParamSentinel, 
-                 advanced=False, semantic="", enabled=True, joinChar=" ", visible=True, exposed=False):
+                 advanced=False, semantic="", enabled=True, joinChar=" ", visible=True, exposed=False, value=None):
         """
         :param elementDesc: the Attribute description of elements to store in that list
+        :param value: default value. Use None to declare a dynamic output ListAttribute
+                      whose content is set during processChunk.
         """
         self._elementDesc = elementDesc
         self._joinChar = joinChar
         commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group
 
-        super(ListAttribute, self).__init__(name=name, label=label, description=description, value=[],
+        super(ListAttribute, self).__init__(name=name, label=label, description=description, value=value,
                                             invalidate=False, commandLineGroup=commandLineGroup, advanced=advanced, semantic=semantic,
                                             enabled=enabled, visible=visible, exposed=exposed)
 

meshroom/core/node.py

@@ -1788,22 +1788,35 @@ def resetOutputAttr(self):
     def loadOutputAttr(self):
         """ Load output attributes with dynamic values from a values.json file.
         """
+
+        # This does not apply to non dynamic output
         if not self.nodeDesc.hasDynamicOutputAttribute:
             return
+        
+        # Check existence of values.json file
         valuesFile = self.valuesFile
         if not os.path.exists(valuesFile):
             logging.warning(f"No output attr file: {valuesFile}")
             return
 
-        # logging.warning("load output attr: {}, value: {}".format(self.name, valuesFile))
+        # Open json file and parse
         with open(valuesFile) as jsonFile:
             data = json.load(jsonFile)
 
-        # logging.warning(data)
+        # loop over all output attributes in the node description
         for output in self.nodeDesc.outputs:
+            # Only consider dynamic values
             if output.isDynamicValue:
                 if self.hasAttribute(output.name) and output.name in data:
-                    self.attribute(output.name).value = data[output.name]
+                    attr = self.attribute(output.name)
+
+                    # Use _populateFromDynamicValue for compatible classes
+                    # (E.g. for ListAttributes) to properly
+                    # create QObject children on the main thread
+                    if hasattr(attr, '_populateFromDynamicValue'):
+                        attr._populateFromDynamicValue(data[output.name])
+                    else:
+                        attr.value = data[output.name]
                 else:
                     if not self.hasAttribute(output.name):
                         logging.warning(f"loadOutputAttr: Missing dynamic output attribute. Node={self.name}, "
@@ -1821,7 +1834,8 @@ def saveOutputAttr(self):
         for output in self.nodeDesc.outputs:
             if output.isDynamicValue:
                 if self.hasAttribute(output.name):
-                    data[output.name] = self.attribute(output.name).value
+                    # Store the primitive value and not the value itself
+                    data[output.name] = self.attribute(output.name).getPrimitiveValue()
                 else:
                     logging.warning(f"saveOutputAttr: Missing dynamic output attribute: {self.name}.{output.name}")
 

tests/test_attributeDescDefaults.py

@@ -95,14 +95,6 @@ def test_param_no_value_is_dynamic(attrDesc):
     assert attrDesc.isDynamicValue is True
 
 
-def test_list_and_group_attributes_not_dynamic():
-    """ListAttribute and GroupAttribute always have a non-None default value."""
-    la = desc.ListAttribute(desc.StringParam(name="elem"), name="items")
-    ga = desc.GroupAttribute([], name="group")
-    assert la.isDynamicValue is False
-    assert ga.isDynamicValue is False
-
-
 def test_label_auto_generated_from_camel_case():
     """Label should be auto-generated from camelCase attribute names."""
     assert desc.File(name="outputFile").label == "Output File"

tests/test_nodeDynamicOutputs.py

@@ -85,6 +85,34 @@ def process(self, node):
         node.floatOutput.value = node.floatInput.value * 2.0
 
 
+class NodeWithDynamicListOutput(desc.Node):
+    """Node with a dynamic output ListAttribute set during processChunk."""
+    inputs = [
+        desc.ListAttribute(
+            name="listInput",
+            label="List Input",
+            description="A list of strings as input.",
+            elementDesc=desc.StringParam(name="value", label="Value", description="", value=""),
+        ),
+    ]
+
+    outputs = [
+        desc.ListAttribute(
+            name="listOutput",
+            label="List Output",
+            description="A dynamic list output set during processing.",
+            elementDesc=desc.StringParam(name="value", label="Value", description="", value=""),
+            value=None,
+        ),
+    ]
+
+    def processChunk(self, chunk):
+        # Read input list and produce an output list with uppercased values
+        inputValues = [attr.value for attr in chunk.node.listInput.value]
+        outputValues = [v.upper() for v in inputValues]
+        chunk.node.listOutput.value = outputValues
+
+
 class InputNodeWithDynamicOutputs(desc.InputNode):
     inputs = [
         desc.File(
@@ -220,3 +248,77 @@ def test_registerInputNodeWithDynamicOutputsV2(self):
             # InputDynamicOutputs has the same description as InputNodeWithDynamicOutputs: had it been valid, it would
             # have been loaded and registered by the plugin manager at the upper level of the test suite.
             graph.addNewNode("InputDynamicOutputs")
+
+
+class TestDynamicListOutputs:
+    """Tests for dynamic output ListAttribute support."""
+
+    @classmethod
+    def setup_class(cls):
+        registerNodeDesc(NodeWithDynamicListOutput)
+
+    @classmethod
+    def teardown_class(cls):
+        unregisterNodeDesc(NodeWithDynamicListOutput)
+
+    def test_dynamicListOutputDescriptor(self):
+        """Check that a ListAttribute with value=None is correctly flagged as dynamic."""
+        nodeDesc = NodeWithDynamicListOutput()
+        assert nodeDesc.hasDynamicOutputAttribute
+        listOutputDesc = nodeDesc.outputs[0]
+        assert listOutputDesc.isDynamicValue
+
+    def test_processWithDynamicListOutput(self, graphSavedOnDisk):
+        """Process a node that sets a dynamic output ListAttribute during processChunk."""
+        graph: Graph = graphSavedOnDisk
+        node = graph.addNewNode(NodeWithDynamicListOutput.__name__)
+
+        node.listInput.value = ["hello", "world"]
+
+        # Execute the node
+        node.process(inCurrentEnv=True)
+
+        # After processChunk, raw values are stored for serialization (thread-safe).
+        # getPrimitiveValue returns the raw values without requiring QObject children.
+        assert node.listOutput.getPrimitiveValue() == ["HELLO", "WORLD"]
+
+        # loadOutputAttr materializes QObject children (normally on the main thread)
+        node.loadOutputAttr()
+        outputValues = [attr.value for attr in node.listOutput.value]
+        assert outputValues == ["HELLO", "WORLD"]
+
+    def test_loadGraphWithComputedDynamicListOutput(self, graphSavedOnDisk):
+        """Check that dynamic list output values are persisted and reloaded correctly."""
+        graph: Graph = graphSavedOnDisk
+        node = graph.addNewNode(NodeWithDynamicListOutput.__name__)
+        name = node.name
+
+        node.listInput.value = ["foo", "bar", "baz"]
+        graph.save()
+
+        # Execute the node
+        node.process(inCurrentEnv=True)
+
+        # Verify raw output was stored
+        assert node.listOutput.getPrimitiveValue() == ["FOO", "BAR", "BAZ"]
+
+        # Reload the graph — loadOutputAttr populates the ListModel on the main thread
+        loadedGraph = loadGraph(graph.filepath)
+        loadedNode = loadedGraph.node(name)
+
+        assert loadedNode
+        loadedOutputValues = [attr.value for attr in loadedNode.listOutput.value]
+        assert loadedOutputValues == ["FOO", "BAR", "BAZ"]
+
+    def test_loadGraphWithUncomputedDynamicListOutput(self, graphSavedOnDisk):
+        """Check that an uncomputed dynamic list output is empty after loading."""
+        graph: Graph = graphSavedOnDisk
+        node = graph.addNewNode(NodeWithDynamicListOutput.__name__)
+        graph.save()
+
+        loadedGraph = loadGraph(graph.filepath)
+        loadedNode = loadedGraph.node(node.name)
+
+        assert loadedNode
+        # Uncomputed dynamic list output should be empty
+        assert len(loadedNode.listOutput) == 0