Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: targetbuffer check and scaling fix #522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: targetbuffer check and scaling fix #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/imageLoader/createImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,7 @@ function convertToIntPixelData(floatPixelData) {
* @param imageFrame
*/
function setPixelDataType(imageFrame, preScale) {
const isScaled = preScale?.scaled;
const scalingParmeters = preScale?.scalingParameters;
let isNegative = false;
if (isScaled && scalingParmeters) {
const rescaleSlope = scalingParmeters?.rescaleSlope || 1;
const rescaleIntercept = scalingParmeters?.rescaleIntercep || 0;
const suvbw = scalingParmeters?.suvbw || 1;
isNegative =
suvbw *
(imageFrame.smallestPixelValue * rescaleSlope + rescaleIntercept) <
0;
}
const isNegative = preScale?.isNegative;

if (imageFrame.bitsAllocated === 32) {
imageFrame.pixelData = new Float32Array(imageFrame.pixelData);
Expand Down Expand Up @@ -174,7 +163,7 @@ function createImage(imageId, pixelData, transferSyntax, options = {}) {
// We can't have done it within the thread incase it was a SharedArrayBuffer.
let alreadyTyped = false;

if (options.targetBuffer) {
if (options.targetBuffer && options.targetBuffer.type) {
let offset, length;
// If we have a target buffer, write to that instead. This helps reduce memory duplication.

Expand Down
15 changes: 8 additions & 7 deletions src/shared/decodeImageFrame.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function postProcessDecodedPixels(imageFrame, options, start, decodeConfig) {

imageFrame.pixelDataLength = imageFrame.pixelData.length;

if (options.targetBuffer) {
if (options.targetBuffer && options.targetBuffer.type) {
let offset, length;
// If we have a target buffer, write to that instead. This helps reduce memory duplication.

Expand Down Expand Up @@ -256,18 +256,19 @@ function postProcessDecodedPixels(imageFrame, options, start, decodeConfig) {
typeof rescaleSlope === 'number' &&
typeof rescaleIntercept === 'number'
) {
if (scaleArray(pixelDataArray, scalingParameters)) {
imageFrame.preScale = {
...options.preScale,
scaled: true,
};
}
scaleArray(
imageFrame,
options.preScale,
pixelDataArray,
scalingParameters
);
}
}

// Handle cases where the targetBuffer is not backed by a SharedArrayBuffer
if (
options.targetBuffer &&
options.targetBuffer.type &&
(!options.targetBuffer.arrayBuffer ||
options.targetBuffer.arrayBuffer instanceof ArrayBuffer)
) {
Expand Down
33 changes: 29 additions & 4 deletions src/shared/scaling/scaleArray.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,45 @@
export default function scaleArray(array, scalingParameters) {
export default function scaleArray(
imageFrame,
preScaleOptions,
array,
scalingParameters
) {
const arrayLength = array.length;
const { rescaleSlope, rescaleIntercept, suvbw } = scalingParameters;
const rescaleSlope = scalingParameters?.rescaleSlope || 1;
const rescaleIntercept = scalingParameters?.rescaleIntercept || 0;
const suvbw = scalingParameters?.suvbw || 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why should we go through headache of the scale and intercept for large arrays if they are undefiend? can we return early instead? Also don't think we get to here at all if rescaleSlope and rescaleIntercept are not numbers because of the check in decodeImageFrame.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So reviewing it looks like there is a check to see if slope/intercept exist before scaleArray is called:

cornerstoneWADOImageLoader/src/shared/decodeImageFrame.js

Lines 255 to 265 in a4d973e

if (
typeof rescaleSlope === 'number' &&
typeof rescaleIntercept === 'number'
) {
scaleArray(
imageFrame,
options.preScale,
pixelDataArray,
scalingParameters
);
}

Additionally, if it is a PET modality and suvbw is not defined the other prescaling is ignored entirely (early return so no prescale). I'm not sure if this desirable because other scale params may exist.

This diff would set defaults and it shouldn't be more computationally expensive than current commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I guess we can say that in this function always typeof rescaleSlope === 'number' is true, so no need for scalingParameters?.rescaleSlope || 1; right?
I guess you are right about the PT scenario

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figured for readability I just follow the PET case since it doesn’t change the functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m okay either way, I can remove the || defaults too


let isNegative = false;

if (scalingParameters.modality === 'PT') {
if (typeof suvbw !== 'number') {
return;
}

for (let i = 0; i < arrayLength; i++) {
array[i] = suvbw * (array[i] * rescaleSlope + rescaleIntercept);
const value = suvbw * (array[i] * rescaleSlope + rescaleIntercept);

array[i] = value;
if (value < 0 && !isNegative) {
isNegative = true;
}
}
} else {
for (let i = 0; i < arrayLength; i++) {
array[i] = array[i] * rescaleSlope + rescaleIntercept;
const value = array[i] * rescaleSlope + rescaleIntercept;

array[i] = value;
if (value < 0 && !isNegative) {
isNegative = true;
}
}
}

imageFrame.preScale = {
...preScaleOptions,
scaled: true,
isNegative,
};

return true;
}