-
Notifications
You must be signed in to change notification settings - Fork 455
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: 1054 perpendicular bidirectional tool on non-square pixel images (…
…#1063) * fix: 1054 perpendicular bidirectional tool on non-square pixel images * Reducing code complexity from bidirectional tool * Reducing code complexity from bidirectional tool * Reducing code complexity from moveLongLine method * Removing amount of arguments from updateLine function * Reducing code complexity from movePerpendicularLine method * Improving code based on PR suggestions * Reducing complexity of getBaseData method * Reducing complexity of updateLine function from moveLongLine method * Reducing complexity of movePerpendicularLine method * Adding jsDoc and some unit tests for movePerpendicularLine method * Adding JSDoc and unit tests for moveLongLine method * Adding JSDoc and unit tests for movePerpendicularLine method * Adding unit tests for updatePerpendicularLineHandles method * Adding comments to the returned attributes from getBaseData * Reverting imageLoader changes to skew the image * Removing test script added for bidirectional tool * Improving tests based on PR suggestions * Removing mocked function and mocking values instead * Removing unneeded proxy function * Changing unit test description * Using toBeCloseTo assertion to prevent precision issues * Using toMatchObject assertion when comparing object values * Started updating the handles from measurementData * Renaming k variable to distanceRatio to make it more meaninful
- Loading branch information
1 parent
3f5e46c
commit e7eaa37
Showing
31 changed files
with
1,544 additions
and
345 deletions.
There are no files selected for viewing
54 changes: 54 additions & 0 deletions
54
src/tools/annotation/bidirectionalTool/moveHandle/getBaseData.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import external from '../../../../externalModules.js'; | ||
import getDistanceWithPixelSpacing from '../utils/getDistanceWithPixelSpacing.js'; | ||
|
||
function createLine(startPoint, endPoint) { | ||
return { | ||
start: startPoint, | ||
end: endPoint, | ||
}; | ||
} | ||
|
||
/** | ||
* Extract and group the base data to be used on bidirectional tool lines | ||
* moving. | ||
* | ||
* @param {*} measurementData Data from current bidirectional tool measurement | ||
* @param {*} eventData Data object associated with the event | ||
* @param {*} fixedPoint Point that is not being moved in line | ||
* | ||
* @returns {*} Grouped that needed for lines moving | ||
*/ | ||
export default function getBaseData(measurementData, eventData, fixedPoint) { | ||
const { lineSegment } = external.cornerstoneMath; | ||
const { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
} = measurementData.handles; | ||
const { columnPixelSpacing = 1, rowPixelSpacing = 1 } = eventData.image; | ||
|
||
const longLine = createLine(start, end); | ||
const perpendicularLine = createLine(perpendicularStart, perpendicularEnd); | ||
const intersection = lineSegment.intersectLine(longLine, perpendicularLine); | ||
|
||
const distanceToFixed = getDistanceWithPixelSpacing( | ||
columnPixelSpacing, | ||
rowPixelSpacing, | ||
fixedPoint, | ||
intersection | ||
); | ||
|
||
return { | ||
columnPixelSpacing, // Width that a pixel represents in mm | ||
rowPixelSpacing, // Height that a pixel represents in mm | ||
start, // Start point of the long line | ||
end, // End point of the long line | ||
perpendicularStart, // Start point of the perpendicular line | ||
perpendicularEnd, // End point of the perpendicular line | ||
longLine, // Long line object containing the start and end points | ||
intersection, // Intersection point between long and perpendicular lines | ||
distanceToFixed, // Distance from intersection to the fixed point | ||
fixedPoint, // Opposite point from the handle that is being moved | ||
}; | ||
} |
102 changes: 102 additions & 0 deletions
102
src/tools/annotation/bidirectionalTool/moveHandle/getBaseData.test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
import getBaseData from './getBaseData'; | ||
|
||
jest.mock('./../../../../externalModules.js', () => { | ||
const intersectLine = () => ({ | ||
x: 4, | ||
y: 4, | ||
}); | ||
|
||
return { | ||
cornerstoneMath: { | ||
lineSegment: { | ||
intersectLine: jest.fn(intersectLine), | ||
}, | ||
}, | ||
}; | ||
}); | ||
|
||
function createPoint(x, y) { | ||
return { | ||
x, | ||
y, | ||
}; | ||
} | ||
|
||
describe('getBaseData.js', () => { | ||
it('returns a baseData object with the correct values', () => { | ||
const start = createPoint(0, 4); | ||
const end = createPoint(8, 4); | ||
const perpendicularStart = createPoint(4, 6); | ||
const perpendicularEnd = createPoint(4, 2); | ||
|
||
const measurementData = { | ||
handles: { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
}, | ||
}; | ||
|
||
const eventData = { | ||
image: { | ||
columnPixelSpacing: 3, | ||
rowPixelSpacing: 2, | ||
}, | ||
}; | ||
|
||
const fixedPoint = measurementData.handles.end; | ||
|
||
const result = getBaseData(measurementData, eventData, fixedPoint); | ||
|
||
expect(result.columnPixelSpacing).toEqual(3); | ||
expect(result.rowPixelSpacing).toEqual(2); | ||
expect(result.start).toMatchObject(start); | ||
expect(result.end).toMatchObject(end); | ||
expect(result.perpendicularStart).toMatchObject(perpendicularStart); | ||
expect(result.perpendicularEnd).toMatchObject(perpendicularEnd); | ||
expect(result.longLine.start).toMatchObject(start); | ||
expect(result.longLine.end).toMatchObject(end); | ||
expect(result.intersection.x).toEqual(4); | ||
expect(result.intersection.y).toEqual(4); | ||
expect(result.distanceToFixed).toEqual(2); | ||
expect(result.fixedPoint).toMatchObject(fixedPoint); | ||
}); | ||
|
||
it('ensure that baseData object is returned with default row and column pixel spacing', () => { | ||
const start = createPoint(0, 4); | ||
const end = createPoint(8, 4); | ||
const perpendicularStart = createPoint(4, 6); | ||
const perpendicularEnd = createPoint(4, 2); | ||
|
||
const measurementData = { | ||
handles: { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
}, | ||
}; | ||
|
||
const eventData = { | ||
image: {}, | ||
}; | ||
|
||
const fixedPoint = measurementData.handles.end; | ||
|
||
const result = getBaseData(measurementData, eventData, fixedPoint); | ||
|
||
expect(result.columnPixelSpacing).toEqual(1); | ||
expect(result.rowPixelSpacing).toEqual(1); | ||
expect(result.start).toMatchObject(start); | ||
expect(result.end).toMatchObject(end); | ||
expect(result.perpendicularStart).toMatchObject(perpendicularStart); | ||
expect(result.perpendicularEnd).toMatchObject(perpendicularEnd); | ||
expect(result.longLine.start).toMatchObject(start); | ||
expect(result.longLine.end).toMatchObject(end); | ||
expect(result.intersection.x).toEqual(4); | ||
expect(result.intersection.y).toEqual(4); | ||
expect(result.distanceToFixed).toEqual(4); | ||
expect(result.fixedPoint).toMatchObject(fixedPoint); | ||
}); | ||
}); |
54 changes: 54 additions & 0 deletions
54
src/tools/annotation/bidirectionalTool/moveHandle/moveLongLine/moveLongLine.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import getDistanceWithPixelSpacing from '../../utils/getDistanceWithPixelSpacing.js'; | ||
import getBaseData from '../getBaseData.js'; | ||
import updatePerpendicularLine from './updatePerpendicularLine.js'; | ||
|
||
/** | ||
* Move the long line updating the perpendicular line handles position. | ||
* | ||
* @param {*} proposedPoint Point that was moved in bidirectional tool | ||
* @param {*} measurementData Data from current bidirectional tool measurement | ||
* @param {*} eventData Data object associated with the event | ||
* @param {*} fixedPoint Point that is not being moved in long line | ||
* | ||
* @returns {boolean} True if perpendicular handles were updated, false if not | ||
*/ | ||
export default function moveLongLine( | ||
proposedPoint, | ||
measurementData, | ||
eventData, | ||
fixedPoint | ||
) { | ||
const baseData = getBaseData(measurementData, eventData, fixedPoint); | ||
const { columnPixelSpacing, rowPixelSpacing, distanceToFixed } = baseData; | ||
|
||
// Calculate the length of the new line, considering the proposed point | ||
const newLineLength = getDistanceWithPixelSpacing( | ||
columnPixelSpacing, | ||
rowPixelSpacing, | ||
fixedPoint, | ||
proposedPoint | ||
); | ||
|
||
// Stop here if the handle tries to move before the intersection point | ||
if (newLineLength <= distanceToFixed) { | ||
return false; | ||
} | ||
|
||
// Calculate the new intersection point | ||
const distanceRatio = distanceToFixed / newLineLength; | ||
const newIntersection = { | ||
x: fixedPoint.x + (proposedPoint.x - fixedPoint.x) * distanceRatio, | ||
y: fixedPoint.y + (proposedPoint.y - fixedPoint.y) * distanceRatio, | ||
}; | ||
|
||
// Calculate and the new position of the perpendicular handles | ||
const newLine = updatePerpendicularLine(baseData, newIntersection); | ||
|
||
// Update the perpendicular line handles | ||
measurementData.handles.perpendicularStart.x = newLine.start.x; | ||
measurementData.handles.perpendicularStart.y = newLine.start.y; | ||
measurementData.handles.perpendicularEnd.x = newLine.end.x; | ||
measurementData.handles.perpendicularEnd.y = newLine.end.y; | ||
|
||
return true; | ||
} |
148 changes: 148 additions & 0 deletions
148
src/tools/annotation/bidirectionalTool/moveHandle/moveLongLine/moveLongLine.test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
import moveLongLine from './moveLongLine'; | ||
|
||
jest.mock('./../../../../../externalModules.js', () => { | ||
const intersectLine = () => ({ | ||
x: 4, | ||
y: 4, | ||
}); | ||
|
||
return { | ||
cornerstoneMath: { | ||
lineSegment: { | ||
intersectLine: jest.fn(intersectLine), | ||
}, | ||
}, | ||
}; | ||
}); | ||
|
||
function createPoint(x, y) { | ||
return { | ||
x, | ||
y, | ||
}; | ||
} | ||
|
||
describe('moveLongLine.js', () => { | ||
it('long line is moved and perpendicular line position is updated', () => { | ||
const proposedPoint = createPoint(12, 6); | ||
|
||
const start = createPoint(0, 4); | ||
const end = createPoint(8, 4); | ||
const perpendicularStart = createPoint(4, 6); | ||
const perpendicularEnd = createPoint(4, 2); | ||
|
||
const measurementData = { | ||
handles: { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
}, | ||
}; | ||
|
||
const eventData = { | ||
image: { | ||
columnPixelSpacing: 1, | ||
rowPixelSpacing: 0.5, | ||
}, | ||
}; | ||
|
||
const fixedPoint = measurementData.handles.start; | ||
|
||
const result = moveLongLine( | ||
proposedPoint, | ||
measurementData, | ||
eventData, | ||
fixedPoint | ||
); | ||
|
||
// Expect line to be moved successfully | ||
expect(result).toEqual(true); | ||
|
||
// Expect perpendicular lines position to be updated | ||
expect(perpendicularStart.x).toBeCloseTo(3.9031375531257786); | ||
expect(perpendicularStart.y).toBeCloseTo(6.657455355319679); | ||
expect(perpendicularEnd.x).toBeCloseTo(4.069228512833258); | ||
expect(perpendicularEnd.y).toBeCloseTo(2.671272322340161); | ||
}); | ||
|
||
it('long line is moved and perpendicular line position stays the same', () => { | ||
const proposedPoint = createPoint(-4, 4); | ||
|
||
const start = createPoint(0, 4); | ||
const end = createPoint(8, 4); | ||
const perpendicularStart = createPoint(4, 6); | ||
const perpendicularEnd = createPoint(4, 2); | ||
|
||
const measurementData = { | ||
handles: { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
}, | ||
}; | ||
|
||
const eventData = { | ||
image: { | ||
columnPixelSpacing: 1, | ||
rowPixelSpacing: 2, | ||
}, | ||
}; | ||
|
||
const fixedPoint = measurementData.handles.end; | ||
|
||
const result = moveLongLine( | ||
proposedPoint, | ||
measurementData, | ||
eventData, | ||
fixedPoint | ||
); | ||
|
||
// Expect line to be moved successfully | ||
expect(result).toEqual(true); | ||
|
||
// Expect perpendicular lines position to be updated | ||
expect(perpendicularStart.x).toEqual(4); | ||
expect(perpendicularStart.y).toEqual(6); | ||
expect(perpendicularEnd.x).toEqual(4); | ||
expect(perpendicularEnd.y).toEqual(2); | ||
}); | ||
|
||
it('long line is not moved (proposed point is before intersection point)', () => { | ||
const proposedPoint = createPoint(3, 6); | ||
|
||
const start = createPoint(0, 4); | ||
const end = createPoint(8, 4); | ||
const perpendicularStart = createPoint(4, 6); | ||
const perpendicularEnd = createPoint(4, 2); | ||
|
||
const measurementData = { | ||
handles: { | ||
start, | ||
end, | ||
perpendicularStart, | ||
perpendicularEnd, | ||
}, | ||
}; | ||
|
||
const eventData = { | ||
image: { | ||
columnPixelSpacing: 1, | ||
rowPixelSpacing: 0.5, | ||
}, | ||
}; | ||
|
||
const fixedPoint = measurementData.handles.start; | ||
|
||
const result = moveLongLine( | ||
proposedPoint, | ||
measurementData, | ||
eventData, | ||
fixedPoint | ||
); | ||
|
||
// Expect line to not be moved | ||
expect(result).toEqual(false); | ||
}); | ||
}); |
Oops, something went wrong.