fix(raw): fixes cropping when the image is rotated

[antond-weta]

Description

This fixes incorrectly positioned crop when the image is rotated.

Tests

I have not added any unit tests specifically for this fix. I'm planning to add some tests later on using generated DNG images, but I doubt that I will get them in time for the v3.2 release. Happy to delay this fix until the tests are ready, if we prefer.

Tested manually with images from several cameras.

Checklist:

• I have read the guidelines on contributions and code review procedures.
• I have read the Policy on AI Coding Assistants
  and if I used AI coding assistants, I have an Assisted-by: TOOL / MODEL
  line in the pull request description above.
• I have updated the documentation if my PR adds features or changes
  behavior.
• I am sure that this PR's changes are tested in the testsuite.
• I have run and passed the testsuite in CI before submitting the
  PR, by pushing the changes to my fork and seeing that the automated CI
  passed there. (Exceptions: If most tests pass and you can't figure out why
  the remaining ones fail, it's ok to submit the PR and ask for help. Or if
  any failures seem entirely unrelated to your change; sometimes things break
  on the GitHub runners.)
• My code follows the prevailing code style of this project and I
  fixed any problems reported by the clang-format CI test.
• If I added or modified a public C++ API call, I have also amended the
  corresponding Python bindings. If altering ImageBufAlgo functions, I also
  exposed the new functionality as oiiotool options.

[lgritz]

LGTM, though do you have any tests?

[antond-weta]

I have added image margins and crop info to the DNG generator, seem to be working. Will add some unit tests shortly.

[antond-weta]

I have added tests

[lgritz]

I'm not sure I understand what happened with the test files. You added crop-36x32.dng, but it also looks like some of the other image test files changed, very slightly, but I'm not sure I understand what changed or why. It doesn't seem like they should be related to the crop fix, or am I misunderstanding?

[antond-weta]

All test images changed, because I'd added 3 new attributes to the DNG header: DefaultCropOrigin, DefaultCropSize, ActiveArea. By default they match the image resolution, so don't affect any other tests (I had to update one of the existing test's reference output as it mentioned the file size, which had changed).

In crop-36x32.dng I have manually crafted the image margins and crop insets so all numbers are different at all stages of calculation. Tests override the orientation with the raw:user_flip hint to avoid needing storing 4 files.

[lgritz]

testsuite/raw/src/make_malformed_dng.py is a script that creates several specific invalid dng files, to test error handling. A special script is needed because... well, they're malformed, so it's hard to create them any other way.

The script was checked into the repo for reference, to document how those malfomed files were made and I guess be able to make other types of malformed files the future. But the existing malformed files are checked into the repo and I think do not need to be changed, even if you use the script to make new, different files.

But is crop-36x32.dng also malformed? Or is it meant to be a legal file?

Maybe if you are making special purpose valid files and need to do so with a python program, they should have their own script?

[antond-weta]

crop-36x32.dng is malformed in a way that it doesn't contain all mandatory DNG attributes. It only contains the ones needed for libraw to calculate cropping. Actually I'm not aware of any other ways of generating DNG files. If I were to do that myself, I would have written a script like yours. If you think a separate script would be better, I'm happy to make it, but it would be 90% identical to make_malformed_dng.py

[lgritz]

I have four concerns, which I will present in the order of both decreasing priority in my mind, and difficulty to address, and you can choose which are worth doing in this PR (or at all):

1. Regardless of how you generate any new test images or alter the generation script, there is no reason to change any of the existing checked-in test images or the output expected from them.
2. I think it's cleaner to make a new script (or a new function within that script) if you have to make new images that are constructed differently than the test images made previously by the existing script, rather than necessarily and retroactively changing how we generate unrelated tests that were doing their job just fine. It's a short amount of code, so there is truly no penalty for some near-duplication.
3. It's cleaner to have a different script or scripts to make images meant to test features working correctly than the one that we use to make intentionally broken images to test error handling. (In retrospect, we probably should have put all the broken images in a separate test directory raw-broken, or something.) If you even need a script at all... it's fine to use imagemagick, libraw, or whatever else is available to construct an image and just check it in without a recipe. It's only in a few cases that we save recipes to construct our test images; most are just gathered from the real world.
4. I'm a little leery of testing our ability to read rotation+crop combinations with a degenerate file that essentially only tests whether we report the metadata correctly. Ideally, a test would be an image with real pixels that we could verify we were loading correctly and putting them in the buffer in the right order. We'd probably prefer not to have to come back to this later if somebody discovers that we report the crop correctly, but the image data is garbled somehow.

[lgritz]

In the last note, I originally said "increasing" when I meant "decreasing". To clarify: Number 1 is easy to fix and should probably be done now. Number 4 is hard and can probably wait. Somewhere in the middle is the dividing line of what we should bother with in this particular PR.

[jinhgkim]

A couple things I thought were worth mentioning

[jinhgkim]

Correct me if I'm wrong, but this seems to diverge from what the doc says here: "The default cropping can be disabled by setting the cropbox to zero size."

We now get the camera crop even when the cropbox is zero sized.

[jinhgkim]

Perhaps the intent was const auto&?