diff --git a/AGENTS.md b/AGENTS.md index 4f2c201..d612aae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,8 +18,8 @@ credit and the "inspired by PhotoSlicer" note. ## Goals of this fork -1. Fix a mirrored-export orientation bug and add an explicit left/right - orientation workflow (see below). This is the primary patch. +1. Fix the mirrored-export orientation bug and add an explicit left/right + orientation workflow (implemented, see below). This is the primary patch. 2. Keep the app usable on the maintainer's own photo scans; afterwards only minimal maintenance is expected. 3. Push to the maintainer's Forgejo instance at `code.dgerdesmann.de` (no @@ -29,40 +29,43 @@ The maintainer is primarily an R user and new to Python/Git. Changes should be small, self-checked, clean, and secure; do not assume the maintainer can vet Python code. -## The orientation bug (primary work item) +## The orientation fix (implemented) -- Exported slices can come out mirrored (left/right swapped). -- Manual workaround: drag-switch the two corners of the top edge so the red - edge (edge 0) becomes the true top. -- Problem: `squarize()` and `reset_rotation()` re-run `normalize_top_left()`, - which re-derives "top" from geometry and silently overwrites the manual fix. +The bug: exported slices could come out mirrored (left/right swapped). Root +cause is `PhotoSlice.normalize_top_left()` (in `slices.py`), which re-derives +"top" from geometry — it picks a top-left corner and resolves winding so edge +0→1 is the "most horizontal" top edge. That inference is ambiguous, and because +`squarize()` and `reset_rotation()` both re-run `normalize_top_left()` at the +end, any manual correction was silently overwritten. -Desired outcome: +The fix adds an explicit left/right orientation workflow: -1. A visual indication of left vs. right edges/corners (in addition to the - existing red top edge). -2. A "swap left/right" button for the active slice. -3. The corrected orientation must persist through `squarize()` / - `reset_rotation()`. +- `PhotoSlice.swapped` — persistent flag (default `False`): "user has explicitly + mirrored left/right". +- `PhotoSlice.swap_left_right()` — toggles `swapped` and reindexes the corners + `[TL,TR,BR,BL] -> [TR,TL,BL,BR]` (i.e. `bbox[[1,0,3,2]]`), a pure left/right + mirror that keeps the top (edge 0→1) and bottom edges in place. NOTE: the + earlier `[0,3,2,1]` reindex was wrong — it rotated the quad 90°. +- `normalize_top_left()` applies the same `[1,0,3,2]` reindex when `swapped` is + true (after its geometric inference), so manual orientation survives + `squarize()` / `reset_rotation()`. +- "Swap L/R" toolbar button (`ui_builders_preview.py`) -> + `CanvasStateMixin.swap_active_slice_left_right()` -> + `ScanSlicerCanvasActionsMixin.swap_active_box_left_right()`. +- Visual indication in `CanvasRenderMixin.paintEvent()` (`canvas_render.py`): + corners color-coded left = orange, right = cyan; edges colored red = top + (0→1), green = right (1→2), orange = bottom (2→3), and the left edge (3→0) + follows lock state (blue locked / lightgreen unlocked). ## Key implementation facts - `PhotoSlice.bbox` is a 4×2 numpy array in the order `[top-left, top-right, bottom-right, bottom-left]`. -- `PhotoSlice.normalize_top_left()` (in `slices.py`) picks a top-left corner and - resolves winding so edge 0→1 is the "most horizontal top" edge. This is the - source of the ambiguity. -- `PhotoSlice.squarize()` and `reset_rotation()` both call - `normalize_top_left()` at the end, discarding any manual orientation. - `Autoslicer.save_slice()` (in `autoslicer.py`) builds the perspective - transform assuming the `[TL,TR,BR,BL]` order; mirroring arises upstream of - this from a wrongly "normalized" quad. -- Edge 0 is drawn red in `CanvasRenderMixin.paintEvent()` (`canvas_render.py`). - -A persistent orientation flag on `PhotoSlice` (respected by -`normalize_top_left`/`squarize`/`reset_rotation`) plus a swap action is the -likely shape of the fix. Do not change the `[TL,TR,BR,BL]` contract of -`save_slice` without updating every consumer. + transform assuming the `[TL,TR,BR,BL]` order. Do NOT change that contract + without updating every consumer. +- `PhotoSlice.squarize()` and `reset_rotation()` call `normalize_top_left()` at + the end; the `swapped` flag makes that safe for manual orientation. ## Repository structure @@ -106,10 +109,28 @@ uv run scanslicer /path/to/scans # or: python -m scanslicer ``` - AppImage (preferred): `bash packaging/appimage/build-appimage.sh` -- Docker: `docker compose up --build` +- AppImage via Docker: `bash packaging/appimage/build-appimage-docker.sh` +- Docker runtime: `docker compose up --build` - Dependencies (from `pyproject.toml`): PySide6, opencv-python, numpy, Pillow. - Python >= 3.10; dev env is Python 3.12 in `.venv`. +### AppImage build: Docker context gotcha + +The maintainer's Docker points at Docker Desktop (`desktop-linux` context). Its +bind-mount filesystem forbids `chown`, but `appimage-builder` needs root +ownership while extracting `.deb` packages, so the build fails with +`dpkg-deb ... Cannot change ownership to uid 0, gid 0: Operation not permitted`. + +Workaround: run the Dockerized build against the native daemon (or switch +context with `docker context use default`): + +```bash +DOCKER_HOST=unix:///var/run/docker.sock bash packaging/appimage/build-appimage-docker.sh +``` + +Output lands in `appimage/dist/ScanSlicer--x86_64.AppImage` (the +`appimage/` dir is git-ignored). + ## Config / persistence Runtime files live in `~/.config/scanslicer/` (or `$XDG_CONFIG_HOME/scanslicer/`) diff --git a/scanslicer/canvas_render.py b/scanslicer/canvas_render.py index 0eff838..e910f85 100644 --- a/scanslicer/canvas_render.py +++ b/scanslicer/canvas_render.py @@ -28,14 +28,22 @@ class CanvasRenderMixin: b = points[(i + 1) % 4] if i == 0: color = QColor("red") + elif i == 1: + color = QColor("green") + elif i == 2: + color = QColor("orange") else: color = QColor("blue") if s.locked else QColor("lightgreen") painter.setPen(QPen(color, 3)) painter.drawLine(a, b) - painter.setPen(QPen(QColor("blue"), 2)) - painter.setBrush(QColor(128, 128, 128, 170)) - for p in points: + # Corners are color-coded: left = orange, right = cyan. + # Edge 0 (top, red) runs TL->TR, so corners 0 and 3 are the left + # side and corners 1 and 2 are the right side. + corner_colors = (QColor("orange"), QColor("cyan"), QColor("cyan"), QColor("orange")) + for p, color in zip(points, corner_colors): + painter.setPen(QPen(QColor("blue"), 2)) + painter.setBrush(color) painter.drawRect(QRectF(p.x() - 5, p.y() - 5, 10, 10)) center = np.mean(s.bbox, axis=0) diff --git a/scanslicer/canvas_state.py b/scanslicer/canvas_state.py index 979d8c9..cf91dc9 100644 --- a/scanslicer/canvas_state.py +++ b/scanslicer/canvas_state.py @@ -192,6 +192,15 @@ class CanvasStateMixin: self.update() return True + # swap_active_slice_left_right: function definition for ScanSlicer behavior. + def swap_active_slice_left_right(self): + active_slice = self.get_active_slice() + if active_slice is None: + return False + active_slice.swap_left_right() + self.update() + return True + # reset_active_slice_rotation: function definition for ScanSlicer behavior. def reset_active_slice_rotation(self): active_slice = self.get_active_slice() diff --git a/scanslicer/controller_canvas_actions.py b/scanslicer/controller_canvas_actions.py index 219d737..b58a42a 100644 --- a/scanslicer/controller_canvas_actions.py +++ b/scanslicer/controller_canvas_actions.py @@ -26,6 +26,7 @@ class ScanSlicerCanvasActionsMixin: self.active_box_lock_button.setEnabled(False) self.active_box_squarize_button.setEnabled(False) self.active_box_reset_rotation_button.setEnabled(False) + self.active_box_swap_button.setEnabled(False) self.active_box_remove_button.setEnabled(False) self.active_box_lock_button.blockSignals(True) self.active_box_lock_button.setChecked(False) @@ -38,6 +39,7 @@ class ScanSlicerCanvasActionsMixin: self.active_box_lock_button.setEnabled(True) self.active_box_squarize_button.setEnabled(True) self.active_box_reset_rotation_button.setEnabled(True) + self.active_box_swap_button.setEnabled(True) self.active_box_remove_button.setEnabled(True) self.active_box_lock_button.blockSignals(True) self.active_box_lock_button.setChecked(active_slice.locked) @@ -69,6 +71,13 @@ class ScanSlicerCanvasActionsMixin: return self.update_statusbar("Active box squarized.") + # swap_active_box_left_right: function definition for ScanSlicer behavior. + def swap_active_box_left_right(self): + if not self.slicing_canvas.swap_active_slice_left_right(): + self.update_statusbar("No active box selected.") + return + self.update_statusbar("Active box left/right swapped.") + # reset_active_box_rotation: function definition for ScanSlicer behavior. def reset_active_box_rotation(self): if not self.slicing_canvas.reset_active_slice_rotation(): diff --git a/scanslicer/main.py b/scanslicer/main.py index 683ed71..ae66cf7 100644 --- a/scanslicer/main.py +++ b/scanslicer/main.py @@ -230,6 +230,14 @@ class QtScanSlicer(QMainWindow, ScanSlicerUiBuilderMixin, ScanSlicerControllerMi "NzRMMjEgMTYiLz48cGF0aCBkPSJNMTYgMTZoNXY1Ii8+PGNpcmNsZSBjeD0iMTIiIGN5PSIxMiIgcj0iMSIv" "Pjwvc3ZnPg==" ) + SWAP_LR_ICON_DATA_URL = ( + "data:image/svg+xml;base64," + "PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIyNCIgaGVpZ2h0PSIyNCIg" + "dmlld0JveD0iMCAwIDI0IDI0IiBmaWxsPSJub25lIiBzdHJva2U9ImN1cnJlbnRDb2xvciIgc3Ryb2tlLXdp" + "ZHRoPSIyIiBzdHJva2UtbGluZWNhcD0icm91bmQiIHN0cm9rZS1saW5lam9pbj0icm91bmQiPjxwYXRoIGQ9" + "Ik04IDMgNCA3bDQgNCIvPjxwYXRoIGQ9Ik00IDdoMTYiLz48cGF0aCBkPSJtMTYgMjEgNC00LTQtNCIvPjxw" + "YXRoIGQ9Ik0yMCAxN0g0Ii8+PC9zdmc+" + ) RESET_ROTATION_ICON_DATA_URL = ( "data:image/svg+xml;base64," "PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIyNCIgaGVpZ2h0PSIyNCIg" diff --git a/scanslicer/slices.py b/scanslicer/slices.py index e253f58..bd0a365 100644 --- a/scanslicer/slices.py +++ b/scanslicer/slices.py @@ -18,6 +18,9 @@ class PhotoSlice: self.locked = False # Side-length handles are an explicit edit mode, enabled by squarize. self.side_handles_enabled = False + # False = default winding, True = explicit left/right swap by the user. + # Persists through squarize/reset_rotation so manual orientation wins. + self.swapped = False @property # bbox: function definition for ScanSlicer behavior. @@ -49,8 +52,23 @@ class PhotoSlice: if horizontal_score(reversed_winding) > horizontal_score(ordered): ordered = reversed_winding + # A user-requested left/right swap overrides the geometric inference, + # so manual orientation survives normalize_top_left(). + if getattr(self, "swapped", False): + ordered = ordered[[1, 0, 3, 2]] + self.bbox = ordered + # swap_left_right: function definition for ScanSlicer behavior. + def swap_left_right(self): + # Mirror left/right: swap the two top corners and the two bottom + # corners, keeping the top (edge 0->1) and bottom edges in place. + # [TL, TR, BR, BL] <-> [TR, TL, BL, BR]. + self.swapped = not self.swapped + self.bbox = self.bbox[[1, 0, 3, 2]] + self._rect_cache = None + self._rect_cache_tol = None + # toggle_locked: function definition for ScanSlicer behavior. def toggle_locked(self, locked=None): if locked is not None: diff --git a/scanslicer/ui_builders_preview.py b/scanslicer/ui_builders_preview.py index fab0dcb..5f8540c 100644 --- a/scanslicer/ui_builders_preview.py +++ b/scanslicer/ui_builders_preview.py @@ -123,6 +123,16 @@ class ScanSlicerUiPreviewMixin(ScanSlicerUiSharedMixin): ) preview_toolbar_layout.addWidget(self.active_box_reset_rotation_button) + self.active_box_swap_button = self._create_toolbar_button( + preview_toolbar, + text="Swap L/R", + tooltip="Swap left/right orientation of the active box", + icon_data_url=self.SWAP_LR_ICON_DATA_URL, + callback=self.swap_active_box_left_right, + enabled=False, + ) + preview_toolbar_layout.addWidget(self.active_box_swap_button) + self.add_box_button = self._create_toolbar_button( preview_toolbar, text="Add box",