From b0a179bd8981ea86bca53c50ba299d619877f193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=C3=A9y=20Ilyushkin?= Date: Thu, 20 Aug 2026 18:34:28 +0200 Subject: [PATCH 1/2] [fix] display feature marker where user clicks --- src/odemis/acq/feature.py | 29 ++++ src/odemis/acq/test/feature_test.py | 6 + src/odemis/gui/comp/overlay/cryo_feature.py | 140 ++++++++++++++++-- src/odemis/gui/cont/acquisition/cryo_acq.py | 5 +- src/odemis/gui/cont/cryo_project.py | 2 + src/odemis/gui/cont/features.py | 51 +++++++ src/odemis/gui/cont/milling.py | 27 +++- src/odemis/gui/cont/test/cryo_project_test.py | 16 +- 8 files changed, 255 insertions(+), 21 deletions(-) diff --git a/src/odemis/acq/feature.py b/src/odemis/acq/feature.py index 90a7747357..534ebea3bf 100644 --- a/src/odemis/acq/feature.py +++ b/src/odemis/acq/feature.py @@ -182,6 +182,13 @@ def __init__(self, name: str, self.stage_position = model.VigilantAttribute(stage_position, unit="m") # stage-bare, in the first posture found # TODO: drop self.fm_focus_position = model.VigilantAttribute(fm_focus_position, unit="m") self.posture_positions: Dict[str, Dict[str, float]] = {} # positions for each posture + # Position of the feature within the saved FIB reference image, in meters + # relative to the image center. The milling posture position remains the + # stage position at the center of that image, used by automated milling. + self.milling_feature_offset = model.TupleVA(None, unit="m") + # Unsaved marker movement while editing at the milling posture. This is + # intentionally not a VA and is not serialized: Save Position commits it. + self.pending_milling_feature_offset: Optional[Tuple[float, float]] = None if milling_tasks is None: # Find the default milling tasks, starting by looking into the config directory, and then @@ -233,6 +240,25 @@ def get_posture_position(self, posture: "Posture") -> Optional[Dict[str, float]] """ return self.posture_positions.get(posture.value, None) + def set_milling_feature_offset(self, + position: Tuple[float, float], + move_patterns: bool = True) -> None: + """Set the feature position relative to the saved FIB image center. + + :param position: Physical (x, y) offset in meters from the saved FIB + reference-image center, expressed in sample-stage axes. + :param move_patterns: If True, snap the milling-pattern stack to the + feature. Manual pattern movement uses a separate controller path. + """ + position = tuple(position) + if move_patterns: + for task in self.milling_tasks.values(): + for pattern in task.patterns: + pattern.center.value = position + # Update this last so redraw subscribers see the complete state. + self.milling_feature_offset.value = position + self.pending_milling_feature_offset = None + def save_milling_task_data(self, stage_position: Dict[str, float], path: str, @@ -300,6 +326,9 @@ def feature_decoder(feature_raw: Dict) -> CryoFeature: feature.status.value = feature_raw['status'] feature.posture_positions = posture_positions feature.milling_tasks = {k: MillingTaskSettings.from_dict(v) for k, v in milling_task_json.items()} + milling_feature_offset = feature_raw.get('milling_feature_offset') + if milling_feature_offset is not None: + feature.milling_feature_offset.value = tuple(milling_feature_offset) feature.path = feature_raw.get('path', None) feature.superz_stream_name = feature_raw.get('superz_stream_name', None) feature.superz_focused = feature_raw.get('superz_focused', None) diff --git a/src/odemis/acq/test/feature_test.py b/src/odemis/acq/test/feature_test.py index 164ac65a84..cc17f4b4c2 100644 --- a/src/odemis/acq/test/feature_test.py +++ b/src/odemis/acq/test/feature_test.py @@ -65,6 +65,7 @@ def test_feature_milling_tasks(self): self.path = os.path.join(os.getcwd(), feature.name.value) reference_image = model.DataArray(numpy.zeros(shape=(1024, 1536)), metadata={}) milling_tasks = load_milling_tasks(DEFAULT_MILLING_TASKS_PATH) + milling_feature_offset = (12e-6, -8e-6) # randomly remove some milling tasks (to simulate user choice) task_name = random.choice(list(milling_tasks.keys())) @@ -77,10 +78,15 @@ def test_feature_milling_tasks(self): reference_image=reference_image, milling_tasks=milling_tasks ) + feature.set_milling_feature_offset(milling_feature_offset) self.assertEqual(feature.path, self.path) self.assertEqual(feature.reference_image.shape, reference_image.shape) self.assertEqual(feature.get_posture_position(Posture.MILLING), stage_position) + self.assertEqual(feature.milling_feature_offset.value, milling_feature_offset) + for task in feature.milling_tasks.values(): + for pattern in task.patterns: + self.assertEqual(pattern.center.value, milling_feature_offset) self.assertEqual(feature.status.value, FEATURE_READY_TO_MILL) self.assertEqual(set(feature.milling_tasks.keys()), set(milling_tasks.keys())) diff --git a/src/odemis/gui/comp/overlay/cryo_feature.py b/src/odemis/gui/comp/overlay/cryo_feature.py index 6b070259fe..951e112c41 100644 --- a/src/odemis/gui/comp/overlay/cryo_feature.py +++ b/src/odemis/gui/comp/overlay/cryo_feature.py @@ -30,11 +30,13 @@ import odemis.gui as gui import odemis.gui.img as guiimg import wx +from odemis import model from odemis.acq.feature import (CryoFeature, FEATURE_ACTIVE, FEATURE_DEACTIVE, FEATURE_READY_TO_MILL, FEATURE_POLISHED, FEATURE_ROUGH_MILLED, TargetType, get_feature_position_at_posture) from odemis.gui.comp.canvas import CAN_DRAG from odemis.gui.comp.overlay.base import DragMixin, WorldOverlay from odemis.gui.comp.overlay.stage_point_select import StagePointSelectOverlay +from odemis.gui.comp.popup import show_message from odemis.gui.model import TabName, TOOL_FEATURE, TOOL_NONE, TOOL_FIDUCIAL, TOOL_REGION_OF_INTEREST, TOOL_SURFACE_FIDUCIAL from odemis.acq.move import Posture, MicroscopePostureManager @@ -79,6 +81,12 @@ def __init__(self, cnvs, tab_data): # get the tab based on the view posture self.tab_name = TabName.METEOR_FIBSEM.value if self.view_posture == Posture.SEM_IMAGING else TabName.CRYOSECOM_LOCALIZATION.value + self._selected_feature = None + self._current_feature = None + self._hover_feature = None + # defer the tool reset until mouse-up to avoid changing modes while the canvas is dragging + self._cancel_move_on_left_up = False + self._label = self.add_label("") self._selected_tool_va = self.tab_data.tool if hasattr(self.tab_data, "tool") else None if self._selected_tool_va: @@ -117,10 +125,7 @@ def __init__(self, cnvs, tab_data): if not hasattr(self.tab_data.main, "currentFeature"): raise ValueError("CryoFeatureOverlay requires currentFeature VA.") self.tab_data.main.currentFeature.subscribe(self._on_current_feature_va, init=True) - - self._selected_feature = None - self._hover_feature = None - self._label = self.add_label("") + self.tab_data.main.tab.subscribe(self._on_tab_change) def _on_tool(self, selected_tool): """ Update the feature mode (show or edit) when the overlay is active and tools change""" @@ -130,14 +135,42 @@ def _on_tool(self, selected_tool): else: self._mode = MODE_SHOW_FEATURES - def _on_current_feature_va(self, _): + def _on_current_feature_va(self, feature): + if self._current_feature is not None and feature is not self._current_feature: + self._discard_pending_milling_feature_offset(self._current_feature) + self._current_feature = feature # Redraw when the current feature is changed, as it's displayed differently wx.CallAfter(self.cnvs.request_drawing_update) + def _on_tab_change(self, tab): + if (self._current_feature is not None + and (tab is None or tab.name != self.tab_name)): + self._discard_pending_milling_feature_offset(self._current_feature) + wx.CallAfter(self.cnvs.request_drawing_update) + + def _discard_pending_milling_feature_offset(self, feature: CryoFeature) -> None: + """Discard an unsaved marker position and notify the user.""" + if feature.pending_milling_feature_offset is None: + return + + feature.pending_milling_feature_offset = None + show_message( + wx.GetApp().main_frame, + "Feature position not saved", + f"The unsaved position for {feature.name.value} was discarded.\n" + "Use \"Save Position\" before switching features or tabs.", + timeout=5.0, + level=logging.WARNING, + ) + def _on_status_change(self, _): # Redraw whenever any feature status changes, as it's reflected in the icon wx.CallAfter(self.cnvs.request_drawing_update) + def _on_milling_feature_offset_change(self, _): + # Redraw whenever the feature/pattern anchor within the FIB image changes. + wx.CallAfter(self.cnvs.request_drawing_update) + def _on_features_changes(self, features): # Redraw if a feature is added/removed wx.CallAfter(self.cnvs.request_drawing_update) @@ -150,6 +183,7 @@ def _on_features_changes(self, features): # a big deal. for f in features: f.status.subscribe(self._on_status_change) + f.milling_feature_offset.subscribe(self._on_milling_feature_offset_change) def on_dbl_click(self, evt): """ @@ -192,10 +226,28 @@ def on_left_down(self, evt): v_pos = evt.Position feature = self._detect_point_inside_feature(v_pos) if self._mode == MODE_EDIT_FEATURES: - if feature: + current_feature = self.tab_data.main.currentFeature.value + if feature is current_feature and current_feature is not None: # move/drag the selected feature self._selected_feature = feature DragMixin._on_left_down(self, evt) + elif feature is not None: + # Only the currently selected feature can be moved. + if current_feature is None: + warning = (f"No feature is selected, but you are trying to move " + f"{feature.name.value}. Select {feature.name.value} first.") + else: + warning = (f"{current_feature.name.value} is selected, but you are trying " + f"to move {feature.name.value}. Select {feature.name.value} first.") + self._cancel_move_on_left_up = True + show_message( + wx.GetApp().main_frame, + "Feature not selected", + warning, + timeout=5.0, + level=logging.WARNING, + ) + evt.Skip() else: # create new feature based on the physical position then disable the feature tool pos = self._view_to_stage_pos(v_pos) @@ -215,6 +267,11 @@ def on_left_up(self, evt): otherwise let the canvas handle the event when the overlay is active. """ if self.active: + if self._cancel_move_on_left_up: + self._cancel_move_on_left_up = False + self._selected_tool_va.value = TOOL_NONE + evt.Skip() + return if self.left_dragging: if self._selected_feature: self._update_selected_feature_position(evt.Position) @@ -232,15 +289,37 @@ def _update_selected_feature_position(self, v_pos): # re-calculate the position for all postures # use current_posture instead of view_posture to support milling posture stage_position = self._view_to_stage_pos(v_pos) - self._selected_feature.stage_position.value = stage_position - self._selected_feature.set_posture_position(self.pm.current_posture.value, stage_position) - self._update_other_postures() + if self._has_saved_milling_reference(self._selected_feature): + # Preview only. Save Position commits the marker and snaps patterns. + self._update_milling_feature_offset(self._selected_feature, stage_position) + else: + self._selected_feature.stage_position.value = stage_position + self._selected_feature.set_posture_position(self.pm.current_posture.value, stage_position) + self._update_other_postures() # Reset the selected tool to signal end of feature moving operation self._selected_feature = None self._selected_tool_va.value = TOOL_NONE self.cnvs.update_drawing() + def _update_milling_feature_offset(self, feature: CryoFeature, stage_position: Dict[str, float]) -> None: + """Preview a feature-marker move relative to the saved FIB image.""" + if self.pm.current_posture.value != Posture.MILLING or feature.reference_image is None: + return + + image_pos = feature.reference_image.metadata.get(model.MD_POS) + if image_pos is None: + logging.warning("Cannot update milling feature offset: reference image has no position metadata.") + return + + sample_pos = self.pm.to_sample_stage_from_stage_position( + stage_position, posture=Posture.MILLING) + feature.pending_milling_feature_offset = (sample_pos["x"] - image_pos[0], + sample_pos["y"] - image_pos[1]) + + def _has_saved_milling_reference(self, feature: CryoFeature) -> bool: + return self.pm.current_posture.value == Posture.MILLING and feature.reference_image is not None + def _update_other_postures(self): """Ask the user to recalculate the feature position for all other postures""" @@ -286,8 +365,7 @@ def in_radius(c_x, c_y, r, x, y): offset = self.cnvs.get_half_buffer_size() # to convert physical feature positions to pixels for feature in self.tab_data.main.features.value: - position = self._get_feature_position_at_view_posture(feature) - view_pos = self.pm.to_sample_stage_from_stage_position(position) + view_pos = self._get_feature_sample_position(feature) fvsp = self.cnvs.phys_to_view((view_pos["x"], view_pos["y"]), offset) if in_radius(fvsp[0], fvsp[1], FEATURE_DIAMETER, v_pos[0], v_pos[1]): return feature @@ -298,13 +376,23 @@ def on_motion(self, evt): v_pos = evt.Position if self.dragging: self.cnvs.set_dynamic_cursor(gui.DRAG_CURSOR) - self._selected_feature.set_posture_position(self.pm.current_posture.value, self._view_to_stage_pos(v_pos)) + stage_position = self._view_to_stage_pos(v_pos) + if self._has_saved_milling_reference(self._selected_feature): + self._update_milling_feature_offset(self._selected_feature, stage_position) + else: + self._selected_feature.set_posture_position(self.pm.current_posture.value, stage_position) self.cnvs.update_drawing() return feature = self._detect_point_inside_feature(v_pos) if feature: self._hover_feature = feature - self.cnvs.set_dynamic_cursor(wx.CURSOR_CROSS) + if self._mode == MODE_EDIT_FEATURES: + if feature is self.tab_data.main.currentFeature.value: + self.cnvs.set_dynamic_cursor(wx.CURSOR_HAND) + else: + self.cnvs.set_dynamic_cursor(wx.CURSOR_NO_ENTRY) + else: + self.cnvs.set_dynamic_cursor(wx.CURSOR_CROSS) else: if self._mode == MODE_EDIT_FEATURES: self.cnvs.set_default_cursor(wx.CURSOR_PENCIL) @@ -334,8 +422,7 @@ def draw(self, ctx, shift=(0, 0), scale=1.0): # (This would automatically take care of the case where the current posture is UNKNOWN, # as it would just return the position in the "ideal" sample coordinates) - position = self._get_feature_position_at_view_posture(feature) - view_pos = self.pm.to_sample_stage_from_stage_position(position) + view_pos = self._get_feature_sample_position(feature) half_size_offset = self.cnvs.get_half_buffer_size() # convert physical position to buffer 'world' coordinates @@ -415,6 +502,29 @@ def _get_feature_position_at_view_posture(self, feature: CryoFeature) -> Dict[st posture=posture, ) + def _get_feature_sample_position(self, feature: CryoFeature) -> Dict[str, float]: + """Return the feature position in sample coordinates for drawing. + + At the milling posture, the stored posture position is the center of the + saved FIB image. The marker itself is drawn at its independent offset + within that image. While dragging, use the live posture position so the + marker follows the pointer until the new offset is committed. + """ + posture = self.pm.current_posture.value + feature_offset = (feature.pending_milling_feature_offset + if feature.pending_milling_feature_offset is not None + else feature.milling_feature_offset.value) + if (posture == Posture.MILLING + and feature_offset is not None + and feature.reference_image is not None): + image_pos = feature.reference_image.metadata.get(model.MD_POS) + if image_pos is not None: + return {"x": image_pos[0] + feature_offset[0], + "y": image_pos[1] + feature_offset[1]} + + position = self._get_feature_position_at_view_posture(feature) + return self.pm.to_sample_stage_from_stage_position(position) + def _on_view_posture_change(self, posture): self.view_posture = posture self.cnvs.update_drawing() diff --git a/src/odemis/gui/cont/acquisition/cryo_acq.py b/src/odemis/gui/cont/acquisition/cryo_acq.py index 2db0151a2e..bbd735c882 100644 --- a/src/odemis/gui/cont/acquisition/cryo_acq.py +++ b/src/odemis/gui/cont/acquisition/cryo_acq.py @@ -900,10 +900,11 @@ def _on_close_dialog(self, z_stack): "z":sample_pos["z"]}, posture=Posture.FM_IMAGING) feature.posture_positions[Posture.FM_IMAGING.value].update(new_feature_stage_bare) feature.fm_focus_position.value = {"z": poi_coords[2]} - # Draw milling position in FIBSEM tab around the projected POI + # Update the shared feature/pattern anchor around the projected POI. + # The saved milling stage position remains the reference-image center. target = correlation_dict.fib_projected_pois[0] rel_pos = pos_to_relative(target.coordinates.value[:2], feature.reference_image) - fibsem_tab.milling_task_controller.move_milling_tasks(rel_pos) + fibsem_tab.milling_task_controller.set_milling_feature_position(rel_pos) @call_in_wx_main def _on_filename(self, name): diff --git a/src/odemis/gui/cont/cryo_project.py b/src/odemis/gui/cont/cryo_project.py index 041503518e..375e6dd2d1 100644 --- a/src/odemis/gui/cont/cryo_project.py +++ b/src/odemis/gui/cont/cryo_project.py @@ -177,6 +177,8 @@ def serialize_project_data(main_data: "CryoMainGUIData") -> Dict: } if feature.path: feature_item['path'] = feature.path + if feature.milling_feature_offset.value is not None: + feature_item['milling_feature_offset'] = feature.milling_feature_offset.value feature_list.append(feature_item) overview_list = serialize_images(overviews, project_dir) diff --git a/src/odemis/gui/cont/features.py b/src/odemis/gui/cont/features.py index 11c2bad79f..7b6d9ee535 100644 --- a/src/odemis/gui/cont/features.py +++ b/src/odemis/gui/cont/features.py @@ -26,6 +26,7 @@ import wx +from odemis import model from odemis.acq.feature import ( FEATURE_ACTIVE, FEATURE_DEACTIVE, @@ -206,6 +207,45 @@ def save_milling_position(self, evt: wx.Event): stream = self._tab.fib_stream # the fib stream + # Preserve the physical feature position before the milling posture is + # updated to the current stage position (the center of the reference + # image). If a reference image already exists, its relative feature + # offset is the authoritative position. + pending_feature_offset = feature.pending_milling_feature_offset + feature_offset = (pending_feature_offset + if pending_feature_offset is not None + else feature.milling_feature_offset.value) + snap_patterns_to_feature = (pending_feature_offset is not None + or feature.milling_feature_offset.value is None) + feature_sample_pos = None + if feature_offset is not None and feature.reference_image is not None: + image_pos = feature.reference_image.metadata.get(model.MD_POS) + if image_pos is not None: + feature_sample_pos = (image_pos[0] + feature_offset[0], + image_pos[1] + feature_offset[1]) + + if feature_sample_pos is None: + milling_pos = feature.get_posture_position(Posture.MILLING) + if milling_pos is not None: + sample_pos = self.pm.to_sample_stage_from_stage_position( + milling_pos, posture=Posture.MILLING) + feature_sample_pos = (sample_pos["x"], sample_pos["y"]) + + if pending_feature_offset is not None and feature_sample_pos is not None: + # Commit the physical marker position only when Save Position is + # pressed. The explicit milling posture is overwritten below with + # the reference-image center, not this marker position. + current_sample_pos = self.pm.to_sample_stage_from_stage_position( + self.pm.stage.position.value, posture=Posture.MILLING) + marker_stage_pos = self.pm.from_sample_stage_to_stage_position( + {"x": feature_sample_pos[0], + "y": feature_sample_pos[1], + "z": current_sample_pos["z"]}, + posture=Posture.MILLING) + updated_stage_pos = dict(feature.stage_position.value) + updated_stage_pos.update(marker_stage_pos) + feature.stage_position.value = updated_stage_pos + # acquire a new fib image for reference from odemis.acq import acqmng self._acq_future = acqmng.acquire( @@ -223,6 +263,17 @@ def save_milling_position(self, evt: wx.Event): path=os.path.join(self._tab.conf.pj_last_path, feature.name.value), reference_image=stream.raw[0]) + # Store the feature within the newly acquired image and position the + # milling patterns around the same point. The milling posture position + # saved above intentionally remains the image-center stage coordinate. + if feature_sample_pos is not None: + image_pos = feature.reference_image.metadata.get(model.MD_POS) + if image_pos is not None: + relative_pos = (feature_sample_pos[0] - image_pos[0], + feature_sample_pos[1] - image_pos[1]) + self._tab.milling_task_controller.set_milling_feature_position( + relative_pos, move_patterns=snap_patterns_to_feature) + save_project(self._tab_data_model.main) # refresh current feature to update reference image and milling tasks diff --git a/src/odemis/gui/cont/milling.py b/src/odemis/gui/cont/milling.py index 07348b1092..17d328f6ff 100644 --- a/src/odemis/gui/cont/milling.py +++ b/src/odemis/gui/cont/milling.py @@ -443,17 +443,38 @@ def _on_selected_tasks(self, tasks: List[str]): def move_milling_tasks(self, pos: Tuple[float, float]): """ - Update the position of the milling patterns for the current feature. - Also updates the saved positions, and redraws the patterns on the viewport. + Update only the milling patterns for the current feature. + + This is the independent Ctrl+Shift+click movement path and must not move + the feature marker. :param pos: the position to draw the patterns at (in m, as relative coordinates to the center of the ion-beam FoV) """ - for task in self.milling_tasks.values(): + feature = self._tab_data.main.currentFeature.value + if feature is None: + logging.warning("Cannot move milling tasks without a selected feature.") + return + + for task in feature.milling_tasks.values(): for pattern in task.patterns: pattern.center.value = pos save_project(self._tab_data.main) self.draw_milling_tasks() + def set_milling_feature_position(self, + pos: Tuple[float, float], + move_patterns: bool = True) -> None: + """Commit the feature marker and optionally snap patterns around it.""" + feature = self._tab_data.main.currentFeature.value + if feature is None: + logging.warning("Cannot position milling feature without a selected feature.") + return + + feature.set_milling_feature_offset(pos, move_patterns=move_patterns) + + save_project(self._tab_data.main) + self.draw_milling_tasks() + @call_in_wx_main def draw_milling_tasks(self, _=None): """Redraw all milling tasks on the canvas. diff --git a/src/odemis/gui/cont/test/cryo_project_test.py b/src/odemis/gui/cont/test/cryo_project_test.py index 4a1e73a9b9..1e2b8c35c1 100644 --- a/src/odemis/gui/cont/test/cryo_project_test.py +++ b/src/odemis/gui/cont/test/cryo_project_test.py @@ -29,7 +29,7 @@ from unittest.mock import MagicMock import odemis.acq.test as acq_test -from odemis.acq.feature import feature_decoder +from odemis.acq.feature import CryoFeature, feature_decoder from odemis.acq.move import Posture from odemis.gui.cont.cryo_project import ( load_project, @@ -114,6 +114,20 @@ def test_v1_0_project(self): for posture_key in project_data["features"][0]["posture_positions"].keys(): self.assertIn(posture_key, posture_values) + def test_milling_feature_offset_roundtrip(self): + """The feature/pattern anchor is persisted without changing legacy projects.""" + feature = CryoFeature("Feature-1", {"x": 0, "y": 0, "z": 0}, {"z": 0}) + feature.milling_feature_offset.value = (12e-6, -8e-6) + main_data = MagicMock() + main_data.tab.value.conf.pj_last_path = self.test_dir + main_data.features.value = [feature] + main_data.overviews.value = [] + + save_project(main_data) + feature_data = load_project(self.test_dir)["features"][0] + self.assertEqual(feature_decoder(feature_data).milling_feature_offset.value, + feature.milling_feature_offset.value) + def test_image_operations(self): """Tests that the image operations work properly.""" images = [] From 93021fd4d84d7355caaebceb312ab9b80db35473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=C3=A9y=20Ilyushkin?= Date: Thu, 27 Aug 2026 18:18:35 +0200 Subject: [PATCH 2/2] [fix] improve feature selection and unsaved position handling --- src/odemis/gui/comp/overlay/cryo_feature.py | 94 +++++++++++++------ src/odemis/gui/cont/features.py | 82 +++++++++++++++- src/odemis/gui/cont/tabs/fibsem_tab.py | 13 ++- src/odemis/gui/cont/tabs/tab.py | 4 + .../gui/cont/tabs/tab_bar_controller.py | 5 + 5 files changed, 162 insertions(+), 36 deletions(-) diff --git a/src/odemis/gui/comp/overlay/cryo_feature.py b/src/odemis/gui/comp/overlay/cryo_feature.py index 951e112c41..cafc30545b 100644 --- a/src/odemis/gui/comp/overlay/cryo_feature.py +++ b/src/odemis/gui/comp/overlay/cryo_feature.py @@ -37,6 +37,7 @@ from odemis.gui.comp.overlay.base import DragMixin, WorldOverlay from odemis.gui.comp.overlay.stage_point_select import StagePointSelectOverlay from odemis.gui.comp.popup import show_message +from odemis.gui.cont.features import confirm_discard_pending_feature_position from odemis.gui.model import TabName, TOOL_FEATURE, TOOL_NONE, TOOL_FIDUCIAL, TOOL_REGION_OF_INTEREST, TOOL_SURFACE_FIDUCIAL from odemis.acq.move import Posture, MicroscopePostureManager @@ -84,8 +85,6 @@ def __init__(self, cnvs, tab_data): self._selected_feature = None self._current_feature = None self._hover_feature = None - # defer the tool reset until mouse-up to avoid changing modes while the canvas is dragging - self._cancel_move_on_left_up = False self._label = self.add_label("") self._selected_tool_va = self.tab_data.tool if hasattr(self.tab_data, "tool") else None @@ -197,6 +196,9 @@ def on_dbl_click(self, evt): v_pos = evt.Position feature = self._detect_point_inside_feature(v_pos) if feature: + if not self._confirm_feature_change(feature): + self.cnvs.cancel_drag() + return logging.info("moving to feature {}".format(feature.name.value)) # convert from stage position to view position position_bare = self._get_feature_position_at_view_posture(feature) @@ -226,29 +228,33 @@ def on_left_down(self, evt): v_pos = evt.Position feature = self._detect_point_inside_feature(v_pos) if self._mode == MODE_EDIT_FEATURES: - current_feature = self.tab_data.main.currentFeature.value - if feature is current_feature and current_feature is not None: - # move/drag the selected feature + if feature is not None: + confirmation_shown = self._feature_change_requires_confirmation(feature) + if not self._confirm_feature_change(feature): + self._finish_interrupted_mouse_gesture() + return + if confirmation_shown: + self.tab_data.main.currentFeature.value = feature + self._finish_interrupted_mouse_gesture() + return + # Select the feature before moving it. + self.tab_data.main.currentFeature.value = feature self._selected_feature = feature DragMixin._on_left_down(self, evt) - elif feature is not None: - # Only the currently selected feature can be moved. - if current_feature is None: - warning = (f"No feature is selected, but you are trying to move " - f"{feature.name.value}. Select {feature.name.value} first.") - else: - warning = (f"{current_feature.name.value} is selected, but you are trying " - f"to move {feature.name.value}. Select {feature.name.value} first.") - self._cancel_move_on_left_up = True - show_message( - wx.GetApp().main_frame, - "Feature not selected", - warning, - timeout=5.0, - level=logging.WARNING, - ) - evt.Skip() else: + current_feature = self.tab_data.main.currentFeature.value + confirmation_shown = (current_feature is not None + and current_feature.pending_milling_feature_offset is not None) + if (current_feature is not None + and not confirm_discard_pending_feature_position( + wx.GetApp().main_frame, + current_feature, + "create a new feature", + )): + self._finish_interrupted_mouse_gesture() + return + if confirmation_shown: + self._finish_interrupted_mouse_gesture() # create new feature based on the physical position then disable the feature tool pos = self._view_to_stage_pos(v_pos) self.tab_data.add_new_feature(stage_position=pos) @@ -256,7 +262,14 @@ def on_left_down(self, evt): self._selected_tool_va.value = TOOL_NONE else: if feature: + confirmation_shown = self._feature_change_requires_confirmation(feature) + if not self._confirm_feature_change(feature): + self._finish_interrupted_mouse_gesture() + return self.tab_data.main.currentFeature.value = feature + if confirmation_shown: + self._finish_interrupted_mouse_gesture() + return evt.Skip() else: super().on_left_down(evt) @@ -267,11 +280,6 @@ def on_left_up(self, evt): otherwise let the canvas handle the event when the overlay is active. """ if self.active: - if self._cancel_move_on_left_up: - self._cancel_move_on_left_up = False - self._selected_tool_va.value = TOOL_NONE - evt.Skip() - return if self.left_dragging: if self._selected_feature: self._update_selected_feature_position(evt.Position) @@ -281,6 +289,33 @@ def on_left_up(self, evt): else: WorldOverlay.on_left_up(self, evt) + def _confirm_feature_change(self, feature: CryoFeature) -> bool: + """Confirm a manual selection change when the current marker was moved.""" + current_feature = self.tab_data.main.currentFeature.value + if current_feature is None or feature is current_feature: + return True + return confirm_discard_pending_feature_position( + wx.GetApp().main_frame, + current_feature, + f"switch to {feature.name.value}", + ) + + def _feature_change_requires_confirmation(self, feature: CryoFeature) -> bool: + """Return whether selecting the feature will show a confirmation dialog.""" + current_feature = self.tab_data.main.currentFeature.value + return (current_feature is not None + and feature is not current_feature + and current_feature.pending_milling_feature_offset is not None) + + def _finish_interrupted_mouse_gesture(self) -> None: + """Finish a canvas gesture interrupted by a confirmation dialog.""" + self.clear_drag() + self.cnvs.cancel_drag() + if self.cnvs.HasCapture(): + self.cnvs.on_mouse_up() + else: + self.cnvs.reset_dynamic_cursor() + def _update_selected_feature_position(self, v_pos): """ Update the selected feature with the newly moved position @@ -387,10 +422,7 @@ def on_motion(self, evt): if feature: self._hover_feature = feature if self._mode == MODE_EDIT_FEATURES: - if feature is self.tab_data.main.currentFeature.value: - self.cnvs.set_dynamic_cursor(wx.CURSOR_HAND) - else: - self.cnvs.set_dynamic_cursor(wx.CURSOR_NO_ENTRY) + self.cnvs.set_dynamic_cursor(wx.CURSOR_HAND) else: self.cnvs.set_dynamic_cursor(wx.CURSOR_CROSS) else: diff --git a/src/odemis/gui/cont/features.py b/src/odemis/gui/cont/features.py index 7b6d9ee535..7656b99538 100644 --- a/src/odemis/gui/cont/features.py +++ b/src/odemis/gui/cont/features.py @@ -41,15 +41,42 @@ ) from odemis.acq.move import Posture from odemis.gui import model as guimod +from odemis.gui.comp.popup import show_message from odemis.gui.conf.licences import LICENCE_MILLING_ENABLED from odemis.gui.cont.cryo_project import save_project -from odemis.gui.model import TOOL_FEATURE +from odemis.gui.model import TOOL_FEATURE, TOOL_NONE from odemis.gui.util import call_in_wx_main from odemis.gui.util.widgets import VigilantAttributeConnector SUPPORTED_POSTURES = [Posture.SEM_IMAGING, Posture.FM_IMAGING, Posture.MILLING, Posture.FIB_IMAGING, Posture.FIB_VIEW_FM] + +def confirm_discard_pending_feature_position(parent: wx.Window, + feature: CryoFeature, + action: str) -> bool: + """Ask before discarding a feature marker position during manual navigation.""" + if feature.pending_milling_feature_offset is None: + return True + + box = wx.MessageDialog( + parent, + message=(f"{feature.name.value} has an unsaved position. " + f"Discard it and {action}?"), + caption="Unsaved feature position", + style=wx.YES_NO | wx.NO_DEFAULT | wx.ICON_WARNING | wx.CENTER, + ) + box.SetYesNoLabels("Discard", "Cancel") + try: + discard = box.ShowModal() == wx.ID_YES + finally: + box.Destroy() + + if discard: + feature.pending_milling_feature_offset = None + return discard + + class CryoFeatureController(object): """ controller to handle the cryo feature panel elements It requires features list VA & currentFeature VA on the tab data to function properly @@ -94,6 +121,7 @@ def __init__(self, tab_data, panel, tab, mode: guimod.AcquiMode): self._panel.btn_create_move_feature.Bind(wx.EVT_BUTTON, self._on_btn_create_move_feature) self._panel.btn_delete_feature.Bind(wx.EVT_BUTTON, self._on_btn_delete_feature) self._panel.btn_go_to_feature.Bind(wx.EVT_BUTTON, self._on_btn_go_to_feature) + self._tab_data_model.tool.subscribe(self._on_tool_change) # specific controls for FM and FIBSEM modes fm_mode = self.acqui_mode is guimod.AcquiMode.FLM @@ -106,9 +134,33 @@ def __init__(self, tab_data, panel, tab, mode: guimod.AcquiMode): self.pm.current_posture.subscribe(self._on_posture_change) def _on_btn_create_move_feature(self, _): - # As this button is identical to clicking the feature tool, - # directly change the tool to feature tool - self._tab_data_model.tool.value = TOOL_FEATURE + if self._tab_data_model.tool.value == TOOL_FEATURE: + self._tab_data_model.tool.value = TOOL_NONE + else: + self._tab_data_model.tool.value = TOOL_FEATURE + + @call_in_wx_main + def _on_tool_change(self, tool: int) -> None: + """Confirm Create/Move activation from either UI control.""" + if tool != TOOL_FEATURE or self._tab_data_model.tool.value != TOOL_FEATURE: + return + + feature = self._tab_data_model.main.currentFeature.value + if feature is None: + return + + had_pending_position = feature.pending_milling_feature_offset is not None + if not confirm_discard_pending_feature_position( + self._tab.main_frame, + feature, + "enter Create/Move mode", + ): + self._tab_data_model.tool.value = TOOL_NONE + return + + if had_pending_position: + for viewport in self._tab.view_controller.viewports: + viewport.canvas.request_drawing_update() def _on_btn_delete_feature(self, _): """ @@ -150,6 +202,18 @@ def _on_btn_go_to_feature(self, _): self._display_go_to_feature_warning() return + if (self.acqui_mode is guimod.AcquiMode.FIBSEM + and current_posture == Posture.MILLING + and feature.reference_image is None): + show_message( + self._tab.main_frame, + "No FIB reference available", + f"No FIB reference image is saved for {feature.name.value}. " + "The stage will move to the feature marker instead.", + timeout=5.0, + level=logging.WARNING, + ) + stage_position = get_feature_position_at_posture(pm=self.pm, feature=feature, posture=current_posture) fm_focus_position = feature.fm_focus_position.value @@ -478,6 +542,16 @@ def _on_cmb_features_change(self, evt): logging.warning("cmb_features selection = -1.") return selected_feature = self._panel.cmb_features.GetClientData(index) + current_feature = self._tab_data_model.main.currentFeature.value + if (selected_feature is not current_feature + and current_feature is not None + and not confirm_discard_pending_feature_position( + self._tab.main_frame, + current_feature, + f"switch to {selected_feature.name.value}", + )): + self._update_feature_cmb_list() + return self._tab_data_model.main.currentFeature.value = selected_feature def _on_cmb_feature_status_change(self): diff --git a/src/odemis/gui/cont/tabs/fibsem_tab.py b/src/odemis/gui/cont/tabs/fibsem_tab.py index 9151e0b484..2050297e64 100644 --- a/src/odemis/gui/cont/tabs/fibsem_tab.py +++ b/src/odemis/gui/cont/tabs/fibsem_tab.py @@ -46,7 +46,7 @@ from odemis.gui.conf.licences import LICENCE_FIBSEM_ENABLED, LICENCE_MILLING_ENABLED from odemis.gui.cont import milling, settings from odemis.gui.cont.acquisition.cryo_acq import CryoAcquiController -from odemis.gui.cont.features import CryoFeatureController +from odemis.gui.cont.features import CryoFeatureController, confirm_discard_pending_feature_position from odemis.gui.cont.stream_bar import CryoFIBAcquiredStreamsController, CryoStreamsController from odemis.gui.cont.tabs.tab import Tab from odemis.gui.model import TabName, TOOL_ACT_ZOOM_FIT @@ -466,6 +466,17 @@ def terminate(self): for s in self.tab_data_model.streams.value: s.is_active.value = False + def query_leave(self) -> bool: + """Ask before leaving the tab with an unsaved feature marker position.""" + feature = self.main_data.currentFeature.value + if feature is None: + return True + return confirm_discard_pending_feature_position( + self.main_frame, + feature, + "switch tabs", + ) + @classmethod def get_display_priority(cls, main_data): if main_data.role == "meteor" and main_data.fibsem and LICENCE_FIBSEM_ENABLED: diff --git a/src/odemis/gui/cont/tabs/tab.py b/src/odemis/gui/cont/tabs/tab.py index 604b9e163b..20014f5776 100644 --- a/src/odemis/gui/cont/tabs/tab.py +++ b/src/odemis/gui/cont/tabs/tab.py @@ -65,6 +65,10 @@ def Show(self, show=True): self.panel.Show(show) + def query_leave(self) -> bool: + """Return whether a manual switch away from this tab may continue.""" + return True + def _connect_22view_event(self): """ If the tab has a 2x2 view, this method will connect it to the 2x2 view menu item (or ensure it's disabled). diff --git a/src/odemis/gui/cont/tabs/tab_bar_controller.py b/src/odemis/gui/cont/tabs/tab_bar_controller.py index e8fd32b4ea..2d53afa656 100644 --- a/src/odemis/gui/cont/tabs/tab_bar_controller.py +++ b/src/odemis/gui/cont/tabs/tab_bar_controller.py @@ -125,6 +125,11 @@ def on_click(self, evt): evt_btn = evt.GetEventObject() for t in self._tab.choices: if evt_btn == t.button: + current_tab = self._tab.value + if t is not current_tab and not current_tab.query_leave(): + t.button.SetToggle(False) + current_tab.button.SetToggle(True) + break self._tab.value = t break else: