Skip to content

Commit

Permalink
Improve comments and consistency of sampling in DownsampleStrategies.
Browse files Browse the repository at this point in the history 
FitCenter, CenterInside, and CenterOutside now all reliably prefer 
quality on versions < KitKat where we can only do power of two 
downsampling.

I’ve also fixed a bug in FitCenter and CenterInside where we would 
downsample to 2x the requested size in some cases where there was
an exact power of 2 match in one dimension but not the other. The logic
to fix the bug isn’t perfect, but it keeps this change small and avoids
making any API changes.
  • Loading branch information
@sjudd
sjudd committed Jun 7, 2019
1 parent cd37a54 commit 3df5445
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 39 deletions.
21 changes: 15 additions & 6 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 @@ -87,6 +87,17 @@ public void calculateScaling_withAtMost() throws IOException {
.with(formats(JPEG, WEBP).expect(13, 100), formats(PNG).expect(12, 100)),
below(VERSION_CODES.N)
.with(formats(JPEG).expect(13, 100), formats(PNG, WEBP).expect(12, 100)))
.givenImageWithDimensionsOf(
801,
100,
below(KITKAT)
.with(
// JPEG is correct because CENTER_INSIDE wants to give a subsequent
// transformation an image that is greater in size than the requested size. On
// Api > VERSION_CODES.KITKAT, CENTER_INSIDE can do the transformation itself.
// On < VERSION_CODES.KITKAT, it has to assume a subsequent transformation will
// be called.
formats(JPEG).expect(50, 6), formats(PNG, WEBP).expect(50, 6)))
.givenImageWithDimensionsOf(87, 78, onAllApisAndAllFormatsExpect(87, 78))
// This set of examples demonstrate that webp uses round on N+ and floor < N.
.setTargetDimensions(13, 13)
Expand Down Expand Up @@ -145,7 +156,6 @@ public void calculateScaling_withCenterInside() throws IOException {
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))
Expand All @@ -164,7 +174,7 @@ public void calculateScaling_withCenterInside() throws IOException {
800,
100,
atAndAbove(KITKAT).with(allFormats().expect(100, 13)),
below(KITKAT).with(allFormats().expect(200, 25)))
below(KITKAT).with(formats(JPEG).expect(100, 13), formats(PNG, WEBP).expect(100, 12)))
.givenImageWithDimensionsOf(
801,
100,
Expand All @@ -184,7 +194,7 @@ public void calculateScaling_withCenterInside() throws IOException {
100,
800,
atAndAbove(KITKAT).with(allFormats().expect(13, 100)),
below(KITKAT).with(allFormats().expect(25, 200)))
below(KITKAT).with(formats(JPEG).expect(13, 100), formats(PNG, WEBP).expect(12, 100)))
.givenImageWithDimensionsOf(87, 78, onAllApisAndAllFormatsExpect(87, 78))
.setTargetDimensions(897, 897)
.givenImageWithDimensionsOf(
Expand Down Expand Up @@ -278,7 +288,6 @@ public void calculateScaling_withFitCenter() throws IOException {
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))
Expand All @@ -298,7 +307,7 @@ public void calculateScaling_withFitCenter() throws IOException {
800,
100,
atAndAbove(KITKAT).with(allFormats().expect(100, 13)),
below(KITKAT).with(allFormats().expect(200, 25)))
below(KITKAT).with(formats(JPEG).expect(100, 13), formats(PNG, WEBP).expect(100, 12)))
.givenImageWithDimensionsOf(
801,
100,
Expand All @@ -318,7 +327,7 @@ public void calculateScaling_withFitCenter() throws IOException {
100,
800,
atAndAbove(KITKAT).with(allFormats().expect(13, 100)),
below(KITKAT).with(allFormats().expect(25, 200)))
below(KITKAT).with(formats(JPEG).expect(13, 100), formats(PNG, WEBP).expect(12, 100)))
.givenImageWithDimensionsOf(
87,
78,
Expand Down
92 changes: 60 additions & 32 deletions library/src/main/java/com/bumptech/glide/load/resource/bitmap/DownsampleStrategy.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.bumptech.glide.load.resource.bitmap;

import android.os.Build;
import com.bumptech.glide.load.Option;
import com.bumptech.glide.util.Synthetic;

Expand All @@ -17,11 +18,34 @@
* com.bumptech.glide.load.ResourceDecoder}s are listed below, but the list is not comprehensive
* because {@link DownsampleStrategy} only controls it's output scale value, not how that output
* value is used.
*
* <p>On some versions of Android, precise scaling is not possible. In those cases, the strategies
* can only pick between downsampling to between 1x the requested size and 2x the requested size and
* between 0.5x the requested size and 1x the requested size because only power of two downsampling
* is supported. To preserve the potential for a {@link com.bumptech.glide.load.Transformation} to
* scale precisely without a loss in quality, all but {@link #AT_MOST} will prefer to downsample to
* between 1x and 2x the requested size.
*/
// Public API.
@SuppressWarnings("WeakerAccess")
public abstract class DownsampleStrategy {

/**
* Downsamples so the image's smallest dimension is between the given dimensions and 2x the given
* dimensions, with no size restrictions on the image's largest dimension.
*
* <p>Does not upscale if the requested dimensions are larger than the original dimensions.
*/
public static final DownsampleStrategy AT_LEAST = new AtLeast();

/**
* Downsamples so the image's largest dimension is between 1/2 the given dimensions and the given
* dimensions, with no restrictions on the image's smallest dimension.
*
* <p>Does not upscale if the requested dimensions are larger than the original dimensions.
*/
public static final DownsampleStrategy AT_MOST = new AtMost();

/**
* Scales, maintaining the original aspect ratio, so that one of the image's dimensions is exactly
* equal to the requested size and the other dimension is less than or equal to the requested
Expand All @@ -31,11 +55,17 @@ public abstract class DownsampleStrategy {
* and height. To avoid upscaling, use {@link #AT_LEAST}, {@link #AT_MOST} or {@link
* #CENTER_INSIDE}.
*
* <p>On pre-KitKat devices, {@link Downsampler} treats this as equivalent to {@link #AT_MOST}
* because only power of two downsampling can be used.
* <p>On pre-KitKat devices, {@code FIT_CENTER} will downsample by a power of two only so that one
* of the image's dimensions is greater than or equal to the requested size. No guarantees are
* made about the second dimensions. This is <em>NOT</em> the same as {@link #AT_LEAST} because
* only one dimension, not both, are greater than or equal to the requested dimensions, the other
* may be smaller.
*/
public static final DownsampleStrategy FIT_CENTER = new FitCenter();

/** Identical to {@link #FIT_CENTER}, but never upscales. */
public static final DownsampleStrategy CENTER_INSIDE = new CenterInside();

/**
* Scales, maintaining the original aspect ratio, so that one of the image's dimensions is exactly
* equal to the requested size and the other dimension is greater than or equal to the requested
Expand All @@ -50,31 +80,6 @@ public abstract class DownsampleStrategy {
*/
public static final DownsampleStrategy CENTER_OUTSIDE = new CenterOutside();

/**
* Downsamples so the image's smallest dimension is between the given dimensions and 2x the given
* dimensions, with no size restrictions on the image's largest dimension.
*
* <p>Does not upscale if the requested dimensions are larger than the original dimensions.
*/
public static final DownsampleStrategy AT_LEAST = new AtLeast();

/**
* Downsamples so the image's largest dimension is between 1/2 the given dimensions and the given
* dimensions, with no restrictions on the image's smallest dimension.
*
* <p>Does not upscale if the requested dimensions are larger than the original dimensions.
*/
public static final DownsampleStrategy AT_MOST = new AtMost();

/**
* Returns the original image if it is smaller than the target, otherwise it will be downscaled
* maintaining its original aspect ratio, so that one of the image's dimensions is exactly equal
* to the requested size and the other is less or equal than the requested size.
*
* <p>Does not upscale if the requested dimensions are larger than the original dimensions.
*/
public static final DownsampleStrategy CENTER_INSIDE = new CenterInside();

/** Performs no downsampling or scaling. */
public static final DownsampleStrategy NONE = new None();

Expand All @@ -92,6 +97,10 @@ public abstract class DownsampleStrategy {
Option.memory(
"com.bumptech.glide.load.resource.bitmap.Downsampler.DownsampleStrategy", DEFAULT);

@Synthetic
static final boolean IS_BITMAP_FACTORY_SCALING_SUPPORTED =
Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;

/**
* Returns a float (0, +infinity) indicating a scale factor to apply to the source width and
* height when displayed in the requested width and height.
Expand Down Expand Up @@ -133,15 +142,31 @@ private static class FitCenter extends DownsampleStrategy {
@Override
public float getScaleFactor(
int sourceWidth, int sourceHeight, int requestedWidth, int requestedHeight) {
float widthPercentage = requestedWidth / (float) sourceWidth;
float heightPercentage = requestedHeight / (float) sourceHeight;
return Math.min(widthPercentage, heightPercentage);
if (IS_BITMAP_FACTORY_SCALING_SUPPORTED) {
float widthPercentage = requestedWidth / (float) sourceWidth;
float heightPercentage = requestedHeight / (float) sourceHeight;

return Math.min(widthPercentage, heightPercentage);
} else {
// Similar to AT_LEAST, but only require one dimension or the other to be >= requested
// rather than both.
int maxIntegerFactor =
Math.max(sourceHeight / requestedHeight, sourceWidth / requestedWidth);
return maxIntegerFactor == 0 ? 1f : 1f / Integer.highestOneBit(maxIntegerFactor);
}
}

@Override
public SampleSizeRounding getSampleSizeRounding(
int sourceWidth, int sourceHeight, int requestedWidth, int requestedHeight) {
return SampleSizeRounding.QUALITY;
if (IS_BITMAP_FACTORY_SCALING_SUPPORTED) {
return SampleSizeRounding.QUALITY;
} else {
// TODO: This doesn't seem right, but otherwise we can skip a sample size because QUALITY
// prefers the smaller of the the width and height scale factor. MEMORY is a hack that
// lets us prefer the larger of the two.
return SampleSizeRounding.MEMORY;
}
}
}

Expand Down Expand Up @@ -246,7 +271,10 @@ public float getScaleFactor(
@Override
public SampleSizeRounding getSampleSizeRounding(
int sourceWidth, int sourceHeight, int requestedWidth, int requestedHeight) {
return SampleSizeRounding.QUALITY;
return getScaleFactor(sourceWidth, sourceHeight, requestedWidth, requestedHeight) == 1.f
? SampleSizeRounding.QUALITY
: FIT_CENTER.getSampleSizeRounding(
sourceWidth, sourceHeight, requestedWidth, requestedHeight);
}
}

Expand Down
3 changes: 3 additions & 0 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 @@ -430,6 +430,9 @@ private static void calculateScaling(
int widthScaleFactor = orientedSourceWidth / outWidth;
int heightScaleFactor = orientedSourceHeight / outHeight;

// TODO: This isn't really right for both CenterOutside and CenterInside. Consider allowing
// DownsampleStrategy to pick, or trying to do something more sophisticated like picking the
// scale factor that leads to an exact match.
int scaleFactor =
rounding == SampleSizeRounding.MEMORY
? Math.max(widthScaleFactor, heightScaleFactor)
Expand Down
2 changes: 1 addition & 1 deletion ...ry/test/src/test/java/com/bumptech/glide/load/resource/bitmap/DownsampleStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = 18)
@Config(sdk = 21)
public class DownsampleStrategyTest {

@Test
Expand Down

0 comments on commit 3df5445

Please sign in to comment.