Skip to content

Commit

Permalink
Always use orientation corrected sizes in Downsampler#calculateScaling.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@sjudd
sjudd committed Jun 1, 2019
1 parent 41877c6 commit ebdf8be
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 27 deletions.
26 changes: 26 additions & 0 deletions ...src/androidTest/java/com/bumptech/glide/load/resource/bitmap/DownsamplerEmulatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down
59 changes: 32 additions & 27 deletions library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: "
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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 =
Expand Down Expand Up @@ -517,6 +520,8 @@ private static void calculateScaling(
+ "x"
+ sourceHeight
+ "]"
+ ", degreesToRotate: "
+ degreesToRotate
+ ", target: ["
+ targetWidth
+ "x"
Expand Down

0 comments on commit ebdf8be

Please sign in to comment.