-
Notifications
You must be signed in to change notification settings - Fork 54
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
WIP: Dicom Parsing Example Proof-of-Concept #252
base: main
Are you sure you want to change the base?
Conversation
4b26014
to
25d535e
Compare
03c6006
to
c41ac4c
Compare
c21b417
to
655c321
Compare
@pieper would you be able to show me where in dcmjs or OHIF are the instance tags extracted to generate the data necessary for volume visualization (pixel data, spacing, origin, orientation, dimensions, etc...)? This is how it is done in Girder, leveraging daikon instead of dicom-parser: DicomViewer.vue. It's done slice per slice however (one image data per slice), and there is no 3d space/ z-depth considered. I believe Slicer does some of that here. I am asking because the work in this branch can be used to organize patient/study/series and extract the files for a specific series, which are then passed to ITK when are once again read and parsed by DCMTK (with emscripten). Ideally, we would avoid reading and parsing them twice, and just extract the data that matters from the series instead of the list of files: this could then be used as a module by ITK, VTK, Girder, even dcmjs and OHIF if there is any use for it. |
39783ea
to
972ce66
Compare
Hi @agirault - It's a complex topic, as you know! Especially the Slicer part, since many cases are handled. Happy to talk if over if that would help. Yes, for sure we should be treating all of this consistently and efficiently and I appreciate the effort you are putting into it. We typically rely on the server to sort patient/study/series in the database (e.g. https://github.com/dcmjs-org/dicomweb-server uses CouchDB and Slicer uses SQLite via CTK https://github.com/commontk/CTK/tree/master/Libs/DICOM/Core.) Once you have the instances for a series, in dcmjs the mapping to volumes is here: https://github.com/dcmjs-org/dcmjs/blob/master/src/normalizers.js The idea is that a "normalized" dicom is a multiframe conversion, often referred to as a 'dataset' in the code. This has all the image data in an array that is effectively exactly what ITK and VTK expect as pixel arrays. Here's the code to map the dataset to a vtkImageData, which is simple as you can see. https://github.com/dcmjs-org/dcmjs/blob/master/examples/vtkDisplay/index.html#L448-L475 I'm going to put on my -Steve |
Hi @pieper. Thanks a lot for sharing this.
Indeed, it seems dcmjs does it right, and I don't want to reinvent the wheel so I'll hold off on continuing implementing this in this current example.
We need to be able to do this sorting client-side since in our use-case the user uploads DICOM files and selects a series that will be rendered on the client, and eventually sent over to the server afterward. My current proof-of-concept in this branch achieves that by leveraging Also worth noting that the current implementation I have in this branch is about 6x (with reslice intercept and/or slope) to 17x faster (without reslice) than the ITK |
@agirault sorry for the slow reply, I was out last week. Regarding the browser-centric workflow I entirely agree. I'd still stick with the database approach to organizing the series though, for consistency. We haven't fleshed it out yet, but I would like to see the same dicomweb-server code applied via PouchDB so that the 'client' side of the browser can use DICOMweb API calls even though it is talking to a local facade. I agree that doing this in native javascript is a better idea than using cross-compiled C++. Not just for speed, but also because there are a lot of use cases for DICOM that still need work to represent well and there's no reason to limit ourselves to what has been implemented for imaging in the past. We can address more general quantitative imaging tasks like segmentation and annotation more cleanly using the dcmjs approach. Would you be interested in having a chat sometime to compare notes? There are several projects that probably share some core needs and maybe we can make progress together. |
b9ab3c6
to
96ebc58
Compare
96ebc58
to
4c43509
Compare
Updated to speed up rescaling and parsing (less function calls, more classic loops), thanks @floryst for the suggestions. |
ca29579
to
d91f87f
Compare
Then provide files to ITK to read the DICOM.
Not when instantiating a DICOMEntity class
TODO: update the tag dictionary to associate the tags with a module (patient, study, serie or image), to automatically pull in all the tags for each module, instead of manually listing the tags to extract (currently that case in the DICOMEntity classes)
Limitations: - does not support compressed/encapsulated pixel data (see non-raw dicom transfer syntaxes, ex: JPEG2000) - assumes slicethickness is spacing - harcode pixel type and dimension in imageType
const direction = { | ||
rows: 3, | ||
columns: 3, | ||
data: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, parsing the same tags and using this code, direction.data ended up being: [...iDirCos, ...jDirCos, ...kDirCos]. Just a heads up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WillCMcC thanks for the note!
In itk-wasm, a direction
ordering issue was addressed -- it may have addressed this.
For compatibility with DICOM, AWS, etc, the metadata could be structured similar to the ImageSet metadata described here: |
Early work in progress showcasing an example that parses and organizes DICOM instances before reading them with ITK.
Opening this PR mainly to enable discussions. I might create a new one once we have a clearer picture of how this will get integrated into ITK (in-source vs npm dependency),
dicomParser
vsdaikon
, etc...Related discussions: itk discourse, #250, #251
cc: @thewtex, @floryst, @zachmullen, @pieper