SciKit-Surgery/scikit-surgeryvtk

Refactor VTKLUSSimulator to accept rot matrices for rendering

Closed this issue · 6 comments

NMontanaBrown commented

Issue Description

Currently, the set pose method for the VTKLUSSimulator accepts Euler angles, which are then converted into a rot matrix for rendering.

It would be useful to be able to directly pass rot matrices into the renderer, to avoid having to convert rot m -> euler angle -> back into rot m if the user is using rot ms.

thompson318 commented

I can't find the setpose method. I found set_camera_pose inherherited from vtk_overlay_window but that seems to use matrices. Can you point to the setpose method please.

mianasbat commented

It is because it is set_pose method I guess which is in
sksurgeryvtk/widgets/vtk_lus_simulator.py
In testing it is tested
tests/camera/test_vtk_camera_model.py and tests/camera/test_lus_simulator.py

  • 👍1
NMontanaBrown commented

I can work on this issue in my fork - any strong feelings about how to refactor? Thinking of just adding a new class method, like set_pose_with_matrices(), which accepts p2c, l2c, angle_of_handle and pokes the actors, effectively duplicating LOCs: 177-187 and 138 - 148.

@mianasbat @thompson318

thompson318 commented

Try and avoid having 2 methods with duplicate code. You could create set_pose_with_matrices, but then refactor the current set_pose method so all it does is convert the the pose parameters to matrices then calls set_pose_with_matrices

  • 👍1
NMontanaBrown commented

@thompson318 see changes in #169