diff --git a/meshroom/common/PySignal.py b/meshroom/common/PySignal.py index 7d0ab40523..0726003e4c 100644 --- a/meshroom/common/PySignal.py +++ b/meshroom/common/PySignal.py @@ -158,6 +158,10 @@ class ClassSignal: """ _map = {} + def __init__(self, *args, **kwargs): + self._args = args + self._kwargs = kwargs + def __get__(self, instance, owner): if instance is None: # When we access ClassSignal element on the class object without any instance, diff --git a/meshroom/core/attribute.py b/meshroom/core/attribute.py index a80a3fb667..18c230a133 100644 --- a/meshroom/core/attribute.py +++ b/meshroom/core/attribute.py @@ -9,6 +9,7 @@ from collections.abc import Iterable, Sequence from string import Template from meshroom.common import BaseObject, Property, Variant, Signal, ListModel, DictModel, Slot +from meshroom.core.desc.validators import NotEmptyValidator from meshroom.core import desc, hashValue from typing import TYPE_CHECKING @@ -73,7 +74,6 @@ def __init__(self, node, attributeDesc: desc.Attribute, isOutput: bool, root=Non self._isOutput: bool = isOutput self._label: str = attributeDesc.label self._enabled: bool = True - self._validValue: bool = True self._description: str = attributeDesc.description self._invalidate = False if self._isOutput else attributeDesc.invalidate @@ -171,24 +171,6 @@ def getUidIgnoreValue(self): """ Value for which the attribute should be ignored during the UID computation. """ return self.attributeDesc.uidIgnoreValue - def getValidValue(self): - """ - Get the status of _validValue: - - If it is a function, execute it and return the result - - Otherwise, simply return its value - """ - if isinstance(self.desc.validValue, types.FunctionType): - try: - return self.desc.validValue(self.node) - except Exception: - return True - return self._validValue - - def setValidValue(self, value): - if self._validValue == value: - return - self._validValue = value - def validateValue(self, value): return self.desc.validateValue(value) @@ -226,7 +208,6 @@ def _set_value(self, value): self.requestNodeUpdate() self.valueChanged.emit() - self.validValueChanged.emit() @Slot() def _onValueChanged(self): @@ -459,7 +440,7 @@ def getPrimitiveValue(self, exportDefault=True): def updateInternals(self): # Emit if the enable status has changed self.setEnabled(self.getEnabled()) - + def _is3D(self) -> bool: """ Return True if the current attribute is considered as a 3d file """ @@ -481,6 +462,43 @@ def _is2D(self) -> bool: return next((imageSemantic for imageSemantic in Attribute.VALID_IMAGE_SEMANTICS if self.desc.semantic == imageSemantic), None) is not None + def getErrorMessages(self) -> list[str]: + """ Execute the validators and aggregate the eventual error messages""" + + result = [] + + for validator in self.desc.validators: + isValid, errorMessages = validator(self.node, self) + + if isValid: + continue + + for errorMessage in errorMessages: + result.append(errorMessage) + + return result + + def _isValid(self) -> bool: + """ Check the validation and return False if any validator return (False, erorrs) + """ + + for validator in self.desc.validators: + isValid, _ = validator(self.node, self) + + if not isValid: + return False + + return True + + def _isMandatory(self) -> bool: + """ An attribute is considered as mandatory it contain a NotEmptyValidator """ + + for validator in self.desc.validators: + if isinstance(validator, NotEmptyValidator): + return True + + return False + name = Property(str, getName, constant=True) fullName = Property(str, getFullName, constant=True) fullNameToNode = Property(str, getFullNameToNode, constant=True) @@ -528,10 +546,12 @@ def _is2D(self) -> bool: enabled = Property(bool, getEnabled, setEnabled, notify=enabledChanged) invalidate = Property(bool, lambda self: self._invalidate, constant=True) uidIgnoreValue = Property(Variant, getUidIgnoreValue, constant=True) - validValueChanged = Signal() - validValue = Property(bool, getValidValue, setValidValue, notify=validValueChanged) root = Property(BaseObject, root.fget, constant=True) + errorMessageChanged = Signal() + errorMessages = Property(Variant, lambda self: self.getErrorMessages(), notify=errorMessageChanged) + isMandatory = Property(bool, _isMandatory, constant=True ) + isValid = Property(bool, _isValid, constant=True) def raiseIfLink(func): """ If Attribute instance is a link, raise a RuntimeError. """ diff --git a/meshroom/core/desc/attribute.py b/meshroom/core/desc/attribute.py index 434d39e731..632c57d6f1 100644 --- a/meshroom/core/desc/attribute.py +++ b/meshroom/core/desc/attribute.py @@ -6,13 +6,24 @@ from meshroom.common import BaseObject, JSValue, Property, Variant, VariantList +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from meshroom.core.desc.validators import AttributeValidator + class Attribute(BaseObject): """ """ - def __init__(self, name, label, description, value, advanced, semantic, group, enabled, invalidate=True, - uidIgnoreValue=None, validValue=True, errorMessage="", visible=True, exposed=False): + def __init__(self, name, label, description, value, advanced, semantic, group, enabled, + invalidate=True, + uidIgnoreValue=None, + visible=True, + exposed=False, + validators:list["AttributeValidator"]=None + ): + super(Attribute, self).__init__() self._name = name self._label = label @@ -24,8 +35,6 @@ def __init__(self, name, label, description, value, advanced, semantic, group, e self._invalidate = invalidate self._semantic = semantic self._uidIgnoreValue = uidIgnoreValue - self._validValue = validValue - self._errorMessage = errorMessage self._visible = visible self._exposed = exposed self._isExpression = (isinstance(self._value, str) and "{" in self._value) \ @@ -33,6 +42,13 @@ def __init__(self, name, label, description, value, advanced, semantic, group, e self._isDynamicValue = (self._value is None) self._valueType = None + if validators is None: + self._validators = [] + elif isinstance(validators, (list, tuple)): + self._validators = validators + else: + raise RuntimeError(f"Validators should be of type 'list[AttributeValidator]', the type '{type(validators)}' is not supported.") + def getInstanceType(self): """ Return the correct Attribute instance corresponding to the description. """ # Import within the method to prevent cyclic dependencies @@ -70,7 +86,11 @@ def matchDescription(self, value, strict=True): except ValueError: return False return True - + + @property + def validators(self): + return self._validators + name = Property(str, lambda self: self._name, constant=True) label = Property(str, lambda self: self._label, constant=True) description = Property(str, lambda self: self._description, constant=True) @@ -89,8 +109,6 @@ def matchDescription(self, value, strict=True): invalidate = Property(Variant, lambda self: self._invalidate, constant=True) semantic = Property(str, lambda self: self._semantic, constant=True) uidIgnoreValue = Property(Variant, lambda self: self._uidIgnoreValue, constant=True) - validValue = Property(Variant, lambda self: self._validValue, constant=True) - errorMessage = Property(str, lambda self: self._errorMessage, constant=True) # visible: # The attribute is not displayed in the Graph Editor if False but still visible in the Node Editor. # This property is useful to hide some attributes that are not relevant for the user. @@ -108,7 +126,7 @@ def matchDescription(self, value, strict=True): class ListAttribute(Attribute): """ A list of Attributes """ def __init__(self, elementDesc, name, label, description, group="allParams", advanced=False, semantic="", - enabled=True, joinChar=" ", visible=True, exposed=False): + enabled=True, joinChar=" ", visible=True, exposed=False, validators=None): """ :param elementDesc: the Attribute description of elements to store in that list """ @@ -116,7 +134,7 @@ def __init__(self, elementDesc, name, label, description, group="allParams", adv self._joinChar = joinChar super(ListAttribute, self).__init__(name=name, label=label, description=description, value=[], invalidate=False, group=group, advanced=advanced, semantic=semantic, - enabled=enabled, visible=visible, exposed=exposed) + enabled=enabled, visible=visible, exposed=exposed, validators=validators) def getInstanceType(self): # Import within the method to prevent cyclic dependencies @@ -160,7 +178,7 @@ def matchDescription(self, value, strict=True): class GroupAttribute(Attribute): """ A macro Attribute composed of several Attributes """ def __init__(self, groupDesc, name, label, description, group="allParams", advanced=False, semantic="", - enabled=True, joinChar=" ", brackets=None, visible=True, exposed=False): + enabled=True, joinChar=" ", brackets=None, visible=True, exposed=False, validators=None): """ :param groupDesc: the description of the Attributes composing this group """ @@ -169,7 +187,7 @@ def __init__(self, groupDesc, name, label, description, group="allParams", advan self._brackets = brackets super(GroupAttribute, self).__init__(name=name, label=label, description=description, value={}, group=group, advanced=advanced, invalidate=False, semantic=semantic, - enabled=enabled, visible=visible, exposed=exposed) + enabled=enabled, visible=visible, exposed=exposed, validators=validators) def getInstanceType(self): # Import within the method to prevent cyclic dependencies @@ -267,21 +285,21 @@ class Param(Attribute): """ """ def __init__(self, name, label, description, value, group, advanced, semantic, enabled, invalidate=True, - uidIgnoreValue=None, validValue=True, errorMessage="", visible=True, exposed=False): + uidIgnoreValue=None, visible=True, exposed=False, validators=None): super(Param, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, uidIgnoreValue=uidIgnoreValue, validValue=validValue, - errorMessage=errorMessage, visible=visible, exposed=exposed) + semantic=semantic, uidIgnoreValue=uidIgnoreValue, visible=visible, exposed=exposed, + validators=validators) class File(Attribute): """ """ def __init__(self, name, label, description, value, group="allParams", advanced=False, invalidate=True, - semantic="", enabled=True, visible=True, exposed=True): + semantic="", enabled=True, visible=True, exposed=True, validators=None): super(File, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, semantic=semantic, - visible=visible, exposed=exposed) + visible=visible, exposed=exposed, validators=validators) self._valueType = str def validateValue(self, value): @@ -304,10 +322,10 @@ class BoolParam(Param): """ """ def __init__(self, name, label, description, value, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", visible=True, exposed=False): + invalidate=True, semantic="", visible=True, exposed=False, validators=None): super(BoolParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, visible=visible, exposed=exposed) + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = bool def validateValue(self, value): @@ -332,12 +350,12 @@ class IntParam(Param): """ """ def __init__(self, name, label, description, value, range=None, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", validValue=True, errorMessage="", visible=True, exposed=False): + invalidate=True, semantic="", visible=True, exposed=False, validators=None): self._range = range super(IntParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, validValue=validValue, errorMessage=errorMessage, - visible=visible, exposed=exposed) + semantic=semantic, + visible=visible, exposed=exposed, validators=validators) self._valueType = int def validateValue(self, value): @@ -362,12 +380,12 @@ class FloatParam(Param): """ """ def __init__(self, name, label, description, value, range=None, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", validValue=True, errorMessage="", visible=True, exposed=False): + invalidate=True, semantic="", visible=True, exposed=False, validators=None): self._range = range super(FloatParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, validValue=validValue, errorMessage=errorMessage, - visible=visible, exposed=exposed) + semantic=semantic, + visible=visible, exposed=exposed, validators=validators) self._valueType = float def validateValue(self, value): @@ -391,10 +409,10 @@ class PushButtonParam(Param): """ """ def __init__(self, name, label, description, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", visible=True, exposed=False): + invalidate=True, semantic="", visible=True, exposed=False, validators=None): super(PushButtonParam, self).__init__(name=name, label=label, description=description, value=None, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, visible=visible, exposed=exposed) + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = None def getInstanceType(self): @@ -428,14 +446,13 @@ class ChoiceParam(Param): _OVERRIDE_SERIALIZATION_KEY_VALUES = "__ChoiceParam_values__" def __init__(self, name: str, label: str, description: str, value, values, exclusive=True, saveValuesOverride=False, - group="allParams", joinChar=" ", advanced=False, enabled=True, invalidate=True, semantic="", - validValue=True, errorMessage="", - visible=True, exposed=False): + group="allParams", joinChar=" ", advanced=False, enabled=True, invalidate=True, semantic="", + visible=True, exposed=False, validators=None): super(ChoiceParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, validValue=validValue, errorMessage=errorMessage, - visible=visible, exposed=exposed) + semantic=semantic, + visible=visible, exposed=exposed, validators=validators) self._values = values self._saveValuesOverride = saveValuesOverride self._exclusive = exclusive @@ -508,12 +525,12 @@ class StringParam(Param): """ """ def __init__(self, name, label, description, value, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", uidIgnoreValue=None, validValue=True, errorMessage="", visible=True, - exposed=False): + invalidate=True, semantic="", uidIgnoreValue=None, visible=True, + exposed=False, validators=None): super(StringParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, uidIgnoreValue=uidIgnoreValue, validValue=validValue, - errorMessage=errorMessage, visible=visible, exposed=exposed) + semantic=semantic, uidIgnoreValue=uidIgnoreValue, + visible=visible, exposed=exposed, validators=validators) self._valueType = str def validateValue(self, value): @@ -534,10 +551,10 @@ class ColorParam(Param): """ """ def __init__(self, name, label, description, value, group="allParams", advanced=False, enabled=True, - invalidate=True, semantic="", visible=True, exposed=False): + invalidate=True, semantic="", visible=True, exposed=False, validators=None): super(ColorParam, self).__init__(name=name, label=label, description=description, value=value, group=group, advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, visible=visible, exposed=exposed) + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = str def validateValue(self, value): diff --git a/meshroom/core/desc/validators.py b/meshroom/core/desc/validators.py new file mode 100644 index 0000000000..943c30225a --- /dev/null +++ b/meshroom/core/desc/validators.py @@ -0,0 +1,66 @@ +from typing import TYPE_CHECKING, TypeVar + +if TYPE_CHECKING: + from meshroom.core.attribute import Attribute + from meshroom.core.node import Node + + +def success() -> tuple[bool, list[str]]: + return (True, []) + +def error(*messages: str) -> tuple[bool, list[str]]: + return (False, messages) + +class AttributeValidator(object): + """ Interface for an attribute validation + You can inherit from this class and override the __call__ methods to implement your own attribute validation logic + + Because it's a callable class, you can also create your own validators on the fly + + .. code-block: python + + lambda node, attribute: success() if attribute.value and attribute.value != "" else error("attribute have no value") + """ + + def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]: + """ + Override this method to implement your custom validation logic. + You can use the success() and error() helpers that encapsulate the returning response. + + :param node: The node that holds the attribute to validate + :param attribute: The atribute to validate + + :returns: The validtion response: (True, []) if it's valid, (False, [errorMessage1, errorMessage2, ...]) if error exists + + """ + raise NotImplementedError() + + +class NotEmptyValidator(AttributeValidator): + """ The attribute value should not be empty + This class is used to determine if an attribute label should be considered as mandatory/required + """ + + def __call__(self, node: "Node", attribute: "Attribute") -> tuple[bool, list[str]]: + + if attribute.value is None or attribute.value == "": + return error("Empty value are not allowed") + + return success() + + +class RangeValidator(AttributeValidator): + """ Check if the attribute value is in the given range + """ + + def __init__(self, min, max): + self._min = min + self._max = max + + def __call__(self, node:"Node", attribute: "Attribute") -> tuple[bool, list[str]]: + + if attribute.value < self._min or attribute.value > self._max: + return error(f"Value should be greater than {self._min} and less than {self._max}", + f"({self._min} < {attribute.value} < {self._max})") + + return success() \ No newline at end of file diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index a5bff2a250..5d34912d8f 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -1593,6 +1593,7 @@ def setVerbose(self, v): updated = Signal() canComputeLeavesChanged = Signal() canComputeLeaves = Property(bool, lambda self: self._canComputeLeaves, notify=canComputeLeavesChanged) + attributeValueChanged = Signal(Attribute) def loadGraph(filepath, strictCompatibility: bool = False) -> Graph: diff --git a/meshroom/core/node.py b/meshroom/core/node.py index 50ee7ba617..65cb95d929 100644 --- a/meshroom/core/node.py +++ b/meshroom/core/node.py @@ -1171,6 +1171,8 @@ def _onAttributeChanged(self, attr: Attribute): if callback: callback(self) + self.hasInvalidAttributeChanged.emit() + if self.graph: # If we are in a graph, propagate the notification to the connected output attributes for edge in self.graph.outEdges(attr): @@ -1622,6 +1624,12 @@ def has3DOutputAttribute(self): return next((attr for attr in self._attributes if attr.enabled and attr.isOutput and attr.is3D), None) is not None + def _hasInvalidAttribute(self): + for attribute in self._attributes: + if len(attribute.errorMessages) > 0: + return True + return False + name = Property(str, getName, constant=True) defaultLabel = Property(str, getDefaultLabel, constant=True) nodeType = Property(str, nodeType.fget, constant=True) @@ -1674,6 +1682,9 @@ def has3DOutputAttribute(self): hasSequenceOutput = Property(bool, hasSequenceOutputAttribute, notify=outputAttrEnabledChanged) has3DOutput = Property(bool, has3DOutputAttribute, notify=outputAttrEnabledChanged) + hasInvalidAttributeChanged = Signal() + hasInvalidAttribute = Property(bool, _hasInvalidAttribute, notify=hasInvalidAttributeChanged) + class Node(BaseNode): """ diff --git a/meshroom/ui/commands.py b/meshroom/ui/commands.py index dcbcac0f49..a75940b7e2 100755 --- a/meshroom/ui/commands.py +++ b/meshroom/ui/commands.py @@ -295,17 +295,21 @@ def redoImpl(self): if self.value == self.oldValue: return False if self.graph.attribute(self.attrName) is not None: - self.graph.attribute(self.attrName).value = self.value + attribute = self.graph.attribute(self.attrName) else: - self.graph.internalAttribute(self.attrName).value = self.value + attribute = self.graph.internalAttribute(self.attrName) + + attribute.value = self.value + return True def undoImpl(self): if self.graph.attribute(self.attrName) is not None: - self.graph.attribute(self.attrName).value = self.oldValue + attribute = self.graph.attribute(self.attrName) else: - self.graph.internalAttribute(self.attrName).value = self.oldValue - + attribute = self.graph.internalAttribute(self.attrName) + + attribute.value = self.oldValue class AddEdgeCommand(GraphCommand): def __init__(self, graph, src, dst, parent=None): diff --git a/meshroom/ui/qml/Application.qml b/meshroom/ui/qml/Application.qml index 5fcf3502e8..16b782f49c 100644 --- a/meshroom/ui/qml/Application.qml +++ b/meshroom/ui/qml/Application.qml @@ -129,6 +129,14 @@ Page { return true; } + function getAllNodes() { + const nodes = [] + for(let i=0; i node.hasInvalidAttribute) ) { + submitWithWarningDialog.nodes = nodes + submitWithWarningDialog.open() + } else { + _reconstruction.submit(nodes) + } } - catch (error) { + catch (error) { const data = ErrorHandler.analyseError(error) - if (data.context === "SUBMITTING") + if (data.context === "SUBMITTING") { computeSubmitErrorDialog.openError(data.type, data.msg, nodes) + } } } } @@ -399,6 +417,26 @@ Page { onAccepted: saveAsAction.trigger() } + MessageDialog { + id: submitWithWarningDialog + + canCopy: false + icon.text: MaterialIcons.warning + parent: Overlay.overlay + preset: "Warning" + title: "Nodes Containing Warnings" + text: "Some nodes contain warnings. Are you sure you want to submit?" + helperText: "Submit even if some nodes have warnings" + standardButtons: Dialog.Cancel | Dialog.Yes + + property var nodes: [] + + onDiscarded: close() + onAccepted: { + _reconstruction.submit(nodes) + } + } + MessageDialog { id: fileModifiedDialog diff --git a/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml b/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml index 7a830d6e3f..53399fd118 100644 --- a/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml +++ b/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml @@ -24,32 +24,24 @@ RowLayout { property int labelWidth // Shortcut to set the fixed size of the Label readonly property bool editable: !attribute.isOutput && !attribute.isLink && !readOnly + property var errorMessages: attribute.errorMessages signal doubleClicked(var mouse, var attr) signal inAttributeClicked(var srcItem, var mouse, var inAttributes) signal outAttributeClicked(var srcItem, var mouse, var outAttributes) signal showInViewer(var attr) - spacing: 2 - - function updateAttributeLabel() { - background.color = attribute.validValue ? Qt.darker(palette.window, 1.1) : Qt.darker(Colors.red, 1.5) - - if (attribute.desc) { - var tooltip = "" - if (!attribute.validValue && attribute.desc.errorMessage !== "") - tooltip += "Error: " + Format.plainToHtml(attribute.desc.errorMessage) + "

" - tooltip += " " + attribute.desc.name + ": " + attribute.type + "
" + Format.plainToHtml(attribute.desc.description) - - parameterTooltip.text = tooltip + Connections { + target: attribute + function onValueChanged() { + root.errorMessages = attribute.errorMessages } + } + spacing: 2 + Pane { - background: Rectangle { - id: background - color: object != undefined && object.validValue ? Qt.darker(parent.palette.window, 1.1) : Qt.darker(Colors.red, 1.5) - } padding: 0 Layout.preferredWidth: labelWidth || implicitWidth Layout.fillHeight: true @@ -93,7 +85,7 @@ RowLayout { padding: 5 wrapMode: Label.WrapAtWordBoundaryOrAnywhere - text: object.label + text: attribute.isMandatory && attribute.isDefault ? `\* ${object.label}` : object.label color: { if (object != undefined && (object.hasOutputConnections || object.isLink) && !object.enabled) @@ -110,11 +102,7 @@ RowLayout { y: parameterMA.mouseY + 10 text: { - var tooltip = "" - if (!object.validValue && object.desc.errorMessage !== "") - tooltip += "Error: " + Format.plainToHtml(object.desc.errorMessage) + "

" - tooltip += "" + object.desc.name + ": " + attribute.type + "
" + Format.plainToHtml(object.description) - return tooltip + return `${object.desc.name} : ${attribute.type}
${Format.plainToHtml(object.description)}` } visible: parameterMA.containsMouse delay: 800 @@ -144,7 +132,6 @@ RowLayout { enabled: root.editable && !attribute.isDefault onTriggered: { _reconstruction.resetAttribute(attribute) - updateAttributeLabel() } } MenuItem { @@ -260,20 +247,20 @@ RowLayout { case "IntParam": case "FloatParam": _reconstruction.setAttribute(root.attribute, Number(value)) - updateAttributeLabel() break case "File": _reconstruction.setAttribute(root.attribute, value) break default: _reconstruction.setAttribute(root.attribute, value.trim()) - updateAttributeLabel() break } } + Loader { Layout.fillWidth: true + id: inputField sourceComponent: { // PushButtonParam always has value == undefined, so it needs to be excluded from this check @@ -337,117 +324,137 @@ RowLayout { Component { id: textFieldComponent - TextField { - id: textField - readOnly: !root.editable - text: attribute.value - - // Don't disable the component to keep interactive features (text selection, context menu...). - // Only override the look by using the Disabled palette. - SystemPalette { - id: disabledPalette - colorGroup: SystemPalette.Disabled - } - states: [ - State { - when: readOnly - PropertyChanges { - target: textField - color: disabledPalette.text - } + RowLayout { + anchors.fill: parent + + TextField { + id: textField + Layout.fillWidth: true + + readOnly: !root.editable + text: attribute.value + placeholderText: attribute.isMandatory ? "This field is required" : "" + + // Don't disable the component to keep interactive features (text selection, context menu...). + // Only override the look by using the Disabled palette. + SystemPalette { + id: disabledPalette + colorGroup: SystemPalette.Disabled } - ] - selectByMouse: true - onEditingFinished: setTextFieldAttribute(text) - persistentSelection: false + background: Rectangle { + visible: errorMessages.length + border.color: "orange" + color: "transparent" + radius: 2 + } - onAccepted: { - setTextFieldAttribute(text) - parameterLabel.forceActiveFocus() - } - Keys.onPressed: function(event) { - if ((event.key == Qt.Key_Escape)) { - event.accepted = true - parameterLabel.forceActiveFocus() + states: [ + State { + when: readOnly + PropertyChanges { + target: textField + color: disabledPalette.text + } + } + ] + + selectByMouse: true + persistentSelection: false + + onEditingFinished: { + setTextFieldAttribute(text) } - } - Component.onDestruction: { - if (activeFocus) + + onAccepted: { setTextFieldAttribute(text) - } - DropArea { - enabled: root.editable - anchors.fill: parent - onDropped: function(drop) { - if (drop.hasUrls) - setTextFieldAttribute(Filepath.urlToString(drop.urls[0])) - else if (drop.hasText && drop.text != '') - setTextFieldAttribute(drop.text) + parameterLabel.forceActiveFocus() } - } - onPressed: (event) => { - if(event.button == Qt.RightButton) { - // Keep selection persistent while context menu is open to - // visualize what is being copied or what will be replaced on paste. - persistentSelection = true; - const menu = textFieldMenuComponent.createObject(textField); - menu.popup(); - - if(selectedText === "") { - cursorPosition = positionAt(event.x, event.y); + Keys.onPressed: function(event) { + if ((event.key == Qt.Key_Escape)) { + event.accepted = true + parameterLabel.forceActiveFocus() } } - } - - Component { - id: textFieldMenuComponent - Menu { - onOpened: { - // Keep cursor visible to see where pasting would happen. - textField.cursorVisible = true; + Component.onDestruction: { + if (activeFocus) + setTextFieldAttribute(text) + } + DropArea { + enabled: root.editable + anchors.fill: parent + onDropped: function(drop) { + if (drop.hasUrls) + setTextFieldAttribute(Filepath.urlToString(drop.urls[0])) + else if (drop.hasText && drop.text != '') + setTextFieldAttribute(drop.text) } - onClosed: { - // Disable selection persistency behavior once menu is closed and - // give focus back to the parent TextField. - textField.persistentSelection = false; - textField.forceActiveFocus(); - destroy(); + } + onPressed: (event) => { + if(event.button == Qt.RightButton) { + // Keep selection persistent while context menu is open to + // visualize what is being copied or what will be replaced on paste. + persistentSelection = true; + const menu = textFieldMenuComponent.createObject(textField); + menu.popup(); + + if(selectedText === "") { + cursorPosition = positionAt(event.x, event.y); + } } - MenuItem { - text: "Copy" - enabled: attribute.value != "" - onTriggered: { - const hasSelection = textField.selectionStart !== textField.selectionEnd; - if(hasSelection) { - // Use `TextField.copy` to copy only the current selection. - textField.copy(); - } - else { - Clipboard.setText(attribute.value); + } + + Component { + id: textFieldMenuComponent + Menu { + onOpened: { + // Keep cursor visible to see where pasting would happen. + textField.cursorVisible = true; + } + onClosed: { + // Disable selection persistency behavior once menu is closed and + // give focus back to the parent TextField. + textField.persistentSelection = false; + textField.forceActiveFocus(); + destroy(); + } + MenuItem { + text: "Copy" + enabled: attribute.value != "" + onTriggered: { + const hasSelection = textField.selectionStart !== textField.selectionEnd; + if(hasSelection) { + // Use `TextField.copy` to copy only the current selection. + textField.copy(); + } + else { + Clipboard.setText(attribute.value); + } } } - } - MenuItem { - text: "Paste" - enabled: !readOnly - onTriggered: { - const clipboardText = Clipboard.getText(); - if (clipboardText.length === 0) { - return; + MenuItem { + text: "Paste" + enabled: !readOnly + onTriggered: { + const clipboardText = Clipboard.getText(); + if (clipboardText.length === 0) { + return; + } + const before = textField.text.substr(0, textField.selectionStart); + const after = textField.text.substr(textField.selectionEnd, textField.text.length); + const updatedValue = before + clipboardText + after; + setTextFieldAttribute(updatedValue); + // Set the cursor at the end of the added text + textField.cursorPosition = before.length + clipboardText.length; } - const before = textField.text.substr(0, textField.selectionStart); - const after = textField.text.substr(textField.selectionEnd, textField.text.length); - const updatedValue = before + clipboardText + after; - setTextFieldAttribute(updatedValue); - // Set the cursor at the end of the added text - textField.cursorPosition = before.length + clipboardText.length; } - } - } - } + } + } } + + } + } Component { @@ -477,6 +484,14 @@ RowLayout { onEditingFinished: setTextFieldAttribute(text) text: attribute.value selectByMouse: true + + background: Rectangle { + visible: errorMessages.length + border.color: "orange" + color: "transparent" + radius: 2 + } + onPressed: { root.forceActiveFocus() } @@ -590,6 +605,7 @@ RowLayout { values: root.attribute.values enabled: root.editable customValueColor: Colors.orange + onToggled: (value, checked) => { var currentValue = root.attribute.value; if (!checked) { @@ -626,6 +642,14 @@ RowLayout { autoScroll: activeFocus validator: attribute.type === "FloatParam" ? doubleValidator : intValidator onEditingFinished: setTextFieldAttribute(text) + + background: Rectangle { + visible: errorMessages.length + border.color: "orange" + color: "transparent" + radius: 2 + } + onAccepted: { setTextFieldAttribute(text) @@ -661,7 +685,6 @@ RowLayout { onPressedChanged: { if (!pressed) { _reconstruction.setAttribute(attribute, formattedValue) - updateAttributeLabel() } } } @@ -832,4 +855,11 @@ RowLayout { } } } + + MaterialLabel { + visible: !attribute.isOutput && root.errorMessages.length + text: MaterialIcons.fmd_bad + ToolTip.text: root.errorMessages.join("\n") + color: "orange" + } } diff --git a/meshroom/ui/qml/GraphEditor/GraphEditor.qml b/meshroom/ui/qml/GraphEditor/GraphEditor.qml index 54300a3a12..e1783b29fc 100755 --- a/meshroom/ui/qml/GraphEditor/GraphEditor.qml +++ b/meshroom/ui/qml/GraphEditor/GraphEditor.qml @@ -870,6 +870,7 @@ Item { mainSelected: uigraph.selectedNode === node hovered: uigraph.hoveredNode === node + hasWarnings: node.hasInvalidAttribute // ItemSelectionModel.hasSelection triggers updates anytime the selectionChanged() signal is emitted. selected: uigraph.nodeSelection.hasSelection ? uigraph.nodeSelection.isRowSelected(index) : false @@ -1006,6 +1007,7 @@ Item { enabled: !nodeRepeater.ongoingDrag NumberAnimation { duration: 100 } } + } } } diff --git a/meshroom/ui/qml/GraphEditor/Node.qml b/meshroom/ui/qml/GraphEditor/Node.qml index 28969f68b3..b027e19125 100755 --- a/meshroom/ui/qml/GraphEditor/Node.qml +++ b/meshroom/ui/qml/GraphEditor/Node.qml @@ -46,6 +46,7 @@ Item { property int directionY: 0; property point mousePosition: Qt.point(mouseArea.mouseX, mouseArea.mouseY) + property bool hasWarnings: false Item { id: m @@ -280,6 +281,11 @@ Item { return 2 } border.color: { + + if(hasWarnings === true) { + return Colors.warning + } + if(root.mainSelected) return activePalette.highlight if(root.selected) @@ -398,6 +404,17 @@ Item { } } + // Attribute warnings + MaterialLabel { + visible: hasWarnings + text: MaterialIcons.fmd_bad + color: Colors.warning + padding: 2 + font.pointSize: 7 + palette.text: Colors.sysPalette.text + ToolTip.text: "Some attribute validation are failing" + } + // Submitted externally indicator MaterialLabel { visible: node.isExternal diff --git a/meshroom/ui/qml/Utils/Colors.qml b/meshroom/ui/qml/Utils/Colors.qml index af27c91ce5..bc108db007 100644 --- a/meshroom/ui/qml/Utils/Colors.qml +++ b/meshroom/ui/qml/Utils/Colors.qml @@ -20,6 +20,7 @@ QtObject { readonly property color lime: "#CDDC39" readonly property color grey: "#555555" readonly property color lightgrey: "#999999" + readonly property color warning: "#FF9800" readonly property var statusColors: { "NONE": "transparent", diff --git a/tests/nodes/test/nodeValidators.py b/tests/nodes/test/nodeValidators.py new file mode 100644 index 0000000000..bc20f1a436 --- /dev/null +++ b/tests/nodes/test/nodeValidators.py @@ -0,0 +1,43 @@ +from meshroom.core import desc +from meshroom.core.desc.validators import NotEmptyValidator, RangeValidator + + +class NodeWithValidators(desc.CommandLineNode): + + inputs = [ + desc.StringParam( + name='mandatory', + label='Mandatory input', + description='''''', + value='', + validators= [ + NotEmptyValidator() + ] + ), + desc.FloatParam( + name='floatRange', + label='range input', + description='''''', + value=0.0, + validators=[ + RangeValidator(min=0.0, max=1.0) + ] + ), + desc.IntParam( + name='intRange', + label='range input', + description='''''', + value=0.0, + ), + + ] + + outputs = [ + desc.File( + name='output', + label='Output', + description='''''', + value='{nodeCacheFolder}/appendText.txt', + ) + ] + diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 9abfd67e43..da9063ad78 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -1,4 +1,7 @@ from meshroom.core.graph import Graph +from tests.utils import registerNodeDesc +from tests.nodes.test.nodeValidators import NodeWithValidators + import pytest import logging @@ -10,6 +13,7 @@ valid2DSemantics= [(semantic, True) for semantic in ('image', 'imageList', 'sequence')] invalid2DSemantics = [(semantic, False) for semantic in ('3d', '', 'multiline', 'color/hue')] +registerNodeDesc(NodeWithValidators) def test_attribute_retrieve_linked_input_and_output_attributes(): """ @@ -65,7 +69,6 @@ def test_attribute_is3D_file_extensions(givenFile, expected): # Then assert n0.input.is3D == expected - def test_attribute_i3D_by_description_semantic(): """ """ @@ -98,4 +101,51 @@ def test_attribute_is2D_file_semantic(givenSemantic, expected): n0.input.desc._semantic = givenSemantic # Then - assert n0.input.is2D == expected \ No newline at end of file + assert n0.input.is2D == expected + +def test_attribute_notEmpty_validation(): + + # Given + g = Graph('') + node = g.addNewNode('NodeWithValidators') + + # When + node.mandatory.value = '' + + # Then + assert not node.mandatory.isValid + assert len(node.mandatory.getErrorMessages()) == 1 + assert node.mandatory.isMandatory is True + assert node.hasInvalidAttribute + + # When + node.mandatory.value = 'test' + + # Then + assert node.mandatory.isValid + assert len(node.mandatory.getErrorMessages()) == 0 + assert not node.hasInvalidAttribute + +def test_attribute_range_validation(): + + # Given + g = Graph('') + node = g.addNewNode('NodeWithValidators') + node.mandatory.value = 'test' + + # When + node.floatRange.value = 2.0 + + # Then + assert not node.floatRange.isValid + assert len(node.floatRange.getErrorMessages()) == 2 + assert node.mandatory.isMandatory is True + assert node.hasInvalidAttribute + + # When + node.floatRange.value = 0.25 + + # Then + assert node.floatRange.isValid + assert len(node.mandatory.getErrorMessages()) == 0 + assert not node.hasInvalidAttribute