 |
Casey: are you available to work on this bug? If not, can Dominik take it? |
 |
If Dominik can take this that would be great. Thank you |
 |
@Dominik: Could you have a look at this in the coming week ? If not we will re-target for 4.4 release. Thanks |
 |
I could not replicate the reported issue about the fault registration matrix or the issue with clearing the inputs. I did discover a new issue with the markups CLI interface that I will investigate further. |
|
|
Before the markups were merged [1], the fiducial registration was working well for me (tested on a version of slicer nightly from June 19 2013), so I think the original issue has been fixed. However, now the issue is that the WriteCLI method [2] is returning all fiducials in a markup list in a single string. So the command line looks right, but actually each command line argument should be a different string in the commandLineAsString vector of strings. If you look at the implementations of the two methods [3] you can see the difference. If I print the arguments from inside the CLI module like this: for (int i = 0; i < argc; i++) PARSE_ARGS; The output is: 0: /Users/pieper/slicer4/latest/Slicer-superbuild/Slicer-build/lib/Slicer-4.2/cli-modules/FiducialRegistration I would say the right solution for this is to refactor a bit and make the signature of WriteCLI accept a vector of strings as the first argument and then it should push back strings onto that list to represent the command line arguments of the node. [1] https://github.com/Slicer/Slicer/commit/f834581be82c1a09d2e74b146294965e696faa9d [3] https://github.com/Slicer/Slicer/blob/master/Base/QTCLI/vtkSlicerCLIModuleLogic.cxx#L1410-1454 |
|
|
From the mailing list, from Brad: https://github.com/Slicer/Slicer/blob/master/Base/QTCLI/vtkSlicerCLIModuleLogic.cxx#L1410 I didn't see how the marksups WriteCLI method even get's this variable. Does it pass all point to in the Markups node? or did I miss something? Should be low impact enough while refactoring for this bug to add a singlePoint flag to WriteCLI. |
|
|
Proposed fix: |
|
|
Tested on Ubuntu 10.04 - Here is the output after I added the debug statement in the FiducialRegistration.cxx: slicer:0x7fdcb22d83a0 --processinformationaddress 0x765d850 --returnparameterfile /tmp/Slicer2/28553_f1OVqjoHRQ.params --fixedLandmarks -25.7469,-9.46105,-10.2143 --fixedLandmarks 6.6209,-44.7714,-10.2143 --fixedLandmarks -35.7515,37.0309,-10.2143 --movingLandmarks -25.7469,-9.46105,-10.2143 --movingLandmarks 6.6209,-44.7714,-10.2143 --movingLandmarks -35.7515,37.0309,-10.2143 --saveTransform /tmp/Slicer2/CIFFD_vtkMRMLLinearTransformNodeE.txt --transformType Rigid 0: slicer:0x7fdcb22d83a0 |
|
|
Should the flag and the value should be on separate lines? When I put in the debugging statement and run the test I get: EDIT (Jc): Having one argument per line is right 475: 0: ModuleEntryPoint While from the GUI I get the same as JC posted: Putting a print statement after the PARSE_ARGS command to print out the fixed landmarks (just the x coord), running the test I get valid points: Running from the GUI, I also get valid points: Looks like PARSE_ARGS is smart enough to deal with both cases, but in vtkSlicerCLIModuleLogic, it does add the flag and the parameter as separate strings. I'm updating the WriteCLI implementations. |
|
|
Rats, wasn't thinking and squashed the two commits so I lost comments on the original one, sorry! Here's the updated topic: |
|
|
Where did you put your debug statement to get the two output reported in note c9721 ? |
|
|
Before and after PARSE_ARGS: PARSE_ARGS; for (int i = 0; i < fixedLandmarks.size(); ++i) |
|
|
Fixed in svn 22369: |
|
|
Closing resolved issues that have not been updated in more than 3 months |
- Anonymous
