Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui] Image Gallery: Fix the display of the intrinsics table with temporary CameraInit nodes #1934

Merged
merged 5 commits into from
Mar 18, 2023

Conversation

cbentejac
Copy link
Contributor

Description

When a temporary CameraInit node was created, its intrinsics (set to be identical to the currently active CameraInit node) were not parsed and the intrinsics table was not updated with them. This issue was not visible if there was a single CameraInit node in the graph, as its intrinsics were still displayed, making it seem like the intrinsics were those of the temporary CameraInit node.

However, when there were several CameraInit groups, switching from one group's temporary CameraInit node to another's would generate a QML error and cause the intrinsics table to be empty.

This PR addresses this issue by correctly parsing the intrinsics when temporary CameraInit nodes are created, thus correctly filling the intrinsics table at all times.

It also handles an issue that was directly caused by this fix: since temporary CameraInit nodes are not part of the graph, they cannot be edited, and attempting to edit a temporary CameraInit's intrinsics from the table would cause errors.

To prevent this from happening, temporary CameraInit nodes are set as locked upon their creation, and the intrinsics table goes in read-only mode when the intrinsics of such a node are displayed. Since nodes are automatically locked while they are involved in computations, this also prevents any edit to the intrinsics of CameraInit nodes in that context.

Finally, three minor changes were made:

  • The width of the intrinsics table's columns has been adjusted to correctly fit the data they contain;
  • Some cleaning has been done to harmonize and have the same indentation everywhere in the Image Gallery code;
  • QML syntax warnings have been fixed in the Image Gallery code.

Features list

  • Parse intrinsics when a temporary CameraInit node is created, and update the intrinsics table accordingly;
  • Set the intrinsics table as read-only if a node, temporary or not, is locked.

When setting a temporary CameraInit node, the intrinsics of the actual
CameraInit node are used, meaning that the temporary CameraInit has all
the intrinsics information we need to fill the intrinsics table.

However, when a temporary CameraInit node is set, the parsing of the
intrinsics and the model update are not performed. If the active CameraInit
group does not change, this is not directly visible as the table keeps on
displaying the intrinsics from the actual CameraInit node.

If the active group changes, we attempt to fill the table with the
intrinsics of the temporary CameraInit node, which is being re-set for the
active group. The intrinsics are thus not available, leading to an empty
table, and the parsing is never retriggered once the temporary CameraInit
has been fully set.

Instead of re-parsing the intrinsics when the CameraInitIndex is updated,
the parsing is triggered when the intrinsics are updated, since this
ensures they will be available and its covers all the cases we could
be facing.
As the temporary CameraInit nodes are not really part of the graph, their
attributes cannot be edited. By default, the intrinsics can be edited for
any CameraInit node from the intrinsics table.

For temporary CameraInit nodes, we want the intrinsics to be displayed in
read-only mode to ensure that the user cannot attempt to edit them, which
would cause errors.

Upon its creation, the temporary CameraInit node is locked (as if it were
computed), and the intrinsics table's component are updated to be displayed
as read-only if the CameraInit node is locked.
@cbentejac cbentejac self-assigned this Mar 15, 2023
@fabiencastan fabiencastan added this to the Meshroom 2023.1.0 milestone Mar 18, 2023
@fabiencastan fabiencastan merged commit ba0541c into develop Mar 18, 2023
@fabiencastan fabiencastan deleted the fix/intrinsicsTmpCamInit branch March 18, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants