diff --git a/meshroom/common/PySignal.py b/meshroom/common/PySignal.py index 94d161fd9d..b2e44185ca 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 b0d394f3e6..a37d71c2e0 100644 --- a/meshroom/core/attribute.py +++ b/meshroom/core/attribute.py @@ -12,6 +12,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 meshroom.core.keyValues import KeyValues from meshroom.core.exception import InvalidEdgeError @@ -401,21 +402,6 @@ def _isDefault(self): else: return self._getValue() == self.getDefaultValue() - def _isValid(self): - """ - Check attribute description validValue: - - If it is a function, execute it and return the result - - Otherwise, simply return true - """ - if callable(self._desc.validValue): - try: - return self._desc.validValue(self.node) - except Exception as exc: - if not self.node.isCompatibilityNode: - logging.warning(f"Failed to evaluate 'isValid' (node lambda) for attribute '{self.fullName}': {exc}") - return True - return True - def _is2dDisplayable(self) -> bool: """ Return True if the current attribute is considered as a displayable 2D file. @@ -487,6 +473,39 @@ def updateInternals(self): # Emit if the enable status has changed self._setEnabled(self._getEnabled()) + def getErrorMessages(self) -> list[str]: + """ Execute the validators and aggregate the error messages if there are any. """ + 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 returns (False, errors). """ + 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 if it contains a NotEmptyValidator. """ + for validator in self.desc.validators: + if isinstance(validator, NotEmptyValidator): + return True + + return False + def _getEnabled(self) -> bool: if callable(self._desc.enabled): try: @@ -742,6 +761,9 @@ def validateIncomingConnection(self, connectingAttribute: Attribute) -> bool: expressionApplied = Signal() + errorMessageChanged = Signal() + errorMessages = Property(Variant, lambda self: self.getErrorMessages(), notify=errorMessageChanged) + isMandatory = Property(bool, _isMandatory, constant=True ) def raiseIfLink(func): """ diff --git a/meshroom/core/desc/attribute.py b/meshroom/core/desc/attribute.py index 5023baaeba..e65eee9e35 100644 --- a/meshroom/core/desc/attribute.py +++ b/meshroom/core/desc/attribute.py @@ -3,9 +3,10 @@ import re from collections.abc import Iterable from enum import auto, Enum +from typing import Sequence from meshroom.common import BaseObject, JSValue, Property, Variant, VariantList, strtobool, deprecated - +from meshroom.core.desc.validators import AttributeValidator # Pre-compile regexes for better performance on repeated calls _ACRONYM_RE = re.compile(r'([A-Z]+)([A-Z][a-z])') @@ -61,8 +62,7 @@ class Attribute(BaseObject): """ def __init__(self, name, label, description, value, advanced, semantic, commandLineGroup, enabled, - keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, - validValue=True, errorMessage="", visible=True, exposed=False): + keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, visible=True, exposed=False, validators: Sequence[AttributeValidator]=None): super(Attribute, self).__init__() self._name = name self._label = convertToLabel(name) if label is None else label @@ -76,8 +76,6 @@ def __init__(self, name, label, description, value, advanced, semantic, commandL 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) \ @@ -85,6 +83,13 @@ def __init__(self, name, label, description, value, advanced, semantic, commandL self._isDynamicValue = (self._value is None) self._valueType = None + if validators is None: + self._validators = [] + elif isinstance(validators, Sequence) and all(isinstance(x, AttributeValidator) for x in validators): + self._validators = validators + else: + raise RuntimeError(f"Validators should be of type 'Sequence[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 @@ -134,7 +139,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) @@ -161,8 +170,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. @@ -181,7 +188,7 @@ class ListAttribute(Attribute): """ A list of Attributes """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, elementDesc, name, label=None, description=None, group="allParams", commandLineGroup=_setParamSentinel, - advanced=False, semantic="", enabled=True, joinChar=" ", visible=True, exposed=False, value=None): + advanced=False, semantic="", enabled=True, joinChar=" ", visible=True, exposed=False, value=None, validators=None): """ :param elementDesc: the Attribute description of elements to store in that list :param value: default value. Use None to declare a dynamic output ListAttribute @@ -193,7 +200,7 @@ def __init__(self, elementDesc, name, label=None, description=None, group="allPa super(ListAttribute, self).__init__(name=name, label=label, description=description, value=value, invalidate=False, commandLineGroup=commandLineGroup, 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 @@ -239,7 +246,7 @@ class GroupAttribute(Attribute): @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, items, name, label=None, description=None, group="allParams", commandLineGroup=_setParamSentinel, advanced=False, semantic="", enabled=True, joinChar=" ", brackets=None, visible=True, - exposed=False): + exposed=False, validators=None): """ :param items: the description of the Attributes composing this group """ @@ -250,7 +257,7 @@ def __init__(self, items, name, label=None, description=None, group="allParams", super(GroupAttribute, self).__init__(name=name, label=label, description=description, value={}, commandLineGroup=commandLineGroup, 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 @@ -349,27 +356,25 @@ class Param(Attribute): """ """ def __init__(self, name, label, description, value, commandLineGroup, advanced, semantic, enabled, - keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, - validValue=True, errorMessage="", visible=True, exposed=False): + keyable=False, keyType=None, invalidate=True, uidIgnoreValue=None, visible=True, exposed=False, validators=None): super(Param, self).__init__(name=name, label=label, description=description, value=value, keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, invalidate=invalidate, semantic=semantic, - uidIgnoreValue=uidIgnoreValue, validValue=validValue, - errorMessage=errorMessage, visible=visible, exposed=exposed) + uidIgnoreValue=uidIgnoreValue, visible=visible, exposed=exposed, validators=validators) class File(Attribute): """ """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") - def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, - advanced=False, invalidate=True, semantic="", enabled=True, visible=True, exposed=True): - + def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, + advanced=False, invalidate=True, semantic="", enabled=True, visible=True, exposed=True, validators=None): + commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group - super(File, self).__init__(name=name, label=label, description=description, value=value, - commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, - invalidate=invalidate, semantic=semantic, visible=visible, exposed=exposed) + super(File, self).__init__(name=name, label=label, description=description, value=value, + commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, + invalidate=invalidate, semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = str def validateValue(self, value): @@ -395,15 +400,15 @@ class BoolParam(Param): """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, name, label=None, description=None, value=None, keyable=False, keyType=None, - group="allParams", commandLineGroup=_setParamSentinel, advanced=False, - enabled=True, invalidate=True, semantic="", visible=True, exposed=False): - + group="allParams", commandLineGroup=_setParamSentinel, advanced=False, + enabled=True, invalidate=True, semantic="", visible=True, exposed=False, validators=None): + commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(BoolParam, self).__init__(name=name, label=label, description=description, value=value, - keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, - advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, visible=visible, exposed=exposed) + keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, + advanced=advanced, enabled=enabled, invalidate=invalidate, + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = bool def validateValue(self, value): @@ -430,17 +435,16 @@ class IntParam(Param): """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, name, label=None, description=None, value=None, range=None, keyable=False, keyType=None, - group="allParams", commandLineGroup=_setParamSentinel, advanced=False, enabled=True, - invalidate=True, semantic="", validValue=True, errorMessage="", visible=True, exposed=False): + group="allParams", commandLineGroup=_setParamSentinel, advanced=False, enabled=True, + invalidate=True, semantic="", visible=True, exposed=False, validators=None): self._range = range commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(IntParam, self).__init__(name=name, label=label, description=description, value=value, - keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, - advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, validValue=validValue, errorMessage=errorMessage, - visible=visible, exposed=exposed) + keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, + advanced=advanced, enabled=enabled, invalidate=invalidate, + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = int def validateValue(self, value): @@ -470,16 +474,15 @@ class FloatParam(Param): """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, name, label=None, description=None, value=None, range=None, keyable=False, keyType=None, - group="allParams", commandLineGroup=_setParamSentinel, advanced=False, enabled=True, - invalidate=True, semantic="", validValue=True, errorMessage="", visible=True, exposed=False): + group="allParams", commandLineGroup=_setParamSentinel, advanced=False, enabled=True, + invalidate=True, semantic="", visible=True, exposed=False, validators=None): self._range = range commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(FloatParam, self).__init__(name=name, label=label, description=description, value=value, - keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, - advanced=advanced, enabled=enabled, invalidate=invalidate, - semantic=semantic, validValue=validValue, errorMessage=errorMessage, - visible=visible, exposed=exposed) + keyable=keyable, keyType=keyType, commandLineGroup=commandLineGroup, + advanced=advanced, enabled=enabled, invalidate=invalidate, + semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._valueType = float def validateValue(self, value): @@ -507,15 +510,15 @@ class PushButtonParam(Param): """ """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") - def __init__(self, name, label=None, description=None, group="allParams", commandLineGroup=_setParamSentinel, - advanced=False, enabled=True, invalidate=True, semantic="", visible=True, exposed=False): - + def __init__(self, name, label=None, description=None, group="allParams", commandLineGroup=_setParamSentinel, + advanced=False, enabled=True, invalidate=True, semantic="", visible=True, exposed=False, validators=None): + commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(PushButtonParam, self).__init__(name=name, label=label, description=description, value=None, - commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, - invalidate=invalidate, semantic=semantic, visible=visible, - exposed=exposed) + commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, + invalidate=invalidate, semantic=semantic, visible=visible, + exposed=exposed, validators=validators) self._valueType = None def getInstanceType(self): @@ -550,15 +553,14 @@ class ChoiceParam(Param): @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") def __init__(self, name: str, label=None, description=None, value=None, values=None, exclusive=True, saveValuesOverride=False, - group="allParams", commandLineGroup=_setParamSentinel, joinChar=" ", advanced=False, enabled=True, - invalidate=True, semantic="", validValue=True, errorMessage="", visible=True, exposed=False): + group="allParams", commandLineGroup=_setParamSentinel, joinChar=" ", advanced=False, enabled=True, + invalidate=True, semantic="", visible=True, exposed=False, validators=None): commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(ChoiceParam, self).__init__(name=name, label=label, description=description, value=value, - commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, - invalidate=invalidate, semantic=semantic, validValue=validValue, - errorMessage=errorMessage, visible=visible, exposed=exposed) + commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, + invalidate=invalidate, semantic=semantic, visible=visible, exposed=exposed, validators=validators) self._values = values if values is not None else [] self._saveValuesOverride = saveValuesOverride self._exclusive = exclusive @@ -635,17 +637,15 @@ class StringParam(Param): """ """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") - def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, - advanced=False, enabled=True, invalidate=True, semantic="", uidIgnoreValue=None, validValue=True, - errorMessage="", visible=True, exposed=False): + def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, + advanced=False, enabled=True, invalidate=True, semantic="", uidIgnoreValue=None, visible=True, exposed=False, validators=None): commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(StringParam, self).__init__(name=name, label=label, description=description, value=value, - commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, - invalidate=invalidate, semantic=semantic, uidIgnoreValue=uidIgnoreValue, - validValue=validValue, errorMessage=errorMessage, visible=visible, - exposed=exposed) + commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, + invalidate=invalidate, semantic=semantic, uidIgnoreValue=uidIgnoreValue, visible=visible, + exposed=exposed, validators=validators) self._valueType = str def validateValue(self, value): @@ -668,14 +668,14 @@ class ColorParam(Param): """ """ @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") - def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, - advanced=False, enabled=True, invalidate=True, semantic="", visible=True, exposed=False): - + def __init__(self, name, label=None, description=None, value=None, group="allParams", commandLineGroup=_setParamSentinel, + advanced=False, enabled=True, invalidate=True, semantic="", visible=True, exposed=False, validators=None): + commandLineGroup = commandLineGroup if commandLineGroup is not _setParamSentinel else group super(ColorParam, self).__init__(name=name, label=label, description=description, value=value, - commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, - invalidate=invalidate, semantic=semantic, visible=visible, exposed=exposed) + commandLineGroup=commandLineGroup, advanced=advanced, enabled=enabled, + invalidate=invalidate, 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..69af1962ad --- /dev/null +++ b/meshroom/core/desc/validators.py @@ -0,0 +1,64 @@ +from typing import TYPE_CHECKING, Protocol, runtime_checkable + +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, list(messages)) + +@runtime_checkable +class AttributeValidator(Protocol): + """ + Interface for an attribute validation. + This class can be inherited, and the __call__ methods overridden to implement any custom attribute validation logic. + + Because it is a callable class, validators can also be created 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]]: + """ + This method can be overridden to implement any custom attribute validation logic. + The `success()` and `error()` helpers can be used to encapsulate the returning responses. + + :param node: The node that holds the attribute to validate + :param attribute: The attribute to validate + + :returns: The validation response: (True, []) if it is valid, (False, [errorMessage1, errorMessage2, ...]) otherwise. + """ + raise NotImplementedError() + + +class NotEmptyValidator(AttributeValidator): + """ + Ensure that the attribute value is not empty. + This class is used to determine if an attribute value 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("An empty value is not allowed.") + + return success() + + +class RangeValidator(AttributeValidator): + """ Check if the attribute value is in a 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/node.py b/meshroom/core/node.py index 86afd7f220..8f23098555 100644 --- a/meshroom/core/node.py +++ b/meshroom/core/node.py @@ -1522,6 +1522,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): @@ -1792,7 +1794,7 @@ def loadOutputAttr(self): # This does not apply to non dynamic output if not self.nodeDesc.hasDynamicOutputAttribute: return - + # Check existence of values.json file valuesFile = self.valuesFile if not os.path.exists(valuesFile): @@ -2160,6 +2162,12 @@ def hasTextOutputAttribute(self) -> bool: """ return next((attr for attr in self._attributes if attr.enabled and attr.isOutput and attr.isTextDisplayable), None) is not None + def _hasInvalidAttribute(self): + for attribute in self._attributes: + if len(attribute.errorMessages) > 0: + return True + return False + def _hasDisplayableShape(self): """ Return True if at least one attribute is a ShapeAttribute, a ShapeListAttribute or a shape File. @@ -2240,6 +2248,9 @@ def _hasDisplayableShape(self): # Whether the node contains a ShapeAttribute, a ShapeListAttribute or a shape File. hasDisplayableShape = Property(bool, _hasDisplayableShape, constant=True) + hasInvalidAttributeChanged = Signal() + hasInvalidAttribute = Property(bool, _hasInvalidAttribute, notify=hasInvalidAttributeChanged) + class Node(BaseNode): """ diff --git a/meshroom/ui/commands.py b/meshroom/ui/commands.py index 4decc8f852..80eba20db0 100755 --- a/meshroom/ui/commands.py +++ b/meshroom/ui/commands.py @@ -314,16 +314,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 AddAttributeKeyValueCommand(GraphCommand): def __init__(self, graph, attribute, key, value, parent=None): diff --git a/meshroom/ui/qml/Application.qml b/meshroom/ui/qml/Application.qml index f78c80ce9f..d829a0e0cf 100644 --- a/meshroom/ui/qml/Application.qml +++ b/meshroom/ui/qml/Application.qml @@ -39,7 +39,7 @@ Page { property alias showImageGallery: imageGalleryVisibilityCB.checked property alias showTextViewer: textViewerVisibilityCB.checked } - + Settings { id: nodeActionsSettings category: "NodeActions" @@ -135,6 +135,14 @@ Page { return true; } + function getAllNodes() { + const nodes = [] + for (let i = 0; i < graphEditor.graph.nodes.count; i++) { + nodes.push(graphEditor.graph.nodes.at(i)) + } + return nodes + } + // File dialogs Platform.FileDialog { id: saveFileDialog @@ -267,14 +275,24 @@ Page { function submit(nodes) { if (!canSubmit) { unsavedSubmitDialog.open() - } else { + } + else { try { - _currentScene.submit(nodes) + if (nodes == null) { + nodes = getAllNodes() + } + if (nodes && nodes.find(node => node.hasInvalidAttribute)) { + submitWithWarningDialog.nodes = nodes + submitWithWarningDialog.open() + } else { + _currentScene.submit(nodes) + } } catch (error) { const data = ErrorHandler.analyseError(error) - if (data.context === "SUBMITTING") + if (data.context === "SUBMITTING") { computeSubmitErrorDialog.openError(data.type, data.msg, nodes) + } } } } @@ -400,6 +418,26 @@ Page { onAccepted: saveAsAction.trigger() } + MessageDialog { + id: submitWithWarningDialog + + canCopy: false + icon.text: MaterialIcons.warning + parent: Overlay.overlay + preset: "Warning" + title: "Nodes With Warnings" + text: "Some nodes have warnings. Are you sure you want to submit them?" + helperText: "Submit nodes even if some of them have warnings." + standardButtons: Dialog.Cancel | Dialog.Yes + + property var nodes: [] + + onDiscarded: close() + onAccepted: { + _currentScene.submit(nodes) + } + } + MessageDialog { id: fileModifiedDialog @@ -733,7 +771,7 @@ Page { model: MeshroomApp.recentProjectFiles MenuItem { enabled: modelData["status"] != 0 - + onTriggered: ensureSaved(function() { openRecentMenu.dismiss() if (_currentScene.load(modelData["path"])) { @@ -887,7 +925,7 @@ Page { id: nodeActionsSettingsMenu title: "NodeActions Settings" implicitWidth: 250 - + MenuItem { id: nodeActionsConfirmDelete checkable: true @@ -1504,7 +1542,7 @@ Page { SplitView.minimumWidth: 350 node: _currentScene ? _currentScene.selectedNode : null - property bool computing: _currentScene ? _currentScene.computing : false + property bool computing: _currentScene ? _currentScene.computing : false property var currentAttributes: [] // Make NodeEditor readOnly when computing diff --git a/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml b/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml index eba44004ad..1e671fb6a7 100644 --- a/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml +++ b/meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml @@ -23,34 +23,29 @@ RowLayout { property alias label: parameterLabel // Accessor to the internal Label (attribute's name) property int labelWidth // Shortcut to set the fixed size of the Label - readonly property bool editable: !attribute.isOutput && !attribute.isLink && + readonly property bool editable: !attribute.isOutput && !attribute.isLink && !readOnly && !(attribute.keyable && _currentScene.selectedViewId === "-1") + 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.isValid ? Qt.darker(palette.window, 1.1) : Qt.darker(Colors.red, 1.5) - - if (attribute.desc) { - var tooltip = "" - if (!attribute.isValid && 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 { visible: attribute.type !== "GroupAttribute" background: Rectangle { id: background - color: object != undefined && object.isValid ? Qt.darker(parent.palette.window, 1.1) : Qt.darker(Colors.red, 1.5) + color: Qt.darker(parent.palette.window, 1.1) } padding: 0 Layout.preferredWidth: labelWidth || implicitWidth @@ -95,7 +90,7 @@ RowLayout { padding: 5 wrapMode: Label.WrapAtWordBoundaryOrAnywhere - text: object.label + text: attribute.isMandatory && attribute.isDefault ? `\* ${object.label}` : object.label color: { if (object != undefined && (object.hasAnyOutputLinks || object.isLink) && !object.enabled) @@ -112,11 +107,7 @@ RowLayout { y: parameterMA.mouseY + 10 text: { - var tooltip = "" - if (!object.isValid && object.desc.errorMessage !== "") - tooltip += "Error: " + Format.plainToHtml(object.desc.errorMessage) + "

" - tooltip += "" + object.desc.name + ": " + attribute.type + "
" + Format.plainToHtml(object.desc.description) - return tooltip + return `${object.desc.name}: ${attribute.type}
${Format.plainToHtml(object.desc.description)}` } visible: parameterMA.containsMouse delay: 800 @@ -146,7 +137,6 @@ RowLayout { enabled: root.editable && !attribute.isDefault onTriggered: { _currentScene.resetAttribute(attribute) - updateAttributeLabel() } } MenuItem { @@ -274,21 +264,20 @@ RowLayout { _currentScene.addAttributeKeyValue(root.attribute, _currentScene.selectedViewId, Number(value)) else _currentScene.setAttribute(root.attribute, Number(value)) - updateAttributeLabel() break case "File": _currentScene.setAttribute(root.attribute, value) break default: _currentScene.setAttribute(root.attribute, value.trim()) - updateAttributeLabel() break } } + Loader { - id: attributeLoader Layout.fillWidth: true + id: inputField sourceComponent: { // PushButtonParam always has value == undefined, so it needs to be excluded from this check @@ -352,115 +341,132 @@ 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 - selectByMouse: true - onEditingFinished: setTextFieldAttribute(text) - persistentSelection: false + TextField { + id: textField + Layout.fillWidth: true - onAccepted: { - setTextFieldAttribute(text) - parameterLabel.forceActiveFocus() - } - Keys.onPressed: function(event) { - if ((event.key == Qt.Key_Escape)) { - event.accepted = true - parameterLabel.forceActiveFocus() + readOnly: !root.editable + text: attribute.value + placeholderText: attribute.isMandatory ? "This field is required" : "" + placeholderTextColor: "gray" + // 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 } - } - 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) + + background: Rectangle { + border.color: errorMessages.length ? "orange" : "transparent" + color: Qt.darker(palette.window, 1.2) + radius: 2 } - } - 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); + states: [ + State { + when: readOnly + PropertyChanges { + target: textField + color: disabledPalette.text + } } + ] + + selectByMouse: true + persistentSelection: false + + onEditingFinished: { + setTextFieldAttribute(text) } - } - Component { - id: textFieldMenuComponent - Menu { - onOpened: { - // Keep cursor visible to see where pasting would happen. - textField.cursorVisible = true; + onAccepted: { + setTextFieldAttribute(text) + parameterLabel.forceActiveFocus() + } + Keys.onPressed: function(event) { + if ((event.key == Qt.Key_Escape)) { + event.accepted = true + parameterLabel.forceActiveFocus() } - onClosed: { - // Disable selection persistency behavior once menu is closed and - // give focus back to the parent TextField. - textField.persistentSelection = false; - textField.forceActiveFocus(); - destroy(); + } + 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) } - 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); - } + } + 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: "Paste" - enabled: !readOnly - onTriggered: { - const clipboardText = Clipboard.getText(); - if (clipboardText.length === 0) { - return; + } + + 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 + } + 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; } } - } + } } } } @@ -492,6 +498,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() } @@ -611,6 +625,7 @@ RowLayout { values: root.attribute.values enabled: root.editable customValueColor: Colors.orange + onToggled: (value, checked) => { var currentValue = root.attribute.value; if (!checked) { @@ -627,13 +642,13 @@ RowLayout { id: sliderComponent RowLayout { ExpressionTextField { - id: expressionTextField + id: expressionTextField implicitWidth: 100 Layout.fillWidth: !slider.active enabled: root.editable // Cast value to string to avoid intrusive scientific notations on numbers - property string displayValue: String(slider.active && slider.item.pressed ? slider.item.formattedValue : - attribute.keyable ? attribute.keyValues.getValueAtKeyOrDefault(_currentScene.selectedViewId) : + property string displayValue: String(slider.active && slider.item.pressed ? slider.item.formattedValue : + attribute.keyable ? attribute.keyValues.getValueAtKeyOrDefault(_currentScene.selectedViewId) : attribute.value) text: displayValue selectByMouse: true @@ -642,7 +657,6 @@ 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: { if (!hasExprError) { setTextFieldAttribute(expressionTextField.evaluatedValue) @@ -650,6 +664,13 @@ RowLayout { expressionTextField.text = Qt.binding(function() { return String(expressionTextField.displayValue); }) } } + + background: Rectangle { + border.color: errorMessages.length ? "orange" : "transparent" + color: Qt.darker(palette.window, 1.2) + radius: 2 + } + onAccepted: { if (!hasExprError) { setTextFieldAttribute(expressionTextField.evaluatedValue) @@ -660,7 +681,7 @@ RowLayout { // (with the most important values and cut the floating point details) ensureVisible(0) } - + Component.onDestruction: { if (activeFocus) { if (!hasExprError) @@ -690,11 +711,10 @@ RowLayout { onPressedChanged: { if (!pressed) { - if(attribute.keyable) + if (attribute.keyable) _currentScene.addAttributeKeyValue(attribute, _currentScene.selectedViewId, formattedValue) else _currentScene.setAttribute(attribute, formattedValue) - updateAttributeLabel() } } } @@ -709,12 +729,12 @@ RowLayout { enabled: root.editable checked: attribute.keyable ? attribute.keyValues.getValueAtKeyOrDefault(_currentScene.selectedViewId) : attribute.value onToggled: { - if(attribute.keyable) + if(attribute.keyable) { const value = attribute.keyValues.getValueAtKeyOrDefault(_currentScene.selectedViewId) _currentScene.addAttributeKeyValue(attribute, _currentScene.selectedViewId, !value) } - else + else { _currentScene.setAttribute(attribute, !attribute.value) } @@ -940,6 +960,13 @@ RowLayout { } } + MaterialLabel { + visible: !attribute.isOutput && root.errorMessages.length + text: MaterialIcons.fmd_bad + ToolTip.text: root.errorMessages.join("\n") + color: "orange" + } + // Add or remove key button for keyable attribute Loader { active: attribute.keyable diff --git a/meshroom/ui/qml/GraphEditor/GraphEditor.qml b/meshroom/ui/qml/GraphEditor/GraphEditor.qml index a4cdc6d357..f5065e3796 100755 --- a/meshroom/ui/qml/GraphEditor/GraphEditor.qml +++ b/meshroom/ui/qml/GraphEditor/GraphEditor.qml @@ -943,6 +943,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 @@ -1329,11 +1330,11 @@ Item { draggable: draggable nodeRepeater: nodeRepeater anchors.fill: parent - + onComputeRequest: function(node) { root.computeRequest([node]) } - + onStopComputeRequest: function(node) { if (node.canBeStopped()) { uigraph.stopNodeComputation(node) @@ -1358,7 +1359,7 @@ Item { uigraph.clearSelectedNodesData() } } - + onSubmitRequest: function(node) { root.submitRequest([node]) } @@ -1373,7 +1374,7 @@ Item { uigraph.restartJobErrorTasks(node) } } - + MessageDialog { id: errorDialog diff --git a/meshroom/ui/qml/GraphEditor/Node.qml b/meshroom/ui/qml/GraphEditor/Node.qml index 85463103b9..6d043d9960 100755 --- a/meshroom/ui/qml/GraphEditor/Node.qml +++ b/meshroom/ui/qml/GraphEditor/Node.qml @@ -47,6 +47,7 @@ Item { property int directionY: 0; property point mousePosition: Qt.point(mouseArea.mouseX, mouseArea.mouseY) + property bool hasWarnings: false Item { id: m @@ -342,9 +343,11 @@ Item { return 2 } border.color: { - if(root.mainSelected) + if (hasWarnings === true) + return Colors.warning + if (root.mainSelected) return activePalette.highlight - if(root.selected) + if (root.selected) return Qt.darker(activePalette.highlight, 1.2) return Qt.lighter(activePalette.base, 3) } @@ -466,6 +469,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 validations are failing for this node." + } + // Submitted externally indicator MaterialLabel { visible: node.isExternal diff --git a/meshroom/ui/qml/Utils/Colors.qml b/meshroom/ui/qml/Utils/Colors.qml index 0d5b2c8936..61deafa402 100644 --- a/meshroom/ui/qml/Utils/Colors.qml +++ b/meshroom/ui/qml/Utils/Colors.qml @@ -22,6 +22,7 @@ QtObject { readonly property color lime: "#CDDC39" readonly property color grey: "#555555" readonly property color lightgrey: "#999999" + readonly property color warning: "#FF9800" readonly property color darkpurple: "#5c4885" readonly property var statusColors: { @@ -66,7 +67,7 @@ QtObject { console.warn("Unknown status : " + chunk.status) return "magenta" } - + function getNodeColor(node, overrides) { if (node === undefined) return "transparent" diff --git a/meshroom/ui/qml/Utils/format.js b/meshroom/ui/qml/Utils/format.js index 846340de12..c86d8e28c7 100644 --- a/meshroom/ui/qml/Utils/format.js +++ b/meshroom/ui/qml/Utils/format.js @@ -9,6 +9,7 @@ function intToString(v) { // Convert a plain text to an html escaped string. function plainToHtml(t) { + if (!t) { return t } var escaped = t.replace(/&/g, '&').replace(//g, '>') // Escape text return escaped.replace(/\n/g, '
') // Replace line breaks } diff --git a/tests/nodes/test/nodeValidators.py b/tests/nodes/test/nodeValidators.py new file mode 100644 index 0000000000..68f06c87b9 --- /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, success, error + + +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, + validators=[lambda node, attr: success() if 0 <= attr.value < 5 else error("Value should be in range 0-5")] + ), + + ] + + 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 22b9a2fecf..84634aca30 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) validTextExtensionFiles = [(f'test{ext}', True) for ext in ('.txt', '.json', '.log', '.csv', '.md')] invalidTextExtensionFiles = [(f'test{ext}', False) for ext in ('', '.exe', '.jpg', '.obj', '.py')] @@ -69,7 +73,6 @@ def test_attribute_is3D_file_extensions(givenFile, expected): # Then assert n0.input.is3dDisplayable == expected - def test_attribute_i3D_by_description_semantic(): """ """ @@ -104,6 +107,54 @@ def test_attribute_is2D_file_semantic(givenSemantic, expected): # Then assert n0.input.is2dDisplayable == 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.floatRange.getErrorMessages()) == 0 + assert not node.hasInvalidAttribute @pytest.mark.parametrize("givenFile,expected", validTextExtensionFiles + invalidTextExtensionFiles)