Merge pull request #134 from alicevision/dev_connectListAttributes

Linkable ListAttribute

meshroom/core/graph.py

@@ -39,7 +39,7 @@ json.JSONEncoder = MyJSONEncoder  # replace the default implementation with our
 stringIsLinkRe = re.compile('^\{[A-Za-z]+[A-Za-z0-9_.]*\}$')
 
 
-def isLink(value):
+def isLinkExpression(value):
     """
     Return whether the given argument is a link expression.
     A link expression is a string matching the {nodeName.attrName} pattern.
@@ -170,7 +170,7 @@ class Attribute(BaseObject):
         if self._value == value:
             return
 
-        if isinstance(value, Attribute) or (isinstance(value, pyCompatibility.basestring) and isLink(value)):
+        if isinstance(value, Attribute) or isLinkExpression(value):
             # if we set a link to another attribute
             self._value = value
         else:
@@ -188,6 +188,9 @@ class Attribute(BaseObject):
             self.requestGraphUpdate()
         self.valueChanged.emit()
 
+    def resetValue(self):
+        self._value = ""
+
     def requestGraphUpdate(self):
         if self.node.graph:
             self.node.graph.markNodesDirty(self.node)
@@ -235,13 +238,13 @@ class Attribute(BaseObject):
             return
         if isinstance(v, Attribute):
             g.addEdge(v, self)
-            self._value = ""
+            self.resetValue()
-        elif self.isInput and isinstance(v, pyCompatibility.basestring) and isLink(v):
+        elif self.isInput and isLinkExpression(v):
             # value is a link to another attribute
             link = v[1:-1]
             linkNode, linkAttr = link.split('.')
             g.addEdge(g.node(linkNode).attribute(linkAttr), self)
-            self._value = ""
+            self.resetValue()
 
     def getExportValue(self):
         if self.isLink:
@@ -279,27 +282,53 @@ class Attribute(BaseObject):
     isDefault = Property(bool, _isDefault, notify=valueChanged)
 
 
+def raiseIfLink(func):
+    """ If Attribute instance is a link, raise a RuntimeError."""
+    def wrapper(attr, *args, **kwargs):
+        if attr.isLink:
+            raise RuntimeError("Can't modify connected Attribute")
+        return func(attr, *args, **kwargs)
+    return wrapper
+
+
 class ListAttribute(Attribute):
 
     def __init__(self, node, attributeDesc, isOutput, root=None, parent=None):
         super(ListAttribute, self).__init__(node, attributeDesc, isOutput, root, parent)
         self._value = ListModel(parent=self)
 
-    def __getitem__(self, item):
-        return self._value.at(item)
-
     def __len__(self):
-        return len(self._value)
+        return len(self.value)
+
+    def at(self, idx):
+        """ Returns child attribute at index 'idx' """
+        # implement 'at' rather than '__getitem__'
+        # since the later is called spuriously when object is used in QML
+        return self.value.at(idx)
+
+    def index(self, item):
+        return self.value.indexOf(item)
+
+    def resetValue(self):
+        self._value = ListModel(parent=self)
 
     def _set_value(self, value):
-        self.desc.validateValue(value)
+        if self.node.graph:
-        self._value.clear()
+            self.remove(0, len(self))
-        self.extend(value)
+        # Link to another attribute
+        if isinstance(value, ListAttribute) or isLinkExpression(value):
+            self._value = value
+        # New value
+        else:
+            self.desc.validateValue(value)
+            self.extend(value)
         self.requestGraphUpdate()
 
+    @raiseIfLink
     def append(self, value):
         self.extend([value])
 
+    @raiseIfLink
     def insert(self, index, value):
         values = value if isinstance(value, list) else [value]
         attrs = [attribute_factory(self.attributeDesc.elementDesc, v, self.isOutput, self.node, self) for v in values]
@@ -308,36 +337,45 @@ class ListAttribute(Attribute):
         self._applyExpr()
         self.requestGraphUpdate()
 
-    def index(self, item):
+    @raiseIfLink
-        return self._value.indexOf(item)
-
     def extend(self, values):
         self.insert(len(self._value), values)
 
+    @raiseIfLink
     def remove(self, index, count=1):
         if self.node.graph:
-            for i in range(index, index + count):
+            # remove potential links
-                attr = self[i]
+            with GraphModification(self.node.graph):
-                if attr.isLink:
+                for i in range(index, index + count):
-                    self.node.graph.removeEdge(attr)  # delete edge if the attribute is linked
+                    attr = self._value.at(i)
+                    if attr.isLink:
+                        # delete edge if the attribute is linked
+                        self.node.graph.removeEdge(attr)
         self._value.removeAt(index, count)
         self.requestGraphUpdate()
         self.valueChanged.emit()
 
     def uid(self, uidIndex):
-        uids = []
+        if isinstance(self.value, ListModel):
-        for value in self._value:
+            uids = []
-            if uidIndex in value.desc.uid:
+            for value in self.value:
-                uids.append(value.uid(uidIndex))
+                if uidIndex in value.desc.uid:
-        return hash(uids)
+                    uids.append(value.uid(uidIndex))
+            return hash(uids)
+        return super(ListAttribute, self).uid(uidIndex)
 
     def _applyExpr(self):
         if not self.node.graph:
             return
-        for value in self._value:
+        if isinstance(self._value, ListAttribute) or isLinkExpression(self._value):
-            value._applyExpr()
+            super(ListAttribute, self)._applyExpr()
+        else:
+            for value in self._value:
+                value._applyExpr()
 
     def getExportValue(self):
+        if self.isLink:
+            return self.getLinkParam().asLinkExpr()
         return [attr.getExportValue() for attr in self._value]
 
     def defaultValue(self):
@@ -353,7 +391,9 @@ class ListAttribute(Attribute):
             return [attr.getPrimitiveValue(exportDefault=exportDefault) for attr in self._value if not attr.isDefault]
 
     def getValueStr(self):
-        return self.attributeDesc.joinChar.join([v.getValueStr() for v in self._value])
+        if isinstance(self.value, ListModel):
+            return self.attributeDesc.joinChar.join([v.getValueStr() for v in self.value])
+        return super(ListAttribute, self).getValueStr()
 
     # Override value property setter
     value = Property(Variant, Attribute._get_value, _set_value, notify=Attribute.valueChanged)

meshroom/ui/commands.py

@@ -93,6 +93,10 @@ class AddNodeCommand(GraphCommand):
         for key, value in self.kwargs.items():
             if isinstance(value, Attribute):
                 self.kwargs[key] = value.asLinkExpr()
+            elif isinstance(value, list):
+                for idx, v in enumerate(value):
+                    if isinstance(v, Attribute):
+                         value[idx] = v.asLinkExpr()
 
     def redoImpl(self):
         node = self.graph.addNewNode(self.nodeType, **self.kwargs)
@@ -133,7 +137,7 @@ class SetAttributeCommand(GraphCommand):
         super(SetAttributeCommand, self).__init__(graph, parent)
         self.attrName = attribute.fullName()
         self.value = value
-        self.oldValue = attribute.getPrimitiveValue(exportDefault=True)
+        self.oldValue = attribute.getExportValue()
         self.setText("Set Attribute '{}'".format(attribute.fullName()))
 
     def redoImpl(self):

meshroom/ui/graph.py

@@ -256,10 +256,10 @@ class UIGraph(QObject):
 
     @Slot(graph.Attribute, graph.Attribute)
     def addEdge(self, src, dst):
-        if isinstance(dst, graph.ListAttribute):
+        if isinstance(dst, graph.ListAttribute) and not isinstance(src, graph.ListAttribute):
             with self.groupedGraphModification("Insert and Add Edge on {}".format(dst.fullName())):
                 self.appendAttribute(dst)
-                self.push(commands.AddEdgeCommand(self._graph, src, dst[-1]))
+                self.push(commands.AddEdgeCommand(self._graph, src, dst.at(-1)))
         else:
             self.push(commands.AddEdgeCommand(self._graph, src, dst))
 
@@ -297,7 +297,7 @@ class UIGraph(QObject):
         with self.groupedGraphModification("Duplicate Node {}".format(srcNode.name)):
             # skip edges: filter out attributes which are links
             if not createEdges:
-                serialized["attributes"] = {k: v for k, v in serialized["attributes"].items() if not graph.isLink(v)}
+                serialized["attributes"] = {k: v for k, v in serialized["attributes"].items() if not graph.isLinkExpression(v)}
             # create a new node of the same type and with the same attributes values
             node = self.addNewNode(serialized["nodeType"], **serialized["attributes"])
         return node
@@ -324,7 +324,7 @@ class UIGraph(QObject):
                 duplicate = self.duplicateNode(srcNode, createEdges=False)
                 duplicates[srcNode.name] = duplicate  # original node to duplicate map
                 # get link attributes
-                links = {k: v for k, v in srcNode.toDict()["attributes"].items() if graph.isLink(v)}
+                links = {k: v for k, v in srcNode.toDict()["attributes"].items() if graph.isLinkExpression(v)}
                 for attr, link in links.items():
                     link = link[1:-1]  # remove starting '{' and trailing '}'
                     # get source node and attribute name

meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml

@@ -59,7 +59,7 @@ RowLayout {
 
                 MenuItem {
                     text: "Reset To Default Value"
-                    enabled: !attribute.isOutput && !attribute.isLink && !attribute.isDefault
+                    enabled: root.editable && !attribute.isDefault
                     onTriggered: _reconstruction.resetAttribute(attribute)
                 }
 
@@ -286,7 +286,7 @@ RowLayout {
                             var cpt = Qt.createComponent("AttributeItemDelegate.qml")
                             var obj = cpt.createObject(item,
                                                        {'attribute': Qt.binding(function() { return item.childAttrib }),
-                                                        'readOnly': Qt.binding(function() { return root.readOnly })
+                                                        'readOnly': Qt.binding(function() { return !root.editable })
                                                        })
                             obj.Layout.fillWidth = true
                             obj.label.text = index

meshroom/ui/qml/GraphEditor/AttributePin.qml

@@ -27,7 +27,8 @@ RowLayout {
 
     // Instantiate empty Items for each child attribute
     Repeater {
-        model: isList ? attribute.value : ""
+        id: childrenRepeater
+        model: isList && !attribute.isLink ? attribute.value : 0
         onItemAdded: {childPinCreated(item.childAttribute, item)}
         onItemRemoved: {childPinDeleted(item.childAttribute, item)}
         delegate: Item {
@@ -57,7 +58,9 @@ RowLayout {
                 if( drag.source.objectName != dragTarget.objectName  // not an edge connector
                    ||  drag.source.nodeItem == dragTarget.nodeItem   // connection between attributes of the same node
                    || dragTarget.isOutput                            // connection on an output
-                   || dragTarget.attribute.isLink)                   // already connected attribute
+                   || dragTarget.attribute.isLink                    // already connected attribute
+                   || childrenRepeater.count                         // attribute has children
+                   )
                 {
                     drag.accepted = false
                 }

meshroom/ui/qml/GraphEditor/GraphEditor.qml

@@ -227,6 +227,7 @@ Item {
                     baseColor: root.selectedNode == node ? Qt.lighter("#607D8B", 1.2) : "#607D8B"
 
                     onAttributePinCreated: registerAttributePin(attribute, pin)
+                    onAttributePinDeleted: unregisterAttributePin(attribute, pin)
 
                     onPressed: {
                         if(mouse.modifiers & Qt.AltModifier)
@@ -299,6 +300,10 @@ Item {
     {
         root._attributeToDelegate[attribute] = pin
     }
+    function unregisterAttributePin(attribute, pin)
+    {
+        delete root._attributeToDelegate[attribute]
+    }
 
     function boundingBox()
     {

meshroom/ui/qml/GraphEditor/Node.qml

@@ -14,6 +14,7 @@ Item {
     signal pressed(var mouse)
     signal doubleClicked(var mouse)
     signal attributePinCreated(var attribute, var pin)
+    signal attributePinDeleted(var attribute, var pin)
 
     signal computeRequest()
     signal submitRequest()
@@ -145,7 +146,9 @@ Item {
                             attribute: object
                             readOnly: root.readOnly
                             Component.onCompleted: attributePinCreated(attribute, inPin)
+                            Component.onDestruction: attributePinDeleted(attribute, inPin)
                             onChildPinCreated: attributePinCreated(childAttribute, inPin)
+                            onChildPinDeleted: attributePinDeleted(childAttribute, inPin)
                         }
                     }
                 }

meshroom/ui/reconstruction.py

@@ -122,7 +122,7 @@ class LiveSfmManager(QObject):
     def imagePathsInCameraInit(self, node):
         """ Get images in the given CameraInit node. """
         assert node.nodeType == 'CameraInit'
-        return [vp.path.value for vp in node.viewpoints]
+        return [vp.path.value for vp in node.viewpoints.value]
 
     def imagesInStep(self):
         """ Get images in the current augmentation step. """
@@ -304,11 +304,11 @@ class Reconstruction(UIGraph):
 
     def allImagePaths(self):
         """ Get all image paths in the reconstruction. """
-        return [vp.path.value for node in self._cameraInits for vp in node.viewpoints]
+        return [vp.path.value for node in self._cameraInits for vp in node.viewpoints.value]
 
     def allViewIds(self):
         """ Get all view Ids involved in the reconstruction. """
-        return [vp.viewId.value for node in self._cameraInits for vp in node.viewpoints]
+        return [vp.viewId.value for node in self._cameraInits for vp in node.viewpoints.value]
 
     @Slot(QObject, graph.Node)
     def handleFilesDrop(self, drop, cameraInit):

tests/test_multiviewPipeline.py

@@ -17,10 +17,10 @@ def test_multiviewPipeline():
         {'path': '/non/existing/file2', 'intrinsicId': 55}
         ])
 
-    assert graph1.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/fileA'
+    assert graph1.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/fileA'
     assert len(graph2.findNode('CameraInit').viewpoints) == 0
-    assert graph3.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/file1'
+    assert graph3.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/file1'
-    assert graph4.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/file1'
+    assert graph4.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/file1'
 
     assert len(graph1.findNode('CameraInit').viewpoints) == 1
     assert len(graph2.findNode('CameraInit').viewpoints) == 0
@@ -28,15 +28,15 @@ def test_multiviewPipeline():
     assert len(graph4.findNode('CameraInit').viewpoints) == 2
 
     viewpoints = graph3.findNode('CameraInit').viewpoints
-    assert viewpoints[0].path.value == '/non/existing/file1'
+    assert viewpoints.at(0).path.value == '/non/existing/file1'
 
-    assert viewpoints[0].path.value == '/non/existing/file1'
+    assert viewpoints.at(0).path.value == '/non/existing/file1'
-    assert viewpoints[0].intrinsicId.value == -1
+    assert viewpoints.at(0).intrinsicId.value == -1
-    assert viewpoints[1].path.value == '/non/existing/file2'
+    assert viewpoints.at(1).path.value == '/non/existing/file2'
-    assert viewpoints[1].intrinsicId.value == -1
+    assert viewpoints.at(1).intrinsicId.value == -1
 
-    assert viewpoints[0].path.isDefault == False
+    assert not viewpoints.at(0).path.isDefault
-    assert viewpoints[0].intrinsicId.isDefault == True
+    assert viewpoints.at(0).intrinsicId.isDefault
     assert viewpoints.getPrimitiveValue(exportDefault=False) == [
         {"path": '/non/existing/file1'},
         {"path": '/non/existing/file2'},
@@ -44,10 +44,10 @@ def test_multiviewPipeline():
 
     for graph in (graph4, graph4b):
         viewpoints = graph.findNode('CameraInit').viewpoints
-        assert viewpoints[0].path.value == '/non/existing/file1'
+        assert viewpoints.at(0).path.value == '/non/existing/file1'
-        assert viewpoints[0].intrinsicId.value == 50
+        assert viewpoints.at(0).intrinsicId.value == 50
-        assert viewpoints[1].path.value == '/non/existing/file2'
+        assert viewpoints.at(1).path.value == '/non/existing/file2'
-        assert viewpoints[1].intrinsicId.value == 55
+        assert viewpoints.at(1).intrinsicId.value == 55
 
     # Ensure that all output UIDs are different as the input is different:
     # graph1 != graph2 != graph3 != graph4