fix: targetbuffer check and scaling fix#522
fix: targetbuffer check and scaling fix#522Ouwen wants to merge 1 commit intocornerstonejs:masterfrom
Conversation
✅ Deploy Preview for cornerstone-wado-image-loader ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| const { rescaleSlope, rescaleIntercept, suvbw } = scalingParameters; | ||
| const rescaleSlope = scalingParameters?.rescaleSlope || 1; | ||
| const rescaleIntercept = scalingParameters?.rescaleIntercept || 0; | ||
| const suvbw = scalingParameters?.suvbw || 1; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Figured for readability I just follow the PET case since it doesn’t change the functionality.
There was a problem hiding this comment.
I’m okay either way, I can remove the || defaults too
src/shared/scaling/scaleArray.js
Outdated
| const value = suvbw * (array[i] * rescaleSlope + rescaleIntercept); | ||
|
|
||
| array[i] = value; | ||
| if (value < 0) { |
There was a problem hiding this comment.
wouldn't it be faster if we don't check on value < 0 if the isNegative is true too ?
Ouwen
force-pushed
the
gradienthealth/fix_prescaling
branch
from
a4d973e to
f5388f7
Compare
2 participants
Prescaling can cause Uint16 type underflows which require the logic in
scaleArrayto account for negative values.