From cabefcefcba463abb1ea9bf346a2f755b2494aed Mon Sep 17 00:00:00 2001 From: Alireza Date: Fri, 4 Nov 2022 10:42:30 -0400 Subject: [PATCH] fix: resetCamera and annotations for flipped viewports (#278) * fix: remove applyTx since flip is not on actor anymore * fix: make annotations appear on flipeed viewports * fix: reset camera for flipped viewports * fix build * fix test * fix: tests * fix build --- common/reviews/api/core.api.md | 8 +- common/reviews/api/tools.api.md | 8 +- .../core/src/RenderingEngine/StackViewport.ts | 8 +- packages/core/src/RenderingEngine/Viewport.ts | 95 +++++++++---------- .../src/RenderingEngine/VolumeViewport.ts | 13 +-- packages/core/src/constants/epsilon.ts | 3 + packages/core/src/constants/index.ts | 3 +- packages/tools/examples/crossHairs/index.ts | 2 +- .../planar/filterAnnotationsWithinSlice.ts | 33 +++++-- .../filterViewportsWithParallelNormals.ts | 26 +++++ .../getViewportIdsWithToolToRender.ts | 10 +- .../src/utilities/viewportFilters/index.ts | 2 + packages/tools/test/segmentationState_test.js | 9 +- 13 files changed, 135 insertions(+), 85 deletions(-) create mode 100644 packages/core/src/constants/epsilon.ts create mode 100644 packages/tools/src/utilities/viewportFilters/filterViewportsWithParallelNormals.ts diff --git a/common/reviews/api/core.api.md b/common/reviews/api/core.api.md index 093ad55f64..b8f7e50318 100644 --- a/common/reviews/api/core.api.md +++ b/common/reviews/api/core.api.md @@ -85,7 +85,8 @@ const colormapsData: CPUFallbackColormapsData; declare namespace CONSTANTS { export { colormapsData as CPU_COLORMAPS, - RENDERING_DEFAULTS + RENDERING_DEFAULTS, + EPSILON } } export { CONSTANTS } @@ -425,6 +426,9 @@ declare namespace Enums { } export { Enums } +// @public (undocumented) +const EPSILON = 0.001; + // @public (undocumented) export enum EVENTS { // (undocumented) @@ -1957,8 +1961,6 @@ export class Viewport implements IViewport { // (undocumented) addActors(actors: Array, resetCameraPanAndZoom?: boolean): void; // (undocumented) - protected applyFlipTx: (worldPos: Point3) => Point3; - // (undocumented) readonly canvas: HTMLCanvasElement; // (undocumented) canvasToWorld: (canvasPos: Point2) => Point3; diff --git a/common/reviews/api/tools.api.md b/common/reviews/api/tools.api.md index 4091f4aba5..c838543b04 100644 --- a/common/reviews/api/tools.api.md +++ b/common/reviews/api/tools.api.md @@ -1634,6 +1634,9 @@ function filterAnnotationsWithinSlice(annotations: Annotations, camera: Types_2. // @public (undocumented) function filterViewportsWithFrameOfReferenceUID(viewports: Array, FrameOfReferenceUID: string): Array; +// @public (undocumented) +function filterViewportsWithParallelNormals(viewports: any, camera: any, EPS?: number): any; + // @public (undocumented) function filterViewportsWithToolEnabled(viewports: Array, toolName: string): Array; @@ -1837,7 +1840,7 @@ function getToolGroupsWithSegmentation(segmentationId: string): string[]; function getToolState(element: HTMLDivElement): CINETypes.ToolData | undefined; // @public (undocumented) -function getViewportIdsWithToolToRender(element: HTMLDivElement, toolName: string, requireSameOrientation?: boolean): string[]; +function getViewportIdsWithToolToRender(element: HTMLDivElement, toolName: string, requireParallelNormals?: boolean): string[]; // @public (undocumented) function getViewportSpecificAnnotationManager(element?: Types_2.IEnabledElement | HTMLDivElement): FrameOfReferenceSpecificAnnotationManager; @@ -4394,7 +4397,8 @@ declare namespace viewportFilters { export { filterViewportsWithToolEnabled, filterViewportsWithFrameOfReferenceUID, - getViewportIdsWithToolToRender + getViewportIdsWithToolToRender, + filterViewportsWithParallelNormals } } diff --git a/packages/core/src/RenderingEngine/StackViewport.ts b/packages/core/src/RenderingEngine/StackViewport.ts index 3c21be9e03..70936575ad 100644 --- a/packages/core/src/RenderingEngine/StackViewport.ts +++ b/packages/core/src/RenderingEngine/StackViewport.ts @@ -2165,7 +2165,7 @@ class StackViewport extends Viewport implements IStackViewport { // The y axis display coordinates are inverted with respect to canvas coords displayCoord[1] = size[1] - displayCoord[1]; - let worldCoord = openGLRenderWindow.displayToWorld( + const worldCoord = openGLRenderWindow.displayToWorld( displayCoord[0], displayCoord[1], 0, @@ -2175,9 +2175,7 @@ class StackViewport extends Viewport implements IStackViewport { // set clipping range back to original to be able vtkCamera.setClippingRange(crange[0], crange[1]); - worldCoord = this.applyFlipTx(worldCoord); - - return worldCoord; + return [worldCoord[0], worldCoord[1], worldCoord[2]]; }; private worldToCanvasGPU = (worldPos: Point3) => { @@ -2199,7 +2197,7 @@ class StackViewport extends Viewport implements IStackViewport { offscreenMultiRenderWindow.getOpenGLRenderWindow(); const size = openGLRenderWindow.getSize(); const displayCoord = openGLRenderWindow.worldToDisplay( - ...this.applyFlipTx(worldPos), + ...worldPos, renderer ); diff --git a/packages/core/src/RenderingEngine/Viewport.ts b/packages/core/src/RenderingEngine/Viewport.ts index de3e52ff3b..566a36ae7b 100644 --- a/packages/core/src/RenderingEngine/Viewport.ts +++ b/packages/core/src/RenderingEngine/Viewport.ts @@ -174,24 +174,6 @@ class Viewport implements IViewport { } } - protected applyFlipTx = (worldPos: Point3): Point3 => { - const actorEntry = this.getDefaultActor(); - - if (!actorEntry || !actorEntry.actor) { - return worldPos; - } - - const actor = actorEntry.actor; - const mat = actor.getMatrix(); - - const newPos = vec3.create(); - const matT = mat4.create(); - mat4.transpose(matT, mat); - vec3.transformMat4(newPos, worldPos, matT); - - return [newPos[0], newPos[1], newPos[2]]; - }; - /** * Flip the viewport on horizontal or vertical axis, this method * works with vtk-js backed rendering pipeline. @@ -207,29 +189,6 @@ class Viewport implements IViewport { return; } - let flipH = false; - let flipV = false; - - if ( - typeof flipHorizontal !== 'undefined' && - ((flipHorizontal && !this.flipHorizontal) || - (!flipHorizontal && this.flipHorizontal)) - ) { - flipH = true; - } - - if ( - typeof flipVertical !== 'undefined' && - ((flipVertical && !this.flipVertical) || - (!flipVertical && this.flipVertical)) - ) { - flipV = true; - } - - if (!flipH && !flipV) { - return; - } - const { viewPlaneNormal, viewUp, focalPoint, position } = this.getCamera(); const viewRight = vec3.create(); @@ -275,7 +234,7 @@ class Viewport implements IViewport { // Flipping horizontal mean that the camera should move // to the other side of the image but looking at the // same direction and same focal point - if (flipH) { + if (flipHorizontal) { // we need to apply the pan value to the new focal point but in the direction // that is mirrored on the viewUp for the flip horizontal and // viewRight for the flip vertical @@ -303,11 +262,13 @@ class Viewport implements IViewport { position: newPosition as Point3, focalPoint: newFocalPoint as Point3, }); + + this.flipHorizontal = !this.flipHorizontal; } // Flipping vertical mean that the camera should negate the view up // and also move to the other side of the image but looking at the - if (flipV) { + if (flipVertical) { viewUpToSet = vec3.negate(viewUpToSet, viewUp); // we need to apply the pan value to the new focal point but in the direction @@ -332,6 +293,8 @@ class Viewport implements IViewport { viewUp: viewUpToSet as Point3, position: newPosition as Point3, }); + + this.flipVertical = !this.flipVertical; } this.render(); @@ -579,6 +542,15 @@ class Viewport implements IViewport { storeAsInitialCamera = true ): boolean { const renderer = this.getRenderer(); + + // fix the flip right away, since we rely on the viewPlaneNormal and + // viewUp for later. Basically, we need to flip back if flipHorizontal + // is true or flipVertical is true + this.setCamera({ + flipHorizontal: false, + flipVertical: false, + }); + const previousCamera = _cloneDeep(this.getCamera()); const bounds = renderer.computeVisiblePropBounds(); @@ -693,8 +665,6 @@ class Viewport implements IViewport { viewAngle: 90, viewUp: viewUpToSet, clippingRange: clippingRangeToUse, - flipHorizontal: this.flipHorizontal ? false : undefined, - flipVertical: this.flipVertical ? false : undefined, }); const modifiedCamera = _cloneDeep(this.getCamera()); @@ -886,8 +856,8 @@ class Viewport implements IViewport { return { viewUp: vtkCamera.getViewUp(), viewPlaneNormal: vtkCamera.getViewPlaneNormal(), - position: this.applyFlipTx(vtkCamera.getPosition() as Point3), - focalPoint: this.applyFlipTx(vtkCamera.getFocalPoint() as Point3), + position: vtkCamera.getPosition(), + focalPoint: vtkCamera.getFocalPoint(), parallelProjection: vtkCamera.getParallelProjection(), parallelScale: vtkCamera.getParallelScale(), viewAngle: vtkCamera.getViewAngle(), @@ -921,8 +891,31 @@ class Viewport implements IViewport { clippingRange, } = cameraInterface; - if (flipHorizontal !== undefined || flipVertical !== undefined) { - this.flip({ flipHorizontal, flipVertical }); + // Note: Flip camera should be two separate calls since + // for flip, we need to flip the viewportNormal, and if + // flipHorizontal, and flipVertical are both true, that would + // the logic would be incorrect. So instead, we handle flip Horizontal + // and flipVertical separately. + if (flipHorizontal !== undefined) { + // flip if not flipped but requested to flip OR if flipped but requested to + // not flip + const flipH = + (flipHorizontal && !this.flipHorizontal) || + (!flipHorizontal && this.flipHorizontal); + + if (flipH) { + this.flip({ flipHorizontal: flipH }); + } + } + + if (flipVertical !== undefined) { + const flipV = + (flipVertical && !this.flipVertical) || + (!flipVertical && this.flipVertical); + + if (flipV) { + this.flip({ flipVertical: flipV }); + } } if (viewUp !== undefined) { @@ -938,11 +931,11 @@ class Viewport implements IViewport { } if (position !== undefined) { - vtkCamera.setPosition(...this.applyFlipTx(position)); + vtkCamera.setPosition(...position); } if (focalPoint !== undefined) { - vtkCamera.setFocalPoint(...this.applyFlipTx(focalPoint)); + vtkCamera.setFocalPoint(...focalPoint); } if (parallelScale !== undefined) { diff --git a/packages/core/src/RenderingEngine/VolumeViewport.ts b/packages/core/src/RenderingEngine/VolumeViewport.ts index 3ec7c0644b..51b5c6f861 100644 --- a/packages/core/src/RenderingEngine/VolumeViewport.ts +++ b/packages/core/src/RenderingEngine/VolumeViewport.ts @@ -26,7 +26,7 @@ import type { } from '../types'; import type { ViewportInput } from '../types/IViewport'; import type IVolumeViewport from '../types/IVolumeViewport'; -import { RENDERING_DEFAULTS } from '../constants'; +import { RENDERING_DEFAULTS, EPSILON } from '../constants'; import { Events, BlendModes, OrientationAxis } from '../enums'; import eventTarget from '../eventTarget'; import type { vtkSlabCamera as vtkSlabCameraType } from './vtkClasses/vtkSlabCamera'; @@ -34,8 +34,6 @@ import { imageIdToURI, triggerEvent } from '../utilities'; import { VoiModifiedEventDetail } from '../types/EventTypes'; import deepFreeze from '../utilities/deepFreeze'; -const EPSILON = 1e-3; - const ORIENTATION = { axial: { viewPlaneNormal: [0, 0, -1], @@ -784,7 +782,7 @@ class VolumeViewport extends Viewport implements IVolumeViewport { const vtkCamera = this.getVtkActiveCamera() as vtkSlabCameraType; /** - * NOTE: this is necessary because we want the coordinate trasformation + * NOTE: this is necessary because we want the coordinate transformation * respect to the view plane (plane orthogonal to the camera and passing to * the focal point). * @@ -826,7 +824,7 @@ class VolumeViewport extends Viewport implements IVolumeViewport { // The y axis display coordinates are inverted with respect to canvas coords displayCoord[1] = size[1] - displayCoord[1]; - let worldCoord = openGLRenderWindow.displayToWorld( + const worldCoord = openGLRenderWindow.displayToWorld( displayCoord[0], displayCoord[1], 0, @@ -835,8 +833,7 @@ class VolumeViewport extends Viewport implements IVolumeViewport { vtkCamera.setIsPerformingCoordinateTransformation(false); - worldCoord = this.applyFlipTx(worldCoord); - return worldCoord; + return [worldCoord[0], worldCoord[1], worldCoord[2]]; }; /** @@ -881,7 +878,7 @@ class VolumeViewport extends Viewport implements IVolumeViewport { offscreenMultiRenderWindow.getOpenGLRenderWindow(); const size = openGLRenderWindow.getSize(); const displayCoord = openGLRenderWindow.worldToDisplay( - ...this.applyFlipTx(worldPos), + ...worldPos, renderer ); diff --git a/packages/core/src/constants/epsilon.ts b/packages/core/src/constants/epsilon.ts new file mode 100644 index 0000000000..f930bcd22b --- /dev/null +++ b/packages/core/src/constants/epsilon.ts @@ -0,0 +1,3 @@ +const EPSILON = 1e-3; + +export default EPSILON; diff --git a/packages/core/src/constants/index.ts b/packages/core/src/constants/index.ts index b901db9a86..f3051f8850 100644 --- a/packages/core/src/constants/index.ts +++ b/packages/core/src/constants/index.ts @@ -1,4 +1,5 @@ import CPU_COLORMAPS from './cpuColormaps'; import RENDERING_DEFAULTS from './rendering'; +import EPSILON from './epsilon'; -export { CPU_COLORMAPS, RENDERING_DEFAULTS }; +export { CPU_COLORMAPS, RENDERING_DEFAULTS, EPSILON }; diff --git a/packages/tools/examples/crossHairs/index.ts b/packages/tools/examples/crossHairs/index.ts index 8724a5c98d..36959a9b24 100644 --- a/packages/tools/examples/crossHairs/index.ts +++ b/packages/tools/examples/crossHairs/index.ts @@ -39,7 +39,7 @@ const toolGroupId = 'MY_TOOLGROUP_ID'; // ======== Set up page ======== // setTitleAndDescription( 'Crosshairs', - 'Here we demonstrate crosshairs linking three orthogonal views of the same data' + 'Here we demonstrate crosshairs linking three orthogonal views of the same data. You can select the blend mode that will be used if you modify the slab thickness of the crosshairs by dragging the control points.' ); const size = '500px'; diff --git a/packages/tools/src/utilities/planar/filterAnnotationsWithinSlice.ts b/packages/tools/src/utilities/planar/filterAnnotationsWithinSlice.ts index 91aad09750..c2ab393eae 100644 --- a/packages/tools/src/utilities/planar/filterAnnotationsWithinSlice.ts +++ b/packages/tools/src/utilities/planar/filterAnnotationsWithinSlice.ts @@ -1,8 +1,12 @@ import { vec3 } from 'gl-matrix'; -import { utilities as csUtils } from '@cornerstonejs/core'; +import { CONSTANTS } from '@cornerstonejs/core'; import type { Types } from '@cornerstonejs/core'; import { Annotations, Annotation } from '../../types'; +const { EPSILON } = CONSTANTS; + +const PARALLEL_THRESHOLD = 1 - EPSILON; + /** * given some `Annotations`, and the slice defined by the camera's normal * direction and the spacing in the normal, filter the `Annotations` which @@ -19,13 +23,28 @@ export default function filterAnnotationsWithinSlice( spacingInNormalDirection: number ): Annotations { const { viewPlaneNormal } = camera; - const annotationsWithSameNormal = annotations.filter((td: Annotation) => { - const annotationViewPlaneNormal = td.metadata.viewPlaneNormal; - return csUtils.isEqual(annotationViewPlaneNormal, viewPlaneNormal); - }); + + // The reason we use parallel normals instead of actual orientation is that + // flipped action is done through camera API, so we can't rely on the + // orientation (viewplaneNormal and viewUp) since even the same image and + // same slice if flipped will have different orientation, but still rendering + // the same slice. Instead, we choose to use the parallel normals to filter + // the annotations and later we fine tune it with the annotation within slice + // logic down below. + const annotationsWithParallelNormals = annotations.filter( + (td: Annotation) => { + const annotationViewPlaneNormal = td.metadata.viewPlaneNormal; + + const isParallel = + Math.abs(vec3.dot(viewPlaneNormal, annotationViewPlaneNormal)) > + PARALLEL_THRESHOLD; + + return annotationViewPlaneNormal && isParallel; + } + ); // No in plane annotations. - if (!annotationsWithSameNormal.length) { + if (!annotationsWithParallelNormals.length) { return []; } @@ -37,7 +56,7 @@ export default function filterAnnotationsWithinSlice( const annotationsWithinSlice = []; - for (const annotation of annotationsWithSameNormal) { + for (const annotation of annotationsWithParallelNormals) { const data = annotation.data; const point = data.handles.points[0]; diff --git a/packages/tools/src/utilities/viewportFilters/filterViewportsWithParallelNormals.ts b/packages/tools/src/utilities/viewportFilters/filterViewportsWithParallelNormals.ts new file mode 100644 index 0000000000..fceb72db7f --- /dev/null +++ b/packages/tools/src/utilities/viewportFilters/filterViewportsWithParallelNormals.ts @@ -0,0 +1,26 @@ +import { vec3 } from 'gl-matrix'; + +/** + * It filters the viewports that are looking in the same view as the camera + * It basically checks if the viewPlaneNormal is parallel to the camera viewPlaneNormal + * @param viewports - Array of viewports to filter + * @param camera - Camera to compare against + * @returns - Array of viewports with the same view + */ +export function filterViewportsWithParallelNormals( + viewports, + camera, + EPS = 0.999 +) { + return viewports.filter((viewport) => { + const vpCamera = viewport.getCamera(); + + const isParallel = + Math.abs(vec3.dot(vpCamera.viewPlaneNormal, camera.viewPlaneNormal)) > + EPS; + + return isParallel; + }); +} + +export default filterViewportsWithParallelNormals; diff --git a/packages/tools/src/utilities/viewportFilters/getViewportIdsWithToolToRender.ts b/packages/tools/src/utilities/viewportFilters/getViewportIdsWithToolToRender.ts index dd47f5397b..0581c4718d 100644 --- a/packages/tools/src/utilities/viewportFilters/getViewportIdsWithToolToRender.ts +++ b/packages/tools/src/utilities/viewportFilters/getViewportIdsWithToolToRender.ts @@ -1,7 +1,7 @@ import { getEnabledElement } from '@cornerstonejs/core'; import filterViewportsWithFrameOfReferenceUID from './filterViewportsWithFrameOfReferenceUID'; import filterViewportsWithToolEnabled from './filterViewportsWithToolEnabled'; -import filterViewportsWithSameOrientation from './filterViewportsWithSameOrientation'; +import filterViewportsWithParallelNormals from './filterViewportsWithParallelNormals'; /** * Given a cornerstone3D enabled `element`, and a `toolName`, find all viewportIds @@ -10,14 +10,14 @@ import filterViewportsWithSameOrientation from './filterViewportsWithSameOrienta * * @param element - The target cornerstone3D enabled element. * @param toolName - The string toolName. - * @param requireSameOrientation - If true, only return viewports matching the orientation of the original viewport + * @param requireParallelNormals - If true, only return viewports that have parallel normals. * * @returns An array of viewportIds. */ export default function getViewportIdsWithToolToRender( element: HTMLDivElement, toolName: string, - requireSameOrientation = true + requireParallelNormals = true ): string[] { const enabledElement = getEnabledElement(element); const { renderingEngine, FrameOfReferenceUID } = enabledElement; @@ -32,8 +32,8 @@ export default function getViewportIdsWithToolToRender( const viewport = renderingEngine.getViewport(enabledElement.viewportId); - if (requireSameOrientation) { - viewports = filterViewportsWithSameOrientation( + if (requireParallelNormals) { + viewports = filterViewportsWithParallelNormals( viewports, viewport.getCamera() ); diff --git a/packages/tools/src/utilities/viewportFilters/index.ts b/packages/tools/src/utilities/viewportFilters/index.ts index e00c10aa42..4dd72b1f97 100644 --- a/packages/tools/src/utilities/viewportFilters/index.ts +++ b/packages/tools/src/utilities/viewportFilters/index.ts @@ -1,9 +1,11 @@ import filterViewportsWithFrameOfReferenceUID from './filterViewportsWithFrameOfReferenceUID'; import filterViewportsWithToolEnabled from './filterViewportsWithToolEnabled'; import getViewportIdsWithToolToRender from './getViewportIdsWithToolToRender'; +import filterViewportsWithParallelNormals from './filterViewportsWithParallelNormals'; export { filterViewportsWithToolEnabled, filterViewportsWithFrameOfReferenceUID, getViewportIdsWithToolToRender, + filterViewportsWithParallelNormals, }; diff --git a/packages/tools/test/segmentationState_test.js b/packages/tools/test/segmentationState_test.js index a6775ad0ce..ccafe07fa3 100644 --- a/packages/tools/test/segmentationState_test.js +++ b/packages/tools/test/segmentationState_test.js @@ -158,11 +158,16 @@ describe('Segmentation State -- ', () => { expect(segRepresentation.segmentationId).toBe(segVolumeId); expect(segRepresentation.type).toBe(LABELMAP); expect(segRepresentation.config).toBeDefined(); - - done(); } ); + // wait for segmentation render to call done to ensure + // all events have been fired and we don't get errors for rendering while + // the data is decached + eventTarget.addEventListener(Events.SEGMENTATION_RENDERED, (evt) => { + done(); + }); + this.segToolGroup.addViewport(vp.id, this.renderingEngine.id); const callback = ({ volumeActor }) =>