diff --git a/dependencies.gradle b/dependencies.gradle index 591c3828d4..1d3f4612e7 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -183,7 +183,9 @@ dependencyResolutionManagement { library('androidx-test-core-ktx', 'androidx.test', 'core-ktx').versionRef('androidx-test') library('androidx-test-ext-junit', 'androidx.test.ext', 'junit').versionRef('androidx-test-ext-junit') library('androidx-test-ext-junit-ktx', 'androidx.test.ext', 'junit-ktx').versionRef('androidx-test-ext-junit') + library('androidx-test-monitor', 'androidx.test:monitor:1.6.1') library('androidx-test-orchestrator', 'androidx.test:orchestrator:1.4.1') + library('androidx-test-runner', 'androidx.test', 'runner').versionRef('androidx-test') library('espresso-core', 'androidx.test.espresso:espresso-core:3.4.0') library('mockito-core', 'org.mockito:mockito-inline:4.6.1') library('mockito-kotlin', 'org.mockito.kotlin:mockito-kotlin:4.0.0') diff --git a/glide-webp/lib/build.gradle.kts b/glide-webp/lib/build.gradle.kts index 3c660a5f70..5b4e2fe0df 100644 --- a/glide-webp/lib/build.gradle.kts +++ b/glide-webp/lib/build.gradle.kts @@ -18,6 +18,8 @@ android { arguments("-DLIBWEBP_PATH=$rootDir/libwebp") } } + + testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } externalNativeBuild { @@ -33,4 +35,11 @@ dependencies { implementation(libs.glide.glide) kapt(libs.glide.compiler) + + androidTestImplementation(testLibs.androidx.test.core) + androidTestImplementation(testLibs.androidx.test.core.ktx) + androidTestImplementation(testLibs.androidx.test.ext.junit) + androidTestImplementation(testLibs.androidx.test.ext.junit.ktx) + androidTestImplementation(testLibs.androidx.test.monitor) + androidTestImplementation(testLibs.androidx.test.runner) } diff --git a/glide-webp/lib/src/androidTest/assets/broken_10Kx10K.webp b/glide-webp/lib/src/androidTest/assets/broken_10Kx10K.webp new file mode 100644 index 0000000000..f2eab03cfa Binary files /dev/null and b/glide-webp/lib/src/androidTest/assets/broken_10Kx10K.webp differ diff --git a/glide-webp/lib/src/androidTest/assets/landscape_550x368.webp b/glide-webp/lib/src/androidTest/assets/landscape_550x368.webp new file mode 100644 index 0000000000..122741b605 Binary files /dev/null and b/glide-webp/lib/src/androidTest/assets/landscape_550x368.webp differ diff --git a/glide-webp/lib/src/androidTest/assets/solid_white_10Kx10K.webp b/glide-webp/lib/src/androidTest/assets/solid_white_10Kx10K.webp new file mode 100644 index 0000000000..f65d7cced4 Binary files /dev/null and b/glide-webp/lib/src/androidTest/assets/solid_white_10Kx10K.webp differ diff --git a/glide-webp/lib/src/androidTest/java/org/signal/glide/webp/WebpDecoderTest.kt b/glide-webp/lib/src/androidTest/java/org/signal/glide/webp/WebpDecoderTest.kt new file mode 100644 index 0000000000..0cb91cbacb --- /dev/null +++ b/glide-webp/lib/src/androidTest/java/org/signal/glide/webp/WebpDecoderTest.kt @@ -0,0 +1,102 @@ +package org.signal.glide.webp + +import android.os.Build +import android.os.Debug +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.signal.core.util.StreamUtil + +@RunWith(AndroidJUnit4::class) +class WebpDecoderTest { + private fun readAsset(name: String): ByteArray { + val source = InstrumentationRegistry.getInstrumentation().context.assets.open(name) + return StreamUtil.readFully(source) + } + + @Test + fun landscapeUnscaled() { + val input = readAsset("landscape_550x368.webp") + + val outputUnconstrained = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000)!! + assertEquals(550, outputUnconstrained.width) + assertEquals(368, outputUnconstrained.height) + + val outputSquareConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 550, 550)!! + assertEquals(550, outputSquareConstraints.width) + assertEquals(368, outputSquareConstraints.height) + + val outputExactConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 550, 368)!! + assertEquals(550, outputExactConstraints.width) + assertEquals(368, outputExactConstraints.height) + } + + @Test + fun landscapeScaledBasedOnWidth() { + val input = readAsset("landscape_550x368.webp") + + val outputUnconstrainedHeight = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 1000)!! + assertEquals(275, outputUnconstrainedHeight.width) + assertEquals(184, outputUnconstrainedHeight.height) + + val outputSquareConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 275)!! + assertEquals(275, outputSquareConstraints.width) + assertEquals(184, outputSquareConstraints.height) + + val outputSmallHeight = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 200)!! + assertEquals(275, outputSmallHeight.width) + assertEquals(184, outputSmallHeight.height) + } + + @Test + fun landscapeScaledBasedOnHeight() { + val input = readAsset("landscape_550x368.webp") + + val outputUnconstrainedWidth = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 184)!! + assertEquals(275, outputUnconstrainedWidth.width) + assertEquals(184, outputUnconstrainedWidth.height) + + val outputSmallWidth = WebpDecoder().nativeDecodeBitmapScaled(input, 300, 184)!! + assertEquals(275, outputSmallWidth.width) + assertEquals(184, outputSmallWidth.height) + } + + private fun getNativeHeapSize(): Long { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + Debug.getNativeHeapSize() + } else { + 0 + } + } + + @Test + fun canScaleMassiveImagesWithoutAllocatingLargeBuffers() { + val nativeHeapSizeAtStart = getNativeHeapSize() + + val input = readAsset("solid_white_10Kx10K.webp") + + val output = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000)!! + assertEquals(1000, output.width) + assertEquals(1000, output.height) + + var nativeHeapSizeAtEnd = getNativeHeapSize() + assertTrue("heap size: " + nativeHeapSizeAtStart + " -> " + nativeHeapSizeAtEnd, nativeHeapSizeAtStart + 10_000 * 10_000 > nativeHeapSizeAtEnd) + } + + @Test + fun canHandleMassiveBrokenImagesWithoutAllocatingLargeBuffers() { + val nativeHeapSizeAtStart = getNativeHeapSize() + + val input = readAsset("broken_10Kx10K.webp") + + val output = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000) + assertNull(output) + + var nativeHeapSizeAtEnd = getNativeHeapSize() + assertTrue("heap size: " + nativeHeapSizeAtStart + " -> " + nativeHeapSizeAtEnd, nativeHeapSizeAtStart + 10_000 * 10_000 > nativeHeapSizeAtEnd) + } +} diff --git a/glide-webp/lib/src/main/cpp/signalwebp.cpp b/glide-webp/lib/src/main/cpp/signalwebp.cpp index d3cae9ee29..dc77efd669 100644 --- a/glide-webp/lib/src/main/cpp/signalwebp.cpp +++ b/glide-webp/lib/src/main/cpp/signalwebp.cpp @@ -1,6 +1,8 @@ +#include #include #include -#include + +#include #include #define TAG "WebpResourceDecoder" @@ -16,7 +18,6 @@ jobject createBitmap(JNIEnv *env, int width, int height, const uint8_t *pixels) jobject argb8888Config = env->GetStaticObjectField(jbitmapConfigClass, jbitmapConfigARGB8888Field); jobject jbitmap = env->CallStaticObjectMethod(jbitmapClass, jbitmapCreateBitmapMethod, intArray, 0, width, width, height, argb8888Config); - env->DeleteLocalRef(argb8888Config); return jbitmap; } @@ -29,14 +30,14 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req WebPBitstreamFeatures features; if (WebPGetFeatures(buffer, bufferLength, &features) != VP8_STATUS_OK) { __android_log_write(ANDROID_LOG_WARN, TAG, "GetFeatures"); - env->ReleaseByteArrayElements(data, javaBytes, 0); + env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT); return nullptr; } WebPDecoderConfig config; if (!WebPInitDecoderConfig(&config)) { __android_log_write(ANDROID_LOG_WARN, TAG, "Init decoder config"); - env->ReleaseByteArrayElements(data, javaBytes, 0); + env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT); return nullptr; } @@ -44,17 +45,13 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req config.output.colorspace = MODE_BGRA; if (requestedWidth > 0 && requestedHeight > 0 && features.width > 0 && features.height > 0 && (requestedWidth < features.width || requestedHeight < features.height)) { - float hRatio = 1.0; - float vRatio = 1.0; - if (features.width >= features.height) { - vRatio = static_cast(features.height) / static_cast(features.width); - } else { - hRatio = static_cast(features.width) / static_cast(features.height); - } + double widthScale = static_cast(requestedWidth) / features.width; + double heightScale = static_cast(requestedHeight) / features.height; + double requiredScale = std::min(widthScale, heightScale); config.options.use_scaling = 1; - config.options.scaled_width = static_cast(static_cast(requestedWidth) * hRatio); - config.options.scaled_height = static_cast(static_cast(requestedHeight) * vRatio); + config.options.scaled_width = static_cast(requiredScale * features.width); + config.options.scaled_height = static_cast(requiredScale * features.height); } uint8_t *pixels = nullptr; @@ -63,8 +60,7 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req VP8StatusCode result = WebPDecode(buffer, bufferLength, &config); if (result != VP8_STATUS_OK) { - __android_log_write(ANDROID_LOG_WARN, TAG, ("Scaled WebPDecode failed (" + std::to_string(result) + ") Trying without scaled").c_str()); - pixels = WebPDecodeBGRA(buffer, bufferLength, &width, &height); + __android_log_write(ANDROID_LOG_WARN, TAG, ("Scaled WebPDecode failed (" + std::to_string(result) + ")").c_str()); } else { pixels = config.output.u.RGBA.rgba; width = config.output.width; @@ -77,7 +73,7 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req } WebPFree(pixels); - env->ReleaseByteArrayElements(data, javaBytes, 0); + env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT); return jbitmap; }