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(color volume viewport): fix incorrect property on volume actor #683

Merged
merged 5 commits into from
Jul 11, 2023
Merged

fix(color volume viewport): fix incorrect property on volume actor #683

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f124e4
fix color independent
sedghi Jul 7, 2023
6c16db5
feat(webLoader): Add support for volume viewports and improved image …
sedghi Jul 10, 2023
13273f3
should not skip color images
sedghi Jul 10, 2023
9a8d72b
apply review comments
sedghi Jul 11, 2023
ab55c52
apply review comments
sedghi Jul 11, 2023
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
95 changes: 83 additions & 12 deletions packages/core/examples/webLoader/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
metaData,
getRenderingEngine,
Types,
setVolumesForViewports,
volumeLoader,
} from '@cornerstonejs/core';
import {
initDemo,
Expand All @@ -28,12 +30,42 @@ setTitleAndDescription(
);

const content = document.getElementById('content');
const element = document.createElement('div');
element.id = 'cornerstone-element';
element.style.width = '500px';
element.style.height = '500px';

content.appendChild(element);
const element1 = document.createElement('div');
element1.id = 'cornerstone-element1';
element1.style.width = '500px';
element1.style.height = '500px';

const paraElement = document.createElement('p');
const paraText = document.createTextNode('volume viewport');
paraElement.appendChild(paraText);

const rowElement = document.createElement('div');
rowElement.style.display = 'flex';
rowElement.style.justifyContent = 'space-between';

const element2 = document.createElement('div');
element2.id = 'cornerstone-element2';
element2.style.width = '500px';
element2.style.height = '500px';

const element3 = document.createElement('div');
element3.id = 'cornerstone-element3';
element3.style.width = '500px';
element3.style.height = '500px';

const element4 = document.createElement('div');
element4.id = 'cornerstone-element4';
element4.style.width = '500px';
element4.style.height = '500px';

rowElement.appendChild(element2);
rowElement.appendChild(element3);
rowElement.appendChild(element4);

content.appendChild(element1);
content.appendChild(paraElement);
content.appendChild(rowElement);

const renderingEngineId = 'myRenderingEngine';
const viewportId = 'COLOR_STACK';
Expand All @@ -58,15 +90,10 @@ const imageIds = [

registerWebImageLoader(imageLoader);

metaData.addProvider(
// @ts-ignore
(type, imageId) => hardcodedMetaDataProvider(type, imageId, imageIds),
10000
);
// ============================= //

addSliderToToolbar({
title: 'SLice Index',
title: 'Slice Index',
range: [0, 9],
defaultValue: 0,
onSelectedValueChange: (value) => {
Expand All @@ -91,21 +118,65 @@ async function run() {
// Init Cornerstone and related libraries
await initDemo();

metaData.addProvider(
// @ts-ignore
(type, imageId) => hardcodedMetaDataProvider(type, imageId, imageIds),
10000
);

// Instantiate a rendering engine
const renderingEngine = new RenderingEngine(renderingEngineId);

const viewportInputArray = [
{
viewportId: 'COLOR_STACK',
type: ViewportType.STACK,
element,
element: element1,
},
{
viewportId: 'COLOR_VOLUME_1',
type: ViewportType.ORTHOGRAPHIC,
element: element2,
},
{
viewportId: 'COLOR_VOLUME_2',
type: ViewportType.ORTHOGRAPHIC,
element: element3,
defaultOptions: {
orientation: Enums.OrientationAxis.CORONAL,
},
},
{
viewportId: 'COLOR_VOLUME_3',
type: ViewportType.ORTHOGRAPHIC,
element: element4,
defaultOptions: {
orientation: Enums.OrientationAxis.SAGITTAL,
},
},
];

const volumeId = 'COLOR_VOLUME';

const volume = await volumeLoader.createAndCacheVolume(volumeId, {
imageIds,
});

renderingEngine.setViewports(viewportInputArray);

// render stack viewport
renderingEngine.getStackViewports()[0].setStack(imageIds);

setVolumesForViewports(
renderingEngine,
[{ volumeId }],
['COLOR_VOLUME_1', 'COLOR_VOLUME_2', 'COLOR_VOLUME_3']
);

volume.load();

// render volume viewports
renderingEngine.render();
}

run();
25 changes: 20 additions & 5 deletions packages/core/examples/webLoader/registerWebImageLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,15 @@ function createImage(image, imageId) {
function getPixelData() {
const imageData = getImageData();

return imageData.data;
// modify original image data and remove alpha channel (RGBA to RGB)
const data = new Uint8Array(imageData.width * imageData.height * 3);
for (let i = 0, j = 0; i < imageData.data.length; i += 4, j += 3) {
data[j] = imageData.data[i];
data[j + 1] = imageData.data[i + 1];
data[j + 2] = imageData.data[i + 2];
}

return data;
}

function getImageData() {
Expand Down Expand Up @@ -70,11 +78,12 @@ function createImage(image, imageId) {
height: rows,
width: columns,
color: true,
rgba: true,
// since the canvas return rgba we should tell the cornerstone that we have 4 components per pixel
sedghi marked this conversation as resolved.
Show resolved Hide resolved
rgba: false,
columnPixelSpacing: 1, // for web it's always 1
rowPixelSpacing: 1, // for web it's always 1
invert: false,
sizeInBytes: rows * columns * 4,
sizeInBytes: rows * columns * 3,
};
}

Expand Down Expand Up @@ -202,11 +211,17 @@ function _loadImageIntoBuffer(
// If we have a target buffer, write to that instead. This helps reduce memory duplication.
const { arrayBuffer, offset, length } = options.targetBuffer;

// since the web image was loaded via canvas api it has rgba format
// below we convert it to rgb since the vtk image data can work
// only with rgb format

// @ts-ignore
const pixelDataRGBA = image.getPixelData();
const pixelDataRGB = image.getPixelData();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need yet another image data array? You don't even appear to use targetArray, so can you throw it away entirely? I don't get this logic here.

Copy link
Member Author

Choose a reason for hiding this comment

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

targetArray is used to view the portion of the full volume at the offset and length, we need it to fill the volume

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I see what you are doing here - the target array is created from the passed in arrayBuffer instance, so it is a view onto the destination object and you are setting it. Missed that and thought you were creating a new instance.
You can avoid the copy entirely by passing targetBuffer into getPixelData and creating the Uint8array view inside the getPixelData. Then, that getPixelData has enough information to do a bounds check to see that nothing is being cut off, and to throw an exception if the provided array is too small.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not about only volume viewport and target buffer, it should handle both cases (Stack and volume) so I don't want to modify the getPixelData since that correctly return the pixelData for stack viewport, does that makes sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is why I would put the logic into getPixelData, setting it up as an optional parameter. The return value will be identical in both cases, just if you are passed an array buffer, it uses that array buffer (+ offsets info), otherwise create new data. That prevents the copy operation entirely, while being 100% compatible with either call method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok applied it

const targetArray = new Uint8Array(arrayBuffer, offset, length);

targetArray.set(pixelDataRGBA, 0);
// TypedArray.Set is api level and ~50x faster than copying elements even for
// Arrays of different types, which aren't simply memcpy ops.
targetArray.set(pixelDataRGB, 0);

resolve(true);
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/RenderingEngine/BaseVolumeViewport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import vtkVolume from '@kitware/vtk.js/Rendering/Core/Volume';
import vtkColorTransferFunction from '@kitware/vtk.js/Rendering/Core/ColorTransferFunction';
import vtkColorMaps from '@kitware/vtk.js/Rendering/Core/ColorTransferFunction/ColorMaps';
import vtkPiecewiseFunction from '@kitware/vtk.js/Common/DataModel/PiecewiseFunction';

import cache from '../cache';
import {
Expand Down Expand Up @@ -50,7 +51,6 @@ import volumeNewImageEventDispatcher, {
import Viewport from './Viewport';
import type { vtkSlabCamera as vtkSlabCameraType } from './vtkClasses/vtkSlabCamera';
import vtkSlabCamera from './vtkClasses/vtkSlabCamera';
import vtkPiecewiseFunction from '@kitware/vtk.js/Common/DataModel/PiecewiseFunction';

/**
* Abstract base class for volume viewports. VolumeViewports are used to render
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/RenderingEngine/helpers/createVolumeActor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ async function createVolumeActor(
const volumeActor = vtkVolume.newInstance();
volumeActor.setMapper(volumeMapper);

const numberOfComponents = imageData
.getPointData()
.getScalars()
.getNumberOfComponents();

if (numberOfComponents === 3) {
volumeActor.getProperty().setIndependentComponents(false);
}

// If the volume is composed of imageIds, we can apply a default VOI based
// on either the metadata or the min/max of the middle slice. Example of other
// types of volumes which might not be composed of imageIds would be e.g., nrrd, nifti
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/loaders/volumeLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function createInternalVTKRepresentation(
imageData.setDirection(direction);
imageData.setOrigin(origin);

// Add scalar datas to 3D or 4D volume
// Add scalar data to 3D or 4D volume
if (volume.isDynamicVolume()) {
const scalarDataArrays = (<Types.IDynamicImageVolume>(
volume
Expand Down
2 changes: 0 additions & 2 deletions packages/core/test/volumeViewport_gpu_render_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@ describe('Volume Viewport GPU -- ', () => {
});

const callback = ({ volumeActor }) => {
volumeActor.getProperty().setIndependentComponents(false);
volumeActor.getProperty().setInterpolationTypeToNearest();
};

Expand Down Expand Up @@ -814,7 +813,6 @@ describe('Volume Viewport GPU -- ', () => {
});

const callback = ({ volumeActor }) => {
volumeActor.getProperty().setIndependentComponents(false);
volumeActor.getProperty().setInterpolationTypeToLinear();
};

Expand Down
6 changes: 1 addition & 5 deletions packages/dicomImageLoader/src/shared/decodeImageFrame.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,7 @@ function postProcessDecodedPixels(
Float32Array,
};

if (
options.targetBuffer &&
options.targetBuffer.type &&
!isColorImage(imageFrame.photometricInterpretation)
) {
if (options.targetBuffer && options.targetBuffer.type) {
pixelDataArray = _handleTargetBuffer(
options,
imageFrame,
Expand Down
9 changes: 6 additions & 3 deletions packages/streaming-image-volume-loader/src/BaseStreamingImageVolume.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export default class BaseStreamingImageVolume extends ImageVolume {
windowCenter,
windowWidth,
color,
// we don't use rgb for the volume actors, so this is always false
sedghi marked this conversation as resolved.
Show resolved Hide resolved
rgba: false,
spacing: this.spacing,
dimensions: this.dimensions,
PhotometricInterpretation,
Expand Down Expand Up @@ -858,9 +860,11 @@ export default class BaseStreamingImageVolume extends ImageVolume {
windowWidth,
voiLUTFunction,
color,
rgba: false,
numComps: numComponents,
rows: dimensions[0],
columns: dimensions[1],
// Note the dimensions were defined as [Columns, Rows, Frames]
rows: dimensions[1],
columns: dimensions[0],
sizeInBytes: imageScalarData.byteLength,
getPixelData: () => imageScalarData,
minPixelValue: minMax.min,
Expand All @@ -871,7 +875,6 @@ export default class BaseStreamingImageVolume extends ImageVolume {
getCanvas: undefined, // todo: which canvas?
height: dimensions[0],
width: dimensions[1],
rgba: undefined, // todo: how
columnPixelSpacing: spacing[0],
rowPixelSpacing: spacing[1],
invert,
Expand Down