RFC for v2.x

[sedghi]

Cornerstone3D beta distribution tag is now live in npm 🎉, and the beta branch will publish to it.

We are excited 😃 to open this RFC for version 2.x of Cornerstone3D. Our vision for these changes includes:

• ~~Removal of cjs in favor of esm and umd: we don't really need all three 🧹~~.
• ~~Dropping ES5 support and targeting newer ES2022: Aligning with the latest standards 🚀~~.
• ~~Upgrade of TypeScript to 5.x: Enhancing the type safety and developer experience 💻~~.
• ~~Rework of the web workers with comlink: Modernizing this critical aspect for better performance and maintainability 🛠️~~.

These changes are intended to make Cornerstone3D more modern and efficient. However, we recognize that they might also lead to breaking changes, particularly in the API 😬.

If there is any part of the API that doesn't make sense to you, or if you have a better idea that you couldn't propose before due to it being a breaking change, now is the time ⏰. We value your input, and we're eager to hear your thoughts 💭.

How to Propose Changes:

• Create an issue in the GitHub and describe your suggestion or concern in detail 📝.
• Post the issue link here as a mention 📎.

[sedghi]

https://github.com/cornerstonejs/cornerstone3D/pull/514 - DONE

[sedghi]

https://github.com/cornerstonejs/cornerstone3D/issues/697 - DONE

[shunia]

Would the rafactor of dicom-image-loader be part of this version?

Since this version is mainly trying to catch up with the modern DX, I'd love to see that the web worker files can be loaded in a more general way, which could makes it possible to be bundled by other modern bundlers like vite. By saying a more general way, I mean the loading path should be implemented with import.meta.url or something like this.

For now I believe users have to manually copy all the worker files from within the dicom-image-loader package into somewhere their bundlers can read and put the files into a path that the dicom-image-loader code can read and load, which is exactly is what you suggested here. Though it definitely works, I think it could be better.

This is what almost 'blocks' me from using this great library the first time, with a project set up by vite.

[sedghi]

Thanks for your comment @shunia Yes, it would be ideally what you are recommending here. Although I have not tried cornerstone3D with vite . Do you have a simple repo with cs3d and default setup of the vite so that I try to make sure in 2.x it is resolved?

[shunia]

Thanks for your comment @shunia Yes, it would be ideally what you are recommending here. Although I have not tried cornerstone3D with vite . Do you have a simple repo with cs3d and default setup of the vite so that I try to make sure in 2.x it is resolved?

I would love to and did try to create a repo for this, but after some digging I find it becoming harder to implement a simple viewer now because of the API change, you have to create an image loader or the viewport will just not work and complains with errors.

If you are interested in how to setup a basic vite project, I think you can simply follow the command:

npm create vite@latest 

And because of the failed experience, I believe the 'image loader' concept should be improved too. I don't have enough time to dig deeper now, but I think the signature of registerImageLoader function is quite intuitive: the second parameter with type ImageLoaderFn is not documented well, and even after reading some of the source codes I still have no clue what should be returned for the promise since it's a Record<string, any> type.

[sedghi]

I hoped the custom image loader how-to-guide adds some guidance on how to write your loaders

https://www.cornerstonejs.org/docs/how-to-guides/custom-image-loader

[sedghi]

DONE

imageCoordinate -> worldCoordinate

function getDataInTime( dynamicVolume: Types.IDynamicImageVolume, options: { frameNumbers?; maskVolumeId?; imageCoordinate?; } ): number[] | number[][] {~~

[sedghi]

DONE

I think there is an inconsistency with how a volume or stack triggers the NEW_VOLUME or NEW_STACK events. In BaseVolumeViewport.ts, the VOLUME_VIEWPORT_NEW_VOLUME event is triggered on the viewport element whereas in StackViewport.ts, the STACK_VIEWPORT_NEW_STACK event is triggered on the cornerstone eventTarget. Is this intentional? Relevant lines below: https://github.com/cornerstonejs/cornerstone3D/blob/c527923a8a1afd65e76f9335ac4cc0bc3a0e924b/packages/core/src/RenderingEngine/BaseVolumeViewport.ts#L779 https://github.com/cornerstonejs/cornerstone3D/blob/c527923a8a1afd65e76f9335ac4cc0bc3a0e924b/packages/core/src/RenderingEngine/StackViewport.ts#[…]2

[shunia]

I hoped the custom image loader how-to-guide adds some guidance on how to write your loaders

https://www.cornerstonejs.org/docs/how-to-guides/custom-image-loader

I believe the most important part is omitted in the example code:

// Request succeeded, Create an image object (logic omitted)
const image = createImageObject(oReq.response);

What is the actual return type?

[sedghi]

DONE

camera rotation should be on the camera not properties

[sedghi]

remove STACK_NEW_IMAGE and VOLUME_NEW_IMAGE after merging the NEW_IMAGE

[sedghi]

DONE

getCalibratedLengthUnits and getCalibratedAreaUnits arugment order

[sedghi]

DONE

resetCamera(resetPan, ....)

[sedghi]

decache, convertToImages etc. in streaming

[sedghi]

DONE

referenceId vs referencedId vs referencedImageId vs referenceImageId

[sedghi]

DONE

export function triggerAnnotationRenderForViewportIds( renderingEngine: Types.IRenderingEngine, viewportIdsToRender: string[] ): void {

Should be rendering engine Id

[sedghi]

DONE

VOLUME_SCROLL_OUT_OF_BOUNDS is VOLUME_VIEWPORT_SCROLL_OUT_OF_BOUNDS

[sedghi]

DONE

createAndCacheVolume might be better to name createAndCacheEmptyVolume

[sedghi]

getSegmentationRepresentationByUID requires toolGroupId which does not makes sense

[sedghi]

getAllSegmentationRepresentations should be array instead of object

[sedghi]

drawpath vs drawpolyline

[sedghi]

Increase line width to 100, OHIF dev experience is much better

[sedghi]

setSegmentationRepresentationSpecificConfig

should not rely on toolGroupId as UIDs should be uids

[sedghi]

drawRect, drawRectByCoordinates,

[kresli]

I can see the types for dicomImageLoader is missing. I've been trying to fix it on main branch but I ended up with issue with @cornerstonejs/core and other types used in the image loader. As you are using monorepo I would expect to use composite tsc feature but thats not the case. I'm happy to contribute if necessary as the image loader is a huge part of our project. https://www.typescriptlang.org/tsconfig/#composite

[sedghi]

@kresli I know it is a pain, we will work on it soon hopefully

[sedghi]

We are not following the lps definition in our slices, volview does it correctly

[wayfarer3130]

Update the volume API to use single slices on browser side and combined slice data on volume side so that the memory is shared between single frames and multiple frames and large volume allocation works. This should allow 2 memory regions on the webGL side to be treated as a single volume for rendering so that larger volumes can be created. This would involve removing all the destination volume code where the offset to store to is included in the imageio, and the imageio would no longer need to use shared memory.

[wayfarer3130]

We might want to consider the API for resizing of windows - there is CSS which automatically selects the right aspect ratio, and will allow resizing the window using the browser native resize so it is really smooth, followed by automatic updates. Unfortunately, it changes the API for the element containing the viewport a tiny bit - the old ones work, but need to be fixed just a bit to work more smoothly, but the new element styling is NOT backwards compatible.

[sedghi]

DONE

We should enable the prescale always to true