Fix and prevent mismatches between an attribute's type and its default value's type #1784

cbentejac · 2022-09-26T22:42:33Z

Description

This PR fixes a long-standing issue with nodes' attributes that was recently highlighted by #1747.

Some nodes' attributes had default values that did not match with the attribute's type, and, provided these attributes could invalidate their node, this would trigger some UID changes when loading a graph with such nodes, compared to their initial values when the graph was written.

One of the direct consequences of this issue was that a pipeline that was created and submitted to the render farm needed to be reloaded in Meshroom (to trigger the undesired change of UIDs) for the nodes' status to be updated; otherwise, the nodes' status remained in the "waiting" state, since these nodes' UIDs did not correspond to those submitted.

We address this issue as follows:

All the attributes that had incorrect default values have been fixed for all the existing nodes in Meshroom.
The unit tests now include a comparison between a graph's nodes' UIDs when said graph is saved, and when it is loaded.
Upon Meshroom's start, when all the nodes are being loaded, every node's description is parsed to ensure that every attribute has a default value and range whose type matches with the attribute's. If no mismatch is detected, the node is loaded as usual. However, if a single attribute has an erroneous default value or range, then the node is rejected altogether: it will not be loaded into Meshroom, and a message will explicitly display which attribute(s) was invalid.

Features list

Fix the type of default values to match with their attribute's type on all nodes
Check, before loading a node in Meshroom, that its description is valid: if there is a single attribute whose default value / range does not match the attribute's type, the node's description is deemed invalid, and that node will not be loaded
Add a check on the UIDs before writing a graph and after loading it in the unit tests

Fix the parameters with default values that do not correspond to the parameters' types. For CameraInit, all the default values of FloatParams that are set with integers are replaced with actual float values. These default values with an erroneous type would cause changes in the CameraInit's UID when the intrinsics' default values were written (with values of the correct type) and when they were loaded (with values of the wrong type).

Fix default values for: - DepthMap: refineSigma (FloatParam); use float instead of int - ImageMasking: hsvMaxSaturation (FloatParam); use float instead of int - ImageMasking: hsvMaxValue (FloatParam); use float instead of int - Meshing: estimateSpaceMinObservationAngle (FloatParam); use float instead of int - PanoramaInit: yawCW (BoolParam); use bool instead of int

meshroom/core/__init__.py

meshroom/core/desc.py

fabiencastan

code LGTM now

meshroom/core/desc.py

At Meshroom's launch, check that every node we attempt to load has a valid description, i.e. that every parameter has a default value that matches its parameter's type. If there is at least one parameter with an incorrect default value, the node is not loaded and a corresponding message will be displayed. This prevents the user from loading erroneous nodes that may lead to unexpected behaviours (such as a change of a node's UID between the moment when it is written and the moment it is loaded).

…ing it For IntParam and FloatParam, which may have ranges, check before loading the node that the non-null ranges are of the same type as the attribute. If at least a range is not correct (e.g. "(0, 10, 0.1)" for a FloatParam, for which "(0.0, 10.0, 0.1)" is expected), then the node is rejected (in the same manner as when a default value is deemed invalid).

cbentejac added 3 commits September 26, 2022 17:48

[tests] Check that UIDs are identical when writing and loading a graph

5b45182

cbentejac force-pushed the fix/uidNodes branch from 1bba0e8 to 2b4a708 Compare September 27, 2022 10:47