From ebdf8beb59df8d62c0a978ad967098dae8f666cf Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Fri, 31 May 2019 17:26:23 -0700 Subject: [PATCH] Always use orientation corrected sizes in Downsampler#calculateScaling. Previously we were sometimes using the rotated size and sometimes using the original size. As a result, some combinations of width, height and exif orientation were decoded with unexpected sizes. Fixes #3673 --- .../bitmap/DownsamplerEmulatorTest.java | 26 ++++++++ .../load/resource/bitmap/Downsampler.java | 59 ++++++++++--------- 2 files changed, 58 insertions(+), 27 deletions(-) diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java index 2ed540dc66..d93272cc27 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java @@ -57,6 +57,9 @@ public class DownsamplerEmulatorTest { @Test public void calculateScaling_withAtMost() throws IOException { new Tester(DownsampleStrategy.AT_MOST) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, onAllApisAndAllFormatsExpect(1512, 2016)) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(200, onAllApisAndAllFormatsExpect(100, 100)) @@ -113,6 +116,9 @@ public void calculateScaling_withAtMost() throws IOException { @Test public void calculateScaling_withAtLeast() throws IOException { new Tester(DownsampleStrategy.AT_LEAST) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, onAllApisAndAllFormatsExpect(3024, 4032)) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(200, onAllApisAndAllFormatsExpect(100, 100)) @@ -133,6 +139,12 @@ public void calculateScaling_withAtLeast() throws IOException { @Test public void calculateScaling_withCenterInside() throws IOException { new Tester(DownsampleStrategy.CENTER_INSIDE) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, + atAndAbove(KITKAT).with(allFormats().expect(1977, 2636)), + // TODO(b/134182995): This shouldn't be preserving quality. + below(KITKAT).with(allFormats().expect(3024, 4032))) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(400, onAllApisAndAllFormatsExpect(100, 100)) @@ -189,6 +201,11 @@ public void calculateScaling_withCenterInside() throws IOException { @Test public void calculateScaling_withCenterOutside() throws IOException { new Tester(DownsampleStrategy.CENTER_OUTSIDE) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, + atAndAbove(KITKAT).with(allFormats().expect(1977, 2636)), + below(KITKAT).with(allFormats().expect(3024, 4032))) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(200, onAllApisAndAllFormatsExpect(100, 100)) @@ -228,6 +245,9 @@ public void calculateScaling_withCenterOutside() throws IOException { @Test public void calculateScaling_withNone() throws IOException { new Tester(DownsampleStrategy.NONE) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, onAllApisAndAllFormatsExpect(3024, 4032)) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(200, onAllApisAndAllFormatsExpect(200, 200)) @@ -248,6 +268,12 @@ public void calculateScaling_withNone() throws IOException { @Test public void calculateScaling_withFitCenter() throws IOException { new Tester(DownsampleStrategy.FIT_CENTER) + // See #3673 + .setTargetDimensions(1977, 2636) + .givenImageWithDimensionsOf(3024, 4032, + atAndAbove(KITKAT).with(allFormats().expect(1977, 2636)), + // TODO(b/134182995): This shouldn't be preserving quality. + below(KITKAT).with(allFormats().expect(3024, 4032))) .setTargetDimensions(100, 100) .givenSquareImageWithDimensionOf(100, onAllApisAndAllFormatsExpect(100, 100)) .givenSquareImageWithDimensionOf(200, onAllApisAndAllFormatsExpect(100, 100)) diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java index 4c2cfd5022..66e247a8e2 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java @@ -384,19 +384,21 @@ private static void calculateScaling( return; } - final float exactScaleFactor; + int orientedSourceWidth = sourceWidth; + int orientedSourceHeight = sourceHeight; + // If we're rotating the image +-90 degrees, we need to downsample accordingly so the image + // width is decreased to near our target's height and the image height is decreased to near + // our target width. + //noinspection SuspiciousNameCombination if (degreesToRotate == 90 || degreesToRotate == 270) { - // If we're rotating the image +-90 degrees, we need to downsample accordingly so the image - // width is decreased to near our target's height and the image height is decreased to near - // our target width. - //noinspection SuspiciousNameCombination - exactScaleFactor = - downsampleStrategy.getScaleFactor(sourceHeight, sourceWidth, targetWidth, targetHeight); - } else { - exactScaleFactor = - downsampleStrategy.getScaleFactor(sourceWidth, sourceHeight, targetWidth, targetHeight); + orientedSourceWidth = sourceHeight; + orientedSourceHeight = sourceWidth; } + final float exactScaleFactor = + downsampleStrategy.getScaleFactor( + orientedSourceWidth, orientedSourceHeight, targetWidth, targetHeight); + if (exactScaleFactor <= 0f) { throw new IllegalArgumentException( "Cannot scale with factor: " @@ -414,18 +416,19 @@ private static void calculateScaling( + targetHeight + "]"); } + SampleSizeRounding rounding = downsampleStrategy.getSampleSizeRounding( - sourceWidth, sourceHeight, targetWidth, targetHeight); + orientedSourceWidth, orientedSourceHeight, targetWidth, targetHeight); if (rounding == null) { throw new IllegalArgumentException("Cannot round with null rounding"); } - int outWidth = round(exactScaleFactor * sourceWidth); - int outHeight = round(exactScaleFactor * sourceHeight); + int outWidth = round(exactScaleFactor * orientedSourceWidth); + int outHeight = round(exactScaleFactor * orientedSourceHeight); - int widthScaleFactor = sourceWidth / outWidth; - int heightScaleFactor = sourceHeight / outHeight; + int widthScaleFactor = orientedSourceWidth / outWidth; + int heightScaleFactor = orientedSourceHeight / outHeight; int scaleFactor = rounding == SampleSizeRounding.MEMORY @@ -458,26 +461,26 @@ private static void calculateScaling( // After libjpegturbo's native rounding, skia does a secondary scale using floor // (integer division). Here we replicate that logic. int nativeScaling = Math.min(powerOfTwoSampleSize, 8); - powerOfTwoWidth = (int) Math.ceil(sourceWidth / (float) nativeScaling); - powerOfTwoHeight = (int) Math.ceil(sourceHeight / (float) nativeScaling); + powerOfTwoWidth = (int) Math.ceil(orientedSourceWidth / (float) nativeScaling); + powerOfTwoHeight = (int) Math.ceil(orientedSourceHeight / (float) nativeScaling); int secondaryScaling = powerOfTwoSampleSize / 8; if (secondaryScaling > 0) { powerOfTwoWidth = powerOfTwoWidth / secondaryScaling; powerOfTwoHeight = powerOfTwoHeight / secondaryScaling; } } else if (imageType == ImageType.PNG || imageType == ImageType.PNG_A) { - powerOfTwoWidth = (int) Math.floor(sourceWidth / (float) powerOfTwoSampleSize); - powerOfTwoHeight = (int) Math.floor(sourceHeight / (float) powerOfTwoSampleSize); + powerOfTwoWidth = (int) Math.floor(orientedSourceWidth / (float) powerOfTwoSampleSize); + powerOfTwoHeight = (int) Math.floor(orientedSourceHeight / (float) powerOfTwoSampleSize); } else if (imageType == ImageType.WEBP || imageType == ImageType.WEBP_A) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - powerOfTwoWidth = Math.round(sourceWidth / (float) powerOfTwoSampleSize); - powerOfTwoHeight = Math.round(sourceHeight / (float) powerOfTwoSampleSize); + powerOfTwoWidth = Math.round(orientedSourceWidth / (float) powerOfTwoSampleSize); + powerOfTwoHeight = Math.round(orientedSourceHeight / (float) powerOfTwoSampleSize); } else { - powerOfTwoWidth = (int) Math.floor(sourceWidth / (float) powerOfTwoSampleSize); - powerOfTwoHeight = (int) Math.floor(sourceHeight / (float) powerOfTwoSampleSize); + powerOfTwoWidth = (int) Math.floor(orientedSourceWidth / (float) powerOfTwoSampleSize); + powerOfTwoHeight = (int) Math.floor(orientedSourceHeight / (float) powerOfTwoSampleSize); } - } else if (sourceWidth % powerOfTwoSampleSize != 0 - || sourceHeight % powerOfTwoSampleSize != 0) { + } else if (orientedSourceWidth % powerOfTwoSampleSize != 0 + || orientedSourceHeight % powerOfTwoSampleSize != 0) { // If we're not confident the image is in one of our types, fall back to checking the // dimensions again. inJustDecodeBounds decodes do obey inSampleSize. int[] dimensions = getDimensions(is, options, decodeCallbacks, bitmapPool); @@ -488,8 +491,8 @@ private static void calculateScaling( powerOfTwoWidth = dimensions[0]; powerOfTwoHeight = dimensions[1]; } else { - powerOfTwoWidth = sourceWidth / powerOfTwoSampleSize; - powerOfTwoHeight = sourceHeight / powerOfTwoSampleSize; + powerOfTwoWidth = orientedSourceWidth / powerOfTwoSampleSize; + powerOfTwoHeight = orientedSourceHeight / powerOfTwoSampleSize; } double adjustedScaleFactor = @@ -517,6 +520,8 @@ private static void calculateScaling( + "x" + sourceHeight + "]" + + ", degreesToRotate: " + + degreesToRotate + ", target: [" + targetWidth + "x"