Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected dimensions while exporting #16692

Conversation

jenshannoschwalm
Copy link
Collaborator

@jenshannoschwalm jenshannoschwalm commented Apr 27, 2024

Depending on pipe.processed_width&height and the given width&height from the export dialog the dimensions for the pixelpipe run could be too small due to flooring.

Instead we do rounding here for the processed width&height and afterwards recalculate the scale to
a) be sure there are no pixels missing
b) avoid out-of-bounds problems.

One report about such rounding errors is in #16689

To reproduce you can take any raw with a size of at least 8288x5520 pixels and crop the size to exactly that in rawprepare while watching the dimensions in the "Image information" section.
darktable -d pipe -d imageio will report the dimension as here

94,1707 [dt_imageio_export] [export] imgid 7871, 8288x5520 --> 1532x1020 (scale 0,1848). upscale=no, hq=yes

keeping at least one dimension for what we want.

This is a tricky part of the pipeline and we had a number of issues related to export dimensions so please test in depth - i did so and couldn't spot anything wrong - watching out for

  1. artifacts at the outermost pixels right and bottom
  2. wrong sizes

So hopefully
Fixes #16689
Fixes #16638
Also seems good for #16133

without introducing a regression.

If good the release note would simply be "Fixed exported image dimensions"

@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes labels Apr 27, 2024
@jenshannoschwalm jenshannoschwalm added this to the 4.8 milestone Apr 27, 2024
@ralfbrown ralfbrown added the scope: image processing correcting pixels label Apr 27, 2024
@jenshannoschwalm
Copy link
Collaborator Author

I know that this has been discussed years ago in #3757 but as we have things fixed in finalscale and the interpolator i thinks it's fine to re-evaluate.

@jenshannoschwalm jenshannoschwalm marked this pull request as draft April 27, 2024 16:55
@jenshannoschwalm jenshannoschwalm force-pushed the finalscale_rounding branch 2 times, most recently from 0b0b916 to a4f7e9e Compare April 28, 2024 06:43
@jenshannoschwalm
Copy link
Collaborator Author

Updated comments about old issues, don't ceil but round.

Tested all of that old issues and current ones, seems good to me now.

@jenshannoschwalm jenshannoschwalm marked this pull request as ready for review April 28, 2024 06:47
@TurboGit
Copy link
Member

I'm not sure for all cases, but there is still issues and it is hard to evaluate if more cases are better covered with this PR.

For example, take the integration test image mire1.cr2. The original size is 3908 x 2602.

Edit it in darkroom and do a square crop taking the whole height (2602 x 2602). Now export with 1920 x 1920 in export module. On my side I get an image of 1920 x 1919.

Now back to crop and resize the crop area to have an even size, for example 2104 x 2104. Again export and you'll get an image of size 1920 x 1919.

Back again to crop and resize the crop area to have an odd size, for example 2565 x 2565. Again export and you'll get an image of size 1920 x 1920.

Also, do a crop size 1917 x 1917, export and you'll get 1916 x 1916.

I found out that even/odd if not really a parameter but more the actual crop size. As I said I'm not sure how to get this right in all cases, a very hard issue to fix to me as the pixelpipe is handled.

@jenshannoschwalm
Copy link
Collaborator Author

Will investigate further next week

Depending on pipe.processed_width&height and the given width&height from the export dialog
the dimensions for the pixelpipe run could be too small due to flooring.

Instead we do rounding here for the processed width&height and afterwards recalculate the scale
to
a) be sure there are no pixels missing
b) avoid out-of-bounds problems.
@jenshannoschwalm
Copy link
Collaborator Author

Closing this, pending work will be based on #16748

@jenshannoschwalm jenshannoschwalm deleted the finalscale_rounding branch May 10, 2024 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: image processing correcting pixels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix cropping when exporting Image export size doesn't respect crop aspect ratio
3 participants