From 26e492cc4b1ac3dd9fcea6e51c78373864fe2cb7 Mon Sep 17 00:00:00 2001 From: Jernej Virag Date: Wed, 30 Mar 2022 14:16:57 +0000 Subject: [PATCH] Fix handling of negative size request for Bitmaps in LocalImageResolver Requesting negative sized bitmap should mean "no restrictions" and not "please resize to -1x-1". This fixes the corner case for pure bitmaps. Also reduce severity of logging when we're falling back to non-ImageDecoder load since that's not a catastrophic failure. Bug:227103943 Bug:227008805 Test: tested on device with Notify1.apk while reproducing the issue atest LocalImageResolver with new tests Change-Id: I581d3e637fcafaba68856738f90cc72c42442d60 --- .../internal/widget/LocalImageResolver.java | 43 ++++++++++------ .../widget/LocalImageResolverTest.java | 50 +++++++++++++++++++ 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/core/java/com/android/internal/widget/LocalImageResolver.java b/core/java/com/android/internal/widget/LocalImageResolver.java index ce27b346567b5..c9953513a97a8 100644 --- a/core/java/com/android/internal/widget/LocalImageResolver.java +++ b/core/java/com/android/internal/widget/LocalImageResolver.java @@ -37,12 +37,18 @@ public class LocalImageResolver { private static final String TAG = "LocalImageResolver"; + /** There's no max size specified, load at original size. */ + public static final int NO_MAX_SIZE = -1; + @VisibleForTesting static final int DEFAULT_MAX_SAFE_ICON_SIZE_PX = 480; /** * Resolve an image from the given Uri using {@link ImageDecoder} if it contains a * bitmap reference. + * Negative or zero dimensions will result in icon loaded in its original size. + * + * @throws IOException if the icon could not be loaded. */ @Nullable public static Drawable resolveImage(Uri uri, Context context) throws IOException { @@ -63,8 +69,10 @@ public class LocalImageResolver { * Get the drawable from Icon using {@link ImageDecoder} if it contains a bitmap reference, or * using {@link Icon#loadDrawable(Context)} otherwise. This will correctly apply the Icon's, * tint, if present, to the drawable. + * Negative or zero dimensions will result in icon loaded in its original size. * - * @return drawable or null if loading failed. + * @return drawable or null if the passed icon parameter was null. + * @throws IOException if the icon could not be loaded. */ @Nullable public static Drawable resolveImage(@Nullable Icon icon, Context context) throws IOException { @@ -76,8 +84,10 @@ public class LocalImageResolver { * Get the drawable from Icon using {@link ImageDecoder} if it contains a bitmap reference, or * using {@link Icon#loadDrawable(Context)} otherwise. This will correctly apply the Icon's, * tint, if present, to the drawable. + * Negative or zero dimensions will result in icon loaded in its original size. * - * @throws IOException if the icon could not be loaded for whichever reason + * @return loaded icon or null if a null icon was passed as a parameter. + * @throws IOException if the icon could not be loaded. */ @Nullable public static Drawable resolveImage(@Nullable Icon icon, Context context, int maxWidth, @@ -144,19 +154,22 @@ public class LocalImageResolver { @Nullable private static Drawable resolveBitmapImage(Icon icon, Context context, int maxWidth, int maxHeight) { - Bitmap bitmap = icon.getBitmap(); - if (bitmap == null) { - return null; - } - if (bitmap.getWidth() > maxWidth || bitmap.getHeight() > maxHeight) { - Icon smallerIcon = icon.getType() == Icon.TYPE_ADAPTIVE_BITMAP - ? Icon.createWithAdaptiveBitmap(bitmap) : Icon.createWithBitmap(bitmap); - // We don't want to modify the source icon, create a copy. - smallerIcon.setTintList(icon.getTintList()) - .setTintBlendMode(icon.getTintBlendMode()) - .scaleDownIfNecessary(maxWidth, maxHeight); - return smallerIcon.loadDrawable(context); + if (maxWidth > 0 && maxHeight > 0) { + Bitmap bitmap = icon.getBitmap(); + if (bitmap == null) { + return null; + } + + if (bitmap.getWidth() > maxWidth || bitmap.getHeight() > maxHeight) { + Icon smallerIcon = icon.getType() == Icon.TYPE_ADAPTIVE_BITMAP + ? Icon.createWithAdaptiveBitmap(bitmap) : Icon.createWithBitmap(bitmap); + // We don't want to modify the source icon, create a copy. + smallerIcon.setTintList(icon.getTintList()) + .setTintBlendMode(icon.getTintBlendMode()) + .scaleDownIfNecessary(maxWidth, maxHeight); + return smallerIcon.loadDrawable(context); + } } return icon.loadDrawable(context); @@ -202,7 +215,7 @@ public class LocalImageResolver { // in some cases despite it not saying so. Rethrow it as an IOException to keep // our API contract. } catch (IOException | Resources.NotFoundException e) { - Log.e(TAG, "Failed to load image drawable", e); + Log.d(TAG, "Couldn't use ImageDecoder for drawable, falling back to non-resized load."); return null; } } diff --git a/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java b/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java index d8b37803f5685..033c3cae5beb7 100644 --- a/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java +++ b/core/tests/coretests/src/com/android/internal/widget/LocalImageResolverTest.java @@ -144,6 +144,56 @@ public class LocalImageResolverTest { assertThat(bd.getBitmap().getHeight()).isLessThan(51); } + @Test + public void resolveImage_largeResourceIcon_negativeWidth_dontResize() { + Icon icon = Icon.createWithResource(mContext, R.drawable.big_a); + Drawable d = LocalImageResolver.resolveImage(icon, mContext, LocalImageResolver.NO_MAX_SIZE, + 50); + + assertThat(d).isInstanceOf(BitmapDrawable.class); + BitmapDrawable bd = (BitmapDrawable) d; + assertThat(bd.getBitmap().getWidth()).isGreaterThan(101); + assertThat(bd.getBitmap().getHeight()).isGreaterThan(51); + } + + @Test + public void resolveImage_largeResourceIcon_negativeHeight_dontResize() { + Icon icon = Icon.createWithResource(mContext, R.drawable.big_a); + Drawable d = LocalImageResolver.resolveImage(icon, mContext, 100, + LocalImageResolver.NO_MAX_SIZE); + + assertThat(d).isInstanceOf(BitmapDrawable.class); + BitmapDrawable bd = (BitmapDrawable) d; + assertThat(bd.getBitmap().getWidth()).isGreaterThan(101); + assertThat(bd.getBitmap().getHeight()).isGreaterThan(51); + } + + @Test + public void resolveImage_largeBitmapIcon_passedNegativeWidth_dontResize() { + Icon icon = Icon.createWithBitmap( + BitmapFactory.decodeResource(mContext.getResources(), R.drawable.big_a)); + Drawable d = LocalImageResolver.resolveImage(icon, mContext, LocalImageResolver.NO_MAX_SIZE, + 50); + + assertThat(d).isInstanceOf(BitmapDrawable.class); + BitmapDrawable bd = (BitmapDrawable) d; + assertThat(bd.getBitmap().getWidth()).isGreaterThan(101); + assertThat(bd.getBitmap().getHeight()).isGreaterThan(51); + } + + @Test + public void resolveImage_largeBitmapIcon_passedNegativeHeight_dontResize() { + Icon icon = Icon.createWithBitmap( + BitmapFactory.decodeResource(mContext.getResources(), R.drawable.big_a)); + Drawable d = LocalImageResolver.resolveImage(icon, mContext, LocalImageResolver.NO_MAX_SIZE, + 50); + + assertThat(d).isInstanceOf(BitmapDrawable.class); + BitmapDrawable bd = (BitmapDrawable) d; + assertThat(bd.getBitmap().getWidth()).isGreaterThan(101); + assertThat(bd.getBitmap().getHeight()).isGreaterThan(51); + } + @Test public void resolveImage_largeBitmapIcon_passedSize_resizeToDefinedSize() { Icon icon = Icon.createWithBitmap(