Update rotation handling to include absolute and relative rotations, #71
Conversation
merge upstream
Refactor out uneeded / duplicate code from interactors, Rename volumeMapper -> volumeActor and change associations, so it's always the actor (rather than wishy washy nonsense)
|
Should probably update the package.json to include commitizen stuff, I couldn't get it to work, so my commit log is probably "incorrect" |
| const _viewUp = [...viewUp]; | ||
|
|
||
| if (model.volumeMapper) { | ||
| let volumeCoordinateSpace = vec9toMat3([1, 0, 0, 0, 1, 0, 0, 0, 1]); |
There was a problem hiding this comment.
Since you're now using an identity matrix here, might as well remove the whole volumeCoordinateSpace math altogether. We did in our project.
|
CC: @JamesAPetts |
|
Also to note: The current rotation does not support screen-space z-Axis rotations ( rotate 90 degrees ), which we also need. Perhaps hold off until I can add that into the function, which WAS in my code that the current logic was cribbed from. /shrug |
I can look at this again when you are ready ^. For now I've had a look through and generally agree with the approach. One issues is that setting the View2D's orientation through the orientation prop no longer works, check the crosshairs example. Its probably a very minor issue with the refactor, would you be able to check + fix that? Cheers. |
|
Fixed. Also realized that the existing logic for onScroll callbacks was wrong, but my code (which uses the stack scroll CB for the axis location updating) was working right; turned out it worked because it wasn't CALLING the getter, but just checking the variable itself, which would always be falsy, never ran the callback. (and none of the React code uses the callback anyways) |
|
Thanks a lot for all of this!
We are using it in OHIF, let me check it works there. |
|
Checked all works in OHIF, and also tested that it doesn't break some future unmerged changes for multi-volume rendering. This is a step in the right direction and also allows us to prevent too much hardforking between the react and vue versions. Super thankful! Thanks :) |
|
🎉 This PR is included in version 0.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Refactor out uneeded / duplicate code from interactors,
Rename volumeMapper -> volumeActor and change associations, so it's always the actor (rather than wishy washy nonsense)