Simpified resizing in export #368
Conversation
AlynxZhou
force-pushed
the
fix-export
branch
3 times, most recently
from
5b5d3e7 to
de508cc
Compare
|
Fixed coding style, added missing |
|
There are mistakes when shrinking user's max size to image ratio, let me fix it. |
AlynxZhou
force-pushed
the
fix-export
branch
from
de508cc to
1101073
Compare
Fixed, the final output size will always be smaller than user's max size and has the same ratio with the original image. |
|
I tried and it works :) 👍🏻 |
src/common/imageio.c
Outdated
| // Keep image ratio even if user does not set max size by ratio. | ||
| // We make sure the calculated size will be smaller than user's max size. | ||
| if((double)max_width / max_height > image_ratio) | ||
| max_width = MIN(round(max_height * image_ratio), max_width); |
There was a problem hiding this comment.
Minor, but indentation is wrong here.
There was a problem hiding this comment.
That's easy to fix, I want to know whether you think my logic is good for you haha
There was a problem hiding this comment.
Do you think we need {} for this if else? I don't find wrong indentation levels.
There was a problem hiding this comment.
Ok, maybe it's just a display issue. No big deal.
src/common/imageio.c
Outdated
| max_height = MIN(max_height, pipe.processed_height); | ||
| // Keep image ratio even if user does not set max size by ratio. | ||
| // We make sure the calculated size will be smaller than user's max size. | ||
| if((double)max_width / max_height > image_ratio) |
There was a problem hiding this comment.
I don't think that's enough.
If the target export ratio is different than the theoritical image ratio (after crop, perspective and all distortion modules), you should:
- pin the largest picture dimension to the max allowed under current constraints,
- define the narrowest picture dimension from the largest and the theoritical image ratio,
- update scaling factor using new dimensions.
This logic prevents rounding errors as much as possible when applying the ratio as floating point.
Here, you don't check which dimension is the largest and don't cover the case where max_with / max_height == image_ratio.
| int width = 0; | ||
| int height = 0; | ||
| double _num, _denum; | ||
| dt_imageio_resizing_factor_get_and_parsing(&_num, &_denum); |
There was a problem hiding this comment.
This function is used to parse the user param only when defining export size as a sub-multiple of the original size (aka is_scaling is TRUE, that case is fairly recent and was hacked in-place). It shouldn't be used here in the general, unconditionnal case, especially since it reads a config key that's irrelevant when defining export size by pixel count (or mm/inches).
This is based on those 3 rules: - Max size is not "max" size acutally but more like "requested" size. - Resizing by scaling factor or by max size are independent and select one will cover the other value, because user cannot see both in UI. - We don't do upscaling when exporting. So the actual logic on determine final size is simple: 1. If user sets both dimensions <= 0 (setting scaling factor is also setting both dimensions), this means don't resize and use the original image size. 2. For max size, if user sets one dimension <= 0, this means calculate the other dimension via image ratio. 3. For max size, if user does not set dimensions according to image ratio, we match the longest dimension, and calculate the other dimension via image ratio. 4. If user sets value > original image size, this also means don't resize and use the original image size. 5. If by scaling factor, calculate max size according to scaling factor. If by max size, calculate scaling factor according to max size. 6. Limit size by max size, and pass correct size and scaling factor to pixelpipe. Fixes <aurelienpierreeng#362>.
|
This is based on those 2 rules:
So the actual logic on determine final size is simple:
Fixes #362.