Add `rotate_about_center_no_crop` to prevent pixel loss during image rotations

[Tikitikitikidesuka]

This PR introduces a new method, rotate_about_center_no_crop, that rotates an image about its center without cropping, ensuring no pixel data is lost in the process. The function calculates the new dimensions to fully accommodate the rotated image and fills any empty space outside the original image boundaries with a user-defined default pixel value.

To support this new method, two auxiliary functions are included:

1. rotate_about_center_into: Same as rotate_about_center_into except it writes the output to an image passed as a mutable reference. Dimensions of the input image and the output do not have to be the same.
2. rotate_into: Same as rotate_into except it writes the output to an image passed as a mutable reference. Dimensions of the input image and the output do not have to be the same.

[cospectrum]

"into" usually used for owned/moved values. For mutation, imageproc prefers "mut" postfix currently, and I think there is even a macro to generate documentation for such "mut" functions. But that's not a big problem, I think

[cospectrum]

Tests would be good

[Tikitikitikidesuka]

"into" usually used for owned/moved values. For mutation, imageproc prefers "mut" postfix currently, and I think there is even a macro to generate documentation for such "mut" functions. But that's not a big problem, I think

The decision to name the functions as "..._into" is based on the already existing warp and warp_into functions which behave similarly. Since it is not that the original image is mutated but that the output image is written to an already existing buffer passed as an additional mutable image argument.

[Tikitikitikidesuka]

Tests would be good

I am having a hard time finding the equivalent function's (rotate_about_center and rotate) tests. What would be an appropriate way of testing these methods? Expecting an exact result does not feel like a good idea. Generating the expected results would be complex.

[cospectrum]

I know, it's hard. Maybe you can start with property testing just to detect panics and limitations, you can introduce invariants. For example, center should be in bounds. Maybe rotate by n * 360 and check similarity(input, out) <= eps. Any ideas!

[Tikitikitikidesuka]

I know, it's hard. Maybe you can start with property testing just to detect panics and limitations, you can introduce invariants. For example, center should be in bounds. Maybe rotate by n * 360 and check similarity(input, out) <= eps. Any ideas!

Ok, thanks! I will get to it :)

[Tikitikitikidesuka]

I am running into some trouble. I have been trying to test rotations for angles multiple of 90 degrees (90º, 180º, 270º, 360º, ...). I figured that testing these would not be as hard as arbitrary angles. But only multiples of 360 degrees work properly...

It is not a problem of my functions, the previous methods behave the same way. Because of floating point precision, the rotation is not exactly around the center and for 90º, 180º and 270º the image is displaced one pixel in either rows or columns.

0º	90º	180º	270º	360º

(Zoom needed, it is only a single pixel of error)

This error displacement makes image comparison a hard problem for testing the functions. Which is why I believe the previously rotation functions have no tests assigned to them except for a zero degree rotation test.

The only test I can think of that might be useful is to check if the non cropping rotation sum of pixel values is similar within some bounds to that of the original. This checks that pixels are not cropped since, if they were, the sum would decrease.

[Tikitikitikidesuka]

I added the test I mentioned previously. It checks that the average color of the image does not change after a 45º rotation, which would happen if pixels were being cropped as is the case with the rotate_about_center_method. It approximates the equality check because of floating point precision and the discreet sampling noise.

#[test]
fn test_rotate_about_center_no_crop() {
    let pixel_val = 255;
    let square_size = 512;

    let image_area = square_size * square_size;

    let image = GrayImage::from_vec(
        square_size,
        square_size,
        vec![pixel_val; image_area as usize],
    )
    .unwrap();

    let expected_proportion = image.iter().map(|&x| x as u32).sum::<u32>() as f32
        / (pixel_val as u32 * image_area) as f32;

    let rotated_image =
        rotate_about_center_no_crop(&image, PI * 0.25, Interpolation::Nearest, Luma([0]));

    let rotated_proportion = rotated_image.iter().map(|&x| x as u32).sum::<u32>() as f32
        / (pixel_val as u32 * image_area) as f32;

    assert_approx_eq!(rotated_proportion, expected_proportion, 0.01);
}

[cospectrum]

You can try to increase epsilon, maybe it will help. Also, for image comparison you can create dedicated function(s), so you will not violate "DRY". It can be any perceptual hash. In your test it's a simple sum, for example.

Another idea I would try is to create a very simple image and attach a very visible watermark. After rotation, you can check that this watermark remains and that it is in a certain place, in a corner. You can even rotate image twice: with theta, and then with -theta.

[cospectrum]

The difference between "no crop" rotation and simple rotation is that the watermark should always be in the output, so such a test would be especially useful.

[Tikitikitikidesuka]

I finally found the associated tests for the previous rotation functions. I feel so stupid for not finding them before. I have used the helper functions and followed the same procedure to test my new rotation function.

[cospectrum]

elephant_rotate_no_crop_bicubic_rgba.png is incorrect

[Tikitikitikidesuka]

You are right elephant_rotate_no_crop_bicubic_rgba.png is wrong but passes the test because it calls the cropping version of the function. Both errors have been fixed.

[cospectrum]

Maybe let's introduce only 1 new public function? Other functions are not particularly relevant to the feature. I just want to avoid breaking API in the future.

[cospectrum]

Not because I'm against those functions. I just think that every public function deserves a separate review from several individuals.

[Tikitikitikidesuka]

I feel like rotate_into and other into functions might be useful as public functions. You are right that they deserve a separate review and their own feature. I have set the all functions except rotate_about_center_no_crop to private.

[cospectrum]

Ok, there's only 1 thread left about the default value. Perhaps the output should be created as let out = Image::from_pixel(out_w, out_h, default);. I don't remember the exact implementation of warp_into, will it fill all output pixels with the default value?

[Tikitikitikidesuka]

Yes, warp_into will guarantee that “Output pixels whose pre-image lies outside the input image are set to default.” So there is no need to create the image with any specific pixel color.

[cospectrum]

@theotherphil You can take a look

[cospectrum]

Yes, warp_into will guarantee that “Output pixels whose pre-image lies outside the input image are set to default.” So there is no need to create the image with any specific pixel color.

Still 1 test would be useful :)