From 8dcce3ced4f4bb34bab4b45729484a313bd7abff Mon Sep 17 00:00:00 2001 From: Zaid Safadi Date: Mon, 23 Apr 2018 21:30:25 -0400 Subject: [PATCH] Fixe viewport scale factor is NaN when row/col pixel spacing is note set. (#264) * Fixes issue #262, scale factor is NaN causing cornerstone-tools to fail with Math. Also fixes in cornerstone tools - createImage columnPixelSpacing (and rowPixelSpacing) is undefined #190 * Refactor, reduce complexity and adding a "getImageSize_test" unit test --- src/fitToWindow.js | 19 +--- src/internal/getDefaultViewport.js | 121 ++++++++++++----------- src/internal/getImageFitScale.js | 40 ++++++++ src/internal/getImageSize.js | 36 +++++-- src/internal/validator.js | 27 +++++ src/resize.js | 10 +- src/version.js | 2 +- test/internal/getDefaultViewport_test.js | 62 +++++++++++- test/internal/getImageFitScale_test.js | 35 +++++++ test/internal/getImageSize_test.js | 59 +++++++++++ 10 files changed, 316 insertions(+), 95 deletions(-) create mode 100644 src/internal/getImageFitScale.js create mode 100644 src/internal/validator.js create mode 100644 test/internal/getImageFitScale_test.js create mode 100644 test/internal/getImageSize_test.js diff --git a/src/fitToWindow.js b/src/fitToWindow.js index 04bbb9be..ec572dd4 100644 --- a/src/fitToWindow.js +++ b/src/fitToWindow.js @@ -1,6 +1,6 @@ import { getEnabledElement } from './enabledElements.js'; import updateImage from './updateImage.js'; -import getImageSize from './internal/getImageSize.js'; +import getImageFitScale from './internal/getImageFitScale.js'; /** * Adjusts an image's scale and translation so the image is centered and all pixels @@ -11,25 +11,10 @@ import getImageSize from './internal/getImageSize.js'; */ export default function (element) { const enabledElement = getEnabledElement(element); - const imageSize = getImageSize(enabledElement); - const { image } = enabledElement; - let verticalRatio = 1; - let horizontalRatio = 1; - - if (image.rowPixelSpacing < image.columnPixelSpacing) { - // we believe that the row pixel is the same as css pixel - horizontalRatio = image.columnPixelSpacing / image.rowPixelSpacing; - } else { - // we believe that the column pixel is the same as css pixel - verticalRatio = image.rowPixelSpacing / image.columnPixelSpacing; - } - - const verticalScale = enabledElement.canvas.height / imageSize.height / verticalRatio; - const horizontalScale = enabledElement.canvas.width / imageSize.width / horizontalRatio; // The new scale is the minimum of the horizontal and vertical scale values - enabledElement.viewport.scale = Math.min(horizontalScale, verticalScale); + enabledElement.viewport.scale = getImageFitScale(enabledElement.canvas, image, enabledElement.viewport.rotation).scaleFactor; enabledElement.viewport.translation.x = 0; enabledElement.viewport.translation.y = 0; diff --git a/src/internal/getDefaultViewport.js b/src/internal/getDefaultViewport.js index 0ef2d070..dc266107 100644 --- a/src/internal/getDefaultViewport.js +++ b/src/internal/getDefaultViewport.js @@ -1,12 +1,64 @@ +import getImageFitScale from './getImageFitScale.js'; + + /** - * Enumeration that describes the displayedArea presentation size mode. + * Creates a new viewport object containing default values + * + * @returns {Viewport} viewport object + * @memberof Internal */ -// const DisplayedAreaSizeMode = Object.freeze({ -// NONE: Symbol('NONE'), -// SCALE_TO_FIT: Symbol('SCALE TO FIT'), -// TRUE_SIZE: Symbol('TRUE SIZE'), -// MAGNIFY: Symbol('MAGNIFY') -// }); +function createViewport () { + const displayedArea = createDefaultDisplayedArea(); + + return { + scale: 1, + translation: { + x: 0, + y: 0 + }, + voi: { + windowWidth: undefined, + windowCenter: undefined + }, + invert: false, + pixelReplication: false, + rotation: 0, + hflip: false, + vflip: false, + modalityLUT: undefined, + voiLUT: undefined, + colormap: undefined, + labelmap: false, + displayedArea + }; +} + + +/** + * Creates the default displayed area. + * C.10.4 Displayed Area Module: This Module describes Attributes required to define a Specified Displayed Area space. + * + * @returns {tlhc: {x,y}, brhc: {x, y},rowPixelSpacing: Number, columnPixelSpacing: Number, presentationSizeMode: Number} displayedArea object + * @memberof Internal + */ + +function createDefaultDisplayedArea () { + return { + // Top Left Hand Corner + tlhc: { + x: 1, + y: 1 + }, + // Bottom Right Hand Corner + brhc: { + x: 1, + y: 1 + }, + rowPixelSpacing: 1, + columnPixelSpacing: 1, + presentationSizeMode: 'NONE' + }; +} /** * Creates a new viewport object containing default values for the image and canvas @@ -22,62 +74,11 @@ export default function (canvas, image) { } if (image === undefined) { - return { - scale: 1, - translation: { - x: 0, - y: 0 - }, - voi: { - windowWidth: undefined, - windowCenter: undefined - }, - invert: false, - pixelReplication: false, - rotation: 0, - hflip: false, - vflip: false, - modalityLUT: undefined, - voiLUT: undefined, - colormap: undefined, - labelmap: false, - - /** - * C.10.4 Displayed Area Module: This Module describes Attributes required to define a Specified Displayed Area space. - */ - displayedArea: { - // Top Left Hand Corner - tlhc: { - x: 1, - y: 1 - }, - // Bottom Right Hand Corner - brhc: { - x: 1, - y: 1 - }, - rowPixelSpacing: 1, - columnPixelSpacing: 1, - presentationSizeMode: 'NONE' - } - }; - } - - let verticalRatio = 1; - let horizontalRatio = 1; - - if (image.rowPixelSpacing < image.columnPixelSpacing) { - // we believe that the row pixel is the same as css pixel - horizontalRatio = image.columnPixelSpacing / image.rowPixelSpacing; - } else { - // we believe that the column pixel is the same as css pixel - verticalRatio = image.rowPixelSpacing / image.columnPixelSpacing; + return createViewport(); } // Fit image to window - const verticalScale = canvas.height / image.rows / verticalRatio; - const horizontalScale = canvas.width / image.columns / horizontalRatio; - const scale = Math.min(horizontalScale, verticalScale); + const scale = getImageFitScale(canvas, image, 0).scaleFactor; return { scale, diff --git a/src/internal/getImageFitScale.js b/src/internal/getImageFitScale.js new file mode 100644 index 00000000..c6f55b1e --- /dev/null +++ b/src/internal/getImageFitScale.js @@ -0,0 +1,40 @@ +import { validateParameterUndefinedOrNull } from './validator.js'; +import getImageSize from './getImageSize.js'; + + +/** + * Calculates the horizontal, vertical and minimum scale factor for an image + @param {{width, height}} windowSize The window size where the image is displayed. This can be any HTML element or structure with a width, height fields (e.g. canvas). + * @param {any} image The cornerstone image object + * @param {Number} rotation Optional. The rotation angle of the image. + * @return {{horizontalScale, verticalScale, scaleFactor}} The calculated horizontal, vertical and minimum scale factor + * @memberof Internal + */ +export default function (windowSize, image, rotation = null) { + + validateParameterUndefinedOrNull(windowSize, 'getImageScale: parameter windowSize must not be undefined'); + validateParameterUndefinedOrNull(image, 'getImageScale: parameter image must not be undefined'); + + const imageSize = getImageSize(image, rotation); + const rowPixelSpacing = image.rowPixelSpacing === undefined ? 1 : image.rowPixelSpacing; + const columnPixelSpacing = image.columnPixelSpacing === undefined ? 1 : image.columnPixelSpacing; + let verticalRatio = 1; + let horizontalRatio = 1; + + if (rowPixelSpacing < columnPixelSpacing) { + horizontalRatio = columnPixelSpacing / rowPixelSpacing; + } else { + // even if they are equal we want to calculate this ratio (the ration might be 0.5) + verticalRatio = rowPixelSpacing / columnPixelSpacing; + } + + const verticalScale = windowSize.height / imageSize.height / verticalRatio; + const horizontalScale = windowSize.width / imageSize.width / horizontalRatio; + + // Fit image to window + return { + verticalScale, + horizontalScale, + scaleFactor: Math.min(horizontalScale, verticalScale) + }; +} diff --git a/src/internal/getImageSize.js b/src/internal/getImageSize.js index c6a322c3..4216ccb9 100644 --- a/src/internal/getImageSize.js +++ b/src/internal/getImageSize.js @@ -1,19 +1,39 @@ +import { validateParameterUndefinedOrNull } from './validator.js'; + +/** + * Check if the angle is rotated + * @param {Number} rotation the rotation angle + * @returns {Boolean} true if the angle is rotated; Otherwise, false. + * @memberof Internal + */ +function isRotated (rotation) { + return !(rotation === null || rotation === undefined || rotation === 0 || rotation === 180); +} + /** * Retrieves the current image dimensions given an enabled element * - * @param {EnabledElement} enabledElement The Cornerstone Enabled Element - * @return {{width, height}} The Image dimensions + * @param {any} image The Cornerstone image. + * @param {Number} rotation Optional. The rotation angle of the image. + * @return {{width:Number, height:Number}} The Image dimensions + * @memberof Internal */ -export default function (enabledElement) { - if (enabledElement.viewport.rotation === 0 || enabledElement.viewport.rotation === 180) { +export default function (image, rotation = null) { + + validateParameterUndefinedOrNull(image, 'getImageSize: parameter image must not be undefined'); + validateParameterUndefinedOrNull(image.width, 'getImageSize: parameter image must have width'); + validateParameterUndefinedOrNull(image.height, 'getImageSize: parameter image must have height'); + + + if (isRotated(rotation)) { return { - width: enabledElement.image.width, - height: enabledElement.image.height + height: image.width, + width: image.height }; } return { - width: enabledElement.image.height, - height: enabledElement.image.width + width: image.width, + height: image.height }; } diff --git a/src/internal/validator.js b/src/internal/validator.js new file mode 100644 index 00000000..fc201e4a --- /dev/null +++ b/src/internal/validator.js @@ -0,0 +1,27 @@ + +/** + * Check if the supplied parameter is undefined and throws and error + * @param {any} checkParam the parameter to validate for undefined + * @param {any} errorMsg the error message to be thrown + * @returns {void} + * @memberof internal + */ +export function validateParameterUndefined (checkParam, errorMsg) { + if (checkParam === undefined) { + throw new Error(errorMsg); + } +} + + +/** + * Check if the supplied parameter is undefined or null and throws and error + * @param {any} checkParam the parameter to validate for undefined + * @param {any} errorMsg the error message to be thrown + * @returns {void} + * @memberof internal + */ +export function validateParameterUndefinedOrNull (checkParam, errorMsg) { + if (checkParam === undefined || checkParam === null) { + throw new Error(errorMsg); + } +} diff --git a/src/resize.js b/src/resize.js index 2e6e71c6..4ab37724 100644 --- a/src/resize.js +++ b/src/resize.js @@ -57,7 +57,7 @@ function setCanvasSize (element, canvas) { */ function wasFitToWindow (enabledElement, oldCanvasWidth, oldCanvasHeight) { const scale = enabledElement.viewport.scale; - const imageSize = getImageSize(enabledElement); + const imageSize = getImageSize(enabledElement.image, enabledElement.viewport.rotation); const imageWidth = Math.round(imageSize.width * scale); const imageHeight = Math.round(imageSize.height * scale); const x = enabledElement.viewport.translation.x; @@ -110,13 +110,7 @@ export default function (element, forceFitToWindow) { return; } - if (forceFitToWindow === true) { - fitToWindow(element); - - return; - } - - if (wasFitToWindow(enabledElement, oldCanvasWidth, oldCanvasHeight)) { + if (forceFitToWindow || wasFitToWindow(enabledElement, oldCanvasWidth, oldCanvasHeight)) { // Fit the image to the window again if it fitted before the resize fitToWindow(element); } else { diff --git a/src/version.js b/src/version.js index d6aa1be8..76874e7c 100644 --- a/src/version.js +++ b/src/version.js @@ -1 +1 @@ -export default '2.2.0'; +export default '2.2.1-rc1'; diff --git a/test/internal/getDefaultViewport_test.js b/test/internal/getDefaultViewport_test.js index 0e538dff..e986802f 100644 --- a/test/internal/getDefaultViewport_test.js +++ b/test/internal/getDefaultViewport_test.js @@ -1,4 +1,4 @@ -import { should } from 'chai'; +import { should, assert } from 'chai'; import getDefaultViewport from '../../src/internal/getDefaultViewport'; should(); @@ -13,4 +13,64 @@ describe('getDefaultViewport', function () { emptyViewport.should.have.all.keys(['scale', 'translation', 'voi', 'invert', 'pixelReplication', 'rotation', 'hflip', 'vflip', 'modalityLUT', 'voiLUT', 'colormap', 'labelmap', 'displayedArea']); }); + + describe('when provided an image with all values available', function () { + beforeEach(function () { + this.canvas = document.createElement('canvas'); + + this.canvas.width = 100; + this.canvas.height = 100; + + this.imageViewport = { + width: 100, + height: 100, + rowPixelSpacing: 1, + columnPixelSpacing: 2, + windowWidth: 255, + windowCenter: 127, + invert: true, + modalityLUT: 'm-lut', + voiLUT: 'voi-lut', + colormap: 'color-map' + }; + }); + + it('should return a viewport with defined values and calculated scale value', function () { + const viewport = getDefaultViewport(this.canvas, this.imageViewport); + + assert.equal(viewport.scale, 0.5); //should take the columnPixelSpacing into consideration and scale down by 1/2 + assert.equal(viewport.displayedArea.presentationSizeMode, 'NONE'); + assert.equal(viewport.displayedArea.rowPixelSpacing, this.imageViewport.rowPixelSpacing); + assert.equal(viewport.displayedArea.columnPixelSpacing, this.imageViewport.columnPixelSpacing); + assert.equal(viewport.voi.windowWidth, this.imageViewport.windowWidth); + assert.equal(viewport.voi.windowCenter, this.imageViewport.windowCenter); + assert.equal(viewport.invert, this.imageViewport.invert); + assert.equal(viewport.modalityLUT, this.imageViewport.modalityLUT); + assert.equal(viewport.voiLUT, this.imageViewport.voiLUT); + assert.equal(viewport.colormap, this.imageViewport.colormap); + }); + + }); + + describe('when provided image is missing the column/row pixel spacing', function () { + beforeEach(function () { + this.canvas = document.createElement('canvas'); + + this.canvas.width = 100; + this.canvas.height = 100; + + this.imageViewport = { + width: 100, + height: 100, + }; + }); + + it('should be smart to set the default values to 1/1', function () { + const viewport = getDefaultViewport(this.canvas, this.imageViewport); + + assert.equal(viewport.displayedArea.rowPixelSpacing, 1); + assert.equal(viewport.displayedArea.columnPixelSpacing, 1); + assert.equal(viewport.scale, 1); + }); + }); }); diff --git a/test/internal/getImageFitScale_test.js b/test/internal/getImageFitScale_test.js new file mode 100644 index 00000000..dac36c60 --- /dev/null +++ b/test/internal/getImageFitScale_test.js @@ -0,0 +1,35 @@ +import { should, assert } from 'chai'; +import getImageFitScale from '../../src/internal/getImageFitScale'; + +should(); + +describe('getImageScale', function () { + it('should throw an error if we don\'t have a canvas', function () { + getImageFitScale.should.throw('getImageScale: parameter windowSize must not be undefined'); + }); + + describe('when provided an image that is larger than the canvas and magnified', function () { + beforeEach(function () { + this.canvas = document.createElement('canvas'); + + this.canvas.width = 100; + this.canvas.height = 100; + + this.imageViewport = { + width: 200, + height: 200, + columnPixelSpacing: 2, + rowPixelSpacing: 1 + }; + }); + + it('should return a viewport with a scale factor to make the image smaller.', function () { + const viewport = getImageFitScale(this.canvas, this.imageViewport, 90); + + assert.equal(viewport.verticalScale, 0.5); + assert.equal(viewport.horizontalScale, 0.25); + assert.equal(viewport.scaleFactor, 0.25); + }); + + }); +}); diff --git a/test/internal/getImageSize_test.js b/test/internal/getImageSize_test.js new file mode 100644 index 00000000..ca10cf89 --- /dev/null +++ b/test/internal/getImageSize_test.js @@ -0,0 +1,59 @@ +import { should, expect } from 'chai'; + +import getImageSize from '../../src/internal/getImageSize.js'; + +should(); + +describe('getImageSize', function () { + + describe('when image parameters is not passed', function () { + it('should throw an error', function () { + + expect(function () { + getImageSize(); + }).to.throw('getImageSize: parameter image must not be undefined'); + + expect(function () { + getImageSize({ width: 50 }); + }).to.throw('getImageSize: parameter image must have height'); + + expect(function () { + getImageSize({ height: 100 }); + }).to.throw('getImageSize: parameter image must have width'); + }); + }); + + describe('when an image is passed with no rotation', function () { + it('should return the image width/height', function () { + const image = { + width: 50, + height:100 + }; + + const imageSizeNoRotationParameter = getImageSize(image); + const imageSize0RotationParameter = getImageSize(image, 0); + const imageSize180RotationParameter = getImageSize(image, 180); + + imageSizeNoRotationParameter.should.be.deep.equal(image); + imageSize0RotationParameter.should.be.deep.equal(image); + imageSize180RotationParameter.should.be.deep.equal(image); + }); + }); + + describe('when an image is passed rotated', function () { + it('should return the image width/height rotated', function () { + let image = { + width: 50, + height:100 + }; + + const returnedImageSize = getImageSize(image, 90); + + // rotate + image.width = 100; + image.height = 50; + + returnedImageSize.should.be.deep.equal(image); + }); + }); +});