Merge pull request #2913 from alicevision/dev/shapesImprovements

Shapes: Various improvements

Author: cbentejac

meshroom/core/attribute.py

@@ -1048,8 +1048,6 @@ def getValueAsDict(self) -> dict:
         """
         from collections import defaultdict
         outValue = defaultdict(dict)
-        if self.isLink:
-            return self._getInputLink().asLinkExpr() 
         if not self.shapeKeyable:
             return super().getSerializedValue()
         for attribute in self.value:
@@ -1062,6 +1060,35 @@ def getValueAsDict(self) -> dict:
                 for pair in attribute.keyValues.pairs:
                     outValue[str(pair.key)][attribute.name] = pair.value
         return dict(outValue)
+
+    def getShapeAsDict(self) -> dict:
+        """
+        Return the shape attribute as dict with the shape file structure.
+        """
+        outDict = { 
+            "name" : self.rootName, 
+            "type" : self.type, 
+            "properties" : { "color": self._color } 
+        }
+
+        if not self.shapeKeyable:
+            # Not keyable shape, use properties.
+            outDict.get("properties").update(super().getSerializedValue())
+        else:
+            # Keyable shape, use observations.
+            from collections import defaultdict
+            outObservations = defaultdict(dict)
+            for attribute in self.value:
+                if isinstance(attribute, ShapeAttribute):
+                    attributeDict = attribute.getValueAsDict()
+                    if attributeDict:
+                        for key, value in attributeDict.items():
+                            outObservations[key][attribute.name] = value
+                else:
+                    for pair in attribute.keyValues.pairs:
+                        outObservations[str(pair.key)][attribute.name] = pair.value
+            outDict.update({ "observations" : dict(outObservations)})
+        return outDict
 
     def _getVisible(self) -> bool:
         """ 
@@ -1225,12 +1252,18 @@ def __init__(self, node, attributeDesc: desc.ShapeList, isOutput: bool,
         self._visible = True
         super().__init__(node, attributeDesc, isOutput, root, parent)
 
-    def getShapesAsDicts(self):
+    def getValuesAsDicts(self):
         """
-        Return the shape list attribute value as dict.
+        Return the values of the children of the shape list attribute.
         """
         return [shapeAttribute.getValueAsDict() for shapeAttribute in self.value]
 
+    def getShapesAsDicts(self):
+        """
+        Return the children of the shape list attribute.
+        """
+        return [shapeAttribute.getShapeAsDict() for shapeAttribute in self.value]
+
     def _getVisible(self) -> bool:
         """ 
         Return whether the shape list is visible for display.

meshroom/ui/components/shapes/shapeFilesHelper.py

@@ -1,8 +1,13 @@
 from meshroom.ui.reconstruction import Reconstruction
 from meshroom.common import BaseObject, Property, Variant, Signal, ListModel, Slot
 from meshroom.core.attribute import GroupAttribute, ListAttribute
+from shiboken6 import isValid
 from .shapeFile import ShapeFile
 
+# Filter runtime warning when closing Meshroom with active shape files
+import warnings
+warnings.filterwarnings("ignore", message=".*Failed to disconnect.*", category=RuntimeWarning)
+
 class ShapeFilesHelper(BaseObject):
     """
     Manages active project selected node shape files.
@@ -11,6 +16,7 @@ class ShapeFilesHelper(BaseObject):
     def __init__(self, activeProject:Reconstruction, parent=None):
         super().__init__(parent)
         self._activeProject = activeProject
+        self._currentNode = activeProject.selectedNode
         self._shapeFiles = ListModel()
         self._activeProject.selectedViewIdChanged.connect(self._onSelectedViewIdChanged)
         self._activeProject.selectedNodeChanged.connect(self._onSelectedNodeChanged)
@@ -28,6 +34,15 @@ def _loadShapeFilesFromAttributes(self, attributes):
                                                   viewId=self._activeProject.selectedViewId,
                                                   parent=self._shapeFiles))
 
+    @Slot()
+    def _loadShapeFiles(self):
+        """Load/Reload active project selected node shape files."""
+        # clear shapeFiles model
+        self._shapeFiles.clear()
+        # load node shape files
+        self._loadShapeFilesFromAttributes(self._activeProject.selectedNode.attributes)
+        self.nodeShapeFilesChanged.emit()
+
     @Slot()
     def _onSelectedViewIdChanged(self):
         """Callback when the active project selected view id changes."""
@@ -37,17 +52,31 @@ def _onSelectedViewIdChanged(self):
     @Slot()
     def _onSelectedNodeChanged(self):
         """Callback when the active project selected node changes."""
-        # clear shapeFiles model
-        self._shapeFiles = ListModel()
-        # check current node
-        if self._activeProject.selectedNode is None:
-            return
-        # check current node has displayable shape
-        if not self._activeProject.selectedNode.hasDisplayableShape:
+        # disconnect internalFolderChanged signal
+        if self._currentNode is not None:
+            try: 
+                self._currentNode.internalFolderChanged.disconnect(self._loadShapeFiles)
+            except RuntimeError:
+                # Signal was already disconnected or never connected
+                pass
+        # check selected node exists and selected node has displayable shape
+        if self._activeProject.selectedNode is None or not self._activeProject.selectedNode.hasDisplayableShape:
+            # clear shapeFiles model
+            if isValid(self._shapeFiles):
+                self._shapeFiles.clear()
+            # clear current node
+            self._currentNode = None
             return
+        # update current node
+        self._currentNode = self._activeProject.selectedNode
+        # connect internalFolderChanged signal
+        try: 
+            self._currentNode.internalFolderChanged.connect(self._loadShapeFiles)
+        except RuntimeError:
+            # Signal was already disconnected or never connected
+            pass
         # load node shape files
-        self._loadShapeFilesFromAttributes(self._activeProject.selectedNode.attributes)
-        self.nodeShapeFilesChanged.emit()
+        self._loadShapeFiles()
 
     # Properties and signals
     nodeShapeFilesChanged = Signal()

meshroom/ui/qml/Shapes/Editor/Items/ShapeAttributeItem.qml

@@ -46,19 +46,5 @@ Column {
         isAttribute: true
     }
 
-    // Expandable list
-    Loader {
-        active: itemHeader.isExpanded
-        width: parent.width
-        height: active ? (item ? item.implicitHeight || item.height : 0) : 0
-
-        sourceComponent: Pane {
-            background: Rectangle { color: "transparent" }
-            padding: 0
-            implicitWidth: parent.width
-            implicitHeight: 20 
-
-            //Shape attribute observation
-        }
-    }
+    // Perhaps add an expandable list for current observations later
 }

meshroom/ui/qml/Shapes/Editor/Items/ShapeDataItem.qml

@@ -28,19 +28,5 @@ Column {
         isAttribute: false
     }
 
-    // Expandable list
-    Loader {
-        active: itemHeader.isExpanded
-        width: parent.width
-        height: active ? (item ? item.implicitHeight || item.height : 0) : 0
-
-        sourceComponent: Pane {
-            background: Rectangle { color: "transparent" }
-            padding: 0
-            implicitWidth: parent.width
-            implicitHeight: 20 
-
-            //Shape data observation
-        }
-    }
+    // Perhaps add an expandable list for current observations later
 }

meshroom/ui/qml/Shapes/Editor/Items/Utils/ItemHeader.qml

@@ -171,22 +171,26 @@ Pane {
         }
 
         // Shape attributes dropdown
-        MaterialToolButton {
-            font.pointSize: 11
-            padding: 2
-            text: {
-                if(isExpanded) {
-                    return (isShape) ?  MaterialIcons.arrow_drop_down : MaterialIcons.keyboard_arrow_down
-                }
-                else {
-                    return (isShape) ?  MaterialIcons.arrow_right : MaterialIcons.keyboard_arrow_right
+        // For now, only for ShapeFile and ShapeListAttribute
+        Loader {
+            active: !isShape 
+            sourceComponent: MaterialToolButton {
+                font.pointSize: 11
+                padding: 2
+                text: {
+                    if(isExpanded) {
+                        return (isShape) ?  MaterialIcons.arrow_drop_down : MaterialIcons.keyboard_arrow_down
+                    }
+                    else {
+                        return (isShape) ?  MaterialIcons.arrow_right : MaterialIcons.keyboard_arrow_right
+                    }
                 }
+                onClicked: { isExpanded = !isExpanded }
+                enabled: true
+                ToolTip.text: isExpanded ? "Collapse" : "Expand"
+                ToolTip.visible: hovered
+                ToolTip.delay: 800
             }
-            onClicked: { isExpanded = !isExpanded }
-            enabled: true
-            ToolTip.text: isExpanded ? "Collapse" : "Expand"
-            ToolTip.visible: hovered
-            ToolTip.delay: 800
         }
 
         // Shape color

meshroom/ui/qml/Shapes/Editor/ShapeEditor.qml

@@ -26,8 +26,12 @@ Item {
         ScrollView {
             anchors.fill: parent
 
-            // Disable horizontal scroll
+            // Disable horizontal scrolling
             ScrollBar.horizontal.policy: ScrollBar.AlwaysOff
+
+            // Ensure that vertical scrolling is always enabled when necessary
+            ScrollBar.vertical.policy: ScrollBar.AlwaysOn
+            ScrollBar.vertical.visible: contentHeight > height
 
             ColumnLayout {
                 anchors.fill: parent
@@ -62,7 +66,7 @@ Item {
                     model: ShapeFilesHelper.nodeShapeFiles
                     delegate: ShapeEditorItem { 
                         model: object
-                        width: ListView.view.width 
+                        width: ListView.view.width
                     }
                 }
             }

meshroom/ui/qml/Shapes/Editor/ShapeEditorItem.qml

@@ -17,7 +17,7 @@ Loader {
 
     // Source component
     sourceComponent: {
-        switch(model.type) {
+        switch(itemLoader.model.type) {
             case "ShapeFile": return shapeFileComponent
             case "ShapeList": return shapeListAttributeComponent
             default:          return shapeAttributeComponent

tests/test_attributeShape.py

@@ -422,7 +422,7 @@ def test_linkAttribute(self):
         assert nodeB.pointList.getSerializedValue() == nodeA.pointList.asLinkExpr()
 
 
-    def test_valueAsDict(self):
+    def test_exportDict(self):
         graph = Graph("")
         node = graph.addNewNode(NodeWithShapeAttributes.__name__)
 
@@ -434,18 +434,48 @@ def test_valueAsDict(self):
 
         # Check uninitialized shape attribute
         # Shape list attribute should be empty list
+        assert node.pointList.getValuesAsDicts() == []
+        assert node.keyablePointList.getValuesAsDicts() == []
         assert node.pointList.getShapesAsDicts() == []
         assert node.keyablePointList.getShapesAsDicts() == []
         # Not keyable shape attribute should be default
         assert node.point.getValueAsDict() == {"x" : -1, "y" : -1}
         assert node.line.getValueAsDict() == {"a" : {"x" : -1, "y" : -1}, "b" : {"x" : -1, "y" : -1}}
         assert node.rectangle.getValueAsDict() == {"center" : {"x" : -1, "y" : -1}, "size" : {"width" : -1, "height" : -1}}
         assert node.circle.getValueAsDict() == {"center" : {"x" : -1, "y" : -1}, "radius" : -1}
+        assert node.point.getShapeAsDict() == {"name" : node.point.rootName, 
+                                               "type" : node.point.type, 
+                                               "properties" : {"color" : node.point.shapeColor, "x" : -1, "y" : -1}}
+        assert node.line.getShapeAsDict() == {"name" : node.line.rootName, 
+                                              "type" : node.line.type, 
+                                              "properties" : {"color" : node.line.shapeColor, "a" : {"x" : -1, "y" : -1}, "b" : {"x" : -1, "y" : -1}}}
+        assert node.rectangle.getShapeAsDict() == {"name" : node.rectangle.rootName, 
+                                                   "type" : node.rectangle.type, 
+                                                   "properties" : {"color" : node.rectangle.shapeColor, "center" : {"x" : -1, "y" : -1}, "size" : {"width" : -1, "height" : -1}}}
+        assert node.circle.getShapeAsDict() == {"name" : node.circle.rootName, 
+                                                "type" : node.circle.type, 
+                                                "properties" : {"color" : node.circle.shapeColor, "center" : {"x" : -1, "y" : -1}, "radius" : -1}}
         # Keyable shape attribute should be empty dict
         assert node.keyablePoint.getValueAsDict() == {}
         assert node.keyableLine.getValueAsDict() == {}
         assert node.keyableRectangle.getValueAsDict() == {}
         assert node.keyableCircle.getValueAsDict() == {}
+        assert node.keyablePoint.getShapeAsDict() == {"name" : node.keyablePoint.rootName, 
+                                                      "type" : node.keyablePoint.type, 
+                                                      "properties" : {"color" : node.keyablePoint.shapeColor},
+                                                      "observations" : {}}
+        assert node.keyableLine.getShapeAsDict() == {"name" : node.keyableLine.rootName, 
+                                                     "type" : node.keyableLine.type, 
+                                                     "properties" : {"color" : node.keyableLine.shapeColor},
+                                                     "observations" : {}}
+        assert node.keyableRectangle.getShapeAsDict() == {"name" : node.keyableRectangle.rootName, 
+                                                          "type" : node.keyableRectangle.type, 
+                                                          "properties" : {"color" : node.keyableRectangle.shapeColor},
+                                                          "observations" : {}}
+        assert node.keyableCircle.getShapeAsDict() == {"name" : node.keyableCircle.rootName, 
+                                                       "type" : node.keyableCircle.type, 
+                                                       "properties" : {"color" : node.keyableCircle.shapeColor},
+                                                       "observations" : {}}
 
         # Add one shape with an observation
         node.pointList.append(observationPoint)
@@ -463,15 +493,25 @@ def test_valueAsDict(self):
 
         # Check shape attribute
         # Shape list attribute should be empty dict
-        assert node.pointList.getShapesAsDicts() == [observationPoint]
-        assert node.keyablePointList.getShapesAsDicts() == [{"0" : observationPoint}]
+        assert node.pointList.getValuesAsDicts() == [observationPoint]
+        assert node.keyablePointList.getValuesAsDicts() == [{"0" : observationPoint}]
+        assert node.pointList.getShapesAsDicts()[0].get("properties") == {"color" : node.keyablePoint.shapeColor} | observationPoint
+        assert node.keyablePointList.getShapesAsDicts()[0].get("observations") == {"0" : observationPoint}
         # Not keyable shape attribute should be default
         assert node.point.getValueAsDict() == observationPoint
         assert node.line.getValueAsDict() == observationLine
         assert node.rectangle.getValueAsDict() == observationRectangle
         assert node.circle.getValueAsDict() == observationCircle
+        assert node.point.getShapeAsDict().get("properties") ==  {"color" : node.point.shapeColor} | observationPoint
+        assert node.line.getShapeAsDict().get("properties") == {"color" : node.line.shapeColor} | observationLine
+        assert node.rectangle.getShapeAsDict().get("properties") == {"color" : node.rectangle.shapeColor} | observationRectangle
+        assert node.circle.getShapeAsDict().get("properties") == {"color" : node.circle.shapeColor} | observationCircle
         # Keyable shape attribute should be empty dict
         assert node.keyablePoint.getValueAsDict() == {"0" : observationPoint}
         assert node.keyableLine.getValueAsDict() == {"0" : observationLine}
         assert node.keyableRectangle.getValueAsDict() == {"0" : observationRectangle}
-        assert node.keyableCircle.getValueAsDict() == {"0" : observationCircle}
+        assert node.keyableCircle.getValueAsDict() == {"0" : observationCircle}
+        assert node.keyablePoint.getShapeAsDict().get("observations") == {"0" : observationPoint}
+        assert node.keyableLine.getShapeAsDict().get("observations") == {"0" : observationLine}
+        assert node.keyableRectangle.getShapeAsDict().get("observations") == {"0" : observationRectangle}
+        assert node.keyableCircle.getShapeAsDict().get("observations") == {"0" : observationCircle}