View Issue Details

IDProjectCategoryView StatusLast Update
0001914Slicer4Module Annotationspublic2017-06-10 08:51
Reporterliuy5 Assigned Tojcfr  
PriorityurgentSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product VersionSlicer 4.1.0 
Target VersionSlicer 4.4.0Fixed in VersionSlicer 4.4.0 
Summary0001914: fiducial points misplaced after switching layouts
Description

Please see attached video

TagsNo tags attached.

Relationships

related to 0001690 assignednicole fiducial shows in wrong lightbox cell 
related to 0001959 closedmillerjv Slicer view messed up after sequential layout switching 
related to 0001742 closedmillerjv autosizing behavior in slicer 4 is not good at all 
related to 0001546 closedmillerjv adjusting images ater loading and reconfiguring leads to unexpected results 

Activities

2012-04-17 07:18

 

fp.ogv (1,302,137 bytes)
nicole

nicole

2012-08-27 15:40

administrator   ~0005819

Tried observing the modified event on the layout node, but it's getting a slice node modified event after that and it's not working quite right.

nicole

nicole

2012-08-29 13:53

administrator   ~0005864

Last edited: 2012-08-29 13:56

Using these steps to reproduce:
download sample data MRHead
Place a fiducial on the red slice
Right click to zoom and move the slice, fiducial moves correctly.
Switch to red slice only layout, fiducial position is okay
Switch back to conventional layout, fiducial position is wrong in red slice viewer.
If I then update the conventional view red slice with right click and drag, the fiducial gets updated.

Digging into the code, the position does get updated, but it's not reflected in the slice viewer. I'm seeing two calls to OnMRMLSliceNodeModifiedEvent that call UpdatePosition, but only the first call is resulting in a changed position. The second call sees the position of the seed widdget in screen space and the fiducial mrml location in display coordinates as teh same.
I suspect that using the slice nodes to differentiate the widgets for each viewer might be failing when a second red slice viewer has been created - I think the red only one is the one that's getting updated invisibly, while the conventional view red slice viewer is seeing the same widget and not updating it's position for a different FOV. When I explicitly change the view on the conventional layout slice node via right click and drag, the fiducial displayable manager correctly updates the fiducial positoin. I'm still trying to confirm this, then to fix it.

nicole

nicole

2012-08-29 14:29

administrator   ~0005868

Re: last note: don't think I have the root cause, I think now that it's due to multiple calls to UpdatePosition because both OnMRMLDisplayableNodeModifiedEvent and OnMRMLSliceNodeModifiedEvent are called when the layout changes, but when the user does a right click and drag, I only get OnMRMLDisplayableNodeModifiedEvent triggered.

nicole

nicole

2012-10-23 16:21

administrator   ~0006714

Happens with fiducial and ruler, but not ROI (linux64 debug build).

millerjv

millerjv

2012-10-25 05:06

developer   ~0006731

Last edited: 2012-10-25 05:08

Fiducial and ruler worked fine on my Mac.

millerjv

millerjv

2012-10-25 05:14

developer   ~0006732

Setting a priority on the events observed in the annotation displayable managers may help. Try setting the priorities low, e.g. -1, so that all the everything else receives the events before the annotation. This may ensure that the SliceNode and window geometry are synchronized before the annotation display coordinate is updated.

I had to put a priority on the MoveEvent in the CrosshairDisplayableManager to get the crosshair to display reliably in the Slice Viewers other than the one being interacted with. In my case, the Crosshair RAS was being updated before other Slice Nodes were updated, so the logic would turn off the Crosshair since it was not "on" the current Slice. Setting the crosshair priority low, the SliceNode would update its position before the Crosshair responded to its change in position.

nicole

nicole

2012-10-25 07:04

administrator   ~0006740

I've been looking into this. The annotations displayable managers respond to slice node modified events, but that's set up in the superclass with no priorities. Seeing if I can get at the observer and adjust the priority on it after the manager is registered.

millerjv

millerjv

2012-10-25 09:07

developer   ~0006743

You could add a priority ivar to the superclass that uses the default value. Then in the subclass, initialize the ivar from the superclass so that when the observers are added they would use the new value.

nicole

nicole

2012-10-25 10:44

administrator   ~0006750

Doesn't seem to have fixed the problem this problem.

jcfr

jcfr

2012-10-25 12:08

administrator   ~0006757

ENH: Easier re-use by adding static sliceView DisplayableManager "Convert" methods

See r21245 / http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=21245

nicole

nicole

2012-10-26 17:12

administrator   ~0006826

The double calls to OnMRMLDisplayableNodeModifiedEvent are caused by the method
qMRMLSliceControllerWidget::setSliceViewSize
calling ResizeSliceNode on the vtkMRMLSliceLogic. That very properly does a Start/End Modify around setting the FOV and Dimensions on the slice node, but the problem is that when EndModify is called, it triggers a modified on the slice node which triggers another call to UpdateMatrices which triggers a modified again (I think).
There's also an issue that there are two paths of modified events happening when the layout is changed and the new widgets are shown:
ctkVTKSliceView::resized
is also triggering modified events.

nicole

nicole

2012-10-26 17:58

administrator   ~0006828

Since working on Mac, which will do the bulk of RSNA demos, retarget for 4.3

nicole

nicole

2013-10-03 12:30

administrator   ~0010126

Test checked in as svn 22587:
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=22587
(the test passes even when it shows the wrong fid location, so as to keep the dashboard green while I continue to debug the problem)

jcfr

jcfr

2014-03-07 21:29

administrator   ~0011394

The following topic is solving the problem:

 https://github.com/jcfr/CTK/tree/slicer-1914-fix-ctkVTKSliceView-resize-event

Now improving the proposed solution to attempt to fix 0001690

jcfr

jcfr

2014-03-10 14:31

administrator   ~0011409

Fixed in r22938
See http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=22938

nicole

nicole

2014-03-19 09:02

administrator   ~0011449

Tested the 2014-03-14 nightly build on Linux and it's fixed there. Running the FiducialLayoutSwitchBug1914 test:
Difference between starting and ending seed display coordinates = 0.980968
Mean of image difference = 15.9848
Image difference is too great!
Expected <= 5.0, but got 15.9848

The fiducial was placed at:
33.4975, 79.4042, -10.2143
mousing over, it doesn't visually move, but checking in the Markups module, the position at the end of the test is:
33.934, 79.408, -10.214

The test when run on my Mac nightly build from the command line is still reporting a difference in starting and ending fiducial display coordinates:

564: Starting seed display coords = 162, 14, 0
564: Fiducial starting position
564: Red Slice only
564: Conventional layout
564: Ending seed display coords = 156, 15, 0
564: Fiducial ending position
564: Difference between starting and ending seed display coordinates = 6.57724
564: Display coordinate difference is too large!
564: Expected < 1.0 but got 6.57724

nicole

nicole

2015-05-13 07:54

administrator   ~0013046

Added a further check to the test (and have it fail on the dashboard properly) to compare RAS positions if display positions are not close enough (resetting the layout can change the FOV):
http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&amp;revision=24251

Related Changesets

Slicer: 2145-support-for-installing-extension-from-file ef7d57e7

2013-10-03 16:29:15

naucoin

Details Diff
BUG: adding a test to show layout switch bug

Fiducial display positions aren't updated correctly
on linux according to bug 1914. This test illustrates this in a
non failing test way so that cross platform behaviour can be
determined.

Issue 0001914


git-svn-id: http://svn.slicer.org/Slicer4/trunk@22587 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Applications/SlicerApp/Testing/Python/CMakeLists.txt Diff File
add - Applications/SlicerApp/Testing/Python/FiducialLayoutSwitchBug1914.py Diff File

Import 2017-06-07 23:51:09: master 02ca2b51

2014-03-10 18:29:18

jcfr

Details Diff
BUG: Update CTK to fix wrong location of fiducials after switching layout.

Jean-Christophe Fillion-Robin (1):
Emit ctkVTKSlicerView::resized after vtkRenderWindow size is set.

Fixes 0001914 - See http://na-mic.org/Mantis/view.php?id=1914

git-svn-id: http://svn.slicer.org/Slicer4/trunk@22938 3bd1e089-480b-0410-8dfb-8563597acbee
mod - SuperBuild/External_CTK.cmake Diff File

Import 2017-06-07 23:51:09: master e4e89922

2015-05-13 11:27:31

naucoin

Details Diff
BUG: update fiducial layout test to check RAS

The former version of this test was set to always pass
since there was still a > 1 pixel difference in starting
and ending fiducial display coordinates. Brainstorming
on the slicer dev hangout brought up the possibility
that since the fiducial RAS position was still correct,
the layout switching might have included some FOV adjustments
to fit the volume to the slice widget. This change will
go on to check the fiducial RAS position against the
volume RAS position if the display positions don't match.

Update the layout switching test to:
- fail on the dashboard if it fails
- show all output for debugging
- check both display coordinates and RAS coordinates
- only fail if both are out of tolerance
- remove the redundant delayDisplay methods and use the slicer.util
version with a shorter time out to speed up the test run time

Issue 0001914

git-svn-id: http://svn.slicer.org/Slicer4/trunk@24251 3bd1e089-480b-0410-8dfb-8563597acbee
mod - Applications/SlicerApp/Testing/Python/FiducialLayoutSwitchBug1914.py Diff File

Issue History

Date Modified Username Field Change
2012-04-17 07:18 liuy5 New Issue
2012-04-17 07:18 liuy5 Status new => assigned
2012-04-17 07:18 liuy5 Assigned To => nicole
2012-04-17 07:18 liuy5 File Added: fp.ogv
2012-05-01 04:44 liuy5 Relationship added related to 0001959
2012-08-20 10:25 nicole Severity minor => major
2012-08-20 10:25 nicole Target Version => Slicer 4.2.0 - Feature freeze Sept 1st 2012
2012-08-20 12:01 jcfr Relationship added related to 0001690
2012-08-27 15:40 nicole Note Added: 0005819
2012-08-29 13:53 nicole Note Added: 0005864
2012-08-29 13:56 nicole Note Edited: 0005864
2012-08-29 14:29 nicole Note Added: 0005868
2012-09-18 12:29 jcfr Summary fudical points misplaced after switching layouts => fiducial points misplaced after switching layouts
2012-10-23 16:21 nicole Note Added: 0006714
2012-10-25 05:06 millerjv Note Added: 0006731
2012-10-25 05:08 millerjv Note Edited: 0006731
2012-10-25 05:14 millerjv Note Added: 0006732
2012-10-25 07:04 nicole Note Added: 0006740
2012-10-25 09:07 millerjv Note Added: 0006743
2012-10-25 10:44 nicole Note Added: 0006750
2012-10-25 12:08 jcfr Note Added: 0006757
2012-10-26 17:12 nicole Note Added: 0006826
2012-10-26 17:58 nicole Note Added: 0006828
2012-10-26 17:59 nicole Target Version Slicer 4.2.0 - coming release => Slicer 4.3.0
2013-07-09 09:01 nicole Target Version Slicer 4.3.0 => Slicer 4.4.0
2013-09-02 19:01 jcfr Target Version Slicer 4.4.0 => Slicer 4.3.1
2013-09-24 12:44 jcfr Target Version Slicer 4.3.1 => Slicer 4.3.2
2013-10-03 12:30 nicole Note Added: 0010126
2014-03-06 10:15 nicole Target Version Slicer 4.3.2 => Slicer 4.4.0
2014-03-07 09:44 pieper Priority normal => urgent
2014-03-07 21:29 jcfr Note Added: 0011394
2014-03-07 21:31 jcfr Assigned To nicole => jcfr
2014-03-10 14:31 jcfr Note Added: 0011409
2014-03-10 14:31 jcfr Status assigned => resolved
2014-03-10 14:31 jcfr Fixed in Version => Slicer 4.4.0
2014-03-10 14:31 jcfr Resolution open => fixed
2014-03-19 09:02 nicole Note Added: 0011449
2014-07-30 14:37 jcfr Relationship added related to 0001742
2014-07-30 14:40 jcfr Relationship added related to 0001546
2014-09-17 22:57 jcfr Status resolved => closed
2015-05-13 07:54 nicole Note Added: 0013046
2017-06-07 23:27 Changeset attached => Slicer 2145-support-for-installing-extension-from-file ef7d57e7
2017-06-10 08:51 Changeset attached => Slicer master e4e89922
2017-06-10 08:51 jcfr Changeset attached => Slicer master 02ca2b51