diff --git a/CLAUDE.md b/CLAUDE.md index 78fe6df..4e33c5e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,12 +143,12 @@ Before opening a PR, check [TODO.md](TODO.md) for related open items to update o - `runtime_setup.py`: `RuntimeContext` dataclass and `create_runtime_context()` — ParaView objects created before Trame is available. `ParaViewRuntime` is constructed in `factory.py` once `state` and `view_controls` exist. - `handler_registration.py`: wires controllers and state handlers. - `state_setup.py`: initializes all Trame state. If adding UI controls, add defaults here. -- `state_handlers.py`: shared `@state.change(...)` callbacks for selected file, color-by, representation, active pipeline node, interaction quality, edit mode. +- `state_handlers.py`: shared `@state.change(...)` callbacks for selected file, color-by, representation, active pipeline node, interaction quality, edit mode. Pipeline node switches are blocked during an active edit session — the handler emits a warning and resets `state.active_pipeline_item` to snap the UI selection back. - `view_controls.py`: central wrapper for Trame view update callbacks and optional ParaView diagnostics. ### ParaView Backend Path -- `paraview_backend.py`: owns ParaView sources/displays, pipeline nodes, filters, coloring, display controls, picking, selection overlays, saving/export. +- `paraview_backend.py`: owns ParaView sources/displays, pipeline nodes, filters, coloring, display controls, picking, selection overlays, saving/export. `set_edit_target_dataset()` builds four remap caches at session begin (`_edit_source_dataset`, `_edit_source_point_indexes`, `_edit_target_cell_map`, `_edit_point_id_map`) so picks avoid per-pick Fetch/rebuild — see `docs/logic_flows.md §5b`. - `paraview_runtime.py`: synchronizes backend state into Trame state, handles render pushes, edit-session overlay sync. Forwards event normalization calls to `paraview_event_utils.py`. - `paraview_event_utils.py`: pure helpers for normalizing ParaView picking event payloads (`normalize_edit_selection_ids`, `summarize_edit_event`, coordinate mapping). No Trame dependency. - `paraview_controllers.py`: user actions from the UI: pipeline actions, filters, edit sessions, selection, field creation, display/color controls. @@ -201,6 +201,12 @@ Do not implement replace as a toggle. ParaView native payloads can contain toggl Selection overlays can interfere with native picking. `paraview_backend.py` clears transient edit-selection overlays before pick queries. +### Pipeline Lock During Edit Session + +`state_handlers.py` blocks `active_pipeline_item` changes while an edit session is active. When blocked, it emits a warning notification and resets `state.active_pipeline_item` to `pv_backend.active_node_id`, which snaps the pipeline tree UI back to the current node. The reset triggers a second `@state.change` event that is a no-op (Trame suppresses events when the value is unchanged), so there is no infinite loop. + +No pre-change hook exists in Vuetify/Trame — `VListItemGroup v_model` writes state immediately on click — so interception must happen inside the callback after the write. + ### EditSession Geometry Modes `edit_session.py` supports three modes selected per-field: diff --git a/TODO.md b/TODO.md index a0f8b32..cfe7415 100644 --- a/TODO.md +++ b/TODO.md @@ -2,9 +2,17 @@ ## Priorita alta +- **Performance selezione su dataset grandi** — bottleneck documentati in + `docs/logic_flows.md` §5a e §6. Piano a step progressivi in [issue #34](https://github.com/2listic/coral-visualizer/issues/34); ogni step + è indipendente e lascia i test esistenti verdi. Step 1 può essere fatto direttamente + su `paraview_backend.py` senza refactor preventivo. + - Separare `paraview_backend.py` in servizi di dominio più piccoli (3900 righe). - Iniziare con `backend/selection.py` + `backend/selection_geometry.py`. - Vedi [docs/backend_refactor.md](docs/backend_refactor.md) per strategia, layout moduli e firme. + Iniziare da moduli con dipendenze unidirezionali (`backend/display.py`, + `backend/coloring.py`, `backend/pipeline.py`) — più self-contained e a minor rischio + di regressione rispetto alla selezione, che dipende trasversalmente dallo stato del + backend. La selezione segue dopo. Vedi [docs/backend_refactor.md](docs/backend_refactor.md) + per strategia, layout moduli e firme. ## Priorita media @@ -20,10 +28,7 @@ ## Miglioramenti futuri -- **Performance selezione su dataset grandi**: identificare il bottleneck (pick - query, overlay rebuild, state sync). Da affrontare dopo aver estratto e - compreso `backend/selection.py`. -- **Riorganizzazione UI/UX**: migliorare l'esperienza utente dopo aver spezzato +- **Riorganizzazione UI/UX**: migliorare l'esperienza utente eventualmente dopo aver spezzato `ui.py` in moduli navigabili. ## Riorganizzazione cartelle diff --git a/docs/logic_flows.md b/docs/logic_flows.md index 543a3f5..4173000 100644 --- a/docs/logic_flows.md +++ b/docs/logic_flows.md @@ -88,37 +88,194 @@ ctrl.pv_add_filter(filter_key) [paraview_controllers.py:689] ctrl.pv_begin_edit_session() ├── pv_backend.export_active_dataset_for_editing() — pulls dataset from ParaView server into a local VTK object (no further server involvement) ├── edit_session.begin(node_id, label, filename, dataset) — DeepCopy into working_dataset, reset all session state, precompute CellCenters array - ├── pv_backend.set_edit_target_dataset(working_dataset) - ├── state.pick_mode = True, mainViewMode = "remote" - └── sync_edit_session_state() + render_and_push() + ├── pv_backend.set_edit_target_dataset(working_dataset) — registers working_dataset so picks can be remapped onto it + ├── state.pick_mode = True, mainViewMode = "remote" — enables hardware picking; forces server-side rendering + └── sync_edit_session_state() + render_and_push() — flush edit state to UI and send first frame [Selection — click or box drag] -ctrl.pv_edit_click_selection(event) / ctrl.pv_edit_box_selection(event) +ctrl.pv_edit_click_selection(event) / ctrl.pv_edit_box_selection(event) — entry point for click and box drag gestures ├── normalize_edit_selection_ids(event) — extract screen coords or composite IDs - ├── _pick_edit_ids_at_coords(mode, x, y) / _pick_edit_ids_in_rect(mode, ...) + ├── _pick_edit_ids_at_coords(mode, x, y) / _pick_edit_ids_in_rect(mode, ...) — dispatch to click or rect pick │ └── pv_backend.pick_visible_cell_ids(x, y) — ParaView hardware pick - └── _apply_selection_ids(picked_ids, ...) - ├── edit_session.replace/add/subtract/flip_selection(picked_ids) - ├── sync_edit_session_state() + └── _apply_selection_ids(picked_ids, ...) — apply selection mode (replace/add/subtract/flip) and update overlay + ├── edit_session.replace/add/subtract/flip_selection(picked_ids) — mutate selected_cell_ids / surface_keys / point_ids + ├── sync_edit_session_state() — update selection count and field readiness in Trame state ├── sync_paraview_edit_selection_overlay() — highlight selected cells in view - └── render_and_push() + └── render_and_push() — send updated frame to browser [Assign value] -ctrl.pv_apply_edit_field() - └── _apply_edit_field() +ctrl.pv_apply_edit_field() — triggered by "Apply" button in Edit Tools panel + └── _apply_edit_field() — resolves field, runs expression, writes values into working_dataset ├── _parse_field_choice() — "cell:FieldName" → (association, name) - ├── edit_session.assign_to_selected(field_name, association, expression) - └── sync_edit_session_state() + ├── edit_session.assign_to_selected(field_name, association, expression) — evaluate expression via vtkArrayCalculator and write results to selected cells/points + └── sync_edit_session_state() — refresh dirty flag and field state in UI [Commit] -ctrl.pv_commit_edit_session() - └── _commit_edit_session() +ctrl.pv_commit_edit_session() — opens save dialog; actual write happens in _commit_edit_session + └── _commit_edit_session() — save, tear down session, reload result as new pipeline node ├── save_paraview_output() — write modified dataset to disk - ├── edit_session.clear() + ├── edit_session.clear() — reset all session state and release working_dataset ├── pv_backend.load_file(output_path) — add saved file back as new pipeline node - ├── update_paraview_ui_state() - └── render_and_push() + ├── update_paraview_ui_state() — full state flush: arrays, display, pipeline tree + └── render_and_push() — send final frame showing new node +``` + +### 5a. Data movement, process model, and bottlenecks + +The diagram below tracks what happens to the mesh data across the same four phases +as section 5. Read section 5 to trace function calls; read this to understand data +ownership, copies, and where the performance costs land. + +The app is **single-process** (one Python process hosts Trame, ParaView pipeline, and +edit logic). VTK C++ filter work uses multi-thread internally via `vtkMultiThreader`; +Python orchestration is single-threaded. There is no MPI: all data lives in one process. +If a pvserver were connected via `simple.Connect()`, `servermanager.Fetch()` would +still collapse all distributed ranks into this one process at the points marked below. + +``` +─── SESSION BEGIN ────────────────────────────────────────────────────────────────── + [paraview_controllers.py:721 pv_begin_edit_session] + [paraview_backend.py:400 export_active_dataset_for_editing] + [edit_session.py:71 EditSession.begin] + + ParaView in-process pipeline (proxy layer, vtkSMProxy stubs) + source proxy → UpdatePipeline() + │ + ▼ + servermanager.Fetch(source) ⚠ BOTTLENECK: entire mesh transferred into + [paraview_backend.py:410] one Python vtkUnstructuredGrid object. + │ In MPI mode all ranks would be merged here. + ▼ + vtkUnstructuredGrid (local Python) + │ + ▼ + vtkUnstructuredGrid.DeepCopy() ⚠ full mesh copy — all points, all cells of + [edit_session.py:85] all dimensions, all point/cell data arrays. + ▼ + edit_session.working_dataset independent copy; field assignments, + │ selections, and expressions operate only + │ on this dataset — picks still go through + │ the ParaView source and are remapped here. + │ + ├── pv_backend.set_edit_target_dataset(working_dataset, source_dataset=exported["dataset"]) + │ [paraview_backend.py:156] + │ registers copy so subsequent picks can be remapped onto it; + │ also pre-builds four remap caches from the already-fetched source: + │ _edit_source_dataset — the fetched source (avoids re-Fetch per pick) + │ _edit_source_point_indexes — coord → point_id index for the source + │ _edit_target_cell_map — cell geometry → cell_id map for working_dataset + │ _edit_point_id_map — source point_id → working_dataset point_id map + │ source_dataset is passed in from export_active_dataset_for_editing so no + │ extra Fetch is needed here + │ + └── _ensure_cell_centers_array() + [edit_session.py:1167] + adds CellCenters cell-data array (XYZ per cell centroid) so + vtkArrayCalculator can reference spatial coords in expressions + +─── DURING SESSION — two parallel structures ────────────────────────────────────── + + ParaView side (pipeline proxy) Edit session side (working_dataset) + ──────────────────────────────── ────────────────────────────────────── + original source rendered in viewport holds all mutations (assigned field values) + picked by SelectSurfaceCells() tracks selected_cell_ids / surface_keys / + selected_point_ids + + cell IDs from rendered source + │ + ▼ + _remap_cell_ids_to_edit_target_dataset() Uses pre-built caches: + [paraview_backend.py:2752] _edit_source_dataset → no Fetch per pick + _remap_surface_keys_to_edit_target_dataset() _edit_target_cell_map → no map rebuild + [paraview_backend.py:2812] _edit_point_id_map → O(1) point lookup + │ + │ Fallback: if caches are absent (e.g. tests + │ that set _edit_target_dataset directly), + │ both methods rebuild the structures inline — + │ same correctness, O(n) per pick as before. + ▼ + IDs on working_dataset + + CellCenters array (cell data) GetPoint() calls (raw mesh geometry) + └─ vtkArrayCalculator reads it └─ _ensure_surface_element_vectors() + when evaluating user expressions [edit_session.py:995] + (e.g. "CellCenters[0] > 2.5") computes unit normals/tangents for + Stripped from output on save. dihedral angle filter during grow. + Never stored as an array. + +─── COMMIT ───────────────────────────────────────────────────────────────────────── + [paraview_controllers.py:108 _commit_edit_session] + + edit_session.working_dataset + │ + ├── _remove_internal_edit_arrays() strips CellCenters before write + │ [edit_session.py:1200] + ▼ + vtkXMLUnstructuredGridWriter.Write() ⚠ full mesh write to disk + │ + ▼ + pv_backend.load_file(output_path) ⚠ full mesh re-read into ParaView pipeline + │ ParaView owns it again as a new node. + ▼ + new pipeline node — normal visualization resumes, edit bridge torn down +``` + +Key constraints that follow from this model: + +- **Edit mode cannot be distributed**: `Fetch()` collapses all data to one process. + For very large meshes the `Fetch` + `DeepCopy` at session begin is the dominant cost. +- **Remap on every pick** *(mitigated)*: coordinate-based cell matching between source + and working dataset was rebuilt on every pick event. Now the structures are built once + at session begin in `set_edit_target_dataset` and reused per pick (O(1) lookups). The + fallback rebuild path remains for correctness when caches are absent. +- **Commit writes the whole mesh**: there is no delta/patch write — the full working + dataset is serialized even if only a handful of cell values changed. +- **Normal visualization (no edit) is unaffected**: filters run in ParaView's proxy + layer with multithread VTK SMP parallelism; only `Fetch()` calls (rare, for metadata) + touch the single-process ceiling. + +--- + +### 5b. Dataset naming conventions + +The edit-session code uses several "dataset" variables and cache fields with similar names. +This is the reference for what each one is, who owns it, how long it lives, and why it exists. + +| Name | Type | Owner | Lifetime | Role | +|---|---|---|---|---| +| `source` | ParaView proxy (`vtkSMProxy`) | `ParaViewBackend` (property) | permanent | Active pipeline node on the server side. Used for hardware picks (`SelectSurfaceCells`) and `Fetch` calls. Never mutated. | +| `source_dataset` | `vtkUnstructuredGrid` | local var (caller) | per call | Result of `servermanager.Fetch(source)`. A read-only local copy used to build indexes or remap IDs. Fetched on demand wherever needed; not stored — except see `_edit_source_dataset`. | +| `working_dataset` | `vtkUnstructuredGrid` | `EditSession` | session | `DeepCopy` of `source_dataset` made once at session begin. The **mutation target**: field assignments, expression results, and selection tracking all land here. | +| `_edit_target_dataset` | reference to `working_dataset` | `ParaViewBackend` | session | Registered via `set_edit_target_dataset()` so remap methods know where to translate picks onto. Same Python object as `working_dataset` — not a copy. | +| `_edit_source_dataset` | `vtkUnstructuredGrid` | `ParaViewBackend` | session | The one `source_dataset` kept alive past its call. Cached at session begin to avoid re-Fetching on every pick. Passed in from `export_active_dataset_for_editing` which already has it. | +| `_edit_source_point_indexes` | `list[dict]` | `ParaViewBackend` | session | Coordinate → point-ID index derived from `_edit_source_dataset`. Lets `_surface_keys_from_selected_dataset` match surface-pick coordinates back to source point IDs without rebuilding on every pick. | +| `_edit_target_cell_map` | `dict` | `ParaViewBackend` | session | Cell-geometry → cell-ID map derived from `working_dataset`. Lets `_remap_cell_ids_between_datasets` translate source cell IDs onto working-dataset cell IDs in O(1) per cell rather than O(n_cells) per pick. | +| `_edit_point_id_map` | `dict[int, int]` | `ParaViewBackend` | session | Source point-ID → working-dataset point-ID map. Lets `_remap_surface_keys_to_edit_target_dataset` translate surface picks from source space to working-dataset space without rebuilding on every pick. | +| `edit_dataset` | alias (local var) | `_pick_surface_keys_native` | per call | Convenience alias within that one method: equals `_edit_target_dataset` when a session is active, falls back to a fresh `source_dataset` fetch otherwise. Read-only. | +| `selected_dataset` | `vtkUnstructuredGrid` | pick methods (local var) | per pick | Result of `ExtractSelection` + `Fetch` after a hardware pick. A small subset of source/GeometryFilter cells that ParaView determined were hit. Deleted after each pick. | + +The four session-scoped `ParaViewBackend` fields (`_edit_source_dataset`, `_edit_source_point_indexes`, +`_edit_target_cell_map`, `_edit_point_id_map`) are built together by `set_edit_target_dataset()` at +session begin and cleared together by `clear_edit_target_dataset()` at session end. + +**Identity chain:** + ``` +source (proxy — server side) + │ servermanager.Fetch() ← pulls data across the proxy boundary + ▼ +source_dataset (vtkUG, read-only) ← also cached as _edit_source_dataset + │ DeepCopy() at session begin + ▼ +working_dataset (vtkUG, mutable) ← also referenced as _edit_target_dataset +``` + +The remap exists because picks go through `source` (proxy) and return IDs relative to +`source_dataset`, but mutations must land on `working_dataset`. Both datasets have the +same geometry but are independent Python objects with independent ID spaces, so +coordinate-based maps (`_edit_target_cell_map`, `_edit_point_id_map`) are needed to +translate IDs from one to the other. These maps are now built once at session begin +(see §5a) rather than on every pick. --- @@ -164,9 +321,9 @@ UI click event │ — clears ParaView's purple selection highlight before returning │ └── _remap_cell_ids_to_edit_target_dataset(picked, source) - — when an edit session is active the source is the pipeline proxy - but the edit session holds a separate local copy of the mesh; - this maps IDs across by matching cell geometry (sorted point coords) + — maps source cell IDs onto working_dataset cell IDs; + uses _edit_source_dataset (no Fetch) and _edit_target_cell_map + (no rebuild) when caches are populated at session begin ``` ### 6b. Box drag pick — cell or point mode @@ -205,26 +362,34 @@ pv_backend.pick_visible_surface_keys(x, y, radius=2) │ ├── hide original display, show surface helper temporarily │ ├── simple.SelectSurfaceCells(rect, on temp_source) │ ├── simple.ExtractSelection(Input=temp_source) + servermanager.Fetch() - │ └── _surface_keys_from_selected_dataset(selected, source_dataset) - │ ├── _iter_leaf_datasets() — flatten composite blocks - │ ├── _point_coordinate_indexes() — multi-precision coord → point_id map - │ ├── for each selected cell: map XYZ corners → source point IDs via - │ │ coordinate lookup (PassThroughPointIds is not used: in ParaView 6.1 - │ │ simple.GeometryFilter's PassThroughPointIds produces an identity - │ │ mapping through the proxy/Fetch round-trip, not source IDs) - │ └── _normalize_surface_keys_to_source_boundary() - │ — ParaView may triangulate quads; resolves partial triangle keys - │ back to the canonical quad key in the source boundary map + │ ├── _surface_keys_from_selected_dataset(selected, source_dataset, + │ │ │ source_point_indexes=_edit_source_point_indexes) + │ │ ├── _iter_leaf_datasets() — flatten composite blocks + │ │ ├── coordinate index: uses pre-built source_point_indexes when provided + │ │ │ (avoids O(n_source_points) rebuild per pick); lazy-builds if absent + │ │ ├── for each selected cell: map XYZ corners → source point IDs via + │ │ │ coordinate lookup (neither PassThroughPointIds nor PassThroughCellIds + │ │ │ is used: in ParaView 6.1 both produce output-geometry indices through + │ │ │ the proxy/Fetch round-trip, not source topology IDs — see PR #31, issue #34) + │ │ └── _normalize_surface_keys_to_source_boundary() + │ │ — ParaView may triangulate quads; resolves partial triangle keys + │ │ back to the canonical quad key in the source boundary map + │ └── _remap_surface_keys_to_edit_target_dataset(keys, source_dataset) + │ — maps source point IDs onto working_dataset point IDs; + │ uses _edit_point_id_map cache when populated at session begin │ └── [fallback path — 2D meshes, or if native path returns None] ├── servermanager.Fetch(source) ├── _cached_boundary_elements(dataset) │ — finds codimension-1 entities (faces for 3D, edges for 2D) that │ appear exactly once: these are the visible boundary elements - └── for each boundary element: - ├── _surface_element_is_visible() — depth-buffer check at centroid - ├── _project_points_to_display() — world → pixel coords via renderer - └── _polyline_intersects_rect() or all-inside check + ├── for each boundary element: + │ ├── _surface_element_is_visible() — depth-buffer check at centroid + │ ├── _project_points_to_display() — world → pixel coords via renderer + │ └── _polyline_intersects_rect() or all-inside check + └── _remap_surface_keys_to_edit_target_dataset(picked, source_dataset=dataset) + — maps source point IDs onto working_dataset point IDs; + uses _edit_point_id_map cache when populated at session begin ``` ### 6d. Overlay update (after any successful pick) diff --git a/edit_session.py b/edit_session.py index 4bde05e..f63f26e 100644 --- a/edit_session.py +++ b/edit_session.py @@ -1,5 +1,7 @@ """Edit-session scaffolding for the ParaView-backed workflow.""" +from __future__ import annotations + from pathlib import Path from math import acos, degrees, sqrt from typing import TypedDict @@ -28,26 +30,32 @@ class EditSession: """Owns the temporary editable dataset derived from the active pipeline node.""" def __init__(self): - self.active = False - self.source_node_id = None - self.source_label = "" - self.source_filename = "" - self.working_dataset = None - self.dirty = False - self.geometry_mode = "volume" - self.field_name = "" - self.expression = "" - self.default_value = "0" - self.selected_cell_ids = set() - self.selected_surface_keys = set() - self.selected_point_ids = set() - self._volume_adjacency = None - self._surface_boundary_map = None - self._existing_codim_keys_cache = {} - self._surface_adjacency = None - self._surface_element_vectors = None + self.active: bool = False + self.source_node_id: str | None = None + self.source_label: str = "" + self.source_filename: str = "" + self.working_dataset: vtkUnstructuredGrid | None = ( + None # DeepCopy of source, mutation target — see docs/logic_flows.md §5b + ) + self.dirty: bool = False + self.geometry_mode: str = "volume" + self.field_name: str = "" + self.expression: str = "" + self.default_value: str = "0" + self.selected_cell_ids: set[int] = set() + self.selected_surface_keys: set[tuple[int, ...]] = set() + self.selected_point_ids: set[int] = set() + self._volume_adjacency: dict[int, set[int]] | None = None + self._surface_boundary_map: dict[tuple[int, ...], tuple] | None = None + self._existing_codim_keys_cache: dict[int, set[tuple[int, ...]]] = {} + self._surface_adjacency: dict[tuple[int, ...], set[tuple[int, ...]]] | None = ( + None + ) + self._surface_element_vectors: ( + dict[tuple[int, ...], tuple[float, float, float]] | None + ) = None - def clear(self): + def clear(self) -> None: """Reset the session to an inactive state.""" self.active = False self.source_node_id = None @@ -104,7 +112,7 @@ def begin( self._surface_element_vectors = None self._ensure_cell_centers_array() - def default_output_filename(self): + def default_output_filename(self) -> str: """Return a suggested filename for the current edit-session output.""" label = ( self.source_label or Path(self.source_filename or "edited").stem or "edited" @@ -118,7 +126,7 @@ def default_output_filename(self): safe_label = f"{safe_label}_edited" return safe_label + ".vtu" - def save(self, output_path): + def save(self, output_path: str | Path) -> str: """Persist the working dataset to disk.""" if not self.active or self.working_dataset is None: raise RuntimeError("No active edit session to save") @@ -143,7 +151,7 @@ def save(self, output_path): raise RuntimeError(f"Failed to write edited dataset to {output_path}") return str(output_path) - def available_cell_variables(self): + def available_cell_variables(self) -> list[str]: """Return cell-data variable names available to the edit calculator.""" if not self.active or self.working_dataset is None: return [] @@ -155,7 +163,7 @@ def available_cell_variables(self): if cell_data.GetArrayName(i) ] - def available_point_variables(self): + def available_point_variables(self) -> list[str]: """Return point-data variable names available to the edit calculator.""" if not self.active or self.working_dataset is None: return [] @@ -166,7 +174,7 @@ def available_point_variables(self): if point_data.GetArrayName(i) ] - def available_fields(self): + def available_fields(self) -> list[dict[str, str]]: """Return both point and cell scalar/vector arrays for field selection.""" fields = [] for name in self.available_cell_variables(): @@ -176,7 +184,7 @@ def available_fields(self): fields.sort(key=lambda item: item["text"].lower()) return fields - def has_cell_field(self, field_name): + def has_cell_field(self, field_name: str) -> bool: """Return True when a cell-data field with ``field_name`` already exists.""" if not self.active or self.working_dataset is None: return False @@ -185,12 +193,12 @@ def has_cell_field(self, field_name): return False return self.working_dataset.GetCellData().GetArray(name) is not None - def has_field(self, field_name, association): + def has_field(self, field_name: str, association: str) -> bool: """Return True when the requested data array already exists.""" array = self._get_data_array((field_name or "").strip(), association) return array is not None - def infer_field_association(self, field_name): + def infer_field_association(self, field_name: str) -> str | None: """Best-effort association lookup for an existing field name.""" name = (field_name or "").strip() if not name or not self.active or self.working_dataset is None: @@ -201,7 +209,7 @@ def infer_field_association(self, field_name): return "point" return None - def selected_count(self): + def selected_count(self) -> int: """Return the number of currently selected cells.""" if self.geometry_mode == "surface": return len(self.selected_surface_keys) @@ -209,13 +217,13 @@ def selected_count(self): return len(self.selected_point_ids) return len(self.selected_cell_ids) - def clear_selection(self): + def clear_selection(self) -> None: """Drop the current volume-cell selection.""" self.selected_cell_ids.clear() self.selected_surface_keys.clear() self.selected_point_ids.clear() - def select_all_cells(self): + def select_all_cells(self) -> int: """Select every cell in the working dataset.""" if not self.active or self.working_dataset is None: return 0 @@ -230,7 +238,12 @@ def select_all_cells(self): self.selected_cell_ids = set(range(self.working_dataset.GetNumberOfCells())) return len(self.selected_cell_ids) - def toggle_cell_selection(self, cell_id, grow=False, angle_threshold=None): + def toggle_cell_selection( + self, + cell_id: int | float | tuple[int, ...], + grow: bool = False, + angle_threshold: float | None = None, + ) -> int: """Toggle a single picked cell, optionally growing by adjacency.""" if not self.active or self.working_dataset is None: return 0 @@ -283,7 +296,9 @@ def toggle_cell_selection(self, cell_id, grow=False, angle_threshold=None): return len(self.selected_cell_ids) - def replace_selection(self, cell_ids, grow=False, angle_threshold=None): + def replace_selection( + self, cell_ids: list, grow: bool = False, angle_threshold: float | None = None + ) -> int: """Replace the current selection with the provided cell IDs.""" if not self.active or self.working_dataset is None: return 0 @@ -318,7 +333,9 @@ def replace_selection(self, cell_ids, grow=False, angle_threshold=None): self.selected_cell_ids = normalized return len(self.selected_cell_ids) - def add_selection(self, cell_ids, grow=False, angle_threshold=None): + def add_selection( + self, cell_ids: list, grow: bool = False, angle_threshold: float | None = None + ) -> int: """Union the provided cell IDs into the current selection.""" if not self.active or self.working_dataset is None: return 0 @@ -354,7 +371,9 @@ def add_selection(self, cell_ids, grow=False, angle_threshold=None): self.selected_cell_ids |= normalized return len(self.selected_cell_ids) - def subtract_selection(self, cell_ids, grow=False, angle_threshold=None): + def subtract_selection( + self, cell_ids: list, grow: bool = False, angle_threshold: float | None = None + ) -> int: """Remove the provided cell IDs from the current selection.""" if not self.active or self.working_dataset is None: return 0 @@ -390,7 +409,9 @@ def subtract_selection(self, cell_ids, grow=False, angle_threshold=None): self.selected_cell_ids -= normalized return len(self.selected_cell_ids) - def flip_selection(self, cell_ids, grow=False, angle_threshold=None): + def flip_selection( + self, cell_ids: list, grow: bool = False, angle_threshold: float | None = None + ) -> int: """Toggle the provided cell IDs against the current selection.""" if not self.active or self.working_dataset is None: return 0 @@ -439,7 +460,13 @@ def flip_selection(self, cell_ids, grow=False, angle_threshold=None): self.selected_cell_ids.add(cell_id) return len(self.selected_cell_ids) - def create_field(self, field_name, association, default_value, overwrite=False): + def create_field( + self, + field_name: str, + association: str, + default_value: float | str, + overwrite: bool = False, + ) -> str: """Create a scalar field across all tuples in the chosen association.""" if not self.active or self.working_dataset is None: raise RuntimeError("No active edit session") @@ -476,7 +503,9 @@ def create_field(self, field_name, association, default_value, overwrite=False): self.dirty = True return field_name - def assign_to_selected(self, field_name, association, expression): + def assign_to_selected( + self, field_name: str, association: str, expression: str + ) -> str: """Assign the expression value to selected entities in the target field.""" if not self.active or self.working_dataset is None: raise RuntimeError("No active edit session") @@ -558,11 +587,11 @@ def _evaluate_expression(self, association, expression): values[idx] = float(result_array.GetTuple1(idx)) return values - def build_selected_volume_dataset(self): + def build_selected_volume_dataset(self) -> vtkUnstructuredGrid | None: """Return a lightweight dataset containing the currently selected cells.""" return self.build_selected_dataset() - def build_selected_dataset(self): + def build_selected_dataset(self) -> vtkUnstructuredGrid | None: """Return a lightweight dataset containing currently selected editable entities.""" if not self.active or self.working_dataset is None: return None @@ -599,7 +628,7 @@ def build_selected_dataset(self): output.DeepCopy(extractor.GetOutput()) return output - def materialize_surface_selection(self): + def materialize_surface_selection(self) -> int: """Append selected boundary faces/edges as missing codim-1 cells.""" if not self.active or self.working_dataset is None: return 0 @@ -642,7 +671,7 @@ def materialize_surface_selection(self): return len(missing_keys) @staticmethod - def _normalize_association(association): + def _normalize_association(association: str) -> str: value = (association or "cell").strip().lower() if value not in {"cell", "point"}: raise ValueError("Field association must be 'cell' or 'point'") @@ -656,7 +685,7 @@ def _dataset_data_for_association(self, association): else self.working_dataset.GetCellData() ) - def _tuple_count_for_association(self, association): + def _tuple_count_for_association(self, association: str) -> int: association = self._normalize_association(association) if association == "point": return self.working_dataset.GetNumberOfPoints() @@ -681,7 +710,7 @@ def _selected_ids_for_association(self, association): return sorted(self._selected_surface_cell_ids()) return sorted(int(cell_id) for cell_id in self.selected_cell_ids) - def _grow_volume_selection(self, seed_ids): + def _grow_volume_selection(self, seed_ids) -> set[int]: """Expand a set of cells by shared-face/shared-edge adjacency.""" seeds = set(seed_ids or []) if not seeds: @@ -701,7 +730,9 @@ def _grow_volume_selection(self, seed_ids): return visited - def _grow_surface_selection(self, seed_keys, angle_threshold=None): + def _grow_surface_selection( + self, seed_keys, angle_threshold: float | None = None + ) -> set[tuple[int, ...]]: """Expand surface keys transitively while respecting neighbor-angle threshold.""" seeds = {tuple(key) for key in (seed_keys or [])} if not seeds: @@ -725,7 +756,7 @@ def _grow_surface_selection(self, seed_keys, angle_threshold=None): pending.append(neighbor) return grown - def _top_dimension(self): + def _top_dimension(self) -> int: if self.working_dataset is None: return 0 return max( @@ -923,7 +954,7 @@ def _invalidate_geometry_caches(self): self._surface_adjacency = None self._surface_element_vectors = None - def _ensure_surface_adjacency(self): + def _ensure_surface_adjacency(self) -> dict[tuple[int, ...], set[tuple[int, ...]]]: """Build and cache codim-1 adjacency graph for surface-mode grow selection.""" if self._surface_adjacency is not None: return self._surface_adjacency @@ -968,7 +999,7 @@ def _ensure_surface_adjacency(self): return self._surface_adjacency @staticmethod - def _normalized_angle_threshold(angle_threshold): + def _normalized_angle_threshold(angle_threshold) -> float | None: if angle_threshold is None: return None try: @@ -981,7 +1012,9 @@ def _normalized_angle_threshold(angle_threshold): return 180.0 return threshold - def _surface_neighbor_angle_degrees(self, key_a, key_b): + def _surface_neighbor_angle_degrees( + self, key_a: tuple[int, ...], key_b: tuple[int, ...] + ) -> float: """Return angle between surface element directions in degrees.""" vectors = self._ensure_surface_element_vectors() va = vectors.get(tuple(key_a)) @@ -992,7 +1025,9 @@ def _surface_neighbor_angle_degrees(self, key_a, key_b): dot = max(-1.0, min(1.0, abs(dot))) return degrees(acos(dot)) - def _ensure_surface_element_vectors(self): + def _ensure_surface_element_vectors( + self, + ) -> dict[tuple[int, ...], tuple[float, float, float]]: """Build and cache representative unit vectors for selectable surface elements.""" if self._surface_element_vectors is not None: return self._surface_element_vectors @@ -1099,7 +1134,7 @@ def _extend_cell_data_for_new_cells(dataset, old_cell_count, owner_cell_ids=None # Best effort: keep tuple resize even if component assignment is unsupported. break - def _ensure_volume_adjacency(self): + def _ensure_volume_adjacency(self) -> dict[int, set[int]]: """Build and cache a face/edge adjacency graph for top-dimensional cells.""" if self._volume_adjacency is not None: return self._volume_adjacency @@ -1164,7 +1199,7 @@ def getter(idx, _cell=cell): self._volume_adjacency = adjacency return self._volume_adjacency - def _ensure_cell_centers_array(self): + def _ensure_cell_centers_array(self) -> None: """Ensure the working dataset exposes ``CellCenters`` in cell data.""" if self.working_dataset is None: return @@ -1197,7 +1232,7 @@ def _ensure_cell_centers_array(self): cell_data.AddArray(array) self.working_dataset.Modified() - def _remove_internal_edit_arrays(self): + def _remove_internal_edit_arrays(self) -> None: """Drop edit-only helper arrays before persisting user data.""" if self.working_dataset is None: return @@ -1207,6 +1242,6 @@ def _remove_internal_edit_arrays(self): self.working_dataset.Modified() @staticmethod - def is_supported_dataset(dataset): + def is_supported_dataset(dataset) -> bool: """Return True when the dataset type is currently editable.""" return isinstance(dataset, vtkUnstructuredGrid) diff --git a/handler_registration.py b/handler_registration.py index 3d64030..df4e7d6 100644 --- a/handler_registration.py +++ b/handler_registration.py @@ -55,6 +55,7 @@ def register_app_handlers( register_state_handlers( state, paraview_runtime=paraview_runtime, + edit_session=edit_session, interaction_quality_presets=interaction_quality_presets, ) diff --git a/paraview_backend.py b/paraview_backend.py index 7fe8641..63794c2 100644 --- a/paraview_backend.py +++ b/paraview_backend.py @@ -143,7 +143,14 @@ def __init__(self, data_directory=None, show_experimental_filters=True, state=No self._edit_selection_overlay = None self._edit_selection_display = None self._scalar_bar_visible = False - self._edit_target_dataset = None + # Edit-session dataset references and remap caches — see docs/logic_flows.md §5b. + self._edit_target_dataset: vtkDataSet | None = None # ref to working_dataset + self._edit_source_dataset: vtkDataSet | None = ( + None # fetched source, cached at begin + ) + self._edit_source_point_indexes: list[dict] | None = None # coord→pt_id index + self._edit_target_cell_map: dict | None = None # cell_key→cell_id + self._edit_point_id_map: dict[int, int] | None = None # src_pt_id→target_pt_id self._surface_selection_helper = None self._last_selection_backend_timing = [] self._boundary_cache = {} @@ -153,15 +160,44 @@ def __init__(self, data_directory=None, show_experimental_filters=True, state=No "QuadraticTetra": "QuadraticTetrahedron", } - def set_edit_target_dataset(self, dataset): - """Set the edit-session dataset used to normalize picker IDs.""" + def set_edit_target_dataset( + self, dataset: vtkDataSet | None, source_dataset: vtkDataSet | None = None + ) -> None: + """Set the edit-session dataset and pre-build remap caches. + + source_dataset may be passed to avoid a redundant Fetch when the caller + already holds the fetched source (e.g. pv_begin_edit_session). + """ self._edit_target_dataset = dataset self._clear_surface_selection_helper() self._clear_boundary_cache() + if source_dataset is None and self.source is not None: + try: + source_dataset = self.servermanager.Fetch(self.source) + except Exception: + source_dataset = None + self._edit_source_dataset = source_dataset + self._edit_source_point_indexes = ( + self._point_coordinate_indexes(source_dataset) + if source_dataset is not None + else None + ) + self._edit_target_cell_map = ( + self._build_target_cell_map(dataset) if dataset is not None else None + ) + self._edit_point_id_map = ( + self._point_id_map_between_datasets(source_dataset, dataset) + if source_dataset is not None and dataset is not None + else None + ) - def clear_edit_target_dataset(self): + def clear_edit_target_dataset(self) -> None: """Clear edit-session dataset normalization context.""" self._edit_target_dataset = None + self._edit_source_dataset = None + self._edit_source_point_indexes = None + self._edit_target_cell_map = None + self._edit_point_id_map = None self._clear_surface_selection_helper() self._clear_boundary_cache() @@ -173,13 +209,21 @@ def consume_selection_backend_timing(self): @property def source(self): - """Return the active ParaView source proxy.""" + """Active pipeline node proxy (vtkSMProxy, server-side). + + Used for hardware picks (SelectSurfaceCells) and Fetch calls. Never mutated. + Not a dataset — call servermanager.Fetch(source) to get a local vtkUnstructuredGrid. + See docs/logic_flows.md §5b for the full dataset naming conventions. + """ node = self._get_active_node() return None if node is None else node["source"] @property def display(self): - """Return the active ParaView display proxy.""" + """Active pipeline node display proxy (vtkSMProxy, server-side). + + Controls representation, coloring, and scalar bar visibility for the active node. + """ node = self._get_active_node() return None if node is None else node["display"] @@ -2128,7 +2172,9 @@ def record_phase(name, start): record_phase("fetch_source_dataset", phase_start) phase_start = time.perf_counter() keys = self._surface_keys_from_selected_dataset( - selected_dataset, source_dataset=source_dataset + selected_dataset, + source_dataset=source_dataset, + source_point_indexes=self._edit_source_point_indexes, ) record_phase("map_selection_keys", phase_start) if keys is None: @@ -2266,13 +2312,22 @@ def _clear_surface_selection_helper(self): self._surface_selection_helper = None @staticmethod - def _surface_keys_from_selected_dataset(selected_dataset, source_dataset=None): + def _surface_keys_from_selected_dataset( + selected_dataset: vtkDataSet | None, + source_dataset: vtkDataSet | None = None, + source_point_indexes: list[dict] | None = None, + ) -> list | None: """Map selected GeometryFilter cells back to original source point-id keys. Builds a coordinate index from source_dataset and maps each selected cell's - XYZ corners back to source point IDs. vtkOriginalPointIds is not used: in - ParaView 6.1, simple.GeometryFilter's PassThroughPointIds does not produce - reliable source IDs through the proxy/server-fetch round-trip. + XYZ corners back to source point IDs. Neither vtkOriginalPointIds nor + vtkOriginalCellIds is used: in ParaView 6.1 both PassThroughPointIds and + PassThroughCellIds on GeometryFilter produce output-geometry indices through + the proxy/server-fetch round-trip rather than source topology IDs (see PR #31 + and issue #34). + + source_point_indexes may be passed as a pre-built index (cached at edit-session + begin, B4) to skip the O(n_source_points) build on each pick. """ if source_dataset is None: return None @@ -2280,22 +2335,18 @@ def _surface_keys_from_selected_dataset(selected_dataset, source_dataset=None): if not selected_blocks: return [] - source_point_indexes = None + # Use mutable container so the closure can lazily populate it when not pre-built. + _indexes = [source_point_indexes] def coordinate_key(block, cell): - nonlocal source_point_indexes if not hasattr(source_dataset, "GetNumberOfPoints"): return None - if source_point_indexes is None: - source_point_indexes = ParaViewBackend._point_coordinate_indexes( - source_dataset - ) + if _indexes[0] is None: + _indexes[0] = ParaViewBackend._point_coordinate_indexes(source_dataset) mapped = [] for point_idx in range(cell.GetNumberOfPoints()): point = block.GetPoint(cell.GetPointId(point_idx)) - point_id = ParaViewBackend._lookup_point_coordinate( - source_point_indexes, point - ) + point_id = ParaViewBackend._lookup_point_coordinate(_indexes[0], point) if point_id is None: return None mapped.append(int(point_id)) @@ -2749,19 +2800,26 @@ def _fetch_selected_original_cell_ids(self, source): return selected_ids - def _remap_cell_ids_to_edit_target_dataset(self, cell_ids, source): + def _remap_cell_ids_to_edit_target_dataset( + self, cell_ids: list[int], source: Any + ) -> list[int]: """Normalize picked source cell IDs onto the active edit-session dataset.""" picked_ids = [int(cell_id) for cell_id in (cell_ids or [])] target_dataset = self._edit_target_dataset if target_dataset is None or not picked_ids: return picked_ids - try: - source_dataset = self.servermanager.Fetch(source) - except Exception: - source_dataset = None + source_dataset = self._edit_source_dataset + if source_dataset is None: + try: + source_dataset = self.servermanager.Fetch(source) + except Exception: + source_dataset = None remapped = self._remap_cell_ids_between_datasets( - picked_ids, source_dataset, target_dataset + picked_ids, + source_dataset, + target_dataset, + _target_cell_map=self._edit_target_cell_map, ) print( "[selection-debug] backend.remap.cells " @@ -2770,18 +2828,24 @@ def _remap_cell_ids_to_edit_target_dataset(self, cell_ids, source): ) return remapped if remapped else picked_ids - def _remap_surface_keys_to_edit_target_dataset(self, keys, source_dataset=None): + def _remap_surface_keys_to_edit_target_dataset( + self, keys: list, source_dataset: vtkDataSet | None = None + ) -> list: """Normalize picked source boundary keys onto edit-session point IDs.""" picked_keys = [tuple(key) for key in (keys or []) if key] target_dataset = self._edit_target_dataset - if target_dataset is None or source_dataset is None or not picked_keys: + if target_dataset is None or not picked_keys: return picked_keys if source_dataset is target_dataset: return picked_keys - point_id_map = self._point_id_map_between_datasets( - source_dataset, target_dataset - ) + point_id_map = self._edit_point_id_map + if point_id_map is None: + if source_dataset is None: + return picked_keys + point_id_map = self._point_id_map_between_datasets( + source_dataset, target_dataset + ) if not point_id_map: return picked_keys @@ -2804,7 +2868,9 @@ def _remap_surface_keys_to_edit_target_dataset(self, keys, source_dataset=None): return normalized @classmethod - def _point_id_map_between_datasets(cls, source_dataset, target_dataset): + def _point_id_map_between_datasets( + cls, source_dataset: vtkDataSet | None, target_dataset: vtkDataSet | None + ) -> dict[int, int]: """Map source point IDs to target point IDs by tolerant coordinates.""" if source_dataset is None or target_dataset is None: return {} @@ -2936,7 +3002,27 @@ def _cell_coordinate_key(dataset, cell): return tuple(sorted(coords)) @staticmethod - def _remap_cell_ids_between_datasets(cell_ids, source_dataset, target_dataset): + def _build_target_cell_map(target_dataset: vtkDataSet) -> dict: + """Build cell-geometry key → cell ID map once for caching the remap lookup.""" + if not hasattr(target_dataset, "GetCell"): + return {} + cell_map = {} + for target_cell_id in range(target_dataset.GetNumberOfCells()): + target_cell = target_dataset.GetCell(target_cell_id) + key = ParaViewBackend._cell_coordinate_key(target_dataset, target_cell) + if key is None: + continue + cell_map.setdefault(key, int(target_cell_id)) + return cell_map + + @staticmethod + def _remap_cell_ids_between_datasets( + cell_ids: list[int], + source_dataset: vtkDataSet | None, + target_dataset: vtkDataSet | None, + *, + _target_cell_map: dict | None = None, + ) -> list[int]: """Map source-dataset cell ids to target-dataset ids by cell geometry.""" if not cell_ids: return [] @@ -2947,14 +3033,17 @@ def _remap_cell_ids_between_datasets(cell_ids, source_dataset, target_dataset): ): return [int(cell_id) for cell_id in cell_ids] - target_cell_map = {} target_count = target_dataset.GetNumberOfCells() - for target_cell_id in range(target_count): - target_cell = target_dataset.GetCell(target_cell_id) - key = ParaViewBackend._cell_coordinate_key(target_dataset, target_cell) - if key is None: - continue - target_cell_map.setdefault(key, int(target_cell_id)) + if _target_cell_map is not None: + target_cell_map = _target_cell_map + else: + target_cell_map = {} + for target_cell_id in range(target_count): + target_cell = target_dataset.GetCell(target_cell_id) + key = ParaViewBackend._cell_coordinate_key(target_dataset, target_cell) + if key is None: + continue + target_cell_map.setdefault(key, int(target_cell_id)) remapped = [] for cell_id in cell_ids: diff --git a/paraview_controllers.py b/paraview_controllers.py index eef61ac..71b231d 100644 --- a/paraview_controllers.py +++ b/paraview_controllers.py @@ -730,7 +730,10 @@ def pv_begin_edit_session(): exported["filename"], exported["dataset"], ) - pv_backend.set_edit_target_dataset(edit_session.working_dataset) + pv_backend.set_edit_target_dataset( + edit_session.working_dataset, + source_dataset=exported["dataset"], + ) pv_backend.apply_representation("Surface with Edges") state.pick_mode = True diff --git a/state_handlers.py b/state_handlers.py index 6aa5c5f..52c1856 100644 --- a/state_handlers.py +++ b/state_handlers.py @@ -17,6 +17,7 @@ def register_state_handlers( state, *, paraview_runtime: ParaViewRuntime, + edit_session, interaction_quality_presets, ): """Register state callbacks for the ParaView backend.""" @@ -58,6 +59,12 @@ def on_active_pipeline_item_change(active_pipeline_item, **kwargs): """Switch active ParaView node when the pipeline selection changes.""" if not active_pipeline_item: return + if edit_session.active: + notify( + state, "Cannot switch pipeline node during an edit session.", "warning" + ) + state.active_pipeline_item = paraview_runtime.pv_backend.active_node_id + return if not paraview_runtime.pv_backend.set_active_node(active_pipeline_item): return diff --git a/tests/test_e2e_edit_selection_playwright.py b/tests/test_e2e_edit_selection_playwright.py index d8f94a8..ef2c741 100644 --- a/tests/test_e2e_edit_selection_playwright.py +++ b/tests/test_e2e_edit_selection_playwright.py @@ -652,6 +652,7 @@ def test_paraview_categorical_annotations_updated_on_file_switch(shared_browser) _wait_for_switch_checked( page, "Interpret values as categories", True, timeout_s=8 ) + time.sleep(1.0) file_a_switch_png = _wait_for_stable_viewport(viewport) _set_switch(page, "Interpret values as categories", False) @@ -667,6 +668,7 @@ def test_paraview_categorical_annotations_updated_on_file_switch(shared_browser) _wait_for_switch_checked( page, "Interpret values as categories", True, timeout_s=8 ) + time.sleep(1.0) file_b_switch_png = _wait_for_stable_viewport(viewport) _set_switch(page, "Interpret values as categories", False) diff --git a/tests/test_paraview_backend.py b/tests/test_paraview_backend.py index a65c305..cb427fd 100644 --- a/tests/test_paraview_backend.py +++ b/tests/test_paraview_backend.py @@ -450,6 +450,10 @@ def make_backend(): backend._edit_selection_display = None backend._scalar_bar_visible = False backend._edit_target_dataset = None + backend._edit_source_dataset = None + backend._edit_source_point_indexes = None + backend._edit_target_cell_map = None + backend._edit_point_id_map = None backend._surface_selection_helper = None backend._boundary_cache = {} backend._last_selection_backend_timing = [] @@ -1759,6 +1763,96 @@ def GetCell(self, cell_id): assert cell_ids == [0] +def test_set_edit_target_dataset_builds_remap_caches(): + source_ds = FakeSurfaceDataset( + points=[(0.0, 0.0, 0.0), (1.0, 0.0, 0.0), (1.0, 1.0, 0.0)], + cells=[(0, 1, 2)], + ) + target_ds = FakeSurfaceDataset( + points=[(1.0, 1.0, 0.0), (1.0, 0.0, 0.0), (0.0, 0.0, 0.0)], + cells=[(2, 1, 0)], + ) + backend = make_backend() + + backend.set_edit_target_dataset(target_ds, source_dataset=source_ds) + + assert backend._edit_source_dataset is source_ds + assert backend._edit_source_point_indexes is not None + assert isinstance(backend._edit_target_cell_map, dict) + assert len(backend._edit_target_cell_map) == 1 + assert isinstance(backend._edit_point_id_map, dict) + assert len(backend._edit_point_id_map) == 3 + + +def test_set_edit_target_dataset_clears_caches_on_clear(): + source_ds = FakeSurfaceDataset( + points=[(0.0, 0.0, 0.0), (1.0, 0.0, 0.0), (1.0, 1.0, 0.0)], + cells=[(0, 1, 2)], + ) + target_ds = FakeSurfaceDataset( + points=[(1.0, 1.0, 0.0), (1.0, 0.0, 0.0), (0.0, 0.0, 0.0)], + cells=[(2, 1, 0)], + ) + backend = make_backend() + backend.set_edit_target_dataset(target_ds, source_dataset=source_ds) + + backend.clear_edit_target_dataset() + + assert backend._edit_target_dataset is None + assert backend._edit_source_dataset is None + assert backend._edit_source_point_indexes is None + assert backend._edit_target_cell_map is None + assert backend._edit_point_id_map is None + + +def test_remap_cell_ids_uses_cached_source_without_fetch(): + fetched = [] + source_ds = FakeSurfaceDataset( + points=[(0.0, 0.0, 0.0), (1.0, 0.0, 0.0), (1.0, 1.0, 0.0)], + cells=[(0, 1, 2)], + ) + target_ds = FakeSurfaceDataset( + points=[(1.0, 1.0, 0.0), (1.0, 0.0, 0.0), (0.0, 0.0, 0.0)], + cells=[(2, 1, 0)], + ) + backend = make_backend() + backend._edit_target_dataset = target_ds + backend._edit_source_dataset = source_ds + backend._edit_target_cell_map = ParaViewBackend._build_target_cell_map(target_ds) + backend.servermanager = SimpleNamespace( + Fetch=lambda _: fetched.append(1) or source_ds + ) + + backend._remap_cell_ids_to_edit_target_dataset([0], object()) + + assert ( + len(fetched) == 0 + ), "Fetch must not be called when _edit_source_dataset is cached" + + +def test_remap_surface_keys_uses_cached_point_id_map(): + # Without a cached map, source_dataset=None triggers an early return (keys unchanged). + # With a cached map, remapping proceeds even when source_dataset is not provided. + # The map shifts each source point ID by 1, so the remapped key differs from the + # input — proving the cache path was taken rather than the early return. + target_ds = FakeSurfaceDataset( + points=[(1.0, 1.0, 0.0), (1.0, 0.0, 0.0), (0.0, 0.0, 0.0)], + cells=[(2, 1, 0)], + ) + # source pt 0 → target pt 1, source pt 1 → target pt 2, source pt 2 → target pt 3 + pre_built = {0: 1, 1: 2, 2: 3} + backend = make_backend() + backend._edit_target_dataset = target_ds + backend._edit_point_id_map = pre_built + + remapped = backend._remap_surface_keys_to_edit_target_dataset( + [(0, 1, 2)], source_dataset=None + ) + + # (0,1,2) → [1,2,3] → sorted → (1,2,3); early return would give (0,1,2) + assert remapped == [(1, 2, 3)] + + def test_remap_cell_ids_between_datasets_matches_by_geometry_coordinates(): source = FakeSurfaceDataset( points=[ diff --git a/tests/test_state_handlers.py b/tests/test_state_handlers.py index 9cafa43..729b6b2 100644 --- a/tests/test_state_handlers.py +++ b/tests/test_state_handlers.py @@ -39,12 +39,21 @@ def _make_runtime(**kwargs): return SimpleNamespace(**defaults) +def _make_edit_session(active=False): + return SimpleNamespace(active=active) + + def _register( - state, *, interaction_quality_presets=INTERACTION_QUALITY_PRESETS, **runtime_kwargs + state, + *, + interaction_quality_presets=INTERACTION_QUALITY_PRESETS, + edit_session=None, + **runtime_kwargs, ): register_state_handlers( state, paraview_runtime=_make_runtime(**runtime_kwargs), + edit_session=edit_session or _make_edit_session(), interaction_quality_presets=interaction_quality_presets, ) @@ -188,6 +197,43 @@ def test_pipeline_and_interaction_callbacks_dispatch(): ) +def test_pipeline_node_switch_blocked_during_edit_session(): + switched = [] + pv_backend = SimpleNamespace( + source=object(), + display=object(), + apply_coloring=lambda v: None, + apply_representation=lambda v: None, + set_active_node=lambda node_id: switched.append(node_id) or True, + set_interactor_rotation=lambda enabled: None, + active_node_id="node-1", + ) + state = FakeState( + notification_message="", + notification_type="info", + notification_show=False, + active_pipeline_item="node-1", + edit_session_active=True, + interactive_quality=0, + interactive_ratio=0, + ) + + _register( + state, + pv_backend=pv_backend, + edit_session=_make_edit_session(active=True), + render_and_push=lambda: None, + ) + + state._handlers["active_pipeline_item"]("node-2") + + assert switched == [], "set_active_node must not be called during an edit session" + assert state.notification_show, "a warning notification must be shown" + assert ( + state.active_pipeline_item == "node-1" + ), "UI selection must snap back to current node" + + def test_pick_mode_in_edit_session_disables_rotation(): calls = [] pv_backend = SimpleNamespace(