darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.74k stars 1.14k forks source link

Rotating cropped image moves cropped area #17631

Open pozix604 opened 3 weeks ago

pozix604 commented 3 weeks ago

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

ralfbrown commented 3 weeks ago

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).

pozix604 commented 3 weeks ago

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.

TurboGit commented 3 weeks ago

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?

pozix604 commented 3 weeks ago

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.

ralfbrown commented 3 weeks ago

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

pozix604 commented 2 weeks ago

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.

AlicVB commented 2 weeks ago

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.