Choose whether or not to use HARDWARE bitmaps based on transformations in Glide.

Previously we were checking only against the requested and actual size
of the image as a heuristic to see if we thought a transformation would
be applied. That doesn't work well for certain types of transformations
that don't necessarily change the size of the image but do change its
shape (rounded corners, circle crop etc).

Now we're explicitly checking to see if a transformation will be applied
to the load and I've removed the heuristic. This should catch cases like
CircleCrop and also allow us to use hardware bitmaps when doing scaling
entirely within Downsampler.

In addition, we're now not applying transformations to Bitmaps for scale
only transformations that can be done with DownsampleStrategies alone.
This is a change in behavior, but since the Transformations were no-ops
previously, it should be low risk.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166253525

library/src/main/java/com/bumptech/glide/load/DecodeFormat.java

@@ -28,7 +28,12 @@ public enum DecodeFormat {
   /**
    * Identical to {@link #PREFER_ARGB_8888} but prevents Glide from using {@link
    * android.graphics.Bitmap.Config#HARDWARE} on Android O+
+   *
+   * @deprecated If you must disable hardware bitmaps, set
+   * {@link com.bumptech.glide.load.resource.bitmap.Downsampler#ALLOW_HARDWARE_CONFIG} to false
+   * instead.
    */
+  @Deprecated
   PREFER_ARGB_8888_DISALLOW_HARDWARE,
 
   /**
@@ -46,5 +51,5 @@ public enum DecodeFormat {
   /**
    * The default value for DecodeFormat.
    */
-  public static final DecodeFormat DEFAULT = PREFER_ARGB_8888_DISALLOW_HARDWARE;
+  public static final DecodeFormat DEFAULT = PREFER_ARGB_8888;
 }

library/src/main/java/com/bumptech/glide/load/engine/DecodeHelper.java

 package com.bumptech.glide.load.engine;
 
+import android.graphics.Bitmap;
 import com.bumptech.glide.GlideContext;
 import com.bumptech.glide.Priority;
 import com.bumptech.glide.Registry;
@@ -132,6 +133,10 @@ final class DecodeHelper<Transcode> {
     return glideContext.getRegistry().getLoadPath(dataClass, resourceClass, transcodeClass);
   }
 
+  boolean isBitmapTransformationSet() {
+    return transformations.get(Bitmap.class) != null;
+  }
+
   @SuppressWarnings("unchecked")
   <Z> Transformation<Z> getTransformation(Class<Z> resourceClass) {
     Transformation<Z> result = (Transformation<Z>) transformations.get(resourceClass);

library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java

 package com.bumptech.glide.load.engine;
 
+import android.os.Build;
 import android.support.v4.os.TraceCompat;
 import android.support.v4.util.Pools;
 import android.util.Log;
@@ -15,6 +16,7 @@ import com.bumptech.glide.load.Transformation;
 import com.bumptech.glide.load.data.DataFetcher;
 import com.bumptech.glide.load.data.DataRewinder;
 import com.bumptech.glide.load.engine.cache.DiskCache;
+import com.bumptech.glide.load.resource.bitmap.Downsampler;
 import com.bumptech.glide.util.LogTime;
 import com.bumptech.glide.util.Synthetic;
 import com.bumptech.glide.util.pool.FactoryPools.Poolable;
@@ -450,12 +452,31 @@ class DecodeJob<R> implements DataFetcherGenerator.FetcherReadyCallback,
     return runLoadPath(data, dataSource, path);
   }
 
+  private Options getOptionsWithHardwareConfig(DataSource dataSource) {
+    Options options = this.options;
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
+      return options;
+    }
+
+    if (options.get(Downsampler.ALLOW_HARDWARE_CONFIG) != null) {
+      return options;
+    }
+
+    if (dataSource == DataSource.RESOURCE_DISK_CACHE
+        || !decodeHelper.isBitmapTransformationSet()) {
+      options = new Options();
+      options.putAll(this.options);
+      options.set(Downsampler.ALLOW_HARDWARE_CONFIG, true);
+    }
+    return options;
+  }
+
   private <Data, ResourceType> Resource<R> runLoadPath(Data data, DataSource dataSource,
       LoadPath<Data, ResourceType, R> path) throws GlideException {
+    Options options = getOptionsWithHardwareConfig(dataSource);
     DataRewinder<Data> rewinder = glideContext.getRegistry().getRewinder(data);
     try {
-      return path.load(rewinder, options, width, height,
-          new DecodeCallback<ResourceType>(dataSource));
+      return path.load(rewinder, options, width, height, new DecodeCallback<>(dataSource));
     } finally {
       rewinder.cleanup();
     }

library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java

@@ -144,6 +144,14 @@ public class LruBitmapPool implements BitmapPool {
 
   @Nullable
   private synchronized Bitmap getDirtyOrNull(int width, int height, Bitmap.Config config) {
+    // Avoid short circuiting on sdk int since it breaks on some versions of Android.
+    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+      if (config == Bitmap.Config.HARDWARE) {
+        throw new IllegalArgumentException("Cannot create a mutable Bitmap with config: " + config
+            + ". Consider setting Downsampler#ALLOW_HARDWARE_CONFIG to false in your RequestOptions"
+            + " and/or in GlideBuilder.setDefaultRequestOptions");
+      }
+    }
     // Config will be null for non public config types, which can lead to transformations naively
     // passing in null as the requested config here. See issue #194.
     final Bitmap result = strategy.get(width, height, config != null ? config : DEFAULT_CONFIG);

library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java

@@ -17,6 +17,7 @@ import com.bumptech.glide.load.engine.Resource;
 import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
 import com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool;
 import com.bumptech.glide.load.resource.bitmap.DownsampleStrategy.SampleSizeRounding;
+import com.bumptech.glide.request.RequestOptions;
 import com.bumptech.glide.request.target.Target;
 import com.bumptech.glide.util.Preconditions;
 import com.bumptech.glide.util.Util;
@@ -64,6 +65,27 @@ public final class Downsampler {
   public static final Option<Boolean> FIX_BITMAP_SIZE_TO_REQUESTED_DIMENSIONS =
       Option.memory("com.bumptech.glide.load.resource.bitmap.Downsampler.FixBitmapSize", false);
 
+  /**
+   * Indicates that it's safe or unsafe to decode {@link Bitmap}s with
+   * {@link Bitmap.Config#HARDWARE}.
+   *
+   * <p>Callers should almost never set this value to {@code true} manually. Glide will already do
+   * so when Glide believes it's safe to do (when no transformations are applied). Instead, callers
+   * can set this value to {@code false} to prevent Glide from decoding hardware bitmaps if Glide
+   * is unable to detect that hardware bitmaps are unsafe. For example, you should set this to
+   * {@code false} if you plan to draw it to a software {@link android.graphics.Canvas} or if you
+   * plan to inspect the {@link Bitmap}s pixels with {@link Bitmap#getPixel(int, int)} or
+   * {@link Bitmap#getPixels(int[], int, int, int, int, int, int)}.
+   *
+   * <p>Callers can disable hardware {@link Bitmap}s for all loads using
+   * {@link com.bumptech.glide.GlideBuilder#setDefaultRequestOptions(RequestOptions)}.
+   *
+   * <p>This option is ignored unless we're on Android O+.
+   */
+  public static final Option<Boolean> ALLOW_HARDWARE_CONFIG =
+      Option.memory(
+          "com.bumtpech.glide.load.resource.bitmap.Downsampler.AllowHardwareDecode", null);
+
   private static final String WBMP_MIME_TYPE = "image/vnd.wap.wbmp";
   private static final String ICO_MIME_TYPE = "image/x-ico";
   private static final Set<String> NO_DOWNSAMPLE_PRE_N_MIME_TYPES =
@@ -172,11 +194,16 @@ public final class Downsampler {
     DecodeFormat decodeFormat = options.get(DECODE_FORMAT);
     DownsampleStrategy downsampleStrategy = options.get(DOWNSAMPLE_STRATEGY);
     boolean fixBitmapToRequestedDimensions = options.get(FIX_BITMAP_SIZE_TO_REQUESTED_DIMENSIONS);
+    boolean isHardwareConfigAllowed = options.get(ALLOW_HARDWARE_CONFIG) != null
+        ? options.get(ALLOW_HARDWARE_CONFIG) : false;
+    if (decodeFormat == DecodeFormat.PREFER_ARGB_8888_DISALLOW_HARDWARE) {
+      isHardwareConfigAllowed = false;
+    }
 
     try {
       Bitmap result = decodeFromWrappedStreams(is, bitmapFactoryOptions,
-          downsampleStrategy, decodeFormat, requestedWidth, requestedHeight,
-          fixBitmapToRequestedDimensions, callbacks);
+          downsampleStrategy, decodeFormat, isHardwareConfigAllowed, requestedWidth,
+          requestedHeight, fixBitmapToRequestedDimensions, callbacks);
       return BitmapResource.obtain(result, bitmapPool);
     } finally {
       releaseOptions(bitmapFactoryOptions);
@@ -186,8 +213,9 @@ public final class Downsampler {
 
   private Bitmap decodeFromWrappedStreams(InputStream is,
       BitmapFactory.Options options, DownsampleStrategy downsampleStrategy,
-      DecodeFormat decodeFormat, int requestedWidth, int requestedHeight,
-      boolean fixBitmapToRequestedDimensions, DecodeCallbacks callbacks) throws IOException {
+      DecodeFormat decodeFormat, boolean isHardwareConfigAllowed, int requestedWidth,
+      int requestedHeight, boolean fixBitmapToRequestedDimensions,
+      DecodeCallbacks callbacks) throws IOException {
 
     int[] sourceDimensions = getDimensions(is, options, callbacks, bitmapPool);
     int sourceWidth = sourceDimensions[0];
@@ -197,13 +225,12 @@ public final class Downsampler {
     int orientation = ImageHeaderParserUtils.getOrientation(parsers, is, byteArrayPool);
     int degreesToRotate = TransformationUtils.getExifOrientationDegrees(orientation);
 
-
     int targetWidth = requestedWidth == Target.SIZE_ORIGINAL ? sourceWidth : requestedWidth;
     int targetHeight = requestedHeight == Target.SIZE_ORIGINAL ? sourceHeight : requestedHeight;
 
     calculateScaling(downsampleStrategy, degreesToRotate, sourceWidth, sourceHeight, targetWidth,
         targetHeight, options);
-    calculateConfig(is, decodeFormat, options, targetWidth, targetHeight);
+    calculateConfig(is, decodeFormat, isHardwareConfigAllowed, options, targetWidth, targetHeight);
 
     boolean isKitKatOrGreater = Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;
     // Prior to KitKat, the inBitmap size must exactly match the size of the bitmap we're decoding.
@@ -366,13 +393,14 @@ public final class Downsampler {
   private void calculateConfig(
       InputStream is,
       DecodeFormat format,
+      boolean isHardwareConfigAllowed,
       BitmapFactory.Options optionsWithScaling,
       int targetWidth,
       int targetHeight)
       throws IOException {
 
     if (hardwareConfigState.setHardwareConfigIfAllowed(
-        targetWidth, targetHeight, optionsWithScaling, format)) {
+        targetWidth, targetHeight, optionsWithScaling, format, isHardwareConfigAllowed)) {
       return;
     }
 

library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java

@@ -50,13 +50,6 @@ final class HardwareConfigState {
    */
   private static final int MAXIMUM_FDS_FOR_HARDWARE_CONFIGS = 700;
 
-  /**
-   * The minimum size that will trigger downsampling in {@link BitmapFactory}.
-   *
-   * <p>From {@link android.graphics.BitmapFactory.Options#inSampleSize}.
-   */
-  private static final int MINIMUM_SAMPLE_SIZE = 2;
-
   private volatile int decodesSinceLastFdCheck;
   private volatile boolean isHardwareConfigAllowed = true;
 
@@ -81,18 +74,19 @@ final class HardwareConfigState {
       int targetWidth,
       int targetHeight,
       BitmapFactory.Options optionsWithScaling,
-      DecodeFormat decodeFormat) {
-    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O
+      DecodeFormat decodeFormat,
+      boolean isHardwareConfigAllowed) {
+    if (!isHardwareConfigAllowed
+        || Build.VERSION.SDK_INT < Build.VERSION_CODES.O
         || decodeFormat == DecodeFormat.PREFER_ARGB_8888_DISALLOW_HARDWARE) {
       return false;
     }
 
-    boolean result = !optionsWithScaling.inScaled
-        && optionsWithScaling.inSampleSize < MINIMUM_SAMPLE_SIZE
-        && targetWidth >= MIN_HARDWARE_DIMENSION
-        && targetHeight >= MIN_HARDWARE_DIMENSION
-        // Make sure to call isFdSizeBelowHardwareLimit last because it has side affects.
-        && isFdSizeBelowHardwareLimit();
+    boolean result =
+        targetWidth >= MIN_HARDWARE_DIMENSION
+            && targetHeight >= MIN_HARDWARE_DIMENSION
+            // Make sure to call isFdSizeBelowHardwareLimit last because it has side affects.
+            && isFdSizeBelowHardwareLimit();
 
     if (result) {
       optionsWithScaling.inPreferredConfig = Bitmap.Config.HARDWARE;

library/src/main/java/com/bumptech/glide/request/RequestOptions.java

@@ -4,6 +4,7 @@ import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
+import android.os.Build;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import com.bumptech.glide.Priority;
@@ -820,55 +821,85 @@ public class RequestOptions implements Cloneable {
   }
 
   /**
-   * Applies {@link com.bumptech.glide.load.resource.bitmap.FitCenter} to all default types, and
-   * ignores unknown types.
    *
-   * <p>This will override previous calls to {@link #dontTransform()}.
+   * Applies {@link FitCenter} and/or {@link DownsampleStrategy#FIT_CENTER} depending on the
+   * android version to all default types and ignores unknown types.
+   *
+   * <p>On Android devices with KitKat or greater, this method only applies a transformation for
+   * {@link GifDrawable}s and uses {@link DownsampleStrategy#FIT_CENTER} only for {@link Bitmap}s.
+   * If you want to customize your {@link DownsampleStrategy} or want to ensure that the
+   * {@link FitCenter} transformation is used, use {@link #transform(Transformation)} or
+   * {@link #downsample(DownsampleStrategy)} directly.
+   *
+   * <p>This will override previous calls to {@link #dontTransform()} and previous calls to
+   * {@link #downsample(DownsampleStrategy)}.
    *
    * @see #optionalTransform(Class, Transformation)
    * @see #fitCenter()
    */
   public RequestOptions optionalFitCenter() {
-    return optionalTransform(DownsampleStrategy.FIT_CENTER, new FitCenter());
+    return optionalScaleOnlyTransform(DownsampleStrategy.FIT_CENTER, new FitCenter());
   }
 
   /**
-   * Applies {@link FitCenter} to all default types and
-   * throws an exception if asked to transform an unknown type.
+   * Applies {@link FitCenter} and/or {@link DownsampleStrategy#FIT_CENTER} depending on the
+   * android version to all default types and throws an exception if asked to transform an unknown
+   * type.
    *
-   * <p>This will override previous calls to {@link #dontTransform()}.
+   * <p>On Android devices with KitKat or greater, this method only applies a transformation for
+   * {@link GifDrawable}s and uses {@link DownsampleStrategy#FIT_CENTER} only for {@link Bitmap}s.
+   * If you want to customize your {@link DownsampleStrategy} or want to ensure that the
+   * {@link FitCenter} transformation is used, use {@link #transform(Transformation)} or
+   * {@link #downsample(DownsampleStrategy)} directly.
+   *
+   * <p>This will override previous calls to {@link #dontTransform()} and previous calls to
+   * {@link #downsample(DownsampleStrategy)}.
    *
    * @see #transform(Class, Transformation)
    * @see #optionalFitCenter()
    */
   public RequestOptions fitCenter() {
-    return transform(DownsampleStrategy.FIT_CENTER, new FitCenter());
+    return scaleOnlyTransform(DownsampleStrategy.FIT_CENTER, new FitCenter());
   }
 
   /**
-   * Applies {@link com.bumptech.glide.load.resource.bitmap.CenterInside} to all default types, and
-   * ignores unknown types.
+   * Applies {@link com.bumptech.glide.load.resource.bitmap.CenterInside} and/or
+   * {@link DownsampleStrategy#CENTER_INSIDE} to all default types, and ignores unknown types.
    *
-   * <p>This will override previous calls to {@link #dontTransform()}.
+   * <p>On Android devices with KitKat or greater, this method only applies a transformation for
+   * {@link GifDrawable}s and uses {@link DownsampleStrategy#CENTER_INSIDE} only for
+   * {@link Bitmap}s. If you want to customize your {@link DownsampleStrategy} or want to ensure
+   * that the {@link CenterInside} transformation is used, use {@link #transform(Transformation)} or
+   * {@link #downsample(DownsampleStrategy)} directly.
+   *
+   * <p>This will override previous calls to {@link #dontTransform()} and previous calls to
+   * {@link #downsample(DownsampleStrategy)}.
    *
    * @see #optionalTransform(Class, Transformation)
    * @see #centerInside()
    */
   public RequestOptions optionalCenterInside() {
-    return optionalTransform(DownsampleStrategy.CENTER_INSIDE, new CenterInside());
+    return optionalScaleOnlyTransform(DownsampleStrategy.CENTER_INSIDE, new CenterInside());
   }
 
   /**
-   * Applies {@link CenterInside} to all default types and
-   * throws an exception if asked to transform an unknown type.
+   * Applies {@link CenterInside} and/or {@link DownsampleStrategy#CENTER_INSIDE} to all default
+   * types and throws an exception if asked to transform an unknown type.
    *
-   * <p>This will override previous calls to {@link #dontTransform()}.
+   * <p>On Android devices with KitKat or greater, this method only applies a transformation for
+   * {@link GifDrawable}s and uses {@link DownsampleStrategy#CENTER_INSIDE} only for
+   * {@link Bitmap}s. If you want to customize your {@link DownsampleStrategy} or want to ensure
+   * that the {@link CenterInside} transformation is used, use {@link #transform(Transformation)} or
+   * {@link #downsample(DownsampleStrategy)} directly.
+   *
+   * <p>This will override previous calls to {@link #dontTransform()} and previous calls to
+   * {@link #downsample(DownsampleStrategy)}.
    *
    * @see #transform(Class, Transformation)
    * @see #optionalCenterInside()
    */
   public RequestOptions centerInside() {
-    return transform(DownsampleStrategy.CENTER_INSIDE, new CenterInside());
+    return scaleOnlyTransform(DownsampleStrategy.CENTER_INSIDE, new CenterInside());
   }
 
   /**
@@ -920,6 +951,51 @@ public class RequestOptions implements Cloneable {
     return transform(transformation);
   }
 
+  /**
+   * On KitKat+ we can apply arbitrary non power of two scaling factors using Downsampler alone,
+   * eliminating the need for transformations that scale and maintain aspect ratios.
+   */
+  private static boolean isDownsampleScalingSupported() {
+    return Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT;
+  }
+
+  private RequestOptions scaleOnlyTransform(
+      DownsampleStrategy strategy, Transformation<Bitmap> transformation) {
+    return scaleOnlyTransform(strategy, transformation, true /*isTransformationRequired*/);
+  }
+
+  private RequestOptions optionalScaleOnlyTransform(
+      DownsampleStrategy strategy, Transformation<Bitmap> transformation) {
+    return scaleOnlyTransform(strategy, transformation, false /*isTransformationRequired*/);
+  }
+
+  private RequestOptions scaleOnlyTransform(
+      DownsampleStrategy strategy,
+      Transformation<Bitmap> transformation,
+      boolean isTransformationRequired) {
+    if (!isDownsampleScalingSupported()) {
+      return isTransformationRequired
+          ? transform(strategy, transformation) : optionalTransform(strategy, transformation);
+    }
+    // Avoid applying an unnecessary Bitmap Transformation if it can be done just with the
+    // downsampler.
+    RequestOptions result =
+        downsample(strategy)
+            // Clear out any previous Bitmap transformations to match the behavior of other
+            // transformation methods that do apply a Bitmap transformation when called.
+            .dontTransform();
+
+    if (isTransformationRequired) {
+      result =
+          result.transform(GifDrawable.class, new GifDrawableTransformation(transformation));
+    } else {
+      result =
+          result.optionalTransform(
+              GifDrawable.class, new GifDrawableTransformation(transformation));
+    }
+    return result;
+  }
+
   /**
    * Applies the given {@link Transformation} for
    * {@link Bitmap Bitmaps} to the default types ({@link Bitmap},
@@ -929,8 +1005,7 @@ public class RequestOptions implements Cloneable {
    *
    * <p>This will override previous calls to {@link #dontTransform()}.
    *
-   * @param transformation Any {@link Transformation} for
-   *                       {@link Bitmap}s.
+   * @param transformation Any {@link Transformation} for {@link Bitmap}s.
    * @see #optionalTransform(Transformation)
    * @see #optionalTransform(Class, Transformation)
    */
@@ -951,7 +1026,7 @@ public class RequestOptions implements Cloneable {
    * {@link android.graphics.drawable.BitmapDrawable}, and
    * {@link com.bumptech.glide.load.resource.gif.GifDrawable})
    * and throws an exception if asked to transform an unknown type.
-   * <p>
+   *
    * <p>This will override previous calls to {@link #dontTransform()}.
    *
    * @param transformations One or more {@link Transformation}s for {@link Bitmap}s.