Page MenuHomePhabricator

Image navigator should show image indices after re-init
Closed, WontfixPublic


Re-init should initialise the renderers in the image space. That is, even if the image is oblique, the renderer geometries should be sliced along the image axes.

Accordingly, it would make sense to see the original image indices in the image navigator for the selected voxel.

Event Timeline

espak created this task.Nov 15 2016, 8:19 PM
espak added a comment.Nov 16 2016, 4:01 PM

The code crashes because of T22122.

You can add a nullptr check for the renderer geometry as a workaround, or mark T22122 as a dependency of this ticket.

espak added a comment.Nov 16 2016, 6:32 PM

A bit smarter solution would be to use ctkSliderWidget for the index positions. ctkSliderWidget integrates a Qt slider and spin box in a single widget. The signals are connected, so that if you change the slider, the spin box changes accordingly and vice versa.

ctkSliderWidget also allows inverting the direction. It means that dragging the slider right or clicking on the up arrow of the spin box will decrease the selected value, and dragging slider left will increase it. (Same with key interactions.)

The idea is that you could invert the slider if the direction is inverted in the stepper. That means, dragging right would be like Stepper::Next() and dragging left would be like Stepper::Previous. In other words, you would move in terms of world coordinates.

Now, dragging right means Stepper::Increase() and dragging left means Stepper::Decrease() that is in terms of image coordinates.

And this would be possible without inverting the StepperAdapater, which means the displayed index would still be the same as in the image.

Note that there is a bit of confusion about naming. The inverted direction property of the stepper is relative to world space. The inverted direction property of the stepper adapter is relative to image space. It ensures that you see the image indices even if you created the geometry for the slice navigation controller in the other way round for some reason.

The inverted direction property of the slider widget should be set based on the inverted direction property of the stepper.

There could also be a preference for this. This might be the kind of stuff that people want to personalise.

I use the ctkSliderWidget in my project exactly for this and it works well.

Let me know if you guys want to see this feature. It is not a priority for us at the moment, but it would be a useful addition and it is easy to do.

espak added a comment.EditedNov 21 2016, 2:38 PM

I updated the PR with adding a nullptr check for now, and with more detailed comment.

It is ready to be merged.

This will be important after merging the PR for T22114, but it is safe for merge also before, as with global reinit nothing will change, and re-init is not working with flipped axes now.

hettich added a subscriber: hettich.Dec 5 2016, 2:33 PM

As a computer scientist, it's quite confusing for me to see values decreasing for one slider but increasing for the other ones (Pic3D.nrrd). Our designer agrees that this is a bad user interface experience. Can you please explain the rationale of this behavior? Your comments above are very technical but don't elaborate on the actual reason/advantage.

espak added a comment.Dec 5 2016, 4:37 PM

The aim is to see the original image indices in the image navigator. If there was global re-init, you are in world coordinate system, so nothing changes. Dragging the sliders from left to right always increases the index, and moves in the same direction in world. (Under 'same' I mean regardless how many and what type of data is loaded.)

However, if you do re-init, you are in image space, so I expect that you want to see original image indices, like what you see on the status bar, or like what you see in other viewers, like ImageJ or FSLView. 'Original image index' means the index in the reference geometry of the renderers.

Inverting the control is a bit different. If we accept that showing the reference geometry indices is the right thing, it has a consequence. Dragging the slider from left to right still always increases the index, but (depending on if the corresponding axis is flipped) it can mean different directions in world. E.g. dragging the axial slider right can mean going superior or inferior depending on if the axial axis is flipped. This can be confusing if someone just wants to scroll through the axial slices by the image navigator let's say from inferior to superior, and for certain images he/she has to drag the slider from left to right but for other images from right to left.

On the other side, I can imagine that someone cares only about the indices but not the actual directions. In this case inverting the controls can be confusing. I can imagine a user preference for this.

So this really depends on what the user is used to or what they expect. But since in MITK the images are rendered in the same orientation (not upside down if an axis is flipped), I assume this should be the same for the image navigator sliders, since this is a big advantage of MITK compared to simple viewers like FSLView.

If we do not accept that the image navigator should show the reference image indices then this issue should be closed as invalid or won't fix. In this case we will keep the changes on our fork.

espak added a comment.Dec 5 2016, 8:43 PM

I tested this again with the latest master and the indices look correct now. I tried images with flipped, permuted and rotated axes, the indices were the same as on the status bar. (After reinit. After global reinit they were inverted iff the corresponding image axis was flipped. This is correct.)

So this seems to be resolved.

The only addition of the PR that you might or might not want to take is the inverted controls feature using the CTK widgets. If you want that I can create a new task and send a new and clean PR with only that change. But this does not affect the correctness of the index coordinates, so this issue can be closed.

kislinsk closed this task as Wontfix.Dec 6 2016, 9:40 AM
kislinsk claimed this task.

Thank you for the explanation. We decided to keep the current behavior, though, as we believe this is a better user experience. Great to hear, that the Geometry issues are resolved!