image-rs / imageproc

Image processing operations
MIT License
716 stars 144 forks source link

Rotation introduces black bars #415

Open chocolatier opened 4 years ago

chocolatier commented 4 years ago

The issue in #391 is also present in the cropping rotate function.

Rotating a square by multiples of pi/2 about centre shouldn't have any pixels with a preimage outside the original square. But calling rotate_about_center introduces bars of 99s.

I've added the following test

    fn test_rotate_half_pi_zero_square_about_center() {
        let image = gray_image!(
            00, 00, 00, 00;
            00, 00, 00, 00;
            00, 00, 00, 00;
            00, 00, 00, 00);

        let expected = gray_image!(
            00, 00, 00, 00;
            00, 00, 00, 00;
            00, 00, 00, 00;
            00, 00, 00, 00);

        let rotated = rotate_about_center(
            &image,
            std::f32::consts::PI / 2f32,
            Interpolation::Nearest,
            Luma([99u8]),
        );
        assert_pixels_eq!(rotated, expected);
    }

which fails because a bar of 99s has been introduced.

thread 'geometric_transformations::tests::test_rotate_half_pi_zero_square_about_center' panicked at 'pixels do not match.
Actual:
       0   1   2 
   +-------------
   | 
  0|  99   0   0 
   | 
  1|  99   0   0 
   | 
  2|  99   0   0 
   | 
  3|  99   0   0 
   | 
Expected:
      0  1  2 
   +----------
   | 
  0|  0  0  0 
   | 
  1|  0  0  0 
   | 
  2|  0  0  0 
   | 
  3|  0  0  0 
   | 
', src/geometric_transformations.rs:895:9

Rotating by pi introduces bars in the top and side.

Actual:
       0   1   2   3 
   +-----------------
   | 
  0|  99  99  99  99 
   | 
  1|  99   0   0   0 
   | 
  2|  99   0   0   0 
   | 
  3|  99   0   0   0 
   | 
Expected:
      0  1  2  3 
   +-------------
   | 
  0|  0  0  0  0 
   | 
  1|  0  0  0  0 
   | 
  2|  0  0  0  0 
   | 
  3|  0  0  0  0 
   | 
', src/geometric_transformations.rs:895:9

Similarly, rotating by 3pi/2 introduces a black bar at the top. Testing with non-zero squares shows that the result square is offset by one pixel.

---- geometric_transformations::tests::test_rotate_half_pi_square_about_center stdout ----
thread 'geometric_transformations::tests::test_rotate_half_pi_square_about_center' panicked at 'pixels do not match.
Actual:
       0   1   2   3 
   +-----------------
   | 
  0|  99  31  21  10 
   | 
  1|  99  32  22  11 
   | 
  2|  99  33  23  12 
   | 
  3|  99  34  25  14 
   | 
Expected:
       0   1   2   3 
   +-----------------
   | 
  0|  31  21  10   0 
   | 
  1|  32  22  11   1 
   | 
  2|  33  23  12   2 
   | 
  3|  34  25  14   3 
   | 
', src/geometric_transformations.rs:899:9

Some of these additional tests are there in this branch https://github.com/chocolatier/imageproc/commits/fix_rotate

chocolatier commented 4 years ago

[I'm not 100% on how the code works, so I might be wrong, but...]

I think this is because the projection matrix takes the origin as the top left pixel (0,0). Since pixels encompass an area 1px*1px wide, directly applying the rotation is taking centre to be the top left pixel (0,0), i.e. (0.5,0.5).

Adding a half pixel offset while rotating fixes this issue, but breaks the regression tests.

Projection::translate(cx -0.5, cy - 0.5) * Projection::rotate(theta) * Projection::translate(-cx + 0.5, -cy + 0.5);
sjaustirni commented 4 years ago

Adding a half-pixel offset while rotating fixes this issue, but breaks the regression tests.

I thought this was the problem, but it turned out not to be. If you fix it as you have suggested, some other rotations get broken.

I have attempted to solve this issue in myriad ways now, but could not find a solution.

chocolatier commented 4 years ago

Could you tell me which other rotations this breaks (other than the regression tests, which are too large for me to compute by hand :).