darktable icon indicating copy to clipboard operation
darktable copied to clipboard

Rotating cropped image moves cropped area

Open pozix604 opened this issue 1 year ago • 7 comments

Describe the bug

The cropped area of a cropped image is shifted to an unexpected area of the image when the image is rotated. It should retain the cropped area even when rotated CW or CCW.

For example, crop an image to be the top half. Now, rotate it CW. After rotation, it should still show the cropped area but rotated. Instead, it shows some other place in the original image.

Steps to reproduce

  1. Crop.
  2. Rotate using orientation module.

Expected behavior

No response

Logfile | Screenshot | Screencast

No response

Commit

No response

Where did you obtain darktable from?

downloaded from www.darktable.org

darktable version

4.8.1

What OS are you using?

Linux

What is the version of your OS?

Debian 12

Describe your system?

No response

Are you using OpenCL GPU in darktable?

Yes

If yes, what is the GPU card and driver?

Intel Iris Xe

Please provide additional context if applicable. You can attach files too, but might need to rename to .txt or .zip

No response

pozix604 avatar Oct 09 '24 16:10 pozix604

Modules are independent, so changing the crop module's input by changing the orientation module's settings will affect what the border sizes are computed from. It's no different from doing a perspective correction or lens correction and then cropping.

Duplicate of #11614 and #14788 (and more recently #17498, but that has basically no discussion since it is itself a duplicate).

ralfbrown avatar Oct 09 '24 17:10 ralfbrown

Just seems like poor and unexpected user experience, which is probably why it has been raised multiple times.

The obvious but unfortunate workaround is to rotate first.

pozix604 avatar Oct 09 '24 17:10 pozix604

Just seems like poor and unexpected user experience, which is probably why it has been raised multiple times.

And nobody came with a fix. Are you willing to work on this?

TurboGit avatar Oct 09 '24 17:10 TurboGit

And nobody came with a fix. Are you willing to work on this?

I read the other tickets. Without knowing anything about the internals of darktable, it appears to me that cropping, orientation, rotation/perspective are all interrelated, codependent aspects of the same operation. But, they are all in their separate modules. So, there must be a reason I don't understand. Maybe that's the way it has to be.

No, you wouldn't want me contributing code. I haven't been paid to write code for 20 years. These days I get paid to focus on product usability, which is why I brought this one up.

pozix604 avatar Oct 09 '24 17:10 pozix604

it appears to me that cropping, orientation, rotation/perspective are all interrelated, codependent aspects of the same operation. But, they are all in their separate modules. So, there must be a reason I don't understand.

They used to all be done by one module (the deprecated "crop & rotate", still in the code at src/iop/clipping.c), which became an unmaintainable mess due to doing too many things at once.... #6736

ralfbrown avatar Oct 09 '24 22:10 ralfbrown

They used to all be done by one module (the deprecated "crop & rotate", still in the code at src/iop/clipping.c), which became an unmaintainable mess due to doing too many things at once.... #6736

Unfortunately, separating what are codependent operations into independent modules for the sake of technical maintainability results in nonsensical user-visible behavior that appears now to be impossible to fix.

Programmer debt has been traded for user debt. In my opinion as a non-contributor to this project, is not the right way forward. My opinion also counts for little to nothing here though, so I guess take it with any amount of salt, or don't at all.

pozix604 avatar Oct 12 '24 16:10 pozix604

Unfortunately, separating what are codependent operations into independent modules for the sake of technical maintainability results in nonsensical user-visible behavior that appears now to be impossible to fix.

not really, "orientation" module as always been a separate module, the split has been with "rotate and perspective"...

btw, there was the same kind of issues inside previous crop&rotate module (esp. with flip and crop or keystone and crop).

I don't say that we don't need to try to fix that issue one day, just that it's not so simple and the split is not the culprit.

AlicVB avatar Oct 12 '24 20:10 AlicVB