Bug 1650692 - ImageDecoder improvements, r=agi,geckoview-reviewers,snorp

Differential Revision: https://phabricator.services.mozilla.com/D82308
2020-07-15 03:14:54 +00:00 · 2020-07-15 03:14:54 +00:00 · f543954b4b
--- a/mobile/android/geckoview/src/androidTest/assets/web_extensions/actions/button/red.png
+++ b/mobile/android/geckoview/src/androidTest/assets/web_extensions/actions/button/red.png
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ImageDecoderTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/ImageDecoderTest.kt
@ -0,0 +1,123 @@
+package org.mozilla.geckoview.test
+
+import android.graphics.Bitmap
+import android.graphics.BitmapFactory
+import android.graphics.Color
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.filters.MediumTest
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mozilla.geckoview.GeckoResult
+
+@MediumTest
+@RunWith(AndroidJUnit4::class)
+class ImageDecoderTest {
+
+    private fun compareBitmap(expectedLocation: String, actual: Bitmap) {
+        val stream = InstrumentationRegistry.getInstrumentation().targetContext.assets
+                .open(expectedLocation)
+
+        val expected = BitmapFactory.decodeStream(stream)
+        for (x in 0 until actual.height) {
+            for (y in 0 until actual.width) {
+                assertEquals(expected.getPixel(x, y), actual.getPixel(x, y))
+            }
+        }
+    }
+
+    @Test(expected = IllegalArgumentException::class)
+    fun decodeNullUri() {
+        ImageDecoder.instance().decode(null)
+    }
+
+    @Test
+    fun decodeIconSvg() {
+        val svgNatural = GeckoResult<Void>()
+        val svg100 = GeckoResult<Void>()
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/icon.svg").accept { actual ->
+            assertEquals(500, actual!!.width)
+            assertEquals(500, actual.height)
+            svgNatural.complete(null)
+        }
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/icon.svg", 100).accept { actual ->
+            assertEquals(100, actual!!.width)
+            assertEquals(100, actual.height)
+            compareBitmap("web_extensions/actions/button/expected.png", actual)
+            svg100.complete(null)
+        }
+
+        UiThreadUtils.waitForResult(svgNatural)
+        UiThreadUtils.waitForResult(svg100)
+    }
+
+    @Test
+    fun decodeIconPng() {
+        val pngNatural = GeckoResult<Void>()
+        val png100 = GeckoResult<Void>()
+        val png38 = GeckoResult<Void>()
+        val png19 = GeckoResult<Void>()
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/geo-19.png").accept { actual ->
+            compareBitmap("web_extensions/actions/button/geo-19.png", actual!!)
+            pngNatural.complete(null)
+        }
+
+        // Raster images shouldn't be upscaled
+        ImageDecoder.instance().decode("web_extensions/actions/button/geo-38.png", 100).accept { actual ->
+            compareBitmap("web_extensions/actions/button/geo-38.png", actual!!)
+            png100.complete(null)
+        }
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/geo-38.png", 38).accept { actual ->
+            compareBitmap("web_extensions/actions/button/geo-38.png", actual!!)
+            png38.complete(null)
+        }
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/geo-19.png", 19).accept { actual ->
+            compareBitmap("web_extensions/actions/button/geo-19.png", actual!!)
+            png19.complete(null)
+        }
+
+        UiThreadUtils.waitForResult(pngNatural)
+        UiThreadUtils.waitForResult(png100)
+        UiThreadUtils.waitForResult(png38)
+        UiThreadUtils.waitForResult(png19)
+    }
+
+    @Test
+    fun decodeIconPngDownscale() {
+        val pngNatural = GeckoResult<Void>()
+        val png16 = GeckoResult<Void>()
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/red.png").accept { actual ->
+            assertEquals(32, actual!!.width)
+            assertEquals(32, actual.height)
+
+            for (x in 0 until actual.height) {
+                for (y in 0 until actual.width) {
+                    assertEquals(Color.RED, actual.getPixel(x, y))
+                }
+            }
+            pngNatural.complete(null)
+        }
+
+        ImageDecoder.instance().decode("web_extensions/actions/button/red.png", 16).accept { actual ->
+            assertEquals(16, actual!!.width)
+            assertEquals(16, actual.height)
+
+            for (x in 0 until actual.height) {
+                for (y in 0 until actual.width) {
+                    assertEquals(Color.RED, actual.getPixel(x, y))
+                }
+            }
+            png16.complete(null)
+        }
+
+        UiThreadUtils.waitForResult(pngNatural)
+        UiThreadUtils.waitForResult(png16)
+    }
+}
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ImageDecoder.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/ImageDecoder.java
@ -16,6 +16,11 @@ import org.mozilla.gecko.annotation.WrapForJNI;

    private ImageDecoder() {}

+    /**
+     * Gets the singleton instance of this class.
+     *
+     * @return the singleton {@link ImageDecoder} instance
+     */
    public static ImageDecoder instance() {
        if (instance == null) {
            instance = new ImageDecoder();
@ -25,7 +30,7 @@ import org.mozilla.gecko.annotation.WrapForJNI;
    }

    @WrapForJNI(dispatchTo = "gecko", stubName = "Decode")
-    private static native void nativeDecode(final String uri, final int desiredLength,
+    private static native void nativeDecode(final String uri, final int maxSize,
                                            GeckoResult<Bitmap> result);

    /**
@ -55,14 +60,13 @@ import org.mozilla.gecko.annotation.WrapForJNI;
     *
     *            e.g. if the image file is locate at /assets/test.png inside the apk, set the uri
     *            to resource://android/assets/test.png.
-     * @param desiredLength Longest size for the image in device pixel units. The resulting image
+     * @param maxSize Longest size for the image in device pixel units. The resulting image
     *                might be slightly different if the image cannot be resized efficiently.
-     *                      If desiredLength is 0 then the image will be decoded to its natural
-     *                      size.
+     *                If maxSize is 0 then the image will be decoded to its natural size.
     * @return A {@link GeckoResult} to the decoded image.
     */
    @NonNull
-    public GeckoResult<Bitmap> decode(final @NonNull String uri, final int desiredLength) {
+    public GeckoResult<Bitmap> decode(final @NonNull String uri, final int maxSize) {
        if (uri == null) {
            throw new IllegalArgumentException("Uri cannot be null");
        }
@ -70,10 +74,10 @@ import org.mozilla.gecko.annotation.WrapForJNI;
        final GeckoResult<Bitmap> result = new GeckoResult<>();

        if (GeckoThread.isStateAtLeast(GeckoThread.State.PROFILE_READY)) {
-            nativeDecode(uri, desiredLength, result);
+            nativeDecode(uri, maxSize, result);
        } else {
            GeckoThread.queueNativeCallUntil(GeckoThread.State.PROFILE_READY, this,
-                    "nativeDecode", String.class, uri, int.class, desiredLength,
+                    "nativeDecode", String.class, uri, int.class, maxSize,
                    GeckoResult.class, result);
        }

--- a/widget/android/ImageDecoderSupport.cpp
+++ b/widget/android/ImageDecoderSupport.cpp
@ -47,8 +47,8 @@ class ImageCallbackHelper : public imgIContainerCallback,
    gDecodeRequests.remove(this);
  }

-  ImageCallbackHelper(java::GeckoResult::Param aResult, int32_t aDesiredLength)
-      : mResult(aResult), mDesiredLength(aDesiredLength), mImage(nullptr) {
+  ImageCallbackHelper(java::GeckoResult::Param aResult, int32_t aMaxSize)
+      : mResult(aResult), mMaxSize(aMaxSize), mImage(nullptr) {
    MOZ_ASSERT(mResult);
  }

@ -63,8 +63,9 @@ class ImageCallbackHelper : public imgIContainerCallback,
    }

    mImage = aImage;
-    return mImage->StartDecoding(
-        imgIContainer::FLAG_SYNC_DECODE | imgIContainer::FLAG_ASYNC_NOTIFY,
+    return mImage->StartDecoding(imgIContainer::FLAG_SYNC_DECODE |
+                                     imgIContainer::FLAG_ASYNC_NOTIFY |
+                                     imgIContainer::FLAG_HIGH_QUALITY_SCALING,
                                 imgIContainer::FRAME_FIRST);
  }

@ -72,11 +73,11 @@ class ImageCallbackHelper : public imgIContainerCallback,
  nsresult SendBitmap() {
    RefPtr<gfx::SourceSurface> surface;

-    if (mDesiredLength > 0) {
+    if (mMaxSize > 0) {
      surface = mImage->GetFrameAtSize(
-          gfx::IntSize(mDesiredLength, mDesiredLength),
-          imgIContainer::FRAME_FIRST,
-          imgIContainer::FLAG_SYNC_DECODE | imgIContainer::FLAG_ASYNC_NOTIFY);
+          gfx::IntSize(mMaxSize, mMaxSize), imgIContainer::FRAME_FIRST,
+          imgIContainer::FLAG_SYNC_DECODE | imgIContainer::FLAG_ASYNC_NOTIFY |
+              imgIContainer::FLAG_HIGH_QUALITY_SCALING);
    } else {
      surface = mImage->GetFrame(
          imgIContainer::FRAME_FIRST,
@ -125,7 +126,7 @@ class ImageCallbackHelper : public imgIContainerCallback,

 private:
  const java::GeckoResult::GlobalRef mResult;
-  int32_t mDesiredLength;
+  int32_t mMaxSize;
  nsCOMPtr<imgIContainer> mImage;
  virtual ~ImageCallbackHelper() {}
 };
@ -136,11 +137,11 @@ NS_IMPL_ISUPPORTS(ImageCallbackHelper, imgIContainerCallback,
 }  // namespace

 /* static */ void ImageDecoderSupport::Decode(jni::String::Param aUri,
-                                              int32_t aDesiredLength,
+                                              int32_t aMaxSize,
                                              jni::Object::Param aResult) {
  auto result = java::GeckoResult::LocalRef(aResult);
  RefPtr<ImageCallbackHelper> helper =
-      new ImageCallbackHelper(result, aDesiredLength);
+      new ImageCallbackHelper(result, aMaxSize);

  nsresult rv = DecodeInternal(aUri->ToString(), helper, helper);
  if (NS_FAILED(rv)) {
--- a/widget/android/ImageDecoderSupport.h
+++ b/widget/android/ImageDecoderSupport.h
@ -15,7 +15,7 @@ namespace widget {
 class ImageDecoderSupport final
    : public java::ImageDecoder::Natives<ImageDecoderSupport> {
 public:
-  static void Decode(jni::String::Param aUri, int32_t aDesiredLength,
+  static void Decode(jni::String::Param aUri, int32_t aMaxSize,
                     jni::Object::Param aResult);

 private: