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.

Sign up for GitHub

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

Jump to bottom

ENH: Set MRML interactor style camera node observing displayable manager #7324

Merged
merged 1 commit into from
Nov 2, 2023
Merged

ENH: Set MRML interactor style camera node observing displayable manager #7324

merged 1 commit into from
Nov 2, 2023

Conversation

jcfr
Copy link
Member

@jcfr jcfr commented Oct 30, 2023

No description provided.

@jcfr jcfr mentioned this pull request Oct 30, 2023
Merged
lassoan
lassoan reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@lassoan lassoan left a comment

Choose a reason for hiding this comment

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

This looks good to me. It is a somewhat more complex design but conceptually more clear. Just added some nitpicks inline.

Libs/MRML/DisplayableManager/vtkMRMLThreeDViewInteractorStyle.cxx Outdated Show resolved Hide resolved
Libs/MRML/DisplayableManager/vtkMRMLThreeDViewInteractorStyle.cxx Outdated Show resolved Hide resolved
@jcfr jcfr force-pushed the revisit-setting-of-cameranode-on-interactorstyle branch from 73a9ade to c3ed96e Compare November 2, 2023 14:52
@jcfr
Copy link
Member Author

jcfr commented Nov 2, 2023

@lassoan I addressed your comment and also created an issue to document possible path forward to improve how we associated view and camera node:

@jcfr
Copy link
Member Author

jcfr commented Nov 2, 2023

Waiting the following pull request is integrated, I cancelled the CI (Build) workflow:

Without it the CI (Lint) workflow is failing.

@jcfr jcfr force-pushed the revisit-setting-of-cameranode-on-interactorstyle branch from c3ed96e to 73532a3 Compare November 2, 2023 14:57
@jcfr jcfr requested a review from lassoan November 2, 2023 14:57
@jcfr jcfr enabled auto-merge (rebase) November 2, 2023 14:58
lassoan
lassoan approved these changes Nov 2, 2023
View reviewed changes
Copy link
Contributor

@lassoan lassoan left a comment

Choose a reason for hiding this comment

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

It looks good to me, thank you!

@jcfr jcfr merged commit 39c5a08 into Slicer:main Nov 2, 2023
5 checks passed
@jcfr jcfr deleted the revisit-setting-of-cameranode-on-interactorstyle branch November 2, 2023 17:35
@jcfr jcfr added the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 5, 2023
@jcfr jcfr mentioned this pull request Nov 9, 2023
Merged
@jcfr jcfr removed the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 15, 2023
This was referenced Dec 21, 2023
Merged
Closed
@jcfr jcfr mentioned this pull request Dec 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lassoan lassoan lassoan approved these changes

@pieper pieper Awaiting requested review from pieper

@cpinter cpinter Awaiting requested review from cpinter

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jcfr @lassoan