 |
@Jim: Could you have a look ? Thanks |
 |
The original design was to support multiple coordinate systems. But the implementation became Slicer specific with an implicit assumption of RAS. It is something that we have intended to revisit. Nicole want to revisit how we communicate points to a CLI, coordinating with that effort would be a good opportunity. |
|
|
@Jim: Thanks for the quick answer. @Yves: Assuming Jim or Nicole provides you with some guidance, would you be willing to help addressing this problem by contributing a patch ? |
 |
If I can be of any help. Can you tell me roughly where to look for in the Slicer code? The plan would be to convert to the desired coordinate system when the command line is generated? I was wondering if it made sense to pass landmarks in a coordinate system when the data is in another? |
|
|
It is tricky. Basically Slicer does everything in RAS. But ITK does everything in LPS. When images are written to disk, Slicer converts its coordinate frame into a a coordinate frame compatible with the file format. When an CLI based on ITK reads that data in, the CLI converts the data to LPS. This same conversion occurs when sending images in memory to CLI as well. But for points, we don't have file format standards defining the coordinate frame of the serialized data. Originally we passed points on the command line. So we planned at attribute in the <point> tag to define what coordinate frame the plugin is expecting the data. Currently plugins receive points in RAS and convert them to LPS internally if needed. Here are some options:
We need to do 0000002 anyway, as we have modules that need to receive a collection of points larger than we want to put on the command line. But I expect for backward compatibility, we will maintain 0000001 is some capacity as it is useful for passing seed points and query points. Let's pull Nicole into the discussion as her plans for points will impact how we resolve this. |
 |
My plan is to move toward 2, using a file format that can pass multiple points. I think I prefer that Slicer does the conversion before passing the points to the CLIs so that they can just specify the coordinate system and then read the points without having to do a conversion. For reference, these CLI's currently use the point parameter and some are expecting LPS coordinates: |
|
|
Would it also make sens to associated the "coordinateSystem" attribute with image input/output also ? |
 |
Will this be fixable for the 4.3 release? Several people have asked about it. |
|
|
I was actually just working on a get markup point as LPS method late last week and was going to debug it today to get it into the Markups check in. The way I went was: Short answer: yes, for markup fiducials. |
|
|
Have the LPS option done locally to markups, but will postpone the IJK one, as it will rely on having an associated node id set for each fiducial point, looking up that volume and doing the RAS to IJK conversion on write/read. If IJK output is critical, I can work on it during the bug fixing phase. To have the CLI use the LPS functionality will work best if the coordinate system selection is defined on the vtkMRMLStorageNode level, otherwise I'm going to have to expand the WriteCLI call on the displayable nodes. There's also the to do point of expanding the point SEM type to support the fileExtension option to use the storage nodes to write files. Jim, can you weigh in? |
|
|
Added a coordinate system flag to the WriteCLI method and use it for annotation points, see the changes here: |
|
|
Support for LPS and RAS annotation/markup points is in the Slicer code base for the 4.3 release. |
|
|
Is this ready to move. |
|
|
Consider supporting --pointfile to write fiducials via file to CLI modules, and get them back, while still supporting individual command line --point (multi supported) for backward compatibility. Look at expanding PARSE_ARGS to do the reading automatically (as a backward compatibility option so that the points would be in the standard array)? If go the PARSE_ARGS way, deprecate the generation of individual points on the command line. The CLI developer would have to write it manually then though if they wanted to return changed points to Slicer. |
|
|
Slicer execution model changes: |
|
|
Thanks Nicole. Within the SlicerExecutionModel, could you add a simple test case to: https://github.com/naucoin/SlicerExecutionModel/blob/2979-support-for-cli-point-coordinateSystem-files/ModuleDescriptionParser/Testing/ModuleDescriptionTest.cxx |
|
|
Trying, but the WriteParameterFile method don't seem to be fully implemented in ModuleDescription. It just produces key = value lines for the top level parameter (though that matches what the Read method expects):
|
|
|
Did a simple module description parser test and a more extensive XML file test for point and pointfile. |
|
|
Execution model pull request: |
|
|
SEM pull request integrated. |
|
|
Slicer pull request: |
|
|
+1 Thanks Nicole. I added comment on the PR. |
|
|
Integrated as of svn 24459 |
- Anonymous
