2285 Crop Coordinates Operation Window: Restrict ROI MaxBounds and Handle Dialog Window Closure (#2302)

Author: samtygier-stfc

docs/release_notes/next/fix-2285-crop-coord-stack-bounds

@@ -0,0 +1 @@
+#2285: Resolve operations windows crop co-ordinates ROI size from being larger than stack bounds. Additionally resolve ROI crop co-ordinate dialog window persistence if parent window closed.

mantidimaging/gui/test/gui_system_operations_test.py

@@ -49,7 +49,6 @@
 ALLOWED_ERRORS = [
     'Negative values found in result preview for slice 0.',
     'Flat-fielding completed. Slices containing negative values in IMAT_Flower_Tomo_000000: all slices.',
-    'Error applying filter for preview: could not broadcast input array from shape (1,80,80) into shape (1,90,90)'
 ]
 
 

mantidimaging/gui/widgets/mi_image_view/view.py

@@ -7,7 +7,7 @@
 from typing import TYPE_CHECKING
 from collections.abc import Callable
 
-from PyQt5.QtCore import Qt
+from PyQt5.QtCore import Qt, QRectF
 from PyQt5.QtWidgets import QApplication, QHBoxLayout, QLabel, QPushButton, QSizePolicy
 from pyqtgraph import ROI, ImageItem, ImageView, ViewBox
 from pyqtgraph.GraphicsScene.mouseEvents import HoverEvent
@@ -148,6 +148,9 @@ def angles(self, angles: ProjectionAngles | None) -> None:
         self._angles = angles
         self._update_message(self._last_mouse_hover_location)
 
+    def _set_roi_max_bounds(self):
+        self.roi.maxBounds = QRectF(0, 0, self.image_data.shape[2], self.image_data.shape[1])
+
     def setImage(self, image: np.ndarray, *args, **kwargs):
         dimensions_changed = self.image_data is None or self.image_data.shape != image.shape
         if image.ndim == 3:
@@ -195,6 +198,7 @@ def roiChanged(self) -> None:
         roi = self._update_roi_region_avg()
         if self.roi_changed_callback and roi is not None:
             self.roi_changed_callback(roi)
+        self._set_roi_max_bounds()
         self._refresh_message()
 
     def _update_roi_region_avg(self) -> SensibleROI | None:

mantidimaging/gui/widgets/roi_selector/view.py

@@ -69,3 +69,7 @@ def toggle_average_images(self) -> None:
         self.roi_view_averaged = not self.roi_view_averaged
         self.roi_view.roi.show()
         self.roi_view.ui.roiPlot.hide()
+
+    def closeEvent(self, event) -> None:
+        self.roi_view.close()
+        event.accept()

mantidimaging/gui/windows/operations/presenter.py

@@ -284,14 +284,9 @@ def _post_filter(self, updated_stacks: list[ImageStack], task):
                 if np.any(stack.data < 0):
                     negative_stacks.append(stack)
 
-            if self.view.roi_view is not None:
-                self.view.roi_view.close()
-                self.view.roi_view = None
-
             self.applying_to_all = False
-            self.do_update_previews()
 
-            if task.error is not None:
+            if task.error:
                 # task failed, show why
                 self.view.show_error_dialog(f"Operation failed: {task.error}")
             elif use_new_data:
@@ -312,6 +307,7 @@ def _post_filter(self, updated_stacks: list[ImageStack], task):
             self.view.filter_applied.emit()
             self._set_apply_buttons_enabled(self.prev_apply_single_state, self.prev_apply_all_state)
             self.filter_is_running = False
+            self.do_update_previews()
 
     def _do_apply_filter(self, apply_to: list[ImageStack]):
         self.filter_is_running = True
@@ -454,11 +450,8 @@ def init_roi_field(self, roi_field: QLineEdit):
         if self.stack is None:
             return
 
-        larger = np.greater(self.stack.data[0].shape, (200, 200))
-        if all(larger):
-            return
-        x = min(self.stack.data[0].shape[0], 200)
-        y = min(self.stack.data[0].shape[1], 200)
+        x = self.stack.data.shape[1] // 2
+        y = self.stack.data.shape[2] // 2
         crop_string = ", ".join(["0", "0", str(y), str(x)])
         roi_field.setText(crop_string)
 

mantidimaging/gui/windows/operations/test/presenter_test.py

@@ -448,14 +448,14 @@ def test_init_roi_field_does_nothing_when_stack_is_none(self):
         self.presenter.init_roi_field(mock_roi_field)
         mock_roi_field.setText.assert_not_called()
 
-    def test_init_roi_field_does_nothing_when_image_is_greater_than_200_by_200(self):
+    def test_init_roi_field_called_with_smaller_values_if_image_is_greater_than_200_by_200(self):
         mock_roi_field = mock.Mock()
         self.presenter.stack = mock.Mock()
         self.presenter.stack.data = np.ones((2, 201, 201))
         self.presenter.init_roi_field(mock_roi_field)
-        mock_roi_field.setText.assert_not_called()
+        mock_roi_field.setText.assert_called_once_with("0, 0, 100, 100")
 
-    @parameterized.expand([(190, 201, "0, 0, 200, 190"), (201, 80, "0, 0, 80, 200"), (200, 200, "0, 0, 200, 200")])
+    @parameterized.expand([(190, 201, "0, 0, 100, 95"), (201, 80, "0, 0, 40, 100"), (200, 200, "0, 0, 100, 100")])
     def test_set_text_called_when_image_not_greater_than_200_by_200(self, shape_x, shape_y, expected):
         mock_roi_field = mock.Mock()
         self.presenter.stack = mock.Mock()

mantidimaging/gui/windows/operations/view.py

@@ -64,6 +64,7 @@ def __init__(self, main_window: MainWindowView):
         self.main_window = main_window
         self.presenter = FiltersWindowPresenter(self, main_window)
         self.roi_view = None
+        self.roi_selector_dialog: ROISelectorView | None = None
         self.roi_view_averaged = False
         self.splitter.setSizes([200, 9999])
         self.splitter.setStretchFactor(0, 1)
@@ -122,8 +123,9 @@ def closeEvent(self, e):
     def cleanup(self):
         self.stackSelector.unsubscribe_from_main_window()
         if self.roi_view is not None:
-            self.roi_view.close()
             self.roi_view = None
+            self.roi_selector_dialog.close()
+
         self.presenter.set_stack(None)
         self.auto_update_triggered.disconnect()
         self.main_window.filters = None
@@ -255,16 +257,17 @@ def roi_changed_callback(callback):
         except ValueError:
             roi_values = None
 
-        window = ROISelectorView(self, self.presenter.stack, self.presenter.model.preview_image_idx, roi_values,
-                                 roi_changed_callback)
+        self.roi_selector_dialog = ROISelectorView(self, self.presenter.stack, self.presenter.model.preview_image_idx,
+                                                   roi_values, roi_changed_callback)
 
         def close_event(event):
             roi_field.setEnabled(True)
             roi_button.setEnabled(True)
             event.accept()
 
-        window.closeEvent = functools.partial(close_event)
-        window.show()
+        self.roi_view = self.roi_selector_dialog.roi_view
+        self.roi_selector_dialog.closeEvent = functools.partial(close_event)
+        self.roi_selector_dialog.show()
 
     def toggle_filters_section(self):
         if self.collapseToggleButton.text() == "<<":