Skip to content

fix(plustek): restore Prescan crop x-mirroring for mirrored sensors - #978

Open
TobbyTravel wants to merge 1 commit into
marcinz606:mainfrom
TobbyTravel:fix/crop-mirror-x
Open

fix(plustek): restore Prescan crop x-mirroring for mirrored sensors#978
TobbyTravel wants to merge 1 commit into
marcinz606:mainfrom
TobbyTravel:fix/crop-mirror-x

Conversation

@TobbyTravel

Copy link
Copy Markdown
Contributor

Summary

Fixes the Plustek Prescan crop being horizontally displaced ("correct crop size, offset one way") on mirror_x devices (OpticFilm 8200i SE / 8100 V2).

Root cause — a regression from #958

  • pyopticfilm flips the image left–right in assemble() (mirror_x) because the sensor reads right-to-left. The Prescan the user frames on is therefore the mirror-corrected image.
  • But NegPy passed the crop window unchanged into TA/hardware space. The hardware crops by sensor x (pre-mirror) while the user drew by display x (post-mirror), so the returned ROI was reflected → right dimensions, displaced x.

The mirror-aware conversion existed and was correct since the Plustek backend shipped (#783) — crop_to_scan_window(..., mirror_x=...) with the x-reflection (x1,y1,x2,y2) → (1-x2, y1, 1-x1, y2). PR #958 ("feat(plustek): upgrade to pyopticfilm 1.1.2") silently replaced it with a plain clamp_scan_area at all three call sites and deleted the capability plumbing. This PR restores that behavior.

Changes

  • negpy/infrastructure/scanners/params.py: restore crop_to_scan_window(crop, *, mirror_x) — self-inverse x-reflection.
  • negpy/infrastructure/scanners/base.py + plustek_backend.py: restore prescan_mirror_x capability, wired from model.mirror_x.
  • negpy/desktop/view/widgets/prescan_dialog.py: apply the conversion at all three crop↔window points (default-crop display, window change, accept).
  • Tests: x-reflection, passthrough, self-inverse; prescan_mirror_x surfaced on 8100 V2 caps.

Validation

  • Full suite: 4882 passed, 12 skipped.
  • Hardware: verified on OpticFilm 8100 V2 — a Prescan crop now lands exactly where drawn.

Scope

Plustek-backend only. PrescanCropDialog is gated on capabilities.prescan, which only Plustek sets; SANE/PieUSB use their own window handling and are unaffected. (Confirmed during investigation.)

PR marcinz606#958 ("feat(plustek): upgrade to pyopticfilm 1.1.2") accidentally flattened
the Prescan crop↔window conversion to a plain clamp, dropping the x-reflection
that had been correct since the Plustek backend first shipped (marcinz606#783).

pyopticfilm flips the image left–right in assemble() (mirror_x, 8200i SE /
8100 V2), so the Prescan the user frames on is the mirror-corrected image while
the hardware crops by sensor x. Passing the display crop straight into TA space
produced correct dimensions but an x-displaced ROI — "correct crop size, offset
one way".

Restore the self-inverse mirror-aware conversion:

- params.py: crop_to_scan_window(crop, *, mirror_x) — reflect x to (1-x2, y1,
  1-x1, y2) when mirrored, passthrough otherwise.
- base.py / plustek_backend.py: re-add prescan_mirror_x capability, wired from
  model.mirror_x.
- prescan_dialog.py: apply the conversion at all three crop↔window points.

Tests: x-reflection, passthrough, self-inverse for crop_to_scan_window, and
prescan_mirror_x surfaced on 8100 V2 caps. Verified on hardware (OpticFilm
8100 V2): a Prescan crop now lands exactly where drawn.
@TobbyTravel

Copy link
Copy Markdown
Contributor Author

Paired with the GL128 first-scan prime (jboneng/pyopticfilm#16) on the OpticFilm 8100 (V2): Prescan crops land exactly where drawn, stable across single-pass, Multi-Exposure and settings changes within one device session.

For review, the pairing has a versioning wrinkle: this PR is fully green against the current pyopticfilm==1.2.0 release (lock) and mergeable as-is. But upstream pyopticfilm main has since renamed bringup.is_opticfilm_8200i_seis_gl128_opticfilm (commit 92d513d, post-1.2.0, and widened to accept the 8100 V2 pid 0x1824 alongside the 8200i SE 0x1825). The next pyopticfilm release after the #16 merge will necessarily carry that rename, at which point the Plustek backend's imports break. Suggested follow-up (small, could also be folded in pre-merge): bump the plustek extra floor to >=<next release>, uv lock, and adopt the new symbol name — the rename is strictly more correct (it matches models by asic == GL128, with the SE pid kept for fallback), so no behaviour change for 8200i SE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant