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.

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(rendering): should still use Float32 when not 16 bit for scaling issues #501

Merged
merged 1 commit into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's probably time to add an options bag here rather than just adding additional parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

This will get removed soon, so can we not do it in this PR?


// @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',
},
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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This option should go in an options object, as there may well be others in the future.

}

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') {
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',
},
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',
},
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