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

Fix computation of interaction transform used in PositionProp #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jcfr
Copy link
Contributor

@jcfr jcfr commented Dec 22, 2023

This adds detailed comments documenting each step involved in computing the interaction transform used in the PositionProp. The corresponding code was initially introduced in commit 503ef1d ("ENH: Add VR interactor style class for transforming objects in MRML", 2018-08-31).

…ionProp

This commit adds detailed comments documenting each step involved in computing
the interaction transform used in the PositionProp. The corresponding code was
initially introduced in commit 503ef1d ("ENH: Add VR interactor style class for
transforming objects in MRML", 2018-08-31).

Co-authored-by: David Allemang <[email protected]>
@jcfr jcfr changed the title DOC: Document steps for computing interaction transform used in PositionProp Fix computation of interaction transform used in PositionProp Dec 22, 2023
@jcfr
Copy link
Contributor Author

jcfr commented Dec 22, 2023

Since the previous attempt at addressing the issue was incorrect1, another approach suggested by @LucasGandel would be to review the implementation of similar computation used in vtkInteractorStyle3D::PositionProp2 function.

Footnotes

  1. https://github.com/KitwareMedical/SlicerVirtualReality/pull/139#issuecomment-1867130977

  2. https://github.com/Kitware/VTK/blob/2579c39f26d6800cea2b5b0ca7b094dd22bc7bf9/Rendering/Core/vtkInteractorStyle3D.cxx#L66-L118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant