From 3f947cf3ef77978d13a18b968d322e1a95710dc8 Mon Sep 17 00:00:00 2001 From: Marcus Bitzl Date: Thu, 29 Apr 2021 16:51:37 +0200 Subject: [PATCH 1/2] Fix jpeg cropping edge case TurboJpeg expects cropping regions to be multiples of the MCU size. In some cases it was possible that the cropping region was larger than the image due to rounding artifacts (less than one MCU). This MR checks if the resulting region would exceed the image and makes it one MCU width and/or height smaller if needed. --- .../turbojpeg/TurboJpeg.java | 3 +- .../imageio/TurboJpegImageReader.java | 31 ++++++++++++++----- .../imageio/TurboJpegImageReaderTest.java | 18 +++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/TurboJpeg.java b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/TurboJpeg.java index 2301db2..6ad8212 100644 --- a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/TurboJpeg.java +++ b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/TurboJpeg.java @@ -272,7 +272,8 @@ public ByteBuffer transform(byte[] jpegData, Info info, Rectangle region, int ro || ((region.y + region.height) != height && region.height % mcuSize.height != 0)) { throw new IllegalArgumentException( String.format( - "Invalid cropping region, width must be divisible by %d, height by %d", + "Invalid cropping region %d×%d, width must be divisible by %d, height by %d", + region.width, region.height, mcuSize.width, mcuSize.height)); } width = region.width; diff --git a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java index 60620ce..1c07425 100644 --- a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java +++ b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java @@ -93,6 +93,11 @@ public int getNumImages(boolean allowSearch) throws IOException { return info.getAvailableSizes().size(); } + private Dimension getDimension(int imageIndex) { + checkIndex(imageIndex); + return info.getAvailableSizes().get(imageIndex); + } + @Override public int getWidth(int imageIndex) throws IOException { checkIndex(imageIndex); @@ -124,13 +129,12 @@ public Iterator getImageTypes(int imageIndex) throws IOExcep * @param region The source region to be cropped * @return The region that needs to be cropped from the image cropped to the expanded rectangle */ - private Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation) - throws IOException { + Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimension imageSize) { if (region == null) { return null; } - final int originalWidth = getWidth(0); - final int originalHeight = getHeight(0); + final int originalWidth = imageSize.width; + final int originalHeight = imageSize.height; final Rectangle originalRegion = (Rectangle) region.clone(); if (rotation == 90) { int x = region.x; @@ -172,11 +176,23 @@ private Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation } } if ((region.x + region.width) != originalWidth && region.width % mcuSize.width != 0) { - region.width = (int) (mcuSize.width * (Math.ceil(region.getWidth() / mcuSize.width))); + 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; } + // TODO is adjusting extraCrop necessary? Could region now be smaller than extraCrop? if ((region.y + region.height) != originalHeight && region.height % mcuSize.height != 0) { - region.height = (int) (mcuSize.height * (Math.ceil(region.getHeight() / mcuSize.height))); + 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; } @@ -259,7 +275,8 @@ public BufferedImage read(int imageIndex, ImageReadParam param) throws IOExcepti region = param.getSourceRegion(); if (!isRegionFullImage(imageIndex, region)) { scaleRegion(imageIndex, region); - extraCrop = adjustRegion(info.getMCUSize(), region, rotation); + // adjustments need native image size → imageIndex == 0 + extraCrop = adjustRegion(info.getMCUSize(), region, rotation, getDimension(0)); } else { region = null; } diff --git a/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java b/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java index 6811e47..763b25b 100644 --- a/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java +++ b/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java @@ -2,6 +2,7 @@ import static de.digitalcollections.turbojpeg.imageio.CustomAssertions.assertThat; +import java.awt.Dimension; import java.awt.Graphics; import java.awt.Rectangle; import java.awt.image.BufferedImage; @@ -249,4 +250,21 @@ public void testCroppingRequiresReallocation() throws IOException { assertThat(img.getWidth()).isEqualTo(365); assertThat(img.getHeight()).isEqualTo(10); } + + @Test + void testAdjustMCURegion() throws IOException { + 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); + +// assertThat(actual).isEqualTo(expected); + assertThat(region.x + region.width).isLessThanOrEqualTo(imageSize.width); + assertThat(region.y + region.height).isLessThanOrEqualTo(imageSize.height); + } } From 1d43cb1338d4471d1575afb5b6ccf833713dcb80 Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Thu, 6 May 2021 13:19:49 +0200 Subject: [PATCH 2/2] Simplify MCU adjustment --- .../imageio/TurboJpegImageReader.java | 42 +++++++++---------- .../imageio/TurboJpegImageReaderTest.java | 12 +++--- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java index 1c07425..6d29c16 100644 --- a/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java +++ b/imageio-turbojpeg/src/main/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReader.java @@ -118,8 +118,9 @@ public Iterator 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). + * This method mutates the region! * *

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 @@ -127,6 +128,8 @@ public Iterator getImageTypes(int imageIndex) throws IOExcep * * @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,12 +160,15 @@ 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; @@ -168,6 +176,7 @@ Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimens 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; @@ -175,30 +184,19 @@ Rectangle adjustRegion(Dimension mcuSize, Rectangle region, int rotation, Dimens 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 diff --git a/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java b/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java index 763b25b..1c64b8e 100644 --- a/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java +++ b/imageio-turbojpeg/src/test/java/de/digitalcollections/turbojpeg/imageio/TurboJpegImageReaderTest.java @@ -252,7 +252,7 @@ public void testCroppingRequiresReallocation() throws IOException { } @Test - void testAdjustMCURegion() throws IOException { + void testAdjustMCURegion() { TurboJpegImageReader reader = new TurboJpegImageReader(null, null); Dimension mcuSize = new Dimension(16, 16); @@ -260,11 +260,11 @@ void testAdjustMCURegion() throws IOException { 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); } }