From 58b1c5e8fb2b6cc62e4ac6c52e3f04dae4c1d628 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 7 Jul 2026 03:16:09 +0600 Subject: [PATCH] fix(mobile): fix 10-bit heic and avif colors on android (#29631) * fix(mobile): fix 10-bit heic and avif colors on android * fix(mobile): share the decode pool across flutter engines * vectorize conversion --------- Co-authored-by: mertalev <101130780+mertalev@users.noreply.github.com> --- .../android/app/src/main/cpp/native_image.c | 64 +++++++++++++++++++ .../kotlin/app/alextran/immich/NativeImage.kt | 10 +++ .../alextran/immich/images/LocalImagesImpl.kt | 39 +++++++++-- .../immich/images/RemoteImagesImpl.kt | 52 ++++++++++++++- .../loaders/remote_image_request.dart | 2 +- 5 files changed, 156 insertions(+), 11 deletions(-) diff --git a/mobile/android/app/src/main/cpp/native_image.c b/mobile/android/app/src/main/cpp/native_image.c index 07bb7e9a2c..3a4f2f11a4 100644 --- a/mobile/android/app/src/main/cpp/native_image.c +++ b/mobile/android/app/src/main/cpp/native_image.c @@ -107,3 +107,67 @@ Java_app_alextran_immich_NativeImage_rotate( } return (jlong) dst; } + +// Convert an RGBA_1010102 buffer to densely-packed RGBA_8888, matching Skia's +// Bitmap.copy(ARGB_8888) byte-for-byte so it's a drop-in for the intermediate 8888 bitmap. +// Each source pixel is a native (little-endian on every Android ABI) u32 with R in bits 0-9, +// G in 10-19, B in 20-29, A in 30-31 (standard RGB10_A2). Each 10-bit channel maps to 8-bit via +// round(v*255/1023). The 2-bit alpha maps to a*85. Both are kept as plain arithmetic as the whole +// loop auto-vectorizes to NEON and measures faster than a LUT on-device. Output is R,G,B,A bytes +// per pixel, i.e. Android ARGB_8888 memory == Dart PixelFormat.rgba8888. +static void convert_1010102(const uint8_t *src, int srcStride, uint32_t *dst, int w, int h) { + for (int y = 0; y < h; y++) { + const uint32_t *srcRow = (const uint32_t *) (src + (size_t) y * srcStride); + uint32_t *dstRow = dst + (size_t) y * w; + for (int x = 0; x < w; x++) { + uint32_t px = srcRow[x]; + uint32_t r = ((px & 0x3FF) * 16336u + 32768u) >> 16; + uint32_t g = (((px >> 10) & 0x3FF) * 16336u + 32768u) >> 16; + uint32_t b = (((px >> 20) & 0x3FF) * 16336u + 32768u) >> 16; + uint32_t a = ((px >> 30) & 0x3) * 85u; + dstRow[x] = r | (g << 8) | (b << 16) | (a << 24); + } + } +} + +// Converts an RGBA_1010102 bitmap (what a 10-bit HEIC/AVIF decodes to on API 33+) into a freshly +// malloc'd RGBA_8888 buffer. Fills outInfo with {width, height, rowBytes} and returns the buffer +// address, or 0 (so the caller falls back to a Skia copy) if the bitmap isn't 1010102 or can't be +// locked. Same ownership contract as rotate: free the returned buffer via NativeBuffer.free. +JNIEXPORT jlong JNICALL +Java_app_alextran_immich_NativeImage_convert1010102( + JNIEnv *env, jclass clazz, jobject bitmap, jintArray outInfo) { + AndroidBitmapInfo info; + if (AndroidBitmap_getInfo(env, bitmap, &info) != ANDROID_BITMAP_RESULT_SUCCESS) { + return 0; + } + if (info.format != ANDROID_BITMAP_FORMAT_RGBA_1010102) { + return 0; + } + + int w = (int) info.width; + int h = (int) info.height; + + uint32_t *dst = (uint32_t *) malloc((size_t) w * h * 4); + if (dst == NULL) { + return 0; + } + + void *srcPixels = NULL; + if (AndroidBitmap_lockPixels(env, bitmap, &srcPixels) != ANDROID_BITMAP_RESULT_SUCCESS) { + free(dst); + return 0; + } + + convert_1010102((const uint8_t *) srcPixels, (int) info.stride, dst, w, h); + + AndroidBitmap_unlockPixels(env, bitmap); + + jint dims[3] = {w, h, w * 4}; + (*env)->SetIntArrayRegion(env, outInfo, 0, 3, dims); + if ((*env)->ExceptionCheck(env)) { + free(dst); + return 0; + } + return (jlong) dst; +} diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt index b1398d84b8..68feaa2d14 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt @@ -16,4 +16,14 @@ object NativeImage { */ @JvmStatic external fun rotate(bitmap: Bitmap, orientation: Int, outInfo: IntArray): Long + + /** + * Converts an RGBA_1010102 [bitmap] (what a 10-bit HEIC/AVIF decodes to on API 33+) to RGBA_8888, + * writing the result into a freshly malloc'd native buffer in one pass, with no intermediate + * ARGB_8888 bitmap. Returns the buffer address (free it with [NativeBuffer.free]) and fills + * [outInfo] with {width, height, rowBytes}. Returns 0 when the bitmap isn't RGBA_1010102 so the + * caller can fall back to a Skia copy. + */ + @JvmStatic + external fun convert1010102(bitmap: Bitmap, outInfo: IntArray): Long } diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt index 6d2499ee0f..2f5db1ca0e 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt @@ -46,22 +46,47 @@ inline fun ImageDecoder.Source.decodeBitmap(target: Size = Size(0, 0)): Bitmap { } fun Bitmap.toNativeBuffer(): Map { - val size = width * height * 4 + // Dart reads the buffer as rgba8888, but 10-bit sources decode to RGBA_1010102, which garbles + // colors when copied verbatim. Convert those straight into the output buffer in native code - + // one pass, no intermediate ARGB_8888 bitmap. + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && config == Bitmap.Config.RGBA_1010102) { + val info = IntArray(3) + val pointer = NativeImage.convert1010102(this, info) + if (pointer != 0L) { + recycle() + return mapOf( + "pointer" to pointer, + "width" to info[0].toLong(), + "height" to info[1].toLong(), + "rowBytes" to info[2].toLong() + ) + } + // native convert declined (OOM/lock) -> fall through to the Skia copy path below. + } + // Other non-8888 configs (e.g. HDR F16) still need Skia's convert; 8-bit is copied as-is. + val bitmap = if (config != Bitmap.Config.ARGB_8888) { + val converted = copy(Bitmap.Config.ARGB_8888, false) + recycle() + converted ?: throw IOException("could not convert bitmap to ARGB_8888") + } else { + this + } + val size = bitmap.width * bitmap.height * 4 val pointer = NativeBuffer.allocate(size) try { val buffer = NativeBuffer.wrap(pointer, size) - copyPixelsToBuffer(buffer) + bitmap.copyPixelsToBuffer(buffer) return mapOf( "pointer" to pointer, - "width" to width.toLong(), - "height" to height.toLong(), - "rowBytes" to (width * 4).toLong() + "width" to bitmap.width.toLong(), + "height" to bitmap.height.toLong(), + "rowBytes" to (bitmap.width * 4).toLong() ) - } catch (e: Exception) { + } catch (e: Throwable) { NativeBuffer.free(pointer) throw e } finally { - recycle() + bitmap.recycle() } } diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt index f7ebc349f6..226305dd85 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt @@ -1,6 +1,8 @@ package app.alextran.immich.images import android.content.Context +import android.graphics.ImageDecoder +import android.os.Build import android.os.CancellationSignal import android.os.OperationCanceledException import app.alextran.immich.INITIAL_BUFFER_SIZE @@ -22,6 +24,7 @@ import java.io.File import java.io.IOException import java.nio.ByteBuffer import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.Executors private const val MAX_PREALLOC_BYTES = 128 * 1024 * 1024 @@ -36,12 +39,16 @@ class RemoteImagesImpl(context: Context) : RemoteImageApi { companion object { val CANCELLED = Result.success?>(null) + + // Shared, process-lifetime pool: RemoteImagesImpl is re-created per FlutterEngine, so a + // per-instance pool would leak threads across engine restarts. + private val decodeExecutor = Executors.newFixedThreadPool(2) } override fun requestImage( url: String, requestId: Long, - @Suppress("UNUSED_PARAMETER") preferEncoded: Boolean, // always returns encoded; setting has no effect on Android + preferEncoded: Boolean, callback: (Result?>) -> Unit ) { val signal = CancellationSignal() @@ -51,12 +58,51 @@ class RemoteImagesImpl(context: Context) : RemoteImageApi { url, signal, onSuccess = { buffer -> - requestMap.remove(requestId) if (signal.isCanceled) { - NativeBuffer.free(buffer.pointer) + requestMap.remove(requestId) + buffer.free() return@fetch callback(CANCELLED) } + // Decode natively when the caller wants pixels: Flutter's fallback decoder copies + // 10-bit bitmaps (RGBA_1010102) as if they were rgba8888, garbling colors. Decode on a + // dedicated pool - the fetch callback threads are shared with video streaming. On any + // decode failure (including OOM on huge originals), hand Flutter the encoded bytes as + // before. + if (!preferEncoded && Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + decodeExecutor.execute { + val res = if (signal.isCanceled) null else try { + val source = ImageDecoder.createSource(NativeBuffer.wrap(buffer.pointer, buffer.offset)) + source.decodeBitmap().toNativeBuffer() + } catch (_: Throwable) { + null + } + requestMap.remove(requestId) + when { + // Deliver even if the request was cancelled meanwhile: re-checking here would orphan + // res's malloc, and Dart frees the buffer itself when it sees the cancel. + res != null -> { + buffer.free() + callback(Result.success(res)) + } + signal.isCanceled -> { + buffer.free() + callback(CANCELLED) + } + else -> callback( + Result.success( + mapOf( + "pointer" to buffer.pointer, + "length" to buffer.offset.toLong() + ) + ) + ) + } + } + return@fetch + } + + requestMap.remove(requestId) callback( Result.success( mapOf( diff --git a/mobile/lib/infrastructure/loaders/remote_image_request.dart b/mobile/lib/infrastructure/loaders/remote_image_request.dart index da135f44d4..40705c16d2 100644 --- a/mobile/lib/infrastructure/loaders/remote_image_request.dart +++ b/mobile/lib/infrastructure/loaders/remote_image_request.dart @@ -12,7 +12,7 @@ class RemoteImageRequest extends ImageRequest { } final info = await remoteImageApi.requestImage(uri, requestId: requestId, preferEncoded: false); - // Android always returns encoded data, so we need to check for both shapes of the response. + // Android falls back to encoded data if native decoding fails, so check for both shapes of the response. final frame = switch (info) { {'pointer': int pointer, 'length': int length} => await _fromEncodedPlatformImage(pointer, length), {'pointer': int pointer, 'width': int width, 'height': int height, 'rowBytes': int rowBytes} =>