diff --git a/PyReconstruct/modules/gui/main/field_widget_4_data.py b/PyReconstruct/modules/gui/main/field_widget_4_data.py index 1f92d9d1..b55677aa 100644 --- a/PyReconstruct/modules/gui/main/field_widget_4_data.py +++ b/PyReconstruct/modules/gui/main/field_widget_4_data.py @@ -215,13 +215,20 @@ def propagateTo(self, to_end : bool = True, log_event=True): ) if modify_section: included_sections.append(snum) + # locked sections are left untouched: warn the user, then exclude them + locked_sections = set() for snum in included_sections: section = self.series.loadSection(snum) if section.align_locked: - if not notifyConfirm("Locked sections will not be modified.\nWould you still like to propagate the transform?"): - return - break - + locked_sections.add(snum) + if locked_sections: + if not notifyConfirm("Locked sections will not be modified.\nWould you still like to propagate the transform?"): + return + included_sections = [ + snum for snum in included_sections + if snum not in locked_sections + ] + # create the progress bar if included_sections: progbar = getProgbar( @@ -232,12 +239,21 @@ def propagateTo(self, to_end : bool = True, log_event=True): final_value = len(included_sections) progbar.setValue(0) + # record a series-wide undo state for the propagation + self.series_states.addState() + try: for snum in included_sections: section = self.series.loadSection(snum) + # capture the section's pre-modification state (no-op if + # its state tracking is already initialized) + self.series_states[section] new_tform = self.stored_tform * section.tform section.tform = new_tform section.save() + # record the undo state so the propagation is undoable + self.series_states[snum].addState(section, self.series) + self.series_states.addSectionUndo(snum) self.propagated_sections.add(snum) if log_event: self.series.addLog(None, snum, "Modify transform") @@ -410,11 +426,11 @@ def corrAlign(self): shift_tform = Transform([1, 0, shift_x, 0, 1, shift_y]) + # apply the field-space correlation shift after the section's own + # transform, and route through changeTform so the shift is recorded + # for propagation when propagation mode is active tform = self.section.tform - self.section.tform = shift_tform * tform - - self.generateView() - self.saveState() + self.changeTform(tform * shift_tform) def calibrateMag(self, trace_lengths : dict, log_event=True): """Calibrate the pixel mag based on the lengths of given traces. diff --git a/tests/test_corr_align_propagate.py b/tests/test_corr_align_propagate.py new file mode 100644 index 00000000..a0024dc7 --- /dev/null +++ b/tests/test_corr_align_propagate.py @@ -0,0 +1,291 @@ +"""Align-by-correlation records its shift so it can be propagated. + +`corrAlign` computes an FFT cross-correlation shift and routes it through +`changeTform` (the same recording path a manual transform uses) instead of +assigning `section.tform` directly. When propagation recording is active the +shift is captured in `stored_tform` and replayed across a range by +`propagateTo`, exactly like a manual translate. + +The correlation shift is composed as `section.tform * shift_tform` -- the +existing tform post-multiplied by a field-space translation, since the shift +is measured in field space and must apply after the section's own transform. +That is bit-for-bit the same transform a manual translate of the same +(dx, dy) produces, so it propagates with identical semantics. + +The methods are exercised against duck-typed stubs plus the real `Transform`, +`changeTform`, and `propagateTo`, so no Qt event loop is required. +""" +import types + +import pytest + +from PyReconstruct.modules.gui.main import field_widget_4_data as fw +from PyReconstruct.modules.datatypes import Transform + + +# --- corrAlign routing ----------------------------------------------------- + +def _corr_stub(tform_list, *, mag, scaling, locked=False, b_layer=True): + section = types.SimpleNamespace( + align_locked=locked, + mag=mag, + tform=Transform(list(tform_list)), + n=3, + ) + stub = types.SimpleNamespace( + section=section, + section_layer=types.SimpleNamespace( + section=section, + generateImageArray=lambda dim, window: None, + ), + b_section_layer=( + types.SimpleNamespace(generateImageArray=lambda dim, window: None) + if b_layer else None + ), + series=types.SimpleNamespace(window=(0, 0, 1, 1)), + pixmap_dim=(4, 4), + scaling=scaling, + changeTform_calls=[], + ) + stub.changeTform = lambda t: stub.changeTform_calls.append(t) + return stub + + +def _patch_corr(monkeypatch, offset): + notified = [] + monkeypatch.setattr(fw, "notify", lambda *a, **k: notified.append(a)) + monkeypatch.setattr( + fw, "cv2", + types.SimpleNamespace(cvtColor=lambda a, code: a, COLOR_RGB2BGR=0), + ) + monkeypatch.setattr(fw, "correlate", lambda a, b: offset) + return notified + + +def test_corr_align_routes_shift_through_changeTform(monkeypatch): + """The computed field-space shift is post-composed onto the existing tform + and handed to changeTform (not written to section.tform directly).""" + _patch_corr(monkeypatch, offset=(10, -4)) + # scaling=2, mag=0.5 -> shift_x = 10/2*0.5 = 2.5 ; shift_y = -(-4/2*0.5) = 1.0 + base = [2, 0, 3, 0, 3, -7] + stub = _corr_stub(base, mag=0.5, scaling=2.0) + original_tform = stub.section.tform + + fw.FieldWidgetData.corrAlign(stub) + + assert len(stub.changeTform_calls) == 1, "corrAlign must record via changeTform" + got = stub.changeTform_calls[0] + # field-space post-shift only touches the translation components + expected = Transform([2, 0, 3 + 2.5, 0, 3, -7 + 1.0]) + assert got.equals(expected) + # equals section.tform * shift_tform (shift applied after the transform) + shift = Transform([1, 0, 2.5, 0, 1, 1.0]) + assert got.equals(original_tform * shift) + # section.tform is NOT mutated directly by corrAlign anymore + assert stub.section.tform is original_tform + + +def test_corr_align_no_b_section_is_noop(monkeypatch): + _patch_corr(monkeypatch, offset=(10, -4)) + stub = _corr_stub([1, 0, 0, 0, 1, 0], mag=1.0, scaling=1.0, b_layer=False) + fw.FieldWidgetData.corrAlign(stub) + assert stub.changeTform_calls == [] + + +def test_corr_align_locked_section_is_noop(monkeypatch): + notified = _patch_corr(monkeypatch, offset=(10, -4)) + stub = _corr_stub([1, 0, 0, 0, 1, 0], mag=1.0, scaling=1.0, locked=True) + fw.FieldWidgetData.corrAlign(stub) + assert stub.changeTform_calls == [] + assert notified, "a locked section should warn the user" + + +# --- capture: correlation shift == manual translate ------------------------ + +def _record_stub(base_tform): + section = types.SimpleNamespace( + align_locked=False, tform=Transform(list(base_tform)), n=1 + ) + stub = types.SimpleNamespace( + section=section, + section_layer=types.SimpleNamespace(section=section), + series=types.SimpleNamespace(addLog=lambda *a, **k: None), + propagate_tform=True, + stored_tform=Transform.identity(), + ) + stub.generateView = lambda *a, **k: None + stub.saveState = lambda *a, **k: None + return stub + + +def test_capture_equivalent_to_manual_translate(): + """Recording a correlation shift yields the same stored_tform as recording + an equivalent manual translate of the same (dx, dy).""" + base = [2, 0, 3, 0, 3, -7] + dx, dy = 2.5, 1.0 + + # correlation path: section.tform * field-space shift + corr = _record_stub(base) + corr_new = corr.section.tform * Transform([1, 0, dx, 0, 1, dy]) + fw.FieldWidgetData.changeTform(corr, corr_new) + + # manual translate path (see translateTform): add dx, dy to translation + man = _record_stub(base) + m = man.section.tform.getList() + m[2] += dx + m[5] += dy + fw.FieldWidgetData.changeTform(man, Transform(m)) + + # the applied transforms are identical... + assert corr.section.tform.equals(man.section.tform) + # ...and so is the captured stored_tform that propagation replays + assert corr.stored_tform.equals(man.stored_tform) + # captured delta is the conjugated change: new * current.inverted() + expected_delta = corr_new * Transform(list(base)).inverted() + assert corr.stored_tform.equals(expected_delta) + + +# --- replay: propagateTo across a range ------------------------------------ + +class _FakeSectionStates: + """Records addState calls the way SectionStates would receive them.""" + + def __init__(self): + self.recorded = [] + + def addState(self, section, series): + self.recorded.append(section.tform) + + +class _FakeSeriesStates: + """Duck-types the SeriesStates surface propagateTo uses.""" + + def __init__(self, snums): + self.section_states = {n: _FakeSectionStates() for n in snums} + self.series_state_count = 0 + self.section_undos = [] + + def addState(self, breakable=True): + self.series_state_count += 1 + + def addSectionUndo(self, snum): + self.section_undos.append(snum) + + def __getitem__(self, index): + if isinstance(index, int): + return self.section_states[index] + return self.section_states[index.n] + + +def _prop_stub(monkeypatch, base_tforms, current_section, locked=()): + """Build a stub with an in-memory set of sections and the real + changeTform/propagateTo wired up.""" + sections = { + n: types.SimpleNamespace( + n=n, + align_locked=(n in locked), + tform=Transform(list(t)), + save_count=0, + ) + for n, t in base_tforms.items() + } + for sec in sections.values(): + def _save(sec=sec): + sec.save_count += 1 + sec.save = _save + cur = sections[current_section] + stub = types.SimpleNamespace( + section=cur, + section_layer=types.SimpleNamespace(section=cur), + series=types.SimpleNamespace( + sections=sections, + current_section=current_section, + loadSection=lambda snum: sections[snum], + addLog=lambda *a, **k: None, + ), + series_states=_FakeSeriesStates(sections.keys()), + propagate_tform=True, + stored_tform=Transform.identity(), + propagated_sections={current_section}, + ) + stub.generateView = lambda *a, **k: None + stub.saveState = lambda *a, **k: None + stub.reload = lambda *a, **k: None + monkeypatch.setattr( + fw, "getProgbar", + lambda *a, **k: types.SimpleNamespace( + setValue=lambda v: None, close=lambda: None + ), + ) + return stub, sections + + +def test_propagate_to_end_same_base_is_field_shift(monkeypatch): + """Sections sharing the current section's base tform get the identical + field-space shift when the recorded correlation align propagates forward.""" + base = [2, 0, 3, 0, 3, -7] + dx, dy = 2.5, 1.0 + stub, sections = _prop_stub( + monkeypatch, {n: base for n in (1, 2, 3, 4, 5)}, current_section=3 + ) + + # record the correlation shift on section 3 + new = stub.section.tform * Transform([1, 0, dx, 0, 1, dy]) + fw.FieldWidgetData.changeTform(stub, new) + + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + shifted = Transform([2, 0, 3 + dx, 0, 3, -7 + dy]) + # forward sections shifted; current section already shifted by changeTform + for n in (3, 4, 5): + assert sections[n].tform.equals(shifted), f"section {n} should be shifted" + # sections before current are untouched + for n in (1, 2): + assert sections[n].tform.equals(Transform(list(base))) + + +def test_propagate_to_start_direction(monkeypatch): + base = [2, 0, 3, 0, 3, -7] + dx, dy = 2.5, 1.0 + stub, sections = _prop_stub( + monkeypatch, {n: base for n in (1, 2, 3, 4, 5)}, current_section=3 + ) + new = stub.section.tform * Transform([1, 0, dx, 0, 1, dy]) + fw.FieldWidgetData.changeTform(stub, new) + + fw.FieldWidgetData.propagateTo(stub, to_end=False) + + shifted = Transform([2, 0, 3 + dx, 0, 3, -7 + dy]) + for n in (1, 2, 3): + assert sections[n].tform.equals(shifted) + for n in (4, 5): # forward sections untouched when propagating to start + assert sections[n].tform.equals(Transform(list(base))) + + +def test_propagate_conjugates_for_differing_base(monkeypatch): + """A section with a *different* base tform receives the stored delta mapped + through its own tform (stored_tform * section.tform), matching the existing + manual-propagation semantics -- NOT a naive identical field shift. + + Current section base scales x2/y3; target section 4 is identity. The stored + delta is a pure translation of (1.25, 1/3), so section 4 shifts by that, + which differs from the current section's (2.5, 1.0) field shift. + """ + dx, dy = 2.5, 1.0 + stub, sections = _prop_stub( + monkeypatch, + {1: [2, 0, 3, 0, 3, -7], 2: [2, 0, 3, 0, 3, -7], + 3: [2, 0, 3, 0, 3, -7], 4: [1, 0, 0, 0, 1, 0]}, + current_section=3, + ) + new = stub.section.tform * Transform([1, 0, dx, 0, 1, dy]) + fw.FieldWidgetData.changeTform(stub, new) + + stored = stub.stored_tform + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + # section 4 (identity base) == stored * identity == stored (a pure translate) + assert sections[4].tform.equals(stored * Transform([1, 0, 0, 0, 1, 0])) + assert sections[4].tform.equals(Transform([1, 0, 1.25, 0, 1, 1.0 / 3.0])) + # and it is NOT the naive field shift the current section received + assert not sections[4].tform.equals(Transform([1, 0, dx, 0, 1, dy])) diff --git a/tests/test_propagate_locked_undo.py b/tests/test_propagate_locked_undo.py new file mode 100644 index 00000000..e90a03b7 --- /dev/null +++ b/tests/test_propagate_locked_undo.py @@ -0,0 +1,283 @@ +"""Tests for two Align-by-correlation propagate fixes. + +1. Locked-section bypass in `propagateTo`: the dialog promises "Locked + sections will not be modified," but the apply loop previously set + `section.tform` and called `section.save()` on every included section with + no `align_locked` check -- silently overwriting and persisting alignments + the user locked. `propagateTo` must skip align-locked sections entirely + (mirroring the early return `changeTform` does on `align_locked`): no tform + change, no save, no log entry. + +2. Missing undo state: the propagate loop only logged the change (`addLog`); + it never recorded undo states, so a propagation could not be undone. + `propagateTo` must record a series-wide undo state plus per-section states + (the same `SeriesStates` pattern `SeriesIterator` uses) so that a series + undo restores every modified section's prior transform. + +Both are exercised two ways: against duck-typed stubs (as in +test_corr_align_propagate) and end-to-end against the real `shapes1.jser` +fixture with a real `SeriesStates`, verifying on-disk bytes of the locked +section are untouched and that `undoState` restores the prior transforms. + +Also confirms the corrAlign composition order (`tform * shift_tform`, i.e. +the field-space shift applied AFTER the existing transform) against a +concrete rotated/scaled transform, where the reversed order visibly differs. +""" +import math +import os +import shutil +import types + +import pytest + +from PyReconstruct.modules.gui.main import field_widget_4_data as fw +from PyReconstruct.modules.datatypes import Transform + +from test_corr_align_propagate import _prop_stub, _patch_corr, _corr_stub + +FIXTURE = os.path.join( + os.path.dirname(__file__), "..", "PyReconstruct", "assets", + "checker", "files", "shapes1.jser", +) + + +def _patch_confirm(monkeypatch, answer): + calls = [] + monkeypatch.setattr( + fw, "notifyConfirm", + lambda *a, **k: (calls.append(a), answer)[1], + ) + return calls + + +# --------------------------------------------------------------------------- +# stub-level: locked sections are skipped +# --------------------------------------------------------------------------- + +def test_propagate_skips_locked_section(monkeypatch): + """A locked section inside the range keeps its tform, is never saved, and + gets no undo/log entry -- while unlocked sections still propagate.""" + confirms = _patch_confirm(monkeypatch, True) + base = [1, 0, 0, 0, 1, 0] + stub, sections = _prop_stub( + monkeypatch, {n: base for n in (1, 2, 3, 4, 5)}, + current_section=3, locked=(4,), + ) + dx, dy = 2.5, 1.0 + fw.FieldWidgetData.changeTform( + stub, stub.section.tform * Transform([1, 0, dx, 0, 1, dy]) + ) + + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + assert len(confirms) == 1, "user is warned exactly once about locked sections" + # locked section 4: untouched and NOT re-saved + assert sections[4].tform.equals(Transform(list(base))) + assert sections[4].save_count == 0 + assert 4 not in stub.propagated_sections + # unlocked section 5: shifted and saved + assert sections[5].tform.equals(Transform([1, 0, dx, 0, 1, dy])) + assert sections[5].save_count == 1 + assert 5 in stub.propagated_sections + # undo recorded only for the modified section + assert stub.series_states.series_state_count == 1 + assert stub.series_states.section_undos == [5] + assert stub.series_states[4].recorded == [] + + +def test_propagate_declined_on_locked_leaves_everything_untouched(monkeypatch): + """Answering 'no' to the locked-section prompt aborts the propagation.""" + _patch_confirm(monkeypatch, False) + base = [1, 0, 0, 0, 1, 0] + stub, sections = _prop_stub( + monkeypatch, {n: base for n in (1, 2, 3)}, + current_section=1, locked=(2,), + ) + stub.stored_tform = Transform([1, 0, 2.5, 0, 1, 1.0]) + + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + for n in (2, 3): + assert sections[n].tform.equals(Transform(list(base))) + assert sections[n].save_count == 0 + assert stub.series_states.series_state_count == 0 + assert stub.series_states.section_undos == [] + + +def test_propagate_records_undo_state_per_modified_section(monkeypatch): + """Every modified section gets a recorded state tied to a series undo.""" + base = [1, 0, 0, 0, 1, 0] + stub, sections = _prop_stub( + monkeypatch, {n: base for n in (1, 2, 3, 4)}, current_section=1, + ) + dx, dy = 2.5, 1.0 + stub.stored_tform = Transform([1, 0, dx, 0, 1, dy]) + + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + assert stub.series_states.series_state_count == 1 + assert stub.series_states.section_undos == [2, 3, 4] + for n in (2, 3, 4): + recorded = stub.series_states[n].recorded + assert len(recorded) == 1 + # the state is recorded after the modification (SeriesIterator pattern: + # SectionStates.addState pushes the pre-mod snapshot internally) + assert recorded[0].equals(Transform([1, 0, dx, 0, 1, dy])) + + +# --------------------------------------------------------------------------- +# end-to-end: real series, real SeriesStates +# --------------------------------------------------------------------------- + +def _load_series(tmp_path): + if not os.path.exists(FIXTURE): + pytest.skip("fixture shapes1.jser not found") + fp = str(tmp_path / "shapes1.jser") + shutil.copyfile(FIXTURE, fp) + + from PySide6.QtWidgets import QApplication + QApplication.instance() or QApplication(["test"]) + from PyReconstruct.modules.datatypes.series import Series + + return Series.openJser(fp) + + +def _set_locked(series, snum, locked): + section = series.loadSection(snum) + section.align_locked = locked + section.save() + + +def _real_stub(monkeypatch, series, current_section, stored_tform): + from PyReconstruct.modules.backend.func.state_manager import SeriesStates + + series.current_section = current_section + cur = series.loadSection(current_section) + stub = types.SimpleNamespace( + section=cur, + section_layer=types.SimpleNamespace(section=cur), + series=series, + series_states=SeriesStates(series), + propagate_tform=True, + stored_tform=stored_tform, + propagated_sections={current_section}, + ) + stub.generateView = lambda *a, **k: None + stub.saveState = lambda *a, **k: None + stub.reload = lambda *a, **k: None + monkeypatch.setattr( + fw, "getProgbar", + lambda *a, **k: types.SimpleNamespace( + setValue=lambda v: None, close=lambda: None + ), + ) + return stub + + +def test_real_series_locked_section_untouched_and_not_resaved( + monkeypatch, tmp_path): + """End-to-end on shapes1.jser: propagating across a range containing a + locked section leaves that section's tform AND its on-disk file untouched, + while the unlocked sections in range are modified and persisted.""" + series = _load_series(tmp_path) + _patch_confirm(monkeypatch, True) + + # unlock all sections except section 2 + for n in series.sections: + _set_locked(series, n, n == 2) + + orig_tforms = { + n: series.loadSection(n).tform.copy() for n in series.sections + } + locked_fp = os.path.join(series.getwdir(), series.sections[2]) + with open(locked_fp, "rb") as f: + locked_bytes_before = f.read() + + stored = Transform([1, 0, 0.5, 0, 1, -0.25]) + stub = _real_stub(monkeypatch, series, current_section=1, + stored_tform=stored) + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + # locked section 2: same tform on a fresh load, file bytes identical + assert series.loadSection(2).tform.equals(orig_tforms[2]) + with open(locked_fp, "rb") as f: + assert f.read() == locked_bytes_before, \ + "locked section file must not be re-saved" + assert 2 not in stub.propagated_sections + + # unlocked sections 3 and 4: persisted stored * original + for n in (3, 4): + expected = stored * orig_tforms[n] + assert series.loadSection(n).tform.equals(expected) + + # sections before the current one: untouched + assert series.loadSection(0).tform.equals(orig_tforms[0]) + + series.close() + + +def test_real_series_propagation_is_undoable(monkeypatch, tmp_path): + """End-to-end: a series undo after propagateTo restores the prior + transforms of every section the propagation modified.""" + series = _load_series(tmp_path) + _patch_confirm(monkeypatch, True) + + for n in series.sections: + _set_locked(series, n, False) + + orig_tforms = { + n: series.loadSection(n).tform.copy() for n in series.sections + } + + stored = Transform([1, 0, 0.5, 0, 1, -0.25]) + stub = _real_stub(monkeypatch, series, current_section=1, + stored_tform=stored) + fw.FieldWidgetData.propagateTo(stub, to_end=True) + + # propagation happened (sanity) + for n in (2, 3, 4): + assert series.loadSection(n).tform.equals(stored * orig_tforms[n]) + + # a series undo must now be available and restore the prior transforms + can_3D, _, _ = stub.series_states.canUndo() + assert can_3D, "propagation must leave an undoable series state" + stub.series_states.undoState() + + for n in series.sections: + assert series.loadSection(n).tform.equals(orig_tforms[n]), \ + f"undo must restore section {n}'s prior transform" + + series.close() + + +# --------------------------------------------------------------------------- +# corrAlign composition order on a rotated/scaled transform +# --------------------------------------------------------------------------- + +def test_corr_align_composition_on_rotated_scaled_tform(monkeypatch): + """For a rotated+scaled section, corrAlign must produce + section.tform * shift (field-space shift applied AFTER the transform: + A * B maps p -> B(A(p)) here) -- demonstrably NOT shift * tform.""" + _patch_corr(monkeypatch, offset=(10, -4)) + # scaling=2, mag=0.5 -> shift = (2.5, 1.0) + dx, dy = 2.5, 1.0 + # rotation by 30 degrees, scaled by 2, translated + c, s = 2 * math.cos(math.pi / 6), 2 * math.sin(math.pi / 6) + base = [c, -s, 1.5, s, c, -0.5] + stub = _corr_stub(base, mag=0.5, scaling=2.0) + tform = Transform(list(base)) + shift = Transform([1, 0, dx, 0, 1, dy]) + + fw.FieldWidgetData.corrAlign(stub) + + assert len(stub.changeTform_calls) == 1 + got = stub.changeTform_calls[0] + assert got.equals(tform * shift), "shift must compose AFTER the tform" + # the reversed (buggy) order is genuinely different here... + assert not got.equals(shift * tform) + # ...and only the correct order shifts mapped points by exactly (dx, dy) + for p in [(0.0, 0.0), (1.0, 2.0), (-3.0, 0.25)]: + bx, by = tform.map(*p) + gx, gy = got.map(*p) + assert gx == pytest.approx(bx + dx) + assert gy == pytest.approx(by + dy)