Separate geometric distortions

[zvezdochiot]

Split the Params into ParamsDeskew and ParamsOblique.

[pablogventura]

Thanks for opening this. Separate geometric distortions overlaps conceptually with deskew/oblique work (#113, #117, #114 on ). Before a new PR, we need a short spec: which transforms stay coupled vs independent. Tracking here until @vigri or maintainers align on scope.

[vigri]

Agreed that a spec is needed before a PR. From a user perspective, Deskew and Oblique already feel like distinct operations in the UI. Deskew corrects rotation along the horizontal axis, while Oblique handles vertical shear. Separating their params in code would reflect that distinction cleanly.

For the spec, one key question: should ParamsDeskew and ParamsOblique be independently applied per page (i.e. a page could have deskew applied but oblique left at default), or are they always applied together as a pair?

[zvezdochiot]

Hi @vigri .

The "problem" of the current design is that the reset of the parameters used for "Auto" cannot be applied if all the parameters {Deskew+Oblique} belong to one class. And you will not solve this issue in any way until you divide the parameters into the two specified classes, and put the aggregator class Params above them.

[pablogventura]

Thanks @vigri and @zvezdochiot for the spec discussion.

Draft spec question (for alignment before a PR):

1. Params model: split into ParamsDeskew + ParamsOblique with a parent Params aggregator (as @zvezdochiot describes), so Auto reset can target one class without clearing the other.
2. Apply semantics: can a page have deskew applied while oblique stays at default (and vice versa), as @vigri asked?

Related work: extended deskew/oblique handling is also in the restored P0 branch (#152, formerly #133). We should align #145 with whatever lands there before a separate PR.

@vigri — when you have a preference on per-page independent apply vs always-coupled defaults, that would unblock a short written spec here.

[pablogventura]

Status: holding — spec before code.

@vigri — your question from 24/05 is still the open point: should ParamsDeskew and ParamsOblique be independently applicable per page (deskew applied, oblique left at default — and vice versa)?

Partial answer may already be in #152 (selective Apply To… for deskew vs oblique), but the fuller ParamsDeskew / ParamsOblique split @zvezdochiot describes is a separate, larger refactor.

We will not start a #145 PR until you confirm whether:

• fix: P0 bundle — Wayland, menus, content box, spread order, deskew/oblique (reopens #133) #152 merge is enough for the near term, with Separate geometric distortions #145 as a follow-up issue, or
• Separate geometric distortions #145 should block or reshape fix: P0 bundle — Wayland, menus, content box, spread order, deskew/oblique (reopens #133) #152 before merge.

No code changes planned on either track until we hear back.

[zvezdochiot]

Current 1.2.0: c2e0222

1. New project
2. Deskew: I didn't do anything, everything is by default.

Status:

1. Oblique is applied even when "Automatic oblique correction" is disabled by default.
2. The "Oblique" levers are incorrectly positioned on the widget in windowed mode. The correct position is only when the full screen.

Good luck.

[pablogventura]

I opened #153 with a fix for the regression described here (24 May):

1. Automatic oblique correction checkbox is now wired to default params and deskew auto-reload — oblique should not run on a new project when that box is unchecked.
2. Oblique handle layout in windowed mode uses the same content-area rect as the rotation handles (scrollbar-aware).

Please re-test on a build from fix/deskew-oblique-regression-145 or once #153 merges:

• New project → Step 3 Deskew, defaults untouched → no oblique shear when auto oblique is off.
• Resize the window (not maximized) → oblique levers line up with the vertical arc.

We are holding the v1.2.1 tag until we have your feedback (or maintainer sign-off). Thanks.