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

fix(StackViewport): Reset camera bug when rotation happens on StackViewport #374

Merged
merged 3 commits into from
Jan 23, 2023
Merged

fix(StackViewport): Reset camera bug when rotation happens on StackViewport #374

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fdbc3d6
fix(StackViewport): #372 Reset camera bug when rotation happens on St…
jbocce Jan 18, 2023
9f1ce66
Save the initialViewUp vector so that it can restored in resetCameraGPU.
jbocce Jan 18, 2023
264ae3d
Set StackViewport.initialViewUp in the constructor.
jbocce Jan 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/core/src/RenderingEngine/StackViewport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ class StackViewport extends Viewport implements IStackViewport {
public modality: string; // this is needed for tools
public scaling: Scaling;

// Camera properties
private initialViewUp: Point3;
sedghi marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Member

Choose a reason for hiding this comment

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

I think we should this.initialViewUp= inside the constructor as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed

/**
* Constructor for the StackViewport class
* @param props - ViewportInput
Expand Down Expand Up @@ -1748,6 +1751,9 @@ class StackViewport extends Viewport implements IStackViewport {

this.setCameraNoEvent({ viewUp, viewPlaneNormal });

// Setting this makes the following comment about resetCameraNoEvent not modifying viewUp true.
this.initialViewUp = viewUp;

// Reset the camera to point to the new slice location, reset camera doesn't
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prior to my changes, this comment was not true. viewUp was getting changed via a call to camera.roll.

// modify the direction of projection and viewUp
this.resetCameraNoEvent();
Expand Down Expand Up @@ -1883,7 +1889,14 @@ class StackViewport extends Viewport implements IStackViewport {
// Todo: we need to make the rotation a camera properties so that
// we can reset it there, right now it is not possible to reset the rotation
// without this
this.getVtkActiveCamera().roll(this.rotationCache);
if (this.initialViewUp) {
Copy link
Member

Choose a reason for hiding this comment

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

if you set it in the constructor you don't need this if here, since it will always be defined

// We do not know the ordering of various flips and rotations that have been applied, so just start like we were at the beginning.
this.setCamera({
flipHorizontal: false,
flipVertical: false,
viewUp: this.initialViewUp,
});
}

// For stack Viewport we since we have only one slice
// it should be enough to reset the camera to the center of the image
Expand Down