[nodes] ExpressionTextField improvements

meshroom/core/evaluation.py

@@ -0,0 +1,51 @@
+#!/usr/bin/env python
+
+import ast, math
+
+
+class MathEvaluator:
+    """ Evaluate math expressions
+
+    ..code::py
+        # Example usage
+        mev = MathEvaluator()
+        print(mev.evaluate("e-1+cos(2*pi)"))
+        print(mev.evaluate("pow(2, 8)"))
+        print(mev.evaluate("round(sin(pi), 3)"))
+    """
+
+    # Allowed math symbols
+    allowed_symbols = {
+        "e": math.e, "pi": math.pi,
+        "cos": math.cos, "sin": math.sin, "tan": math.tan, "exp": math.exp,
+        "pow": pow, "round": round, "abs": abs, "min": min, "max": max,
+        "sqrt": math.sqrt, "log": math.log
+    }
+
+    # Allowed AST node types
+    allowed_nodes = (
+        ast.Expression, ast.BinOp, ast.UnaryOp, ast.Call, ast.Name, ast.Load,
+        ast.Add, ast.Sub, ast.Mult, ast.Div, ast.Pow, ast.Mod, ast.FloorDiv,
+        ast.USub, ast.UAdd, ast.BitXor, ast.BitOr, ast.BitAnd,
+        ast.LShift, ast.RShift, ast.Invert,
+        ast.Constant
+    )
+
+    def _validate_ast(self, node):
+        for child in ast.walk(node):
+            if not isinstance(child, self.allowed_nodes):
+                raise ValueError(f"Bad expression: {ast.dump(child)}")
+            # Check that all variable/function names are whitelisted
+            if isinstance(child, ast.Name):
+                if child.id not in self.allowed_symbols:
+                    raise ValueError(f"Unknown symbol: {child.id}")
+
+    def evaluate(self, expr: str):
+        if any(bad in expr for bad in ('\n', '#')):
+            raise ValueError(f"Invalid expression: {expr}")
+        try:
+            node = ast.parse(expr.strip(), mode="eval")
+            self._validate_ast(node)
+            return eval(compile(node, "<expr>", "eval"), {"__builtins__": {}}, self.allowed_symbols)
+        except Exception:
+            raise ValueError(f"Invalid expression: {expr}")

meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml

@@ -622,12 +622,18 @@ RowLayout {
                     isInt: attribute.type === "FloatParam" ? false : true
 
                     onEditingFinished: {
-                        if (!hasExprError)
+                        if (!hasExprError) {
                             setTextFieldAttribute(expressionTextField.evaluatedValue)
+                            // Restore binding
+                            expressionTextField.text = Qt.binding(function() { return String(expressionTextField.displayValue); })
+                        }
                     }
                     onAccepted: {
-                        if (!hasExprError)
+                        if (!hasExprError) {
                             setTextFieldAttribute(expressionTextField.evaluatedValue)
+                            // Restore binding
+                            expressionTextField.text = Qt.binding(function() { return String(expressionTextField.displayValue); })
+                        }
                         // When the text is too long, display the left part
                         // (with the most important values and cut the floating point details)
                         ensureVisible(0)

meshroom/ui/qml/Utils/ExpressionTextField.qml

@@ -6,6 +6,7 @@ TextField {
 
     // evaluated numeric value (NaN if invalid)
     // It helps keeping the connection that text has so that we don't loose ability to undo/reset
+    property bool textChanged: false
     property real evaluatedValue: 0
 
     property bool hasExprError: false
@@ -30,26 +31,16 @@ TextField {
         hasExprError = false
     }
 
-    function reset(_value) {
-        clearError()
-        evaluatedValue = _value
-        if (isInt) {
-            root.text = _value.toFixed(0)
-        } else {
-            root.text = _value.toFixed(decimals)
-        }
-    }
-
     function getEvalExpression(_text) {
-        try {
-            var result = MathEvaluator.eval(_text)
+        var [_res, _err] = _reconstruction.evaluateMathExpression(_text)
+        if (_err == false) {
             if (isInt)
-                result = Math.round(result)
+                _res = Math.round(_res)
             else
-                result = result.toFixed(decimals)
-            return result
-        } catch (err) {
-            console.error("Error evaluating expression (", _text, "):", err)
+                _res = _res.toFixed(decimals)
+            return _res
+        } else {
+            console.error("Error: Expression", _text, "is invalid")
             return NaN
         }
     }
@@ -63,34 +54,61 @@ TextField {
     }
 
     function updateExpression() {
+        var previousEvaluatedValue = evaluatedValue
         var result = getEvalExpression(root.text)
         if (!isNaN(result)) {
             evaluatedValue = result
             clearError()
-            return result
         } else {
-            evaluatedValue = NaN
+            evaluatedValue = previousEvaluatedValue
             raiseError()
-            return NaN
         }
+        textChanged = false
     }
 
-    // When user commits input, evaluate but do NOT overwrite text
+    // onAccepted and onEditingFinished will break the bindings to text
+    // so if used on fields that needs to be driven by sliders or other qml element,
+    // the binding needs to be restored
+    // No need to restore the binding if the expression has an error because we don't break it
+
     onAccepted: {
-        updateExpression()
+        if (textChanged)
+        {
+            updateExpression()
+            if (!hasExprError && !isNaN(evaluatedValue)) {
+                // Commit the result value to the text field
+                if (isInt)
+                    root.text = Number(evaluatedValue).toFixed(0)
+                else
+                    root.text = Number(evaluatedValue).toFixed(decimals)
+            }
+        }
     }
 
     onEditingFinished: {
-        updateExpression()
+        if (textChanged)
+        {
+            updateExpression()
+            if (!hasExprError && !isNaN(evaluatedValue)) {
+                if (isInt)
+                    root.text = Number(evaluatedValue).toFixed(0)
+                else
+                    root.text = Number(evaluatedValue).toFixed(decimals)
+            }
+        }
     }
 
     onTextChanged: {
         if (!activeFocus) {
             refreshStatus()
+        } else {
+            textChanged = true
         }
     }
 
-    Component.onCompleted: {
-        refreshStatus()
+    Component.onDestruction: {
+        if (textChanged) {
+            root.accepted()
+        }
     }
 }

meshroom/ui/qml/Utils/qmldir

@@ -11,5 +11,4 @@ singleton ExifOrientation 1.0 ExifOrientation.qml
 # singleton Filepath 1.0 Filepath.qml
 # singleton Scene3DHelper 1.0 Scene3DHelper.qml
 # singleton Transformations3DHelper 1.0 Transformations3DHelper.qml
-MathEvaluator 1.0 mathEvaluator.js
 ExpressionTextField 1.0 ExpressionTextField.qml

meshroom/ui/qml/Viewer/HdrImageToolbar.qml

@@ -112,8 +112,8 @@ FloatingPane {
                 ToolTip.text: "Reset Gain"
 
                 onClicked: {
-                    gainCtrl.value = gainDefaultValue
-                    gainLabel.reset(gainValue)
+                    gainLabel.text = gainDefaultValue
+                    gainCtrl.value = gainLabel.text
                 }
             }
             ExpressionTextField {
@@ -129,15 +129,12 @@ FloatingPane {
                 selectByMouse: true
                 onAccepted: {
                     if (!gainLabel.hasExprError) {
-                        if (gainLabel.evaluatedValue <= 0) {
+                        if (gainLabel.text <= 0) {
                             gainLabel.evaluatedValue = 0
                             gainCtrl.value = gainLabel.evaluatedValue
                         } else {
                             gainCtrl.value = Math.pow(Number(gainLabel.evaluatedValue), 1.0 / slidersPowerValue)
                         }
-                    } else {
-                        gainLabel.evaluatedValue = 0
-                        gainCtrl.value = gainLabel.evaluatedValue
                     }
                 }
             }
@@ -148,7 +145,9 @@ FloatingPane {
                 to: 2
                 value: gainDefaultValue
                 stepSize: 0.01
-                onMoved: gainLabel.reset(Math.pow(value, slidersPowerValue))
+                onMoved: {
+                    gainLabel.text = Math.pow(value, slidersPowerValue).toFixed(2)
+                }
             }
         }
 
@@ -164,8 +163,8 @@ FloatingPane {
                 ToolTip.text: "Reset Gamma"
 
                 onClicked: {
-                    gammaCtrl.value = gammaDefaultValue;
-                    gammaLabel.reset(gammaValue)
+                    gammaLabel.text = gammaDefaultValue
+                    gammaCtrl.value = gammaLabel.text;
                 }
             }
             ExpressionTextField {
@@ -187,9 +186,6 @@ FloatingPane {
                         } else {
                             gammaCtrl.value = Math.pow(Number(gammaLabel.evaluatedValue), 1.0 / slidersPowerValue)
                         }
-                    } else {
-                        gainLabel.evaluatedValue = 0
-                        gainCtrl.value = gainLabel.evaluatedValue
                     }
                 }
             }
@@ -200,7 +196,9 @@ FloatingPane {
                 to: 2
                 value: gammaDefaultValue
                 stepSize: 0.01
-                onMoved: gammaLabel.reset(Math.pow(value, slidersPowerValue))
+                onMoved: {
+                    gammaLabel.text = Math.pow(value, slidersPowerValue).toFixed(2)
+                }
             }
         }
 

meshroom/ui/reconstruction.py

@@ -18,6 +18,7 @@
 from meshroom.core import Version
 from meshroom.core.node import Node, CompatibilityNode, Status, Position, CompatibilityIssue
 from meshroom.core.taskManager import TaskManager
+from meshroom.core.evaluation import MathEvaluator
 
 from meshroom.ui import commands
 from meshroom.ui.graph import UIGraph
@@ -1166,6 +1167,21 @@ def setCurrentViewPath(self, path):
         self._currentViewPath = path
         self.currentViewPathChanged.emit()
 
+    @Slot(str, result="QVariantList")
+    def evaluateMathExpression(self, expr):
+        """ Evaluate a mathematical expression and return the result as a string 
+        Returns a list of 2 values :
+        - the result value
+        - a boolean that indicate if an error occured
+        """
+        mev = MathEvaluator()
+        try:
+            res = mev.evaluate(expr)
+            return [res, False]
+        except Exception as err:
+            self.parent().showMessage(f"Invalid field expression: {expr}", "error")
+            return [None, True]
+
     selectedViewIdChanged = Signal()
     selectedViewId = Property(str, lambda self: self._selectedViewId, setSelectedViewId, notify=selectedViewIdChanged)
     selectedViewpointChanged = Signal()