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

Future of @cornerstonejs/dicom-image-loader #612

Open
sedghi opened this issue May 16, 2023 · 1 comment
Open

Future of @cornerstonejs/dicom-image-loader #612

sedghi opened this issue May 16, 2023 · 1 comment

Comments

@sedghi
Copy link
Member

sedghi commented May 16, 2023

This feature request is to incorporate two important advancements into the codebase: ECMAScript module builds and a centralized web worker manager. These enhancements will provide significant benefits in terms of code organization, performance, and scalability.

Feature 1: ECMAScript Module Builds

ECMAScript modules offer a standardized approach to modularizing JavaScript code, allowing for better code organization, encapsulation, and reusability. We tried a bit following our recipe for other packages in the codebase and reused them for the dicom-image-loader, however, since the dicom-image-loader utilizes WASM codecs and webworkers, it didn't work properly.

Potential Solutions (?)

  • I think we should get rid of the worker-loader from webpack in our codebase. Currently, we are using the older 4.x styles, while webpack 5.x offers better support for workers.
  • In webpack 5, a more efficient approach for using workers is available. According to the webpack documentation, we can use new Worker(new URL('./worker.js', import.meta.url)). However, there is a caveat when the application, such as OHIF, is served from a subroute (e.g., not from the root directory, but from '/myviewer'). In such cases, webpack encounters difficulties in locating the workers.

Feature 2: Centralized Web Worker Manager

The current webworker manager in the cornerstone ecosystem is exclusively located within the dicom-image-loader. However, there is an increasing need to offload resource-intensive tasks to webworkers. To address this, we should refactor the code and move the webworker manager to the core. By doing so, other libraries and applications like OHIF can also take advantage of this functionality.

@sedghi sedghi pinned this issue Jun 20, 2023
@Ragnar-Oock
Copy link

Ragnar-Oock commented Sep 6, 2024

Now that cornerstone is built within a monorepo it would be wise to use it as an advantage. I think you should avoid having a monolithic core that does it all and instead have smaller modules that implement each systems in isolation. For example there's no need for a worker manager to know about VTK or images.
That also means it is easier to find, patch and update a specific system if it is compartmentalized, and having it in it's own module discourages the use of intuitive patterns like silently augmenting a class instance with new members.

You could then have a @cornerstone3d/core that is built from @cornerstone3d/web-worker-manager, @cornerstone3d/rendering-engine, @cornerstone3d/request-pooling and so on.

It makes the code easier to read, to reason about and create a "preferred way of implementing new stuff"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants