Skip to content

Commit

Permalink
Automated g4 rollback of changelist 250816270.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Rollforward with fix

*** Original change description ***

Automated g4 rollback of changelist 250798154.

*** Reason for rollback ***

Doesn't handle Build.MODEL with lengths < 7

*** Original change description ***

Disable hardware bitmaps for certain Samsung devices

***

PiperOrigin-RevId: 250943789
  • Loading branch information
@sjudd @glide-copybara-robot
sjudd authored and glide-copybara-robot committed May 31, 2019
1 parent 5e89210 commit 6d833d9
Show file tree
Hide file tree
Showing 3 changed files with 270 additions and 11 deletions.
1 change: 0 additions & 1 deletion 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 @@ -592,7 +592,6 @@ private void calculateConfig(
targetWidth,
targetHeight,
optionsWithScaling,
format,
isHardwareConfigAllowed,
isExifOrientationRequired)) {
return;
Expand Down
52 changes: 42 additions & 10 deletions library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.os.Build;
import androidx.annotation.GuardedBy;
import androidx.annotation.VisibleForTesting;
import android.util.Log;
import com.bumptech.glide.load.DecodeFormat;
import java.io.File;

/**
Expand All @@ -22,7 +23,7 @@ final class HardwareConfigState {
*
* @see #FD_SIZE_LIST
*/
private static final int MIN_HARDWARE_DIMENSION = 128;
@VisibleForTesting static final int MIN_HARDWARE_DIMENSION = 128;

/**
* Allows us to check to make sure we're not exceeding the FD limit for a process with hardware
Expand Down Expand Up @@ -53,8 +54,13 @@ final class HardwareConfigState {

private static volatile HardwareConfigState instance;

private volatile int decodesSinceLastFdCheck;
private volatile boolean isHardwareConfigAllowed = true;
private final boolean isHardwareConfigAllowedByDeviceModel;

@GuardedBy("this")
private int decodesSinceLastFdCheck;

@GuardedBy("this")
private boolean isFdSizeBelowHardwareLimit = true;

static HardwareConfigState getInstance() {
if (instance == null) {
Expand All @@ -67,20 +73,21 @@ static HardwareConfigState getInstance() {
return instance;
}

private HardwareConfigState() {
@VisibleForTesting
HardwareConfigState() {
isHardwareConfigAllowedByDeviceModel = isHardwareConfigAllowedByDeviceModel();
// Singleton constructor.
}

@TargetApi(Build.VERSION_CODES.O)
@SuppressWarnings("deprecation")
boolean setHardwareConfigIfAllowed(
int targetWidth,
int targetHeight,
BitmapFactory.Options optionsWithScaling,
DecodeFormat decodeFormat,
boolean isHardwareConfigAllowed,
boolean isExifOrientationRequired) {
if (!isHardwareConfigAllowed
|| !isHardwareConfigAllowedByDeviceModel
|| Build.VERSION.SDK_INT < Build.VERSION_CODES.O
|| isExifOrientationRequired) {
return false;
Expand All @@ -99,13 +106,38 @@ boolean setHardwareConfigIfAllowed(
return result;
}

private static boolean isHardwareConfigAllowedByDeviceModel() {
if (Build.MODEL == null || Build.MODEL.length() < 7) {
return true;
}
switch (Build.MODEL.substring(0, 7)) {
case "SM-N935":
// Fall through
case "SM-J720":
// Fall through
case "SM-G960":
// Fall through
case "SM-G965":
// Fall through
case "SM-G935":
// Fall through
case "SM-G930":
// Fall through
case "SM-A520":
// Fall through
return Build.VERSION.SDK_INT != Build.VERSION_CODES.O;
default:
return true;
}
}

private synchronized boolean isFdSizeBelowHardwareLimit() {
if (++decodesSinceLastFdCheck >= MINIMUM_DECODES_BETWEEN_FD_CHECKS) {
decodesSinceLastFdCheck = 0;
int currentFds = FD_SIZE_LIST.list().length;
isHardwareConfigAllowed = currentFds < MAXIMUM_FDS_FOR_HARDWARE_CONFIGS;
isFdSizeBelowHardwareLimit = currentFds < MAXIMUM_FDS_FOR_HARDWARE_CONFIGS;

if (!isHardwareConfigAllowed && Log.isLoggable(Downsampler.TAG, Log.WARN)) {
if (!isFdSizeBelowHardwareLimit && Log.isLoggable(Downsampler.TAG, Log.WARN)) {
Log.w(
Downsampler.TAG,
"Excluding HARDWARE bitmap config because we're over the file descriptor limit"
Expand All @@ -116,6 +148,6 @@ private synchronized boolean isFdSizeBelowHardwareLimit() {
}
}

return isHardwareConfigAllowed;
return isFdSizeBelowHardwareLimit;
}
}
228 changes: 228 additions & 0 deletions ...y/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
package com.bumptech.glide.load.resource.bitmap;

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;

import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.os.Build;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowBuild;

@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class HardwareConfigStateTest {

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void
setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsFalse() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertThat(result).isTrue();
assertThat(options.inMutable).isFalse();
assertThat(options.inPreferredConfig).isEqualTo(Bitmap.Config.HARDWARE);
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_doesNotSetValues() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION - 1,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertThat(result).isFalse();
assertThat(options.inMutable).isTrue();
assertThat(options.inPreferredConfig).isNull();
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doesNotSetValues() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION - 1,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertThat(result).isFalse();
assertThat(options.inMutable).isTrue();
assertThat(options.inPreferredConfig).isNull();
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void
setHardwareConfigIfAllowed_withHardwareConfigDisallowed_returnsFalse_doesNotSetValues() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ false,
/*isExifOrientationRequired=*/ false);

assertThat(result).isFalse();
assertThat(options.inMutable).isTrue();
assertThat(options.inPreferredConfig).isNull();
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void
setHardwareConfigIfAllowed_withExifOrientationRequired_returnsFalse_doesNotSetValues() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ true);

assertThat(result).isFalse();
assertThat(options.inMutable).isTrue();
assertThat(options.inPreferredConfig).isNull();
}

@Config(sdk = Build.VERSION_CODES.N_MR1)
@Test
public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetValues() {
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertThat(result).isFalse();
assertThat(options.inMutable).isTrue();
assertThat(options.inPreferredConfig).isNull();
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void
setHardwareConfigIfAllowed_withDisallowedSamsungDevices_returnsFalse_doesNotSetValues() {
for (String model :
new String[] {
"SM-N9351", "SM-J72053", "SM-G9600", "SM-G965ab", "SM-G935.", "SM-G930", "SM-A5204"
}) {
ShadowBuild.setModel(model);
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertWithMessage("model: " + model).that(result).isFalse();
assertWithMessage("model: " + model).that(options.inMutable).isTrue();
assertWithMessage("model: " + model).that(options.inPreferredConfig).isNull();
}
}

@Config(sdk = Build.VERSION_CODES.O_MR1)
@Test
public void setHardwareConfigIfAllowed_withDisallowedSamsungDevices_OMR1_returnsTrue() {
for (String model :
new String[] {
"SM-N9351", "SM-J72053", "SM-G9600", "SM-G965ab", "SM-G935.", "SM-G930", "SM-A5204"
}) {
ShadowBuild.setModel(model);
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertWithMessage("model: " + model).that(result).isTrue();
assertWithMessage("model: " + model).that(options.inMutable).isFalse();
assertWithMessage("model: " + model)
.that(options.inPreferredConfig)
.isEqualTo(Bitmap.Config.HARDWARE);
}
}

@Config(sdk = Build.VERSION_CODES.O)
@Test
public void setHardwareConfigIfAllowed_withShortEmptyOrNullModelNames_returnsTrue() {
for (String model :
new String[] {null, ".", "-", "", "S", "SM", "SM-", "SM-G", "SM-G9.", "SM-G93"}) {
ShadowBuild.setModel(model);
HardwareConfigState state = new HardwareConfigState();
BitmapFactory.Options options = new BitmapFactory.Options();
options.inPreferredConfig = null;
options.inMutable = true;

boolean result =
state.setHardwareConfigIfAllowed(
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION,
options,
/*isHardwareConfigAllowed=*/ true,
/*isExifOrientationRequired=*/ false);

assertWithMessage("model: " + model).that(result).isTrue();
assertWithMessage("model: " + model).that(options.inMutable).isFalse();
assertWithMessage("model: " + model)
.that(options.inPreferredConfig)
.isEqualTo(Bitmap.Config.HARDWARE);
}
}
}

0 comments on commit 6d833d9

Please sign in to comment.