From 27bfacc2ee65612edd1a511cc01ebe7f64528e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Alves?= Date: Fri, 21 Jul 2023 08:04:48 -0300 Subject: [PATCH 1/2] fix(SegRendering): preserve canvas transform matrix after seg render Previously after rendering the segmentation cornerstone was always transforming the canvas to follow the identity matrix, but in cases where we had an overlay using a different transformation matrix, it old undesirably affect the original overlay. --- .../internals/renderSegmentationFill.js | 33 ++++------ .../internals/renderSegmentationOutline.js | 2 + .../renderSegmentationOutline.test.js | 64 ++++++++++++++++--- 3 files changed, 71 insertions(+), 28 deletions(-) diff --git a/src/eventListeners/internals/renderSegmentationFill.js b/src/eventListeners/internals/renderSegmentationFill.js index f4f1f69cf..92d0a6365 100644 --- a/src/eventListeners/internals/renderSegmentationFill.js +++ b/src/eventListeners/internals/renderSegmentationFill.js @@ -1,9 +1,5 @@ import { getModule } from '../../store/index.js'; -import { - getNewContext, - resetCanvasContextTransform, - transformCanvasContext, -} from '../../drawing/index.js'; +import { getNewContext, transformCanvasContext } from '../../drawing/index.js'; import external from '../../externalModules'; const segmentationModule = getModule('segmentation'); @@ -80,25 +76,25 @@ export function getLabelmapCanvas(evt, labelmap3D, labelmap2D) { export function renderFill(evt, labelmapCanvas, isActiveLabelMap) { const { configuration } = segmentationModule; const eventData = evt.detail; - const context = getNewContext(eventData.canvasContext.canvas); + const { canvasContext, element, image, viewport } = eventData; - const canvasTopLeft = external.cornerstone.pixelToCanvas(eventData.element, { + const previousTransform = canvasContext.getTransform(); + const context = getNewContext(canvasContext.canvas); + + const canvasTopLeft = external.cornerstone.pixelToCanvas(element, { x: 0, y: 0, }); - const canvasTopRight = external.cornerstone.pixelToCanvas(eventData.element, { - x: eventData.image.width, + const canvasTopRight = external.cornerstone.pixelToCanvas(element, { + x: image.width, y: 0, }); - const canvasBottomRight = external.cornerstone.pixelToCanvas( - eventData.element, - { - x: eventData.image.width, - y: eventData.image.height, - } - ); + const canvasBottomRight = external.cornerstone.pixelToCanvas(element, { + x: image.width, + y: image.height, + }); const cornerstoneCanvasWidth = external.cornerstoneMath.point.distance( canvasTopLeft, @@ -109,8 +105,7 @@ export function renderFill(evt, labelmapCanvas, isActiveLabelMap) { canvasBottomRight ); - const canvas = eventData.canvasContext.canvas; - const viewport = eventData.viewport; + const canvas = canvasContext.canvas; const previousImageSmoothingEnabled = context.imageSmoothingEnabled; const previousGlobalAlpha = context.globalAlpha; @@ -137,7 +132,7 @@ export function renderFill(evt, labelmapCanvas, isActiveLabelMap) { context.globalAlpha = previousGlobalAlpha; context.imageSmoothingEnabled = previousImageSmoothingEnabled; - resetCanvasContextTransform(context); + context.setTransform(previousTransform); } /** diff --git a/src/eventListeners/internals/renderSegmentationOutline.js b/src/eventListeners/internals/renderSegmentationOutline.js index 379d6e050..fd096bb32 100644 --- a/src/eventListeners/internals/renderSegmentationOutline.js +++ b/src/eventListeners/internals/renderSegmentationOutline.js @@ -44,6 +44,7 @@ export function renderOutline( const lineWidth = configuration.outlineWidth || 1; + const previousTransform = canvasContext.getTransform(); const context = getNewContext(canvasContext.canvas); const colorLutTable = state.colorLutTables[colorLUTIndex]; @@ -74,6 +75,7 @@ export function renderOutline( }); context.globalAlpha = previousAlpha; + context.setTransform(previousTransform); } /** diff --git a/src/eventListeners/internals/renderSegmentationOutline.test.js b/src/eventListeners/internals/renderSegmentationOutline.test.js index 2cd048dfc..a98d5c28b 100644 --- a/src/eventListeners/internals/renderSegmentationOutline.test.js +++ b/src/eventListeners/internals/renderSegmentationOutline.test.js @@ -5,16 +5,21 @@ import * as drawing from '../../drawing/index.js'; const { state } = getModule('segmentation'); -jest.mock('../../drawing/index.js', () => ({ - getNewContext: () => ({ +jest.mock('../../drawing/index.js', () => { + const getNewContextMock = { globalAlpha: 1.0, - }), - draw: (context, callback) => { - callback(context); - }, - drawLines: jest.fn(), - drawJoinedLines: jest.fn(), -})); + setTransform: jest.fn(), + }; + + return { + getNewContext: jest.fn(() => getNewContextMock), + draw: (context, callback) => { + callback(context); + }, + drawLines: jest.fn(), + drawJoinedLines: jest.fn(), + }; +}); jest.mock('../../externalModules', () => ({ cornerstone: { @@ -132,6 +137,15 @@ function resetEvents() { lineWidth = 1; + const canvasTransformState = { + a: 0.078, + b: 0, + c: 0, + d: 0.078, + e: 52.9, + f: 0, + }; + eventData = { element: null, image: { @@ -154,6 +168,7 @@ function resetEvents() { width: canvasScale * width, height: canvasScale * width, }, + getTransform: jest.fn(() => canvasTransformState), }, }; @@ -224,8 +239,14 @@ function setCanvasTransform(options = {}) { } describe('renderSegmentationOutline.js', () => { + let getNewContext; + beforeEach(() => { resetEvents(); + + getNewContext = drawing.getNewContext; + + jest.clearAllMocks(); }); describe('Initialization', () => { @@ -365,6 +386,31 @@ describe('renderSegmentationOutline.js', () => { }); describe('renderOutline', () => { + it('should preserve the canvas transform matrix', () => { + const outline = getOutline(evt, labelmap3D, labelmap2D, lineWidth); + + // Fake colormap to stop renderOutline breaking. + state.colorLutTables[0] = [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]; + + const beforeCallTransformMatrix = evt.detail.canvasContext.getTransform(); + const newContextSetTransform = getNewContext().setTransform; + + evt.detail.canvasContext.getTransform.mockClear(); + getNewContext.mockClear(); + + renderOutline(evt, outline, 0, true); + + expect(evt.detail.canvasContext.getTransform).toHaveBeenCalledTimes(1); + expect(getNewContext).toHaveBeenCalledTimes(1); + expect(getNewContext).toHaveBeenCalledWith( + evt.detail.canvasContext.canvas + ); + expect(newContextSetTransform).toHaveBeenCalledTimes(1); + expect(newContextSetTransform).toHaveBeenCalledWith( + beforeCallTransformMatrix + ); + }); + it('Should call drawLines twice', () => { const outline = getOutline(evt, labelmap3D, labelmap2D, lineWidth); From 4363285db54820972adf143faf54b138c53e0677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Alves?= Date: Fri, 21 Jul 2023 08:06:20 -0300 Subject: [PATCH 2/2] improvement(SegRendering): added unit test file to test renderFill function --- .../internals/renderSegmentationFill.test.js | 326 ++++++++++++++++++ 1 file changed, 326 insertions(+) create mode 100644 src/eventListeners/internals/renderSegmentationFill.test.js diff --git a/src/eventListeners/internals/renderSegmentationFill.test.js b/src/eventListeners/internals/renderSegmentationFill.test.js new file mode 100644 index 000000000..d62842bd8 --- /dev/null +++ b/src/eventListeners/internals/renderSegmentationFill.test.js @@ -0,0 +1,326 @@ +import { getModule } from '../../store/index.js'; +import { getLabelmapCanvas, renderFill } from './renderSegmentationFill.js'; +import external from '../../externalModules.js'; +import * as drawing from '../../drawing/index.js'; + +const { state } = getModule('segmentation'); + +jest.mock('../../drawing/index.js', () => { + const getNewContextMock = { + drawImage: jest.fn(), + globalAlpha: 1.0, + setTransform: jest.fn(), + putImageData: jest.fn(), + }; + + return { + getNewContext: jest.fn(() => getNewContextMock), + transformCanvasContext: jest.fn(), + }; +}); + +jest.mock('../../externalModules', () => ({ + cornerstone: { + pixelToCanvas: (element, imageCoord) => { + // Mock some transformation. + const { viewport, canvas } = mockGetEnabledElement(element); + + const m = [1, 0, 0, 1, 0, 0]; + + function translate(x, y) { + m[4] += m[0] * x + m[2] * y; + m[5] += m[1] * x + m[3] * y; + } + + function rotate(rad) { + const c = Math.cos(rad); + const s = Math.sin(rad); + const m11 = m[0] * c + m[2] * s; + const m12 = m[1] * c + m[3] * s; + const m21 = m[0] * -s + m[2] * c; + const m22 = m[1] * -s + m[3] * c; + + m[0] = m11; + m[1] = m12; + m[2] = m21; + m[3] = m22; + } + + function scale(sx, sy) { + m[0] *= sx; + m[1] *= sx; + m[2] *= sy; + m[3] *= sy; + } + + // Move to center of canvas + translate(canvas.width / 2, canvas.height / 2); + + // Apply the rotation before scaling + const angle = viewport.rotation; + + if (angle !== 0) { + rotate((angle * Math.PI) / 180); + } + + // Apply the scale + const widthScale = viewport.scale; + const heightScale = viewport.scale; + + const width = + viewport.displayedArea.brhc.x - (viewport.displayedArea.tlhc.x - 1); + const height = + viewport.displayedArea.brhc.y - (viewport.displayedArea.tlhc.y - 1); + + scale(widthScale, heightScale); + + // Unrotate to so we can translate unrotated + if (angle !== 0) { + rotate((-angle * Math.PI) / 180); + } + + // Apply the pan offset + translate(viewport.translation.x, viewport.translation.y); + + // Rotate again + if (angle !== 0) { + rotate((angle * Math.PI) / 180); + } + + // Apply Flip if required + if (viewport.hflip) { + scale(-1, 1); + } + + if (viewport.vflip) { + scale(1, -1); + } + + // Move back from center of image + translate(-width / 2, -height / 2); + + const x = imageCoord.x; + const y = imageCoord.y; + + const px = x * m[0] + y * m[2] + m[4]; + const py = x * m[1] + y * m[3] + m[5]; + + return { + x: px, + y: py, + }; + }, + }, + cornerstoneMath: { + point: { + distance: ({ x: x1, y: y1 }, { x: x2, y: y2 }) => + Math.sqrt(Math.pow(x2 - x1, 2) + Math.pow(y2 - y1, 2)), + }, + }, +})); + +// Mock implementation for ImageData +class MockImageData { + constructor(width, height) { + this.width = width; + this.height = height; + this.data = new Uint8ClampedArray(width * height * 4); + } +} +Object.defineProperty(window, 'ImageData', { value: MockImageData }); + +let eventData; +let evt; +let labelmap3D; +let labelmap2D; + +const mockGetEnabledElement = element => ({ + element, + viewport: eventData.viewport, + image: eventData.image, + canvas: eventData.canvasContext.canvas, +}); + +function resetEvents() { + const width = 256; + const length = width * width; + const currentImageIdIndex = 0; + + const canvasScale = 1.0; + + const canvasTransformState = { + a: 0.078, + b: 0, + c: 0, + d: 0.078, + e: 52.9, + f: 0, + }; + + eventData = { + element: null, + image: { + width: 256, + height: 256, + }, + viewport: { + rotation: 0, + scale: canvasScale, + translation: { x: 0, y: 0 }, + hflip: false, + vflip: false, + displayedArea: { + brhc: { x: 256, y: 256 }, + tlhc: { x: 1, y: 1 }, + }, + }, + canvasContext: { + canvas: { + width: canvasScale * width, + height: canvasScale * width, + }, + getTransform: jest.fn(() => canvasTransformState), + }, + }; + + evt = { + detail: eventData, + }; + + labelmap3D = { + buffer: new ArrayBuffer(length * 2), + colorLUTIndex: 0, + labelmaps2D: [], + metadata: [], + activeSegmentIndex: 0, + segmentsHidden: [], + }; + + labelmap2D = { + pixelData: new Uint16Array(labelmap3D.buffer, 0, length), + segmentsOnLabelmap: [0, 1, 2], + }; + + const pixelData = labelmap2D.pixelData; + const cols = eventData.image.width; + + // Add segment 1 as an L shape, so should have 1 interior corner. + pixelData[64 * cols + 64] = 1; + pixelData[65 * cols + 64] = 1; + pixelData[65 * cols + 65] = 1; + + // Add segment 2 as a rectangle. + for (let x = 201; x <= 210; x++) { + pixelData[200 * cols + x] = 2; + pixelData[201 * cols + x] = 2; + } + + labelmap3D.labelmaps2D[currentImageIdIndex] = labelmap2D; +} + +describe('renderSegmentationFill.js', () => { + let getNewContext; + + beforeEach(() => { + resetEvents(); + + getNewContext = drawing.getNewContext; + + jest.clearAllMocks(); + }); + + describe('renderFill', () => { + it('should preserve the canvas transform matrix', () => { + // Fake colormap to stop renderFill breaking. + state.colorLutTables[0] = [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]; + + const labelMapCanvas = getLabelmapCanvas(evt, labelmap3D, labelmap2D); + + const beforeCallTransformMatrix = evt.detail.canvasContext.getTransform(); + const newContextSetTransform = getNewContext().setTransform; + + evt.detail.canvasContext.getTransform.mockClear(); + getNewContext.mockClear(); + + renderFill(evt, labelMapCanvas, true); + + expect(evt.detail.canvasContext.getTransform).toHaveBeenCalledTimes(1); + expect(getNewContext).toHaveBeenCalledTimes(1); + expect(getNewContext).toHaveBeenCalledWith( + evt.detail.canvasContext.canvas + ); + expect(newContextSetTransform).toHaveBeenCalledTimes(1); + expect(newContextSetTransform).toHaveBeenCalledWith( + beforeCallTransformMatrix + ); + }); + + it('should draw segmentation fill', () => { + // Fake colormap to stop renderFill breaking. + state.colorLutTables[0] = [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]; + + const labelMapCanvas = getLabelmapCanvas(evt, labelmap3D, labelmap2D); + + const { canvasContext, element, image, viewport } = evt.detail; + const mockContext = getNewContext(canvasContext.canvas); + + jest.spyOn(external.cornerstone, 'pixelToCanvas'); + jest.spyOn(external.cornerstoneMath.point, 'distance'); + + renderFill(evt, labelMapCanvas, true); + + const topLeft = { + x: 0, + y: 0, + }; + const topRight = { + x: image.width, + y: 0, + }; + const bottomRight = { + x: image.width, + y: image.height, + }; + + expect(external.cornerstone.pixelToCanvas).toHaveBeenCalledTimes(3); + expect(external.cornerstone.pixelToCanvas).toHaveBeenCalledWith( + element, + topLeft + ); + expect(external.cornerstone.pixelToCanvas).toHaveBeenCalledWith( + element, + topRight + ); + expect(external.cornerstone.pixelToCanvas).toHaveBeenCalledWith( + element, + bottomRight + ); + + expect(external.cornerstoneMath.point.distance).toHaveBeenCalledTimes(2); + expect(external.cornerstoneMath.point.distance).toHaveBeenCalledWith( + topLeft, + topRight + ); + expect(external.cornerstoneMath.point.distance).toHaveBeenCalledWith( + topRight, + bottomRight + ); + + expect(drawing.transformCanvasContext).toHaveBeenCalledTimes(1); + expect(drawing.transformCanvasContext).toHaveBeenCalledWith( + mockContext, + canvasContext.canvas, + viewport + ); + + expect(mockContext.drawImage).toHaveBeenCalledTimes(1); + expect(mockContext.drawImage).toHaveBeenCalledWith( + labelMapCanvas, + 0, + 0, + image.width, + image.height + ); + }); + }); +});