MRview: ROI editing

[Lestropie]

Main capabilities:

• Handle multiple ROIs in a file list
• Assign colour to each ROI, either manually or random
• Transparency (per ROI?)
• Global on/off switch & individual ROI on/off switches
• Create ROI from template image (make sure to handle 4D images sensibly - perhaps if the template image is 4D prompt the user as to whether they want a 3D or 4D mask)

Editing:

• Adjustable brush size
• Adjustable 2D / 3D brush
• Adjustable square/circular (cubic / spherical) brush
• Extra buttons e.g. undo / redo, copy contents to slice above/below, fill function

Remaining / unresolved discussion (will do my best to condense what has been discussed in length on this topic): Need to decide how to properly handle editing of ROIs either when the view is not snap-focused, or when the ROI does not match the displayed image. Suggestions include:

• Making the ROI the main image, using the reference image as an overlay, and potentially modifying the render order so that the ROI is not 'underneath' the reference image
• Have a button in the ROI tool that snaps the camera position to the ROI image axes to allow editing; don't allow ROI editing if the ROI image does not align with the main reference image / this explicit alignment step has not been applied

[Lestropie]

Since we now have something working, I'll make a list of bugs / outstanding features that we can plug away at. Will try to keep this to what needs to be done before the master merge, wishlist items can wait.

1. 3D brush currently missing; since the rectangular drawer tool can't be 3D, could have 'rectangle' / 'circle' / 'sphere', with the last one being 3D. --> Just played with this, the whole UndoEntry struct is designed around 2D modifications, would need to store a 3D texture per entry to support 3D brushes. Not sure whether or not this will slow the whole thing down too much... If it does, an alternative might be 'copy from slice above' / 'copy from slice below' buttons.
2. Tend to think the 2D brush should be the default, rather than the rectangle tool.
3. Do we think people will still want access to square / cubic brush? If so, we could have a pair of toggle boxes, one for square / circular and one for 2D / 3D for the brush.
4. When drawing, the mouse mode needs to be set to change focus on left mouse. Either of the other two will produce unwanted behaviour (e.g. drawing ROI as you rotate the camera).
5. The 'Lock to ROI axes' button feels unusual to me. Looks like when it is set, the viewer will snap to the ROI axes upon ROI draw; but initially the user is sitting there clicking the button and wondering why nothing is happening. To me a more sensible UI would be to have this as a momentary button, immediately snapping the camera to the selected ROI when the button is pressed. If the user then rotates the camera, they can just push the button again.
6. Seems that toggling between 'draw' and 'erase' sometimes requires two clicks rather than one.
7. The explicit ROI close button prompts the user to save an edited image, but closing the whole mrview window does not.
8. Hitting 'cancel' when setting a path to save a ROI results in error message 'No name supplied to open image'.
9. If a ROI is selected, and a new ROI is created, both are selected in the list view. This then fails assertions.
10. Newly-created ROIs all have the same name; should add a counter.
11. roi_analysis.cpp is mammoth, should give the ROI tool its own sub-directory and break it up.
12. If the focus is not in the middle of the slice, sometimes the draw will do nothing, as even the closest voxel is further away than the specified radius. Solution: If the brush size is set to the minimum, draw_circle() should be bypassed, instead doing a simple nearest-neighbour interp.

I have some things to get out of the way, but will hopefully help out with these soon.

[Lestropie]

f008b5bd:

7. Fixed; though the prompt appears after the main window has disappeared, it would be preferable for it to appear beforehand.
8. Fixed.
9. Fixed.
10. Fixed.

31cd7896:

6. Fixed, though a little hack-ish. Seem to recall having run into a similar issue before with QT elements not being fully updated by function calls, as opposed to user inputs.

[Lestropie]

Played around a bit with 4, needs a bit more work. Here's what I'm thinking:

• When the ROI tool is in edit mode (either draw or erase), the mouse mode buttons should be disabled, and the cursor should change to one with a pencil logo. Can still access the other mouse modes using keyboard shortcuts though.
• Not sure I'm a fan of the focus point being updated when you first click during ROI editing.
• Rather than separate draw & erase buttons in the tool bar, should the erase functionality be accessed using RMB when in edit mode? If this were the case, the logic behind the drawing functionality would sit nicer with the other three mouse modes.

[Lestropie]

OK, some more commits up. 2, 11 and 12 should be covered. Also enabled erasing using RMB and made some cursors.

Remaining issues:

• Absence of 3D brush. But we can probably merge to master without it.
• 4 and 5 are kind of together. I tend to think a 'press this button to immediately snap to ROI axes' makes more sense, but curious to hear from others. Mouse mode issue: still looking into it, it seems if you have snap to ROI axis off, and are in mouse camera rotation mode, and activate draw mode in the ROI tool, it will both rotate the camera and ROI draw at the same time.

Edit: Not a mouse mode issue. If you're off-axis, and try to draw a ROI, the editing is constrained to be on a single image slice because of the UndoEntry design, so the camera focus point is going up and down w.r.t. the screen normal in order to stay on that slice. Makes sense to me; won't to a user.

[jdtournier]

Sorry about lack of feedback, newborn in house doesn't leave a lot of headspace to think about this - also, internet is down at home at the moment, makes it tricky to respond...

I won't be able review your changes for a few days at least, but here's a few comments on what you've raised so far.

The design of the UI being restricted to the slice is very deliberate. There is simply no way to edit the ROI sensibly when at an angle, in my opinion it would lead to even more confusion/frustration for the user. The other option is to force the UI to lock to the axes when editing, which would also be frustrating if you simply wanted to tilt the plane to inspect the ROI from a different angle or something like that (assuming you force lock as soon as you're in edit mode), or snap the plane back to the ROI axes for every draw (which is what it does now when the snap button is active). It's really hard to come up with a behaviour that remains flexible in terms of viewing and maintaining access to all the other functionality in MRView (namely the off-axis display), and ensuring the ROI editing behaves intuitively...

The undo stack is obviously designed on that basis. I had initially intended for the slice data to be kept on the GPU, but it turns out not to be possible without OpenGL 4. I thought briefly about using a voxel list like you say, but figured the savings weren't worth the extra complexity. This is really simple, and more importantly, quick - just a couple of bulk read/writes, barely any CPU involvement in that.

It does mean the 3D brush can't be done in this framework, but then my experience with the 3D brush is that it's pretty close to useless anyway. It was originally implemented as a way of having a wider brush without having to worry about the slice axis, but in practice it's very unwieldy. I don't expect anyone will miss it. So the decision to not implement it was again a very conscious one.

I've designed the UI to lock onto the plane currently being edited as a way of both simplifying the undo stack (only need to worry about that one ROI plane), and as a reminder to the user that they're editing off-axis. I agree this will probably come across as confusing to users initially, but I think they'll quickly figure it out. I'm not aware of any other software that would allow tilted display and editing of the ROI anyway... Besides, in most cases I expect users won't even encounter the issue, since it doesn't happen with the default workflow (open an image, create a new ROI, start editing).

Basically, I've been thinking about this for so long, and never managed to come up with a fully satisfactory approach. I think this is the best of a bad bunch... The only real alternative that removes all weirdnesses is to have a separate, dedicated ROI app - but then you'd lose all of the other functionality when drawing your ROI, I think that would be too high a price to pay.

I was aware of the glitches with the draw/erase buttons, but as far as I could tell from the docs, it sounds like a Qt bug... I was going on the premise that they would eventually fix it.

I like your idea of introducing an additional mouse mode. I'll look into that when I get the chance...

[jdtournier]

By the way, I'm not a fan of setting the focus on first LMB either - but it makes the math needed to lock the viewing plane to within the ROI plane much easier... It's probably not required, but I couldn't figure it out at the time, so decided to make my life easier. Happy to change if you can figure it out...

[Lestropie]

I'd thought there were a number of users who wanted a 3D brush? I've used it on occasion myself, but won't cry over its absence. I reckon we merge and see how many people complain.

I suspect anyone who tries to draw with the view rotated and the lock to axis on draw button off is going to think it's a bug. Even if it's something they never really wanted to do in the first place. Another option might be to lose the lock to ROI axis on draw button, perform the axis lock when the ROI draw button is clicked, and prevent camera rotations (other than 90 degrees) while it is depressed. That way the user can never enter a state of ROI-drawing where the expected behaviour is ill-posed.

-> Edit: Just realised you raised this already (locking on entering edit mode). Given that editing off-axis isn't going to do what the user expects, maybe we should keep the behaviour as it is, but lose the 'lock to ROI axis on draw' button, and just force that behaviour.

RE the mouse mode, essentially now the ROI draw button acts as a fourth mouse mode: left click to draw, right click to erase, and the other three modes are still accessible through the keyboard shortcuts. I thought about moving the ROI draw button to live alongside the other mouse modes, or greying out the mouse mode buttons while ROI draw is enabled, but I'm not convinced there's any benefit.

Hadn't thought of requiring to set the focus in order to determine the draw plane. Thinking maybe if we went for the idea in paragraph 2 it wouldn't be required...?

[Lestropie]

Tried it without the lock to ROI axes on draw button, I think it makes more sense. The camera locks to the ROI on draw, and there's nothing the user can do about it. Otherwise, there's a great big button just begging to be pressed, and all it does is introduce weirdness because the behaviour in such a state is ill-defined in the first place.

I thought about enforcing the lock and forbidding rotations when entering draw mode rather than on draw, but you already said you don't like that idea.

Committed, but it's your baby so feel free to revert if you think it actually serves a purpose.

[jdtournier]

I take it we can close this, given that it's been merged a while back...