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] Preserve last CameraInit index when updating the CameraInits list #2145

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

cbentejac
Copy link
Contributor

@cbentejac cbentejac commented Aug 7, 2023

Description

Prior to this PR, when a CameraInit node was added to or removed from the list of CameraInit nodes, the active CameraInit was always reset to the first one in the list, independently from its previous value. This meant that whenever a CameraInit node was added to or removed from the graph while the Image Gallery was displaying a CameraInit node that was not the first one in the list, the Image Gallery was automatically updated to display the first CameraInit node without any intervention from the user, as shown below:

This pull request changes that behaviour by only modifying the active CameraInit node if no CameraInit node has been assigned yet, or if the active CameraInit node does not exist anymore (meaning it has been removed).

This requires to emit the cameraInitChanged() signal every single time the list of existing CameraInit nodes is modified even if the active one remains the same, and to connect it to the ImageGallery in order to ensure the index from the combo box always corresponds to the currently active CameraInit node. Indeed, when the list of CameraInits is updated, the
model for the combo box is reset, and it needs to be re-updated with the correct non-default value.

@cbentejac cbentejac added this to the Meshroom 2023.3.0 milestone Aug 7, 2023
@cbentejac cbentejac self-assigned this Aug 7, 2023
meshroom/ui/reconstruction.py Outdated Show resolved Hide resolved
Prior to this commit, when a `CameraInit` node was added to or removed
from the list of `CameraInit` nodes, the active `CameraInit` was always
reset to the first one in the list, independently from its previous
value.

This commit changes that behaviour by only modifying the active
`CameraInit` if no `CameraInit` node has been assigned yet, or if the
active `CameraInit` does not exist anymore (meaning it has been removed).

This requires to emit the `cameraInitChanged` signal every single time
the list of existing `CameraInit` nodes is modified even if the active one
is not changed, and to connect it to the ImageGallery in order to ensure
the index from the combo box always corresponds to the currently active
`CameraInit` node. Indeed, when the list of CameraInits is updated, the
model for the combo box is reset, and it needs to be re-updated with the
correct non-default value.
Copy link
Member

@fabiencastan fabiencastan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check if we have removed all cameraInit before accessing [0]?

@cbentejac
Copy link
Contributor Author

Do we need to check if we have removed all cameraInit before accessing [0]?

If all the CameraInits have been removed, then the test in self.cameraInit = cameraInits[0] if cameraInits else None will fail, so there's no risk of accessing an index that does not exist.

@fabiencastan fabiencastan merged commit c02aa1f into develop Oct 19, 2023
@fabiencastan fabiencastan deleted the fix/cameraInitIdx branch October 19, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants