Simplify MCU adjustment

imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java

@@ -118,15 +118,18 @@ public Iterator<ImageTypeSpecifier> getImageTypes(int imageIndex) throws IOExcep
 
   /**
    * Since TurboJPEG can only crop to values divisible by the MCU size, we may need to expand the
-   * cropping area to get a suitable rectangle. Thus, cropping becomes a two-stage process: - Crop
-   * to to nearest MCU boundaries (TurboJPEG) - Crop to the actual region (Java)
+   * cropping area to get a suitable rectangle. Thus, cropping becomes a two-stage process: 1. Crop
+   * to to nearest MCU boundaries (TurboJPEG) 2. Crop to the actual region (Java).
+   * <strong>This method <em>mutates</em> the region!</strong>
    *
    * <p>Additionally, since TurboJPEG applies rotation **before** cropping, but the ImageIO API is
    * based on the assumption that rotation occurs **after** cropping, we have to transform the
    * cropping region accordingly.
    *
    * @param mcuSize The size of the MCUs
    * @param region The source region to be cropped
+   * @param rotation Degrees the image is supposed to be rotated.
+   * @param imageSize Dimensions of the image the cropping region targets
    * @return The region that needs to be cropped from the image cropped to the expanded rectangle
    */
   Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimension imageSize) {
@@ -135,6 +138,8 @@ Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimens
     }
     final int originalWidth = imageSize.width;
     final int originalHeight = imageSize.height;
+
+    // Recalculate the cropping region based on the desired rotation.
     final Rectangle originalRegion = (Rectangle) region.clone();
     if (rotation == 90) {
       int x = region.x;
@@ -155,50 +160,43 @@ Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimens
       region.width = region.height;
       region.height = w;
     }
+
+    // Calculate how much of the region returned from libjpeg has to be cropped on the JVM-side
     Rectangle extraCrop =
         new Rectangle(
             0,
             0,
             region.width == 0 ? originalWidth - region.x : region.width,
             region.height == 0 ? originalHeight - region.y : region.height);
+    // X-Offset + Width
     if (region.x % mcuSize.width != 0) {
       extraCrop.x = region.x % mcuSize.width;
       region.x -= extraCrop.x;
       if (region.width > 0) {
         region.width = Math.min(region.width + extraCrop.x, originalWidth - region.x);
       }
     }
+    // Y-Offset + Height
     if (region.y % mcuSize.height != 0) {
       extraCrop.y = region.y % mcuSize.height;
       region.y -= extraCrop.y;
       if (region.height > 0) {
         region.height = Math.min(region.height + extraCrop.y, originalHeight - region.y);
       }
     }
+
     if ((region.x + region.width) != originalWidth && region.width % mcuSize.width != 0) {
-      int adjustedWidth = (int) (mcuSize.width * (Math.ceil(region.getWidth() / mcuSize.width)));
-      // ceil makes the region bigger than requested so we can crop it later. Sometimes that exceeds the image boundaries.
-      if (region.x + adjustedWidth > originalWidth) {
-        adjustedWidth = (int) (mcuSize.width * Math.floor(region.getWidth() / mcuSize.width));
-      }
-      region.width = adjustedWidth;
+      region.width = Math.min(
+            (int) (mcuSize.width * (Math.ceil(region.getWidth() / mcuSize.width))),
+            imageSize.width - region.x);
     }
-    // TODO is adjusting extraCrop necessary? Could region now be smaller than extraCrop?
+
     if ((region.y + region.height) != originalHeight && region.height % mcuSize.height != 0) {
-      int adjustedHeight = (int) (mcuSize.height * (Math.ceil(region.getHeight() / mcuSize.height)));
-      // ceil makes the region bigger than requested so we can crop it later. Sometimes that exceeds the image boundaries.
-      if (region.y + adjustedHeight > originalHeight) {
-        adjustedHeight = (int) (mcuSize.height * Math.floor(region.getHeight() / mcuSize.height));
-      }
-      region.height = adjustedHeight;
-    }
-    // TODO is adjusting extraCrop necessary? Could region now be smaller than extraCrop?
-    if (region.height > originalHeight) {
-      region.height = originalHeight;
-    }
-    if (region.width > originalWidth) {
-      region.width = originalWidth;
+      region.height = Math.min(
+          (int) (mcuSize.height * (Math.ceil(region.getHeight() / mcuSize.height))),
+          imageSize.height - region.y);
     }
+
     boolean modified =
         originalRegion.x != region.x
             || originalRegion.y != region.y

imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java

@@ -252,19 +252,19 @@ public void testCroppingRequiresReallocation() throws IOException {
   }
 
   @Test
-  void testAdjustMCURegion() throws IOException {
+  void testAdjustMCURegion() {
     TurboJpegImageReader reader = new TurboJpegImageReader(null, null);
 
     Dimension mcuSize = new Dimension(16, 16);
     Rectangle region = new Rectangle(1185, 327, 309, 36);
     int rotation = 0;
     Dimension imageSize = new Dimension(1500, 2260);
 
-    Rectangle actual = reader.adjustRegion(mcuSize, region, rotation, imageSize);
-    Rectangle expected = new Rectangle(1184, 320, 320, 48);
+    Rectangle extraCrop = reader.adjustRegion(mcuSize, region, rotation, imageSize);
+    Rectangle regionExpected = new Rectangle(1184, 320, 316, 48);
+    Rectangle extraCropExpected = new Rectangle(1, 7, 309, 36);
 
-//    assertThat(actual).isEqualTo(expected);
-    assertThat(region.x + region.width).isLessThanOrEqualTo(imageSize.width);
-    assertThat(region.y + region.height).isLessThanOrEqualTo(imageSize.height);
+    assertThat(region).isEqualTo(regionExpected);
+    assertThat(extraCrop).isEqualTo(extraCropExpected);
   }
 }