Bug 1403653 - Part 1 - Refactor getDominantColor. r?nechen draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Sat, 14 Oct 2017 19:23:03 +0200
changeset 684468 569449686e191f6ede4a85887a393d6cd529b3e3
parent 684467 dd0813b1e01fa253b0ce83a955f2c81cc18ac591
child 684469 49607810d82f26b580484561cc5f2d05c24e68fc
push id85620
push usermozilla@buttercookie.de
push dateSun, 22 Oct 2017 13:13:39 +0000
reviewersnechen
bugs1403653, 1318667
milestone58.0a1
Bug 1403653 - Part 1 - Refactor getDominantColor. r?nechen We want to use the Palette library for getting a fallback accent colour for lightweight themes, however because of bug 1318667, we might have to continue using our own implementation of getDominantColor on x86 devices. Therefore we move this into BitmapUtils, so we can have a central location from which to switch between our own and the Palette library implementation. MozReview-Commit-ID: 52WsfZbW12x
mobile/android/app/build.gradle
mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
mobile/android/base/java/org/mozilla/gecko/util/ShortcutUtils.java
mobile/android/base/moz.build
mobile/android/geckoview/build.gradle
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/BitmapUtils.java
mobile/android/search/java/org/mozilla/search/autocomplete/SearchBar.java
--- a/mobile/android/app/build.gradle
+++ b/mobile/android/app/build.gradle
@@ -224,17 +224,16 @@ android {
 
 dependencies {
     compile "com.android.support:support-v4:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
     compile "com.android.support:appcompat-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
     compile "com.android.support:cardview-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
     compile "com.android.support:recyclerview-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
     compile "com.android.support:design:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
     compile "com.android.support:customtabs:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
-    compile "com.android.support:palette-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
 
     if (mozconfig.substs.MOZ_NATIVE_DEVICES) {
         compile "com.android.support:mediarouter-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
         compile "com.google.android.gms:play-services-basement:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
         compile "com.google.android.gms:play-services-base:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
         compile "com.google.android.gms:play-services-cast:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
     }
 
--- a/mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/processing/ColorProcessor.java
@@ -2,23 +2,20 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.icons.processing;
 
 import android.graphics.Bitmap;
 import android.support.annotation.ColorInt;
-import android.support.v7.graphics.Palette;
-import android.util.Log;
 
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.icons.IconRequest;
 import org.mozilla.gecko.icons.IconResponse;
-import org.mozilla.gecko.util.HardwareUtils;
 
 /**
  * Processor implementation to extract the dominant color from the icon and attach it to the icon
  * response object.
  */
 public class ColorProcessor implements Processor {
     private static final String LOGTAG = "GeckoColorProcessor";
     private static final int DEFAULT_COLOR = 0xFFB1B1B3; // 0 == No color, here we use photon color
@@ -32,46 +29,18 @@ public class ColorProcessor implements P
         final Bitmap bitmap = response.getBitmap();
 
         final @ColorInt Integer edgeColor = getEdgeColor(bitmap);
         if (edgeColor != null) {
             response.updateColor(edgeColor);
             return;
         }
 
-        if (HardwareUtils.isX86System()) {
-            // (Bug 1318667) We are running into crashes when using the palette library with
-            // specific icons on x86 devices. They take down the whole VM and are not recoverable.
-            // Unfortunately our release icon is triggering this crash. Until we can switch to a
-            // newer version of the support library where this does not happen, we are using our
-            // own slower implementation.
-            extractColorUsingCustomImplementation(response);
-        } else {
-            extractColorUsingPaletteSupportLibrary(response);
-        }
-    }
-
-    private void extractColorUsingPaletteSupportLibrary(final IconResponse response) {
-        try {
-            final Palette palette = Palette.from(response.getBitmap()).generate();
-            response.updateColor(palette.getVibrantColor(DEFAULT_COLOR) & 0x7FFFFFFF);
-        } catch (ArrayIndexOutOfBoundsException e) {
-            // We saw the palette library fail with an ArrayIndexOutOfBoundsException intermittently
-            // in automation. In this case lets just swallow the exception and move on without a
-            // color. This is a valid condition and callers should handle this gracefully (Bug 1318560).
-            Log.e(LOGTAG, "Palette generation failed with ArrayIndexOutOfBoundsException", e);
-
-            response.updateColor(DEFAULT_COLOR);
-        }
-    }
-
-    private void extractColorUsingCustomImplementation(final IconResponse response) {
-        final int dominantColor = BitmapUtils.getDominantColor(response.getBitmap());
-
-        response.updateColor(dominantColor);
+        final @ColorInt int dominantColor = BitmapUtils.getDominantColor(response.getBitmap(), DEFAULT_COLOR);
+        response.updateColor(dominantColor & 0x7FFFFFFF);
     }
 
     /**
      * If a bitmap has a consistent edge colour (i.e. if all the border pixels have the same colour),
      * return that colour.
      * @param bitmap
      * @return The edge colour. null if there is no consistent edge color.
      */
--- a/mobile/android/base/java/org/mozilla/gecko/util/ShortcutUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/ShortcutUtils.java
@@ -134,17 +134,17 @@ public class ShortcutUtils {
         } else if (aSource.getWidth() >= insetSize || aSource.getHeight() >= insetSize) {
             // Otherwise, if the icon is large enough, just draw it.
             final Rect iconBounds = new Rect(0, 0, size, size);
             canvas.drawBitmap(aSource, null, iconBounds, null);
             return bitmap;
         } else {
             // Otherwise, use the dominant color from the icon +
             // a layer of transparent white to lighten it somewhat.
-            final int color = BitmapUtils.getDominantColor(aSource);
+            final int color = BitmapUtils.getDominantColorCustomImplementation(aSource);
             paint.setColor(color);
             canvas.drawRoundRect(new RectF(kOffset, kOffset, size - kOffset, size - kOffset),
                                            kRadius, kRadius, paint);
             paint.setColor(Color.argb(100, 255, 255, 255));
             canvas.drawRoundRect(new RectF(kOffset, kOffset, size - kOffset, size - kOffset),
                                  kRadius, kRadius, paint);
         }
 
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -478,16 +478,17 @@ if CONFIG['MOZ_ANDROID_HLS_SUPPORT']:
         'media/Utils.java',
     ]]
 
 
 gvjar.extra_jars += [
     CONFIG['ANDROID_SUPPORT_ANNOTATIONS_JAR_LIB'],
     CONFIG['ANDROID_SUPPORT_V4_AAR_LIB'],
     CONFIG['ANDROID_SUPPORT_V4_AAR_INTERNAL_LIB'],
+    CONFIG['ANDROID_PALETTE_V7_AAR_LIB'],
     'constants.jar',
     'gecko-mozglue.jar',
     'gecko-util.jar',
 ]
 
 gvjar.javac_flags += [
     '-Xlint:all,-deprecation,-fallthrough',
     '-J-Xmx512m',
@@ -1169,17 +1170,16 @@ if CONFIG['MOZ_INSTALL_TRACKING']:
 
 gbjar.extra_jars += [CONFIG['ANDROID_APPCOMPAT_V7_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_SUPPORT_VECTOR_DRAWABLE_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_ANIMATED_VECTOR_DRAWABLE_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_CARDVIEW_V7_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_DESIGN_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_RECYCLERVIEW_V7_AAR_LIB']]
 gbjar.extra_jars += [CONFIG['ANDROID_CUSTOMTABS_AAR_LIB']]
-gbjar.extra_jars += [CONFIG['ANDROID_PALETTE_V7_AAR_LIB']]
 
 gbjar.javac_flags += ['-Xlint:all,-deprecation,-fallthrough', '-J-Xmx512m', '-J-Xms128m']
 
 # gecko-thirdparty is a good place to put small independent libraries
 gtjar = add_java_jar('gecko-thirdparty')
 gtjar.sources += [ thirdparty_source_dir + f for f in [
     'com/booking/rtlviewpager/PagerAdapterWrapper.java',
     'com/booking/rtlviewpager/RtlViewPager.java',
--- a/mobile/android/geckoview/build.gradle
+++ b/mobile/android/geckoview/build.gradle
@@ -99,16 +99,17 @@ android {
             assets {
             }
         }
     }
 }
 
 dependencies {
     compile "com.android.support:support-v4:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
+    compile "com.android.support:palette-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
 }
 
 task syncPreprocessedCode(type: Sync, dependsOn: rootProject.generateCodeAndResources) {
     into("${project.buildDir}/generated/source/preprocessed_code")
     from("${topobjdir}/mobile/android/base/generated/preprocessed") {
         // These constants files are included in the main app project.
         exclude '**/AdjustConstants.java'
         exclude '**/MmaConstants.java'
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/BitmapUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/BitmapUtils.java
@@ -14,19 +14,23 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.graphics.Canvas;
 import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
+import android.support.annotation.ColorInt;
+import android.support.v7.graphics.Palette;
 import android.util.Base64;
 import android.util.Log;
 
+import org.mozilla.gecko.util.HardwareUtils;
+
 public final class BitmapUtils {
     private static final String LOGTAG = "GeckoBitmapUtils";
 
     private BitmapUtils() {}
 
     public static Bitmap decodeByteArray(byte[] bytes) {
         return decodeByteArray(bytes, null);
     }
@@ -130,23 +134,49 @@ public final class BitmapUtils {
         try {
             return BitmapFactory.decodeResource(resources, id, options);
         } catch (OutOfMemoryError e) {
             Log.e(LOGTAG, "decodeResource() OOM! Resource id=" + id, e);
             return null;
         }
     }
 
-    public static int getDominantColor(Bitmap source) {
-        return getDominantColor(source, true);
+    public static @ColorInt int getDominantColor(Bitmap source, @ColorInt int defaultColor) {
+        if (HardwareUtils.isX86System()) {
+            // (Bug 1318667) We are running into crashes when using the palette library with
+            // specific icons on x86 devices. They take down the whole VM and are not recoverable.
+            // Unfortunately our release icon is triggering this crash. Until we can switch to a
+            // newer version of the support library where this does not happen, we are using our
+            // own slower implementation.
+            return getDominantColorCustomImplementation(source, true, defaultColor);
+        } else {
+            try {
+                final Palette palette = Palette.from(source).generate();
+                return palette.getVibrantColor(defaultColor);
+            } catch (ArrayIndexOutOfBoundsException e) {
+                // We saw the palette library fail with an ArrayIndexOutOfBoundsException intermittently
+                // in automation. In this case lets just swallow the exception and move on without a
+                // color. This is a valid condition and callers should handle this gracefully (Bug 1318560).
+                Log.e(LOGTAG, "Palette generation failed with ArrayIndexOutOfBoundsException", e);
+
+                return defaultColor;
+            }
+        }
     }
 
-    public static int getDominantColor(Bitmap source, boolean applyThreshold) {
-      if (source == null)
-        return Color.argb(255, 255, 255, 255);
+    public static @ColorInt int getDominantColorCustomImplementation(Bitmap source) {
+        return getDominantColorCustomImplementation(source, true, Color.WHITE);
+    }
+
+    public static @ColorInt int getDominantColorCustomImplementation(Bitmap source,
+                                                                     boolean applyThreshold,
+                                                                     @ColorInt int defaultColor) {
+      if (source == null) {
+          return defaultColor;
+      }
 
       // Keep track of how many times a hue in a given bin appears in the image.
       // Hue values range [0 .. 360), so dividing by 10, we get 36 bins.
       int[] colorBins = new int[36];
 
       // The bin with the most colors. Initialize to -1 to prevent accidentally
       // thinking the first bin holds the dominant color.
       int maxBin = -1;
@@ -188,18 +218,19 @@ public final class BitmapUtils {
 
           // Keep track of the bin that holds the most colors.
           if (maxBin < 0 || colorBins[bin] > colorBins[maxBin])
             maxBin = bin;
         }
       }
 
       // maxBin may never get updated if the image holds only transparent and/or black/white pixels.
-      if (maxBin < 0)
-        return Color.argb(255, 255, 255, 255);
+      if (maxBin < 0) {
+          return defaultColor;
+      }
 
       // Return a color with the average hue/saturation/value of the bin with the most colors.
       hsv[0] = sumHue[maxBin] / colorBins[maxBin];
       hsv[1] = sumSat[maxBin] / colorBins[maxBin];
       hsv[2] = sumVal[maxBin] / colorBins[maxBin];
       return Color.HSVToColor(hsv);
     }
 
--- a/mobile/android/search/java/org/mozilla/search/autocomplete/SearchBar.java
+++ b/mobile/android/search/java/org/mozilla/search/autocomplete/SearchBar.java
@@ -134,21 +134,21 @@ public class SearchBar extends FrameLayo
     public void setEngine(SearchEngine engine) {
         final String iconURL = engine.getIconURL();
         final Bitmap bitmap = BitmapUtils.getBitmapFromDataURI(iconURL);
         final BitmapDrawable d = new BitmapDrawable(getResources(), bitmap);
         engineIcon.setImageDrawable(d);
         engineIcon.setContentDescription(engine.getName());
 
         // Update the focused background color.
-        int color = BitmapUtils.getDominantColor(bitmap);
+        int color = BitmapUtils.getDominantColorCustomImplementation(bitmap);
 
-        // BitmapUtils#getDominantColor ignores black and white pixels, but it will
-        // return white if no dominant color was found. We don't want to create a
-        // white underline for the search bar, so we default to black instead.
+        // BitmapUtils#getDominantColorCustomImplementation ignores black and white pixels,
+        // but it will return white if no dominant color was found. We don't want to create
+        // a white underline for the search bar, so we default to black instead.
         if (color == Color.WHITE) {
             color = Color.BLACK;
         }
         focusedBackground.setColorFilter(new PorterDuffColorFilter(color, PorterDuff.Mode.MULTIPLY));
 
         editText.setHint(getResources().getString(R.string.search_bar_hint, engine.getName()));
     }