[ui] ExpressionTextField : fix issues with undostack, error status, number parsing, ...

Author: Alxiice

meshroom/core/desc/attribute.py

@@ -3,7 +3,6 @@
 import os
 import types
 from collections.abc import Iterable
-import math
 
 from meshroom.common import BaseObject, JSValue, Property, Variant, VariantList
 
@@ -342,7 +341,7 @@ def __init__(self, name, label, description, value, range=None, group="allParams
         self._valueType = int
 
     def validateValue(self, value):
-        if value is None or math.isnan(value):
+        if value is None or isinstance(value, str):
             return value
         # Handle unsigned int values that are translated to int by shiboken and may overflow
         try:
@@ -372,7 +371,7 @@ def __init__(self, name, label, description, value, range=None, group="allParams
         self._valueType = float
 
     def validateValue(self, value):
-        if value is None:
+        if value is None or isinstance(value, str):
             return value
         try:
             return float(value)

meshroom/ui/qml/Controls/ExpressionTextField.qml

@@ -1,6 +1,5 @@
 module Controls
 
-MathEvaluator 1.0 mathEvaluator.js
 ColorChart 1.0 ColorChart.qml
 ColorSelector 1.0 ColorSelector.qml
 ExpandableGroup 1.0 ExpandableGroup.qml
@@ -22,4 +21,3 @@ SelectionBox 1.0 SelectionBox.qml
 SelectionLine 1.0 SelectionLine.qml
 DelegateSelectionBox 1.0 DelegateSelectionBox.qml
 DelegateSelectionLine 1.0 DelegateSelectionLine.qml
-ExpressionTextField 1.0 ExpressionTextField.qml

meshroom/ui/qml/Controls/qmldir

@@ -259,7 +259,9 @@ RowLayout {
         switch (attribute.type) {
             case "IntParam":
             case "FloatParam":
-                _reconstruction.setAttribute(root.attribute, Number(value))
+                // We don't set a number because we want to keep the invalid expression
+                // _reconstruction.setAttribute(root.attribute, Number(value))
+                _reconstruction.setAttribute(root.attribute, value)
                 updateAttributeLabel()
                 break
             case "File":
@@ -606,6 +608,7 @@ RowLayout {
             id: sliderComponent
             RowLayout {
                 ExpressionTextField {
+                    id: expressionTextField 
                     implicitWidth: 100
                     Layout.fillWidth: !slider.active
                     enabled: root.editable
@@ -618,17 +621,30 @@ RowLayout {
                     // of the number. When we are editing (item is in focus), the content should follow the editing.
                     autoScroll: activeFocus
                     isInt: attribute.type === "FloatParam" ? false : true
-                    onEditingFinished: setTextFieldAttribute(text)
+                    
+                    onEditingFinished: {
+                        if (hasExprError)
+                            setTextFieldAttribute(expressionTextField.text)  // On the undo stack we keep the expression
+                        else
+                            setTextFieldAttribute(expressionTextField.evaluatedValue)
+                    }
                     onAccepted: {
-                        setTextFieldAttribute(text)
-
+                        if (hasExprError)
+                            setTextFieldAttribute(expressionTextField.text)  // On the undo stack we keep the expression
+                        else
+                            setTextFieldAttribute(expressionTextField.evaluatedValue)
                         // When the text is too long, display the left part
                         // (with the most important values and cut the floating point details)
                         ensureVisible(0)
                     }
+                    
                     Component.onDestruction: {
-                        if (activeFocus)
-                            setTextFieldAttribute(text)
+                        if (activeFocus) {
+                            if (hasExprError)
+                                setTextFieldAttribute(expressionTextField.text)
+                            else
+                                setTextFieldAttribute(expressionTextField.evaluatedValue)
+                        }
                     }
                     Component.onCompleted: {
                         // When the text is too long, display the left part

meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml

@@ -0,0 +1,96 @@
+import QtQuick
+import QtQuick.Controls
+
+TextField {
+    id: root
+
+    // 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 real evaluatedValue: 0
+
+    property bool hasExprError: false
+    property bool isInt: false
+    property int decimals: 2
+
+    // Overlay for error state (red border on top of default background)
+    Rectangle {
+        anchors.fill: parent
+        radius: 4
+        border.color: "red"
+        color: "transparent"
+        visible: root.hasExprError
+        z: 1
+    }
+
+    function raiseError() {
+        hasExprError = true
+    }
+
+    function clearError() {
+        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)
+            if (isInt)
+                result = Math.round(result)
+            else
+                result = result.toFixed(decimals)
+            return result
+        } catch (err) {
+            console.error("Error evaluating expression (", _text, "):", err)
+            return NaN
+        }
+    }
+
+    function refreshStatus() {
+        if (isNaN(getEvalExpression(root.text))) {
+            raiseError()
+        } else {
+            clearError()
+        }
+    }
+
+    function updateExpression() {
+        var result = getEvalExpression(root.text)
+        if (!isNaN(result)) {
+            evaluatedValue = result
+            clearError()
+            return result
+        } else {
+            evaluatedValue = NaN
+            raiseError()
+            return NaN
+        }
+    }
+
+    // When user commits input, evaluate but do NOT overwrite text
+    onAccepted: {
+        updateExpression()
+    }
+
+    onEditingFinished: {
+        updateExpression()
+    }
+
+    onTextChanged: {
+        if (!activeFocus) {
+            refreshStatus()
+        }
+    }
+
+    Component.onCompleted: {
+        refreshStatus()
+    }
+}

meshroom/ui/qml/Utils/ExpressionTextField.qml

@@ -20,21 +20,34 @@ var functions = {
 };
 
 
+function removeLeadingZeros(str) {
+    return str.replace(/\b0*(\d+(?:\.\d*)?)/g, (match, number) => {
+        // If the number starts with a decimal, add a leading zero
+        if (number.startsWith('.')) {
+            return '0' + number;
+        }
+        // Otherwise just return the number without leading zeros
+        return number;
+    });
+}
+
+
 /**
  * Evaluate an expression
  * 
  * @param {*} expr Math expression
  * @returns        float or int
  */
 function eval(expr) {
-    // Additionally replace the "," to "."
-    expr = expr.replace(",", ".").replace(" ", "")
+    // Warning : commas are for separating function args and dot to indicate decimals
 
     // Only allow numbers, operators, parentheses, and function names
     if (!/^[0-9+\-*/^()e.,\s]*$/.test(expr.replace(/\b[a-zA-Z]+\b/g, ""))) {
         throw "Invalid characters in expression";
     }
 
+    expr = removeLeadingZeros(expr);
+
     // Replace symbols and functions
     for (var symbol in symbols) {
         expr = expr.replace(new RegExp("\\b" + symbol + "\\b", "g"), symbols[symbol]);

meshroom/ui/qml/Controls/mathEvaluator.js renamed to meshroom/ui/qml/Utils/mathEvaluator.js

@@ -11,3 +11,5 @@ 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/Utils/qmldir

@@ -3,6 +3,7 @@ import QtQuick.Controls
 import QtQuick.Layouts
 
 import Controls 1.0
+import Utils 1.0
 
 FloatingPane {
     id: root
@@ -16,8 +17,8 @@ FloatingPane {
 
 
     property real slidersPowerValue: 4.0
-    property real gainValue: Math.pow(gainCtrl.value, slidersPowerValue)
-    property real gammaValue: Math.pow(gammaCtrl.value, slidersPowerValue)
+    property real gainValue: Math.pow(gainCtrl.value, slidersPowerValue).toFixed(2)
+    property real gammaValue: Math.pow(gammaCtrl.value, slidersPowerValue).toFixed(2)
     property alias channelModeValue: channelsCtrl.value
     property variant colorRGBA: null
     property variant mousePosition: ({x:0, y:0})
@@ -112,6 +113,7 @@ FloatingPane {
 
                 onClicked: {
                     gainCtrl.value = gainDefaultValue
+                    gainLabel.reset(gainValue)
                 }
             }
             ExpressionTextField {
@@ -121,11 +123,19 @@ FloatingPane {
                 ToolTip.delay: 100
                 ToolTip.text: "Color Gain (in linear colorspace)"
 
-                text: gainValue.toFixed(2)
+                text: gainValue
+                decimals: 2
                 Layout.preferredWidth: textMetrics_gainValue.width
                 selectByMouse: true
                 onAccepted: {
-                    gainCtrl.value = Math.pow(Number(gainLabel.text), 1.0 / slidersPowerValue)
+                    if (!gainLabel.hasExprError) {
+                        if (gainLabel.evaluatedValue <= 0) {
+                            gainLabel.evaluatedValue = 0
+                            gainCtrl.value = gainLabel.evaluatedValue
+                        } else {
+                            gainCtrl.value = Math.pow(Number(gainLabel.evaluatedValue), 1.0 / slidersPowerValue)
+                        }
+                    }
                 }
             }
             Slider {
@@ -135,6 +145,7 @@ FloatingPane {
                 to: 2
                 value: gainDefaultValue
                 stepSize: 0.01
+                onMoved: gainLabel.reset(Math.pow(value, slidersPowerValue))
             }
         }
 
@@ -151,6 +162,7 @@ FloatingPane {
 
                 onClicked: {
                     gammaCtrl.value = gammaDefaultValue;
+                    gammaLabel.reset(gammaValue)
                 }
             }
             ExpressionTextField {
@@ -160,11 +172,19 @@ FloatingPane {
                 ToolTip.delay: 100
                 ToolTip.text: "Apply Gamma (after Gain and in linear colorspace)"
 
-                text: gammaValue.toFixed(2)
+                text: gammaValue
+                decimals: 2
                 Layout.preferredWidth: textMetrics_gainValue.width
                 selectByMouse: true
                 onAccepted: {
-                    gammaCtrl.value = Math.pow(Number(gammaLabel.text), 1.0 / slidersPowerValue)
+                    if (!gammaLabel.hasExprError) {
+                        if (gammaLabel.evaluatedValue <= 0) {
+                            gammaLabel.evaluatedValue = 0
+                            gammaCtrl.value = gammaLabel.evaluatedValue
+                        } else {
+                            gammaCtrl.value = Math.pow(Number(gammaLabel.evaluatedValue), 1.0 / slidersPowerValue)
+                        }
+                    }
                 }
             }
             Slider {
@@ -174,6 +194,7 @@ FloatingPane {
                 to: 2
                 value: gammaDefaultValue
                 stepSize: 0.01
+                onMoved: gammaLabel.reset(Math.pow(value, slidersPowerValue))
             }
         }