search

microscope-cockpit / cockpit

Cockpit is a microscope graphical user interface. It is a flexible and easy to extend platform aimed at life scientists using bespoke microscopes.
https://microscope-cockpit.org
GNU General Public License v3.0
37 stars 26 forks source link

UI to set ROI #837

Closed carandraug closed 1 year ago

carandraug commented 1 year ago

Currently, there only way to set a ROI on a camera is through the settings button (and manually type in the ROI). This goes through the "roi" setting instead of the set_roi and get_roi methods but in Python-Microscope we would like to remove that "roi" setting (see python-microscope/microscope#260).

  1. add a UI to draw a ROI on the camera view: @dstoychev has a patch about this that needs to be cleared

  2. ability to select between preconfigured ROIs

Originally posted by @juliomateoslangerak in https://github.com/python-microscope/microscope/issues/260#issuecomment-1375583498

it would still nice if Cockpit had some GUI to draw a ROI.

Definitely. I miss very much the menu to choose the ROI from a per-configured list in the depot.

  1. ability to have a default ROI on the depot file:

Originally posted by @iandobbie in https://github.com/python-microscope/microscope/issues/260#issuecomment-1375686107

I think we need to ensure there is still an easy way to define the ROI programmatically, say from depot.conf. Some have cameras which are bigger than the optical field of view so some of the camera is never used and a predefined ROI is a massive win to save doing it every time you restart.

This should be very easy to implement in the same manner that transform is already a config option in depot for cameras.

juliomateoslangerak commented 1 year ago

This feature used to be there in cockpit through a dedicated menu on the camera in the main pannel.

It would be nice to be able to trigger some other action upon changing the roi. I'm thinking about triggering a change in a rectangular window adapting the size of the excitation to the ROI size.

iandobbie commented 1 year ago

I would think that this should go through the events system. The camera view definitely triggers some stuff if the camera image size changes but this only happens once an image is returned. It would be better to have whatever is adjusting the RIO fire an ROI event and get any interested devices to listen to such an event.

carandraug commented 1 year ago

3. ability to have a default ROI on the depot file:

Originally posted by @iandobbie in python-microscope/microscope#260 (comment)

I think we need to ensure there is still an easy way to define the ROI programmatically, say from depot.conf. Some have cameras which are bigger than the optical field of view so some of the camera is never used and a predefined ROI is a massive win to save doing it every time you restart.

This should be very easy to implement in the same manner that transform is already a config option in depot for cameras.

This (default initial ROI in the depot file) is now implemented on my branch 837-camera-default-roi.

dstoychev commented 1 year ago

Here is the branch for the interactive ROI changes that Matthew did: dstoychev/cockpit/interactive_roi

carandraug commented 1 year ago

Here is the branch for the interactive ROI changes that Matthew did: dstoychev/cockpit/interactive_roi

I've quickly looked at it and only have one question. The code seems to assume that ROI coordinates are always relative to the sensor original shape. So a ROI with width of 10 will return an image of width 5px if binning is 2x2. But did you actually test that? (if it doesn't work there may also be an issue in Microscope). Also, if the ROI has width 1 but binning is 2x2, what should happen (may be a discussion for Microscope)?

iandobbie commented 1 year ago

I would need to check the code but I think a camera with 10x10 sensor and binned 2x2 returns an image that is 5x5 and the camera view adjusts to that. This solves the 1 pixel width on a 2x2 binned image, but we then need to define how ROI are interpreted in microscope, or expand the ROI from image pixels to camera pixels before we pass it over the wire.

juliomateoslangerak commented 1 year ago

Just to note that I think that is the expected behavior from the user perspective. A user will most of the time associate the ROI with FOV and 'zoom' rather than the number of pixels of the final image. There are less thinking steps for the average user that is not using binning. I discussed with my colleagues here and they agree. They mentioned though that users coming from the confocal might have to adapt.

dstoychev commented 1 year ago

I've quickly looked at it and only have one question. The code seems to assume that ROI coordinates are always relative to the sensor original shape. So a ROI with width of 10 will return an image of width 5px if binning is 2x2. But did you actually test that?

Not at the time, but I tested it now and it works in the following (expected) way:

carandraug commented 1 year ago

Not at the time, but I tested it now and it works in the following (expected) way:

  • If I have an ROI of 10x10 and set the binning to 2x2, the image in the Camera View will be 5x5.
  • If I have binning of 2x2, set ROI to 5x5 and change the binning to 1x1, the image in the Camera View will be 10x10

I'm not sure about a thing. You set binning to 2x2, then acquire an image. Then you draw a 5x5 ROI on the camera view. Does it send a 5x5px or 10x10px ROI to Microscope? I think cockpit should be sending a 10x10px ROI (but I may be wrong on what actually happens, it's not documented anywhere).

iandobbie commented 1 year ago

There is an obvious clash between the current settings and the settings when the images was taken here. If you set the camera to bin 2x2, acquire and image, set it back to 1x1 then set a ROI you will get the wrong result but I think this is such a side issue that we should ignore it.

To be honest Mike removed my scale bar in the camera views over exactly the same issue but I think it is such a corner case that we can document it and ignore it.

iandobbie commented 1 year ago

I have played a little with this and I can see some improvements. I have started to make them in my copy of this branch as well as rebasing it on the latest master.

1) Once the roi is set the mosaic and touchscreen updates to reflect the image size and shape.

2) why can we only do square ROIs?

3) once you have set the roi, I think it should display in some way where the roi is on the current image, currently you are left hanging until you snap another image.

iandobbie commented 1 year ago

Just to make it clear I have fixed point 1 above in my branch.

iandobbie commented 1 year ago

I have created a new event UPDATE_ROI. The code I implemented fired a MOSAIC_UPDATE event which deals with the mosaic image outline.

I have also added flag to keep displaying the selected ROI until a new image is collected or if it cleared. Finally the code also now uses the real coords to draw the ROI, with shift used to force a square roi.

For Julio's question above about changing illumination area on change of ROI, we should subscribe to the UPDATE_ROI event which is passed the camera name.

We should add a trigger to ask if other cameras should have the same ROI set. My current issue with this is where do we do this? We could have every camera handler subscribe to the UPDATE_ROI and check the name, if this cam is active and it was not this camera then offer to set the ROI, the problem with this if you have 4 cameras active and you set an ROI you get 3 dialogs.

iandobbie commented 1 year ago

Added the dialog to apply it to other active cameras. Closing issue as complete.