Skip to content

Commit

Permalink
fix(rendering): should still use Float32 when not 16 bit for scaling …
Browse files Browse the repository at this point in the history
…issues (#501)
  • Loading branch information
sedghi authored Mar 22, 2023
1 parent c68040a commit 448baf2
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 26 deletions.
6 changes: 5 additions & 1 deletion common/reviews/api/core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ export abstract class BaseVolumeViewport extends Viewport implements IVolumeView
// (undocumented)
setVolumes(volumeInputArray: Array<IVolumeInput>, immediate?: boolean, suppressEvents?: boolean): Promise<void>;
// (undocumented)
use16BitTexture: boolean;
// (undocumented)
useCPURendering: boolean;
// (undocumented)
static get useCustomRenderingPipeline(): boolean;
Expand Down Expand Up @@ -497,7 +499,7 @@ function createUint16SharedArray(length: number): Uint16Array;
function createUint8SharedArray(length: number): Uint8Array;

// @public (undocumented)
export function createVolumeActor(props: createVolumeActorInterface, element: HTMLDivElement, viewportId: string, suppressEvents?: boolean): Promise<VolumeActor>;
export function createVolumeActor(props: createVolumeActorInterface, element: HTMLDivElement, viewportId: string, suppressEvents?: boolean, use16BitTexture?: boolean): Promise<VolumeActor>;

// @public (undocumented)
export function createVolumeMapper(imageData: any, vtkOpenGLTexture: any): any;
Expand Down Expand Up @@ -2448,6 +2450,8 @@ export class Viewport implements IViewport {
// (undocumented)
sHeight: number;
// (undocumented)
protected _shouldUse16BitTexture(): boolean;
// (undocumented)
readonly suppressEvents: boolean;
// (undocumented)
sWidth: number;
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/RenderingEngine/BaseVolumeViewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import vtkSlabCamera from './vtkClasses/vtkSlabCamera';
*/
abstract class BaseVolumeViewport extends Viewport implements IVolumeViewport {
useCPURendering = false;
use16BitTexture = false;
private _FrameOfReferenceUID: string;

// Viewport Properties
Expand All @@ -62,6 +63,7 @@ abstract class BaseVolumeViewport extends Viewport implements IVolumeViewport {
super(props);

this.useCPURendering = getShouldUseCPURendering();
this.use16BitTexture = this._shouldUse16BitTexture();

if (this.useCPURendering) {
throw new Error(
Expand Down Expand Up @@ -368,7 +370,8 @@ abstract class BaseVolumeViewport extends Viewport implements IVolumeViewport {
volumeInputArray[i],
this.element,
this.id,
suppressEvents
suppressEvents,
this.use16BitTexture
);

// We cannot use only volumeId since then we cannot have for instance more
Expand Down Expand Up @@ -433,7 +436,8 @@ abstract class BaseVolumeViewport extends Viewport implements IVolumeViewport {
volumeInputArray[i],
this.element,
this.id,
suppressEvents
suppressEvents,
this.use16BitTexture
);

if (visibility === false) {
Expand Down
16 changes: 7 additions & 9 deletions packages/core/src/RenderingEngine/StackViewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import vtkCamera from '@kitware/vtk.js/Rendering/Core/Camera';
import { vec2, vec3, mat4 } from 'gl-matrix';
import vtkImageMapper from '@kitware/vtk.js/Rendering/Core/ImageMapper';
import vtkImageSlice from '@kitware/vtk.js/Rendering/Core/ImageSlice';
import { getConfiguration } from '../init';
import * as metaData from '../metaData';
import Viewport from './Viewport';
import eventTarget from '../eventTarget';
Expand Down Expand Up @@ -179,7 +178,7 @@ class StackViewport extends Viewport implements IStackViewport {
this.scaling = {};
this.modality = null;
this.useCPURendering = getShouldUseCPURendering();

this.use16BitTexture = this._shouldUse16BitTexture();
this._configureRenderingPipeline();

if (this.useCPURendering) {
Expand Down Expand Up @@ -1431,7 +1430,6 @@ class StackViewport extends Viewport implements IStackViewport {
TypedArray,
}): void {
let pixelArray;
this.use16BitTexture = this._shouldUse16BitTexture();
switch (bitsAllocated) {
case 8:
pixelArray = new Uint8Array(numVoxels * numComps);
Expand Down Expand Up @@ -1739,6 +1737,9 @@ class StackViewport extends Viewport implements IStackViewport {
const requestType = RequestType.Interaction;
const additionalDetails = { imageId };
const options = {
targetBuffer: {
type: this.use16BitTexture ? undefined : 'Float32Array',
},
preScale: {
enabled: true,
},
Expand Down Expand Up @@ -1823,6 +1824,9 @@ class StackViewport extends Viewport implements IStackViewport {
const requestType = RequestType.Interaction;
const additionalDetails = { imageId };
const options = {
targetBuffer: {
type: this.use16BitTexture ? undefined : 'Float32Array',
},

This comment has been minimized.

Copy link
@Ouwen

Ouwen Mar 26, 2023

Contributor

@sedghi this will probably need to be changed to

    targetBuffer: use16BitTexture
      ? undefined
      : {
          type: 'Float32Array',
        },

Otherwise an error will get thrown due to CSWIL calling undefined an invalid type

preScale: {
enabled: true,
},
Expand Down Expand Up @@ -1992,12 +1996,6 @@ class StackViewport extends Viewport implements IStackViewport {
}
}

private _shouldUse16BitTexture() {
const { useNorm16Texture, preferSizeOverAccuracy } =
getConfiguration().rendering;
return useNorm16Texture || preferSizeOverAccuracy;
}

private _getInitialVOIRange(image: IImage) {
if (this.voiRange && this.voiUpdatedWithSetProperties) {
return this.voiRange;
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/RenderingEngine/Viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
} from '../types';
import type { ViewportInput, IViewport } from '../types/IViewport';
import type { vtkSlabCamera } from './vtkClasses/vtkSlabCamera';
import { getConfiguration } from '../init';

/**
* An object representing a single viewport, which is a camera
Expand Down Expand Up @@ -1119,6 +1120,12 @@ class Viewport implements IViewport {
return { widthWorld: maxX - minX, heightWorld: maxY - minY };
}

protected _shouldUse16BitTexture() {
const { useNorm16Texture, preferSizeOverAccuracy } =
getConfiguration().rendering;
return useNorm16Texture || preferSizeOverAccuracy;
}

_getCorners(bounds: Array<number>): Array<number>[] {
return [
[bounds[0], bounds[2], bounds[4]],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ async function createVolumeActor(
props: createVolumeActorInterface,
element: HTMLDivElement,
viewportId: string,
suppressEvents = false
suppressEvents = false,
use16BitTexture = false
): Promise<VolumeActor> {
const { volumeId, callback, blendMode } = props;

Expand Down Expand Up @@ -61,7 +62,7 @@ async function createVolumeActor(
// types of volumes which might not be composed of imageIds would be e.g., nrrd, nifti
// format volumes
if (imageVolume.imageIds) {
await setDefaultVolumeVOI(volumeActor, imageVolume);
await setDefaultVolumeVOI(volumeActor, imageVolume, use16BitTexture);
}

if (callback) {
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/RenderingEngine/helpers/setDefaultVolumeVOI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ const REQUEST_TYPE = RequestType.Prefetch;
*/
async function setDefaultVolumeVOI(
volumeActor: VolumeActor,
imageVolume: IImageVolume
imageVolume: IImageVolume,
use16BitTexture: boolean
): Promise<void> {
let voi = getVOIFromMetadata(imageVolume);

if (!voi) {
voi = await getVOIFromMinMax(imageVolume);
voi = await getVOIFromMinMax(imageVolume, use16BitTexture);
}

if (!voi || voi.lower === undefined || voi.upper === undefined) {
Expand Down Expand Up @@ -114,7 +115,10 @@ function getVOIFromMetadata(imageVolume: IImageVolume): VOIRange {
* @param imageVolume - The image volume that we want to get the VOI from.
* @returns The VOIRange with lower and upper values
*/
async function getVOIFromMinMax(imageVolume: IImageVolume): Promise<VOIRange> {
async function getVOIFromMinMax(
imageVolume: IImageVolume,
use16BitTexture: boolean
): Promise<VOIRange> {
const { imageIds } = imageVolume;
const scalarData = imageVolume.getScalarData();

Expand Down Expand Up @@ -152,6 +156,9 @@ async function getVOIFromMinMax(imageVolume: IImageVolume): Promise<VOIRange> {
const byteOffset = imageIdIndex * bytesPerImage;

const options = {
targetBuffer: {
type: use16BitTexture ? undefined : 'Float32Array',
},
priority: PRIORITY,
requestType: REQUEST_TYPE,
preScale: {
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/loaders/volumeLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import eventTarget from '../eventTarget';
import triggerEvent from '../utilities/triggerEvent';
import { uuidv4 } from '../utilities';
import { Point3, Metadata, EventTypes, Mat3 } from '../types';
import { getConfiguration } from '../init';

interface VolumeLoaderOptions {
imageIds: Array<string>;
Expand Down Expand Up @@ -273,6 +274,8 @@ export async function createAndCacheDerivedVolume(

let numBytes, TypedArray;

const { useNorm16Texture } = getConfiguration().rendering;

// If target buffer is provided
if (targetBuffer) {
if (targetBuffer.type === 'Float32Array') {
Expand All @@ -281,10 +284,10 @@ export async function createAndCacheDerivedVolume(
} else if (targetBuffer.type === 'Uint8Array') {
numBytes = scalarLength;
TypedArray = Uint8Array;
} else if (targetBuffer.type === 'Uint16Array') {
} else if (useNorm16Texture && targetBuffer.type === 'Uint16Array') {
numBytes = scalarLength * 2;
TypedArray = Uint16Array;
} else if (targetBuffer.type === 'Int16Array') {
} else if (useNorm16Texture && targetBuffer.type === 'Int16Array') {

This comment has been minimized.

Copy link
@Ouwen

Ouwen Mar 26, 2023

Contributor

it might be better to use _shouldUse16BitTexture on the Viewport here

numBytes = scalarLength * 2;
TypedArray = Uint16Array;
} else {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/utilities/loadImageToCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as metaData from '../metaData';
import { RequestType } from '../enums';
import imageLoadPoolManager from '../requestPool/imageLoadPoolManager';
import renderToCanvas from './renderToCanvas';
import { getConfiguration } from '../init';

/**
* Loads and renders an imageId to a Canvas. It will use the CPU rendering pipeline
Expand Down Expand Up @@ -55,9 +56,14 @@ export default function loadImageToCanvas(
);
}

const { useNorm16Texture } = getConfiguration().rendering;

// IMPORTANT: Request type should be passed if not the 'interaction'
// highest priority will be used for the request type in the imageRetrievalPool
const options = {
targetBuffer: {
type: useNorm16Texture ? undefined : 'Float32Array',
},

This comment has been minimized.

Copy link
@Ouwen

Ouwen Mar 26, 2023

Contributor

Similar issue

    targetBuffer: use16BitTexture
      ? undefined
      : {
          type: 'Float32Array',
        }
preScale: {
enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"@mdx-js/react": "^1.6.21",
"@svgr/webpack": "^6.2.1",
"clsx": "^1.1.1",
"cornerstone-wado-image-loader": "^4.8.0",
"cornerstone-wado-image-loader": "^4.10.2",
"dcmjs": "^0.24.4",
"detect-gpu": "^4.0.45",
"dicom-parser": "^1.8.11",
Expand Down
4 changes: 2 additions & 2 deletions packages/streaming-image-volume-loader/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@
},
"dependencies": {
"@cornerstonejs/core": "^0.36.1",
"cornerstone-wado-image-loader": "^4.10.0"
"cornerstone-wado-image-loader": "^4.10.2"
},
"peerDependencies": {
"@cornerstonejs/calculate-suv": "1.0.2"
},
"devDependencies": {
"@cornerstonejs/calculate-suv": "1.0.2",
"cornerstone-wado-image-loader": "^4.10.0"
"cornerstone-wado-image-loader": "^4.10.2"
},
"contributors": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,22 +177,36 @@ function cornerstoneStreamingImageVolumeLoader(
break;

case 16:
// Temporary fix for 16 bit images to use Float32
// until the new dicom image loader handler the conversion
// correctly
if (!use16BitDataType) {
sizeInBytes = length * 4;
scalarData = useSharedArrayBuffer
? createFloat32SharedArray(length)
: new Float32Array(length);

break;
}

sizeInBytes = length * 2;
if (use16BitDataType && (signed || hasNegativeRescale)) {
if (signed || hasNegativeRescale) {
handleCache(sizeInBytes);
scalarData = useSharedArrayBuffer
? createInt16SharedArray(length)
: new Int16Array(length);
break;
}

if (use16BitDataType && !signed && !hasNegativeRescale) {
if (!signed && !hasNegativeRescale) {
handleCache(sizeInBytes);
scalarData = useSharedArrayBuffer
? createUint16SharedArray(length)
: new Uint16Array(length);
break;
}

// Default to Float32 again
sizeInBytes = length * 4;
handleCache(sizeInBytes);
scalarData = useSharedArrayBuffer
Expand Down
7 changes: 6 additions & 1 deletion packages/tools/src/utilities/stackPrefetch/stackPrefetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
eventTarget,
imageLoadPoolManager,
cache,
Types,
getConfiguration as getCoreConfiguration,
} from '@cornerstonejs/core';
import { addToolState, getToolState } from './state';

Expand Down Expand Up @@ -243,10 +243,15 @@ function prefetch(element) {
const requestFn = (imageId, options) =>
imageLoader.loadAndCacheImage(imageId, options);

const { useNorm16Texture } = getCoreConfiguration().rendering;

imageIdsToPrefetch.forEach((imageId) => {
// IMPORTANT: Request type should be passed if not the 'interaction'
// highest priority will be used for the request type in the imageRetrievalPool
const options = {
targetBuffer: {
type: useNorm16Texture ? undefined : 'Float32Array',
},

This comment has been minimized.

Copy link
@Ouwen

Ouwen Mar 26, 2023

Contributor
    targetBuffer: use16BitTexture
      ? undefined
      : {
          type: 'Float32Array',
        },
preScale: {
enabled: true,
},
Expand Down
18 changes: 17 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8908,7 +8908,23 @@ core-util-is@~1.0.0:
resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.3.tgz#a6042d3634c2b27e9328f837b965fac83808db85"
integrity sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==

cornerstone-wado-image-loader@^4.10.0, cornerstone-wado-image-loader@^4.8.0:
cornerstone-wado-image-loader@^4.10.2:
version "4.10.2"
resolved "https://registry.npmjs.org/cornerstone-wado-image-loader/-/cornerstone-wado-image-loader-4.10.2.tgz#139956654324fd2b01fe5b4900d0f4ed8c52633d"
integrity sha512-qj9dThELqYCm3jAZfg9qnUl8d76gngOl55kYJabY5lh/dFeVIxno/hYxy3ydE7RtG2c/TUGXb+EMUl0CJSqKBQ==
dependencies:
"@babel/eslint-parser" "^7.19.1"
"@cornerstonejs/codec-charls" "^1.2.3"
"@cornerstonejs/codec-libjpeg-turbo-8bit" "^1.2.2"
"@cornerstonejs/codec-openjpeg" "^1.2.2"
"@cornerstonejs/codec-openjph" "^2.4.2"
coverage-istanbul-loader "^3.0.5"
date-format "^4.0.14"
dicom-parser "^1.8.9"
pako "^2.0.4"
uuid "^9.0.0"

cornerstone-wado-image-loader@^4.8.0:
version "4.10.0"
resolved "https://registry.npmjs.org/cornerstone-wado-image-loader/-/cornerstone-wado-image-loader-4.10.0.tgz#25c367cfc54a2c92ebbb5c64dba4fe38439112a1"
integrity sha512-XZcgB8DpUxnsTA3vU/zbPtB2uFLL4ght70BPrQHykkX/Jlg/r6Ob7ztX2dEEAAb4ELE/d4icfGuSy10Acg/kyw==
Expand Down

0 comments on commit 448baf2

Please sign in to comment.