-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Add DICOM SR Support for additional/common annotations #1215
Comments
There are 2D (image) relative coordinates in DICOM SR, and 3D (frame of reference e.g., patient, volume, slide). All of Graphic Types specified for those that are co-planar with a referenced image orientation should be supported (3D types that are not in the same plane as the image would require volume "awareness", which may be out of scope for now): 2D: POINT, MULTIPOINT, POLYLINE, CIRCLE, ELLIPSE See: http://dicom.nema.org/medical/dicom/current/output/chtml/part03/sect_C.18.6.html In DICOM SR TID 1500 (which is the template to focus on), see examples of use in these templates: http://dicom.nema.org/medical/dicom/current/output/chtml/part16/chapter_A.html#sect_TID_1410 Examples at: http://dicom.nema.org/medical/dicom/current/output/chtml/part17/sect_RRR.5.html David PS. Note that "generic" SR annotation rendering (as opposed to authoring) code may theoretically "ignore" what template is defined to be or appears to be used, and just search the tree for any SCOORD or SCOORD3D content item, and then explore the local neighborhood for children that are images, and parents that describe its context (i.e., any associated description, identification, measurements, etc.). That approach is robust and can support all sorts of SR templates and applications from the generic TID 1500 to Mammography CAD, as well as private templates. |
Agree "arrows" should be single point. |
Thanks the input David, I thought as much for the arrow. Yeah the inital scope for this issue would be the in plane annotations that cornerstone supports by default. Out of plane annotations as well as TID 1411 (?, off the top of my head as I'm on a train, the volumetric measurements for ROIs) will come down the line as separate issues. |
This is a good note. When our application is not the author of the report, and without additional assurances/constraints, this may be the best we can do without introducing hazards. OverviewCurrent approachWe parse and render the contents of a DICOM SR by doing the following: import * as dcmjs from 'dcmjs';
const { DicomMetaDictionary, DicomMessage } = dcmjs.data;
setContentFromByteArray(byteArray) {
const arrayBuffer = byteArray.buffer;
const dicomData = DicomMessage.readFile(arrayBuffer);
const dataset = DicomMetaDictionary.naturalizeDataset(dicomData.dict);
dataset._meta = DicomMetaDictionary.namifyDataset(dicomData.meta);
const mainData = getMainData(dataset);
const contentSequence = getContentSequence(dataset);
const content = (
<>
{mainData}
{contentSequence}
</>
);
this.setState({
content,
});
} A Issues with current approach
Goals
Blockers
PhasesThis task is too large to tackle in one go. Here are some reasonable milestones we can use to iteratively add value: Shift where/how we query and store DICOM SR informationThe MeasurementService works well for our critical path, but it doesn't encapsulate all of the information contained in an SR and it's not immutable. We likely want to extend the existing
Display DICOM SR measurements in a table and on a 2D canvasThe simplest approach here is likely what @dclunie outlined above. We may want to limit the (initial) supported measurements to SCOORD as they'll have a reference image and are easier to map to Cornerstone's
This may sound more straightforward than it is. It's worth looking over the various templates and how they group measurements. We'll need to account for related measurements that are linked by UID (and span measurement groups), as well as a few other scenarios. This may impact how we display information in our table. Some measurements will explicitly map to a POINT or POLYLINE, while others are implied or derived from neighboring information. Notes:
When a TID 1500 Report originated from a supported OHIF version/schema, map to edit toolsWe currently accomplish this by keying off of a Measurement's "Code Meaning". For example, we encode a POLYLINE that has a CodeMeaning of The TID 1500 includes Author Observer Sequence. This would allow us to encode information about the application that generated the report and to make further assumptions about the encoded data. For example, if we see that there is an Author Observer (there may be a better field for this) that has:
We may be able to reasonably assert that because our application supports Application Y and Software Version X that the encoded measurements are no longer generic, but rather, map The biggest change here is identifying what information we key off of, and how we might go about supporting schemas as we continue to version/grow what we do support. Ideally, this would be extensible as we grow this to support multi-step and composite tools, but I'm likely getting ahead of myself.
This may sound more straightforward than it is. There are several "gotchas" that may not be immediately obvious. For example: There is no "rectangle" value type. A rectangle ROI would be encoded as a POLYLINE. Rendering a POLYLINE is simple enough, but if we need to know this should map to a "rectangle tool", we would need to look at the points more closely to identify that. Knowing when a square POLYLINE maps to a square tool, a rectangle tool, or a freehand roi tool is an interesting problem. POLYLINEs are only "closed" if the first point is the same as the end point. POLYLINEs do not guarantee a "winding" order. You cannot assume that the points are read from left to right, or right to left. This makes encoding an "arrow" with an end point an interesting problem. We may need to use a single point (end point), and a magnitude/direction Support for persisting tools outside the scope of primitive value types (far future)We will likely need to create a custom "Code Scheme Designator", "Code Meaning"s, and "Code Value"s when persisting tool state for tools that can't be encoded using a single value type. A GitHub repository (and npm package) can describe these concepts, and create an avenue for other systems that would like to consume the data beyond simply rendering/listing the values. I am currently unsure of how we might provide a path to add these dynamically or ad-hoc through extensions. Interoperability beyond our application may only be possible with published definitions and a concerted effort. Not DiscussedSCOORD3D SupportWhile we have a frame of reference tool state manager in cornerstone tools, it's not commonly used. For most measurements (created in OHIF), we'll want to tie them to a specific instance. As we broaden VTK annotation support, SCOORD3D support will becomes more pressing. Pathway for rendering w/ VTK Viewports... Automatic mappingWhile we could make optimistic guesses to map SCOORD and SCOORD3D to edit tools, the outcome is likely too hazardous to pursue. A future enhancement could be made to allow manually transferring the state to a tool that supports that same general schema, but with a very blatant warning. If there is an obvious pattern where MANUFACTURER maps to these tools, we could build this in as a more robust feature where users can setup the automatic mappings. Akin to using ImagerPixelspacing or calibrating an image's pixel spacing. "You can do this, but at your own risk." I am doubtful we will have the bandwidth to tackle this anytime soon, but I believe it's worth listing as it may catch the right financier's eye <3 Parting WordsPlease feel free to push on any/all of this. This is my best guess at our path forward with a few sparse implementation details peppered in. I'm fairly new to DICOM SR, and curious to hear how other implementers are tackling these same problems. Note: I've created a |
Just so we are on the same page, the HTML render will still be available, just accessed from elsewhere?
Duplicating the tools is overkill I reckon? We could add a locked state for all toolState that makes them immutable. This may be too intrusive to CST (unless this a feature others would value?). I think the “SR measurement render only tool” you suggested is better.
I feel like this is a mix of concerns that will only confuse intergation, and is better left as a separate ticket/project later.
Ideally yes, if they contain different data and not just a “corrected” version, but how could we possibly work this out? :thinking_face: . I don’t think we can. Maybe we have to limit to only one SR per series (even that is a little complicated as SRs can reference lots of images from a study I believe). Of course other than a PWA, a more complicated backend could triage time versioned SR.
I think the OHIF standard we make must be consumed and added to a Code Scheme library. An extension could add its on CSD + definitions to this library (e.g. an extension adds a tool and how to store it?). |
The HTML Viewport Extension would use the Improved programmtic control of the Viewport Grid and Viewports is something we have slated.
csTools.addTool(csTools.LengthTool, { name: 'DicomSrLength' });
csTools.setToolEnabled('DicomSrLength'); But I agree. I think a render only tool would be cleaner.
I agree. A pipe dream of mine.
👍 |
Regarding the way we display the SRs and how to get old SRs
As SR is also used for more things other than just measurements, we could filter the DICOM SR with measurements from thumbnail, maybe using
We could have a runtime load of a new SR, similar to what we have on
Now a days, we use dcmjs and store one SR per Study, doing one by series, I don't personally don't see any gain on having it by series. Regarding the way we display the tools, I like the idea of render only tool. Have you guys thought about tool versioning and synchronization with what we have on a DICOM SR? I mean, what if the data we stored in the past, new app does not know how to render anymore, like the structure has changed.. |
Wow, that's a lot of stuff guys! I have to say I could not follow all of the points in the discussion due to lacking understanding of the OHIF internals perhaps. My overall suggestion would be to consider focusing on specific use cases, find a very specific customer that will be invested in using this functionality and provide feedback to guide development. Just one TID 1500 template has so many capabilities and usage flavors that even exhaustively covering those is a huge project! On a positive side, I guess (considering the "I am doubtful we will have the bandwidth to tackle this anytime soon" comment!), planar annotations are not an immediate priority for the IDC project. Our current target is development of the MVP (see our plans in the IDC MVP document linked from here), which will provide static view of a subset of TCIA collections. None of those collections (with perhaps the exception of the annotations produced by CCC for some of them) have planar annotations. It is much more important for IDC to be able to handle measurements and SR objects derived from volumetric segmentations - plenty of datasets of that kind, and more on the way. I promised to @pieper and @swederik that I will submit all issues that I encountered and also create issues for functionality we need for IDC MVP. We can discuss those after that in a call to hopefully bring everyone to the same page. |
Also just for context: the effort to serialize and deserialize OHIF annotations is not just for IDC, although there's great synergy with the efforts to use SR for this purpose especially given the guidance from @dclunie 's white paper. Currently we have IDC:candidate and IDC:priority labels, but maybe this would fall into something like an IDC:collaboration category (don't want to invent too many labels, but if needed we can). |
@pieper I agree, and adding "IDC:collaboration" makes sense to me. |
Its not a bug itself in the implementation, but in how the data is presented in Measurement Panel. @fedorov , correct me if i am wrong, but the want to filter the SRs in the panel to the ones related to the active viewport, right? |
Scenarios: Given I'm a user So the idea here is to parse the SR, display the measurements in the measurement panel (the exact number of measurements present in the SR, no duplication allowed) and when the user clicks on a given measurement, that measurement is displayed on the active viewport (the viewer jumps to that slice). This means that the measurement could be rendered on any series if it shares a frame of reference and is close enough to a particular slice given a tolerance value. |
Thank you @igoroctaviano - this is exactly correct. |
This scenario has a problem, if there is no slice near to the SCOORD3D points in the active viewport, nothing will show. What if we change this scenario a little bit, by filtering the Measurements Table based on the current active viewport. If there is annotations they will be displayed, if not the measurements panel will be empty. |
For now, we can focus on the happy path and later decide on the case you mentioned. For the future, If the measurement is not fit for the active viewport it's better to have it disabled (grayed out) than filtered out. This way the user knows that the content of the SR is there but it's not valid because of reason X. Maybe later we could even allow the user to change/tune the tolerance value but this is all future things. |
Yes, I agree with @igoroctaviano. Showing a subset of items from SR based on what can be visualized would be very misleading.
Instead, we could explore a variety of other ways to communicate this to the user (in addition to disabling, as suggested by Igor):
I would implement the easiest of those ideas for now, and revisit later. |
Are you updating a different env? I tried with https://d3w0jbfdn2j3pj.cloudfront.net/projects/idc-sandbox-000/locations/us/datasets/idc-dicom-test-inventory/dicomStores/test-samples/study/1.3.6.1.4.1.14519.5.2.1.7310.5101.860473186348887719777907797922 First series (measurement not active for viewport)Second series (blank / no measurement although it jumps to a frame)Third series (measurement not active for viewport) |
Just series 6, 8, 9 has measurements. For me it goes ok, though I saw clicking with no measurement display, and I click again and it is there. Maybe its a bug in OHIF, but can you try these series and if there is no annotation displaying try again clicking in the same annotation |
@fedorov is this correct? I thought the measurements should be rendered for other planes too. |
They are translated from SCOORD3D to SCOORD. Each SCOORD3D can generate multiple SCOORD annotations, depending on the proximity of the slice |
I do not understand what this means. Translated by whom and when? |
@fedorov I'll have a meeting with @rodrigobasilio2022 to discuss this. We'll get back to you ASAP. |
Thank you Igor. @rodrigobasilio2022 the DICOM data received from the server must be shown as received from the server in the DICOM tag viewer. It is absolutely unacceptable to modify and present modified data in the DICOM tag viewer! The details of how you manage internal to the implementation data structures must be completely separate from the DICOM data representation. |
Understood. I will change the implementation. |
Per discussion today, we will test this once a sandbox instance with these features is deployed. |
Dears, any updates on this? |
@dclunie I hope you remember this issue. Why does Arrow need to be "single" point" rather than "two points"? Because of this, everytime changing viewport (resizing), arrows are randomly rotated.. Or is there any logics behind it? |
Because SR is about semantics, not appearance. I.e., for an arrow, what matters is what is at the tip, not the direction of the arrow. |
@dclunie Thank you for the prompt response :) Yes, but in my case, a user saves the status with the screenshot, and they load next time. Their expectation is that the arrow should be loaded exactly as they saved. And I think that makes sense as well. And I think if I refer how the line (length) is saved as SR, probably I can edit how arrow is saved and may achieve what I expect. What do you think? |
@seandoyle FYI seems like a response to our recent discussion |
I agree with David here about the interpretation of the SR spec. I'm not sure how the data could be encoded in the SR that is consistent with it being a point. Another approach would be to keep it as a point in the SR but somehow have rendering rules that constrain the presentation but I don't have any solid ideas on what these rules would be. Right now if draw an arrow that is pointed up at a 45 degree angle from the horizontal it will be displayed at roughly 45 + 90 degrees when restored from the SR (the point that is being pointed at by the arrow is correct). The only way I can see making this consistent is that when the user has entered the text for the arrow that it automatically moves to the location constrained by the rendering algorithm. This would make it consistent when the SR is re-rendered. But users will find it startling that their arrow moved. |
I think I found a way to fix this arrow issue and made the following PRs several weeks ago. Fortunately, A point for TID300 can store additional point, which makes the implementation easier. If you can find some issues in the following PRs, please let me know :) |
I think on first pass we should support:
@pieper has kindly provided some guidance on how to approach these mappings in a discussion with @galelis, @JamesAPetts, and @dannyrb
@galelis, if you transition off this issue, please provide insights/next-steps in a follow-up comment.
The text was updated successfully, but these errors were encountered: