From 11a767db1a0cd1835dbbdbc8ad00505f6bffb8ee Mon Sep 17 00:00:00 2001 From: Leonardo Campos Date: Mon, 3 Jun 2024 10:50:19 -0300 Subject: [PATCH] fix(livewire): polyline and handles (winding direction) * fix(livewire): polyline and handles (winding direction) * fixed lint issues * update-api --- common/reviews/api/tools.api.md | 9 ++-- .../contourSegmentationCompleted.ts | 27 ++++++---- .../LivewireContourSegmentationTool.ts | 2 +- .../tools/annotation/LivewireContourTool.ts | 53 ++++++++++--------- .../tools/src/tools/base/ContourBaseTool.ts | 1 + .../contours/updateContourPolyline.ts | 23 ++++++-- .../utilities/math/polyline/getSignedArea.ts | 6 +-- 7 files changed, 75 insertions(+), 46 deletions(-) diff --git a/common/reviews/api/tools.api.md b/common/reviews/api/tools.api.md index dae07c41da..d72c89de12 100644 --- a/common/reviews/api/tools.api.md +++ b/common/reviews/api/tools.api.md @@ -3404,7 +3404,7 @@ export class LivewireContourTool extends ContourSegmentationBaseTool { lastCanvasPoint?: Types_2.Point2; confirmedPath?: LivewirePath; currentPath?: LivewirePath; - confirmedPathRight?: LivewirePath; + confirmedPathNext?: LivewirePath; closed?: boolean; worldToSlice?: (point: Types_2.Point3) => Types_2.Point2; sliceToWorld?: (point: Types_2.Point2) => Types_2.Point3; @@ -3412,7 +3412,7 @@ export class LivewireContourTool extends ContourSegmentationBaseTool { contourHoleProcessingEnabled?: boolean; } | null; // (undocumented) - editHandle(worldPos: Types_2.Point3, element: any, annotation: any, handleIndex: number): void; + editHandle(worldPos: Types_2.Point3, element: any, annotation: LivewireContourAnnotation, handleIndex: number): void; // (undocumented) _endCallback: (evt: EventTypes_2.InteractionEventType, clearAnnotation?: boolean) => void; // (undocumented) @@ -3440,9 +3440,9 @@ export class LivewireContourTool extends ContourSegmentationBaseTool { // (undocumented) protected scissors: LivewireScissors; // (undocumented) - protected scissorsRight: LivewireScissors; + protected scissorsNext: LivewireScissors; // (undocumented) - protected setupBaseEditData(worldPos: any, element: any, annotation: any, rightPos?: any, contourHoleProcessingEnabled?: any): void; + protected setupBaseEditData(worldPos: any, element: any, annotation: any, nextPos?: any, contourHoleProcessingEnabled?: any): void; // (undocumented) static toolName: string; // (undocumented) @@ -6074,6 +6074,7 @@ function updateContourPolyline(annotation: ContourAnnotation, polylineData: { targetWindingDirection?: ContourWindingDirection; }, transforms: { canvasToWorld: (point: Types_2.Point2) => Types_2.Point3; + worldToCanvas: (point: Types_2.Point3) => Types_2.Point2; }, options?: { decimate?: { enabled?: boolean; diff --git a/packages/tools/src/eventListeners/annotations/contourSegmentation/contourSegmentationCompleted.ts b/packages/tools/src/eventListeners/annotations/contourSegmentation/contourSegmentationCompleted.ts index ff1aca9782..b11f1612e5 100644 --- a/packages/tools/src/eventListeners/annotations/contourSegmentation/contourSegmentationCompleted.ts +++ b/packages/tools/src/eventListeners/annotations/contourSegmentation/contourSegmentationCompleted.ts @@ -216,17 +216,26 @@ export function createPolylineHole( const { windingDirection: holeWindingDirection } = holeAnnotation.data.contour; - // Check if both normals are pointing to the same direction because the - // polyline for the hole needs to be in a different direction - // if (glMatrix.equals(1, dotNormals)) { - if (targetWindingDirection === holeWindingDirection) { - holeAnnotation.data.contour.polyline.reverse(); - holeAnnotation.data.contour.windingDirection = targetWindingDirection * -1; - } - addChildAnnotation(targetAnnotation, holeAnnotation); contourSegUtils.removeContourSegmentationAnnotation(holeAnnotation); + const { contour: holeContour } = holeAnnotation.data; + const holePolyline = convertContourPolylineToCanvasSpace( + holeContour.polyline, + viewport + ); + + // Calling `updateContourPolyline` method instead of reversing the polyline + // locally because it is also responsible for checking/fixing the winding direction. + contourUtils.updateContourPolyline( + holeAnnotation, + { + points: holePolyline, + closed: holeContour.closed, + }, + viewport + ); + const { element } = viewport; const enabledElement = getEnabledElement(element); const { renderingEngine } = enabledElement; @@ -393,7 +402,7 @@ function combinePolylines( }; // Calling `updateContourPolyline` method instead of setting it locally - // because it is also responsible for checking/setting the winding direction. + // because it is also responsible for checking/fixing the winding direction. contourUtils.updateContourPolyline( newAnnotation, { diff --git a/packages/tools/src/tools/annotation/LivewireContourSegmentationTool.ts b/packages/tools/src/tools/annotation/LivewireContourSegmentationTool.ts index b81bbf7c50..20a528f8e2 100644 --- a/packages/tools/src/tools/annotation/LivewireContourSegmentationTool.ts +++ b/packages/tools/src/tools/annotation/LivewireContourSegmentationTool.ts @@ -96,7 +96,7 @@ class LivewireContourSegmentationTool extends LivewireContourTool { // Now, update the rendering this.updateAnnotation(acceptedPath); this.scissors = null; - this.scissorsRight = null; + this.scissorsNext = null; this.editData = null; annotation.data.handles.interpolationSources = null; diff --git a/packages/tools/src/tools/annotation/LivewireContourTool.ts b/packages/tools/src/tools/annotation/LivewireContourTool.ts index a297ed9d9a..5d3deb65c3 100644 --- a/packages/tools/src/tools/annotation/LivewireContourTool.ts +++ b/packages/tools/src/tools/annotation/LivewireContourTool.ts @@ -38,8 +38,8 @@ const CLICK_CLOSE_CURVE_SQR_DIST = 10 ** 2; // px class LivewireContourTool extends ContourSegmentationBaseTool { public static toolName: string; protected scissors: LivewireScissors; - /** The scissors from the right handle, used for editing */ - protected scissorsRight: LivewireScissors; + /** The scissors from the next handle, used for editing */ + protected scissorsNext: LivewireScissors; touchDragCallback: any; mouseDragCallback: any; @@ -53,7 +53,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { confirmedPath?: LivewirePath; currentPath?: LivewirePath; /** The next path segment, on the other side of the handle */ - confirmedPathRight?: LivewirePath; + confirmedPathNext?: LivewirePath; closed?: boolean; worldToSlice?: (point: Types.Point3) => Types.Point2; sliceToWorld?: (point: Types.Point2) => Types.Point3; @@ -141,7 +141,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { worldPos, element, annotation, - rightPos?, + nextPos?, contourHoleProcessingEnabled? ) { const enabledElement = getEnabledElement(element); @@ -217,21 +217,21 @@ class LivewireContourTool extends ContourSegmentationBaseTool { height, voiRange ); - if (rightPos) { - this.scissorsRight = LivewireScissors.createInstanceFromRawPixelData( + if (nextPos) { + this.scissorsNext = LivewireScissors.createInstanceFromRawPixelData( scalarData as Float32Array, width, height, voiRange ); - this.scissorsRight.startSearch(worldToSlice(rightPos)); + this.scissorsNext.startSearch(worldToSlice(nextPos)); } // Scissors always start at the startPos for both editing handles and // for initial rendering this.scissors.startSearch(startPos); - const newAnnotation = !rightPos; + const newAnnotation = !nextPos; const confirmedPath = new LivewirePath(); const currentPath = new LivewirePath(); @@ -255,7 +255,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { lastCanvasPoint, confirmedPath, currentPath, - confirmedPathRight: currentPathNext, + confirmedPathNext: currentPathNext, closed: false, handleIndex: this.editData?.handleIndex ?? annotation.handles?.activeHandleIndex, @@ -467,7 +467,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { protected clearEditData() { this.editData = null; this.scissors = null; - this.scissorsRight = null; + this.scissorsNext = null; this.isDrawing = false; } @@ -628,7 +628,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { public editHandle( worldPos: Types.Point3, element, - annotation, + annotation: LivewireContourAnnotation, handleIndex: number ) { const { data } = annotation; @@ -638,11 +638,11 @@ class LivewireContourTool extends ContourSegmentationBaseTool { handlePoints[(handleIndex - 1 + numHandles) % numHandles]; const nextHandle = handlePoints[(handleIndex + 1) % numHandles]; - if (!this.editData?.confirmedPathRight) { + if (!this.editData?.confirmedPathNext) { this.setupBaseEditData(previousHandle, element, annotation, nextHandle); const { polyline } = data.contour; const confirmedPath = new LivewirePath(); - const confirmedPathRight = new LivewirePath(); + const confirmedPathNext = new LivewirePath(); const { worldToSlice } = this.editData; const previousIndex = findHandlePolylineIndex( annotation, @@ -660,23 +660,19 @@ class LivewireContourTool extends ContourSegmentationBaseTool { // For this case, the next/previous indices are swapped, and the // path data gets inserted in between the newly generated data, so // handle this case specially - confirmedPathRight.addPoints( + confirmedPathNext.addPoints( polyline.slice(nextIndex + 1, previousIndex).map(worldToSlice) ); - } else if (nextIndex < previousIndex) { - throw new Error( - `Expected right index after left index, but were: ${previousIndex} ${nextIndex}` - ); } else { confirmedPath.addPoints( polyline.slice(0, previousIndex + 1).map(worldToSlice) ); - confirmedPathRight.addPoints( + confirmedPathNext.addPoints( polyline.slice(nextIndex, polyline.length).map(worldToSlice) ); } this.editData.confirmedPath = confirmedPath; - this.editData.confirmedPathRight = confirmedPathRight; + this.editData.confirmedPathNext = confirmedPathNext; } const { editData, scissors } = this; const { worldToSlice, sliceToWorld } = editData; @@ -702,7 +698,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { handlePoints[handleIndex] = sliceToWorld(slicePos); const pathPointsLeft = scissors.findPathToPoint(slicePos); - const pathPointsRight = this.scissorsRight.findPathToPoint(slicePos); + const pathPointsRight = this.scissorsNext.findPathToPoint(slicePos); const currentPath = new LivewirePath(); // Merge the "confirmed" path that goes from the first control point to the @@ -713,7 +709,7 @@ class LivewireContourTool extends ContourSegmentationBaseTool { currentPath.addPoints(pathPointsLeft); } currentPath.addPoints(pathPointsRight.reverse()); - currentPath.appendPath(editData.confirmedPathRight); + currentPath.appendPath(editData.confirmedPathNext); if (handleIndex === 0) { currentPath.addPoints(pathPointsLeft); } @@ -928,22 +924,29 @@ class LivewireContourTool extends ContourSegmentationBaseTool { return; } - const { annotation, sliceToWorld } = this.editData; + const { annotation, sliceToWorld, worldToSlice, closed, newAnnotation } = + this.editData; let { pointArray: imagePoints } = livewirePath; if (imagePoints.length > 1) { imagePoints = [...imagePoints, imagePoints[0]]; } + // Save the annotation in clockwise winding direction only after closing it + // because reversing the handle points may cause some weird issues + const targetWindingDirection = + newAnnotation && closed ? ContourWindingDirection.Clockwise : undefined; + this.updateContourPolyline( annotation, { points: imagePoints, - closed: annotation.data.contour.closed, - targetWindingDirection: ContourWindingDirection.Clockwise, + closed, + targetWindingDirection, }, { canvasToWorld: sliceToWorld, + worldToCanvas: worldToSlice, } ); } diff --git a/packages/tools/src/tools/base/ContourBaseTool.ts b/packages/tools/src/tools/base/ContourBaseTool.ts index d5b104c920..a7de8aac24 100644 --- a/packages/tools/src/tools/base/ContourBaseTool.ts +++ b/packages/tools/src/tools/base/ContourBaseTool.ts @@ -219,6 +219,7 @@ abstract class ContourBaseTool extends AnnotationTool { }, transforms: { canvasToWorld: (point: Types.Point2) => Types.Point3; + worldToCanvas: (point: Types.Point3) => Types.Point2; } ) { const decimateConfig = this.configuration?.decimate || {}; diff --git a/packages/tools/src/utilities/contours/updateContourPolyline.ts b/packages/tools/src/utilities/contours/updateContourPolyline.ts index b42d9943db..ddb6d1a148 100644 --- a/packages/tools/src/utilities/contours/updateContourPolyline.ts +++ b/packages/tools/src/utilities/contours/updateContourPolyline.ts @@ -30,6 +30,7 @@ export default function updateContourPolyline( }, transforms: { canvasToWorld: (point: Types.Point2) => Types.Point3; + worldToCanvas: (point: Types.Point3) => Types.Point2; }, options?: { decimate?: { @@ -38,7 +39,7 @@ export default function updateContourPolyline( }; } ) { - const { canvasToWorld } = transforms; + const { canvasToWorld, worldToCanvas } = transforms; const { data } = annotation; const { targetWindingDirection } = polylineData; let { points: polyline } = polylineData; @@ -54,7 +55,8 @@ export default function updateContourPolyline( let { closed } = polylineData; const numPoints = polyline.length; const polylineWorldPoints = new Array(numPoints); - const currentWindingDirection = math.polyline.getWindingDirection(polyline); + const currentPolylineWindingDirection = + math.polyline.getWindingDirection(polyline); const parentAnnotation = getParentAnnotation(annotation) as ContourAnnotation; if (closed === undefined) { @@ -79,11 +81,24 @@ export default function updateContourPolyline( : targetWindingDirection; if (windingDirection === undefined) { - windingDirection = currentWindingDirection; - } else if (windingDirection !== currentWindingDirection) { + windingDirection = currentPolylineWindingDirection; + } + + if (windingDirection !== currentPolylineWindingDirection) { polyline.reverse(); } + const handlePoints = data.handles.points.map((p) => worldToCanvas(p)); + + if (handlePoints.length > 2) { + const currentHandlesWindingDirection = + math.polyline.getWindingDirection(handlePoints); + + if (currentHandlesWindingDirection !== windingDirection) { + data.handles.points.reverse(); + } + } + for (let i = 0; i < numPoints; i++) { polylineWorldPoints[i] = canvasToWorld(polyline[i]); } diff --git a/packages/tools/src/utilities/math/polyline/getSignedArea.ts b/packages/tools/src/utilities/math/polyline/getSignedArea.ts index f7e4eb3fce..f24f1f79a8 100644 --- a/packages/tools/src/utilities/math/polyline/getSignedArea.ts +++ b/packages/tools/src/utilities/math/polyline/getSignedArea.ts @@ -5,9 +5,9 @@ import type { Types } from '@cornerstonejs/core'; * https://www.youtube.com/watch?v=GpsKrAipXm8&t=1900s * * This functions has a runtime very close to `getArea` and it is recommended to - * be called only if you need the area signal (eg: calculate polygon normal). If - * you do not need the area signal you should always call `getArea`. - * + * be called only if you need the area signal (eg: calculate polygon normal or + * winding direction). If you do not need the area signal you should always call + * `getArea`. * * @param polyline - Polyline points (2D) * @returns Area of the polyline (with signal)