nibabel icon indicating copy to clipboard operation
nibabel copied to clipboard

BUG: wrong origin location in orthoview, update OrthoSlicer3D._set_position in viewers.py

Open gjpcbecq opened this issue 1 year ago • 3 comments

correct a bug with image.orthoview() from class nibabel.viewers.OrthoSlicer3D

There is a wrong selection of indices from original data using the inverse affine transform, leading to weird selection of voxels

This bug is also related to strange behavior with special acquisition, for example with small animal settings such as rodents where PA and IS axis are transposed (LSA system), leading to wrong location of origin (0,0,0) with image.orthoview()

For example, this small example will generate wrong localization of the referential with the main code and will result in good representation with the proposed correction:

data1 = np.random.rand(20, 20, 40)
data1[10, 10, :] = 0
data1[10, :, 30] = 0
data1[:, 10, 30] = 0
aff1 = np.array([[1, 0, 0, -10], [0, 0, 1, -30], [0, 1, 0, -10], [0, 0, 0, 1]])
nib.viewers.OrthoSlicer3D(data1, aff1)

gjpcbecq avatar Apr 25 '24 14:04 gjpcbecq

Thanks for this! Would it be possible to get a test that clearly demonstrates the error before and is fixed after?

effigies avatar Apr 29 '24 13:04 effigies

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.35%. Comparing base (d15ec58) to head (cdc788d). Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1319   +/-   ##
=======================================
  Coverage   95.34%   95.35%           
=======================================
  Files         207      207           
  Lines       29481    29504   +23     
  Branches     4981     4982    +1     
=======================================
+ Hits        28110    28133   +23     
  Misses        930      930           
  Partials      441      441           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 29 '24 13:04 codecov[bot]

Thanks for this! Would it be possible to get a test that clearly demonstrates the error before and is fixed after?

Hi Chris, You can try this code in a module for example test_ortho.py:

import matplotlib
matplotlib.use('Qt5Agg') # or any functional backend. 
import matplotlib.pyplot as plt
import nibabel as nib
import numpy as np
data1 = np.random.rand(10, 20, 40)
data1[5, 10, :] = 0
data1[5, :, 30] = 0
data1[:, 10, 30] = 0
aff1 = np.array([[1, 0, 0, -5], [0, 0, 1, -30], [0, 1, 0, -10], [0, 0, 0, 1]])
o1 = nib.viewers.OrthoSlicer3D(data1, aff1)
o1.show()

This creates a 3D array containing random values, with more elements in the third dimension than in the others dimensions. Values for the axes of the frame of reference and origin at (5, 30, 10) are set to 0. The affine transform to the RAS system associates LR with dim 0, PA to dim 2 and IS to dim 1.

\mbox{Aff}_1 = 
\left(\begin{array}{cccc}
1 & 0 & 0 & -5 \\
0 & 0 & 1 & -30 \\
0 & 1 & 0 & -10 \\
0 & 0 & 0 & 1 \end{array}\right)

using OrthoSlicer3D the referential in green should override the black lines in the volume. The visual result of the use of this code is given in Fig.1 : a) with the initial version of the code, the green lines are not aligned with the black lines in the volume. b) with the proposed correction, the green lines are aligned with the black lines in the volume.

Fig.1

Fig.1

It seems to have other bugs in the OrthoSlicer3D that I try to fix. This will be proposed in other pull requests.

gjpcbecq avatar Apr 30 '24 12:04 gjpcbecq

Hi @gjpcbecq! I'm going to try to turn your MRE into a test.

Apologies for taking so long to get back to this. I'm commenting here to try to pressure myself to spend some time on it. :-)

effigies avatar Sep 05 '24 16:09 effigies

Okay, so I've had a go at reproducing this, and your fix resolves the initial placement at the origin, but if you attempt to click around to other locations with the mouse, you no longer go the place you click. Removing your patch makes the mouse work.

So I think there's something more fundamentally wrong here. It's possible that we just need to do the inverse of this in the _on_mouse() function... I'm going to spend a bit more time on this, but if you have a chance to look at it, feel free to take a stab.

effigies avatar Sep 05 '24 17:09 effigies

Thank you Chris for working on this. I was working how to check for the data in the axes for the test, but your solution is very nice. There is yet the problem with the _on_mouse function. I am working on it and I will propose you next week a solution in a new pull request with a test on the test_viewers.py to check it. Best regards, Guillaume

gjpcbecq avatar Sep 06 '24 15:09 gjpcbecq