From 2786cc16a161f1e5ec8bf53aaef2cb79971d5c6a Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Mon, 20 Jul 2026 20:11:39 +0600 Subject: [PATCH] fix(mobile): target minSdk for the native lib, move memory-card thumbhash to the core, add native CI --- .github/workflows/build-mobile.yml | 54 ++++++ .github/workflows/static_analysis.yml | 1 + .github/workflows/test.yml | 71 ++++++++ mise.toml | 1 + .../app/alextran/immich/NativeBuffer.kt | 9 +- .../kotlin/app/alextran/immich/NativeImage.kt | 19 +-- .../alextran/immich/images/LocalImagesImpl.kt | 37 +++-- mobile/integration_test/native_core_test.dart | 33 ++-- mobile/integration_test/native_jni_test.dart | 120 ++++++++++---- mobile/ios/Runner/Core/NativeCore.swift | 46 +++++- .../ios/Runner/Images/LocalImagesImpl.swift | 13 +- mobile/ios/fastlane/Fastfile | 4 +- mobile/lib/utils/hooks/blurhash_hook.dart | 70 +++++++- mobile/mise.toml | 5 +- mobile/pubspec.lock | 8 - mobile/pubspec.yaml | 1 - .../test/utils/hooks/blurhash_hook_test.dart | 56 +++++++ native/.gitignore | 3 - native/Cargo.toml | 18 +- native/README.md | 84 +++------- native/crates/immich_core/Cargo.toml | 4 +- native/crates/immich_core/src/image.rs | 154 ++++++++++-------- native/crates/immich_core/src/lib.rs | 7 +- native/crates/immich_core/src/thumbhash.rs | 32 +--- native/crates/immich_core_ffi/Cargo.toml | 8 +- native/crates/immich_core_ffi/build.rs | 9 +- native/crates/immich_core_ffi/cbindgen.toml | 2 +- .../immich_core_ffi/include/immich_core.h | 35 ++-- .../immich_core_ffi/rust-toolchain.toml | 13 +- .../immich_core_ffi/src/android/buffer.rs | 30 +--- .../immich_core_ffi/src/android/image.rs | 57 ++++--- .../src/android/jnigraphics.rs | 3 - .../crates/immich_core_ffi/src/android/log.rs | 3 - .../crates/immich_core_ffi/src/android/mod.rs | 17 +- .../crates/immich_core_ffi/src/capi/image.rs | 42 ++--- native/crates/immich_core_ffi/src/capi/mod.rs | 20 +-- .../immich_core_ffi/src/capi/thumbhash.rs | 31 +--- native/crates/immich_core_ffi/src/ios/log.rs | 8 +- native/crates/immich_core_ffi/src/ios/mod.rs | 6 - native/crates/immich_core_ffi/src/lib.rs | 13 +- native/crates/immich_core_ffi/src/log.rs | 6 - native/crates/immich_core_ffi/src/runtime.rs | 10 +- native/crates/immich_core_ffi/tests/c_abi.rs | 15 +- native/crates/immich_core_napi/src/lib.rs | 6 +- native/immich_native_core/.gitignore | 9 - native/immich_native_core/README.md | 44 +---- .../immich_native_core/analysis_options.yaml | 3 - native/immich_native_core/ffigen.yaml | 7 +- native/immich_native_core/hook/build.dart | 87 +++++++--- .../lib/immich_native_core.dart | 5 - .../lib/src/ffi/bindings.g.dart | 33 +--- native/immich_native_core/pubspec.yaml | 11 +- .../test/native_core_test.dart | 30 ++-- native/mise.toml | 14 +- native/scripts/build-linux.sh | 9 +- native/smoke/dart_smoke.dart | 25 ++- native/smoke/node_smoke.mjs | 1 - 57 files changed, 760 insertions(+), 702 deletions(-) create mode 100644 mobile/test/utils/hooks/blurhash_hook_test.dart diff --git a/.github/workflows/build-mobile.yml b/.github/workflows/build-mobile.yml index 42a86431db..a9b2c37e34 100644 --- a/.github/workflows/build-mobile.yml +++ b/.github/workflows/build-mobile.yml @@ -65,6 +65,7 @@ jobs: filters: | mobile: - 'mobile/**' + - 'native/**' force-filters: | - '.github/workflows/build-mobile.yml' force-events: 'workflow_call,workflow_dispatch' @@ -154,6 +155,59 @@ jobs: flutter build apk --release fi + - name: Verify native Android compatibility + run: | + apk=mobile/build/app/outputs/flutter-apk/app-release.apk + sdk=${ANDROID_SDK_ROOT:-${ANDROID_HOME:?Android SDK path is not set}} + min_sdk=$(sed -nE 's/^[[:space:]]*minSdk[[:space:]]*=[[:space:]]*([0-9]+)[[:space:]]*$/\1/p' mobile/android/app/build.gradle) + [[ $min_sdk =~ ^[0-9]+$ ]] || { printf 'Could not parse minSdk from mobile/android/app/build.gradle\n' >&2; exit 1; } + readelf=$(find "$sdk/ndk" -path '*/toolchains/llvm/prebuilt/*/bin/llvm-readelf' -print | sort -V | tail -n 1) + [[ -n $readelf ]] || { printf 'No llvm-readelf found under %s\n' "$sdk/ndk" >&2; exit 1; } + dir=$(mktemp -d) + trap 'rm -rf "$dir"' EXIT + + test -f "$apk" + [[ -x $readelf ]] || { printf 'llvm-readelf is not executable: %s\n' "$readelf" >&2; exit 1; } + + for abi in armeabi-v7a arm64-v8a x86_64; do + so="$dir/$abi.so" + unzip -p "$apk" "lib/$abi/libimmich_core_ffi.so" > "$so" + test -s "$so" + + notes=$("$readelf" -n "$so") + headers=$("$readelf" -lW "$so") + printf '%s notes:\n%s\n' "$abi" "$notes" + printf '%s LOAD headers:\n%s\n' "$abi" "$(printf '%s\n' "$headers" | awk '/^[[:space:]]*LOAD[[:space:]]/')" + + bytes=$(printf '%s\n' "$notes" | awk ' + /^[[:space:]]*Android[[:space:]]/ { android = 1; next } + android && /description data:/ { + sub(/^.*description data:[[:space:]]*/, "") + print $1, $2, $3, $4 + exit + } + ') + read -r b0 b1 b2 b3 <<< "$bytes" + for byte in "$b0" "$b1" "$b2" "$b3"; do + [[ $byte =~ ^[0-9a-fA-F]{2}$ ]] + done + api=$((16#$b0 + (16#$b1 << 8) + (16#$b2 << 16) + (16#$b3 << 24))) + printf '%s Android API: %d\n' "$abi" "$api" + if ((api > min_sdk)); then + printf '%s Android API %d exceeds minSdk %d\n' "$abi" "$api" "$min_sdk" >&2 + exit 1 + fi + + alignments=$(printf '%s\n' "$headers" | awk '/^[[:space:]]*LOAD[[:space:]]/ { print $NF }') + test -n "$alignments" + while read -r alignment; do + if [[ $alignment != 0x4000 ]]; then + printf '%s LOAD alignment %s is not 0x4000\n' "$abi" "$alignment" >&2 + exit 1 + fi + done <<< "$alignments" + done + - name: Publish Android Artifact id: upload-apk uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/.github/workflows/static_analysis.yml b/.github/workflows/static_analysis.yml index 8f39906507..746a0f9157 100644 --- a/.github/workflows/static_analysis.yml +++ b/.github/workflows/static_analysis.yml @@ -35,6 +35,7 @@ jobs: mobile: - 'mobile/**' - 'i18n/en.json' + - 'native/**' force-filters: | - '.github/workflows/static_analysis.yml' force-events: 'workflow_dispatch,release' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4fac7a152b..0ab378c9ba 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -59,6 +59,10 @@ jobs: - 'mise.toml' mobile: - 'mobile/**' + - 'native/**' + - 'mise.toml' + native: + - 'native/**' - 'mise.toml' machine-learning: - 'machine-learning/**' @@ -69,6 +73,73 @@ jobs: - '.github/workflows/test.yml' force-events: 'workflow_dispatch' + native-tests: + name: Test & Lint Native Core + needs: pre-job + if: ${{ fromJSON(needs.pre-job.outputs.should_run).native == true }} + runs-on: ubuntu-latest + permissions: + contents: read + defaults: + run: + working-directory: ./native + steps: + - id: token + uses: immich-app/devtools/actions/create-workflow-token@1af396ae134e4bc3b63d947e672bc68bf4ff9dc5 # create-workflow-token-action-v3.0.0 + with: + client-id: ${{ secrets.PUSH_O_MATIC_APP_CLIENT_ID }} + private-key: ${{ secrets.PUSH_O_MATIC_APP_KEY }} + permission-contents: read + + - name: Checkout code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + token: ${{ steps.token.outputs.token }} + + - name: Setup Mise + uses: immich-app/devtools/actions/use-mise@3bca63ca3c15020293b36b51737a3ee2c773340b # use-mise-action-v3.1.0 + with: + github_token: ${{ steps.token.outputs.token }} + working_directory: ./native + + - name: Install Flutter dependencies + working-directory: ./native/immich_native_core + run: flutter pub get + + - name: Run Dart analyze + working-directory: ./native/immich_native_core + run: dart analyze + + - name: Check formatting + run: cargo fmt --all --check + + - name: Run Clippy + run: cargo clippy --workspace --all-targets --locked -- -D warnings + + - name: Run tests + run: cargo test --workspace --locked + + - name: Generate native bindings + run: mise //native:codegen + + - name: Find generated file changes + uses: tj-actions/verify-changed-files@a1c6acee9df209257a246f2cc6ae8cb6581c1edf # v20.0.4 + id: verify-native-generated-files + with: + files: | + native/crates/immich_core_ffi/include/immich_core.h + native/immich_native_core/lib/src/ffi/bindings.g.dart + + - name: Verify generated files have not changed + if: steps.verify-native-generated-files.outputs.files_changed == 'true' + env: + CHANGED_FILES: ${{ steps.verify-native-generated-files.outputs.changed_files }} + run: | + echo "ERROR: Native generated files not up to date! Run 'mise //native:codegen'" + echo "Changed files: ${CHANGED_FILES}" + exit 1 + script-unit-tests: name: Scripts unit tests needs: pre-job diff --git a/mise.toml b/mise.toml index 2744b6605d..62d95b7266 100644 --- a/mise.toml +++ b/mise.toml @@ -12,6 +12,7 @@ config_roots = [ "docs", ".github", "machine-learning", + "native", ] [tools] diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt index 9c0f7000b4..619afefa80 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt @@ -6,7 +6,6 @@ const val INITIAL_BUFFER_SIZE = 32 * 1024 object NativeBuffer { init { - // All native code lives in the shared Rust core (built by the Flutter build hook). System.loadLibrary("immich_core_ffi") } @@ -33,8 +32,12 @@ class NativeByteBuffer(initialCapacity: Int) { inline fun ensureHeadroom() { if (offset == capacity) { - capacity *= 2 - pointer = NativeBuffer.realloc(pointer, capacity) + check(capacity <= Int.MAX_VALUE / 2) { "Native buffer capacity overflow" } + val newCapacity = capacity * 2 + val newPointer = NativeBuffer.realloc(pointer, newCapacity) + check(newPointer != 0L) { "Native buffer realloc failed" } + pointer = newPointer + capacity = newCapacity } } 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 4c56f0be5e..47896237cb 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 @@ -4,33 +4,26 @@ import android.graphics.Bitmap object NativeImage { init { - // The image functions are JNI exports of the shared Rust core. System.loadLibrary("immich_core_ffi") } /** - * Rotates an RGBA_8888 [bitmap] to the given EXIF [orientation], writing the result into a freshly - * malloc'd native buffer. Returns the buffer address (free it with [NativeBuffer.free]) and fills - * [outInfo] with {width, height, rowBytes}. Returns 0 when the bitmap can't be handled (e.g. a - * non-8888 config) so the caller can fall back. + * Rotates an RGBA_8888 [bitmap] and returns a malloc'd buffer, or 0 on failure. + * [outInfo] receives width, height, and row bytes. */ @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. + * Converts an RGBA_1010102 [bitmap] to RGBA_8888 and returns a malloc'd buffer, or 0 on failure. + * [outInfo] receives width, height, and row bytes. */ @JvmStatic external fun convert1010102(bitmap: Bitmap, outInfo: IntArray): Long /** - * Decodes a ThumbHash placeholder into a freshly malloc'd RGBA_8888 native buffer. Returns the - * buffer address (free it with [NativeBuffer.free]) and fills [outInfo] with - * {width, height, rowBytes}. Returns 0 when the hash is malformed. + * Decodes a ThumbHash into a malloc'd RGBA_8888 buffer, or 0 on failure. + * [outInfo] receives width, height, and row bytes. */ @JvmStatic external fun thumbhash(hash: ByteArray, 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 54a220a4e6..5aa64d5155 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 @@ -13,6 +13,7 @@ import android.provider.MediaStore.Video import android.util.Size import androidx.annotation.RequiresApi import androidx.exifinterface.media.ExifInterface +import app.alextran.immich.BuildConfig import app.alextran.immich.NativeBuffer import app.alextran.immich.NativeImage import kotlin.math.* @@ -49,17 +50,23 @@ fun Bitmap.toNativeBuffer(): Map { // 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 source1010102 = + Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && config == Bitmap.Config.RGBA_1010102 + if (source1010102) { 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() - ) + return buildMap { + put("pointer", pointer) + put("width", info[0].toLong()) + put("height", info[1].toLong()) + put("rowBytes", info[2].toLong()) + if (BuildConfig.DEBUG) { + put("source1010102", 1L) + put("converted1010102", 1L) + } + } } // native convert declined (OOM/lock) -> fall through to the Skia copy path below. } @@ -70,12 +77,16 @@ fun Bitmap.toNativeBuffer(): Map { try { val buffer = NativeBuffer.wrap(pointer, size) bitmap.copyPixelsToBuffer(buffer) - return mapOf( - "pointer" to pointer, - "width" to bitmap.width.toLong(), - "height" to bitmap.height.toLong(), - "rowBytes" to (bitmap.width * 4).toLong() - ) + return buildMap { + put("pointer", pointer) + put("width", bitmap.width.toLong()) + put("height", bitmap.height.toLong()) + put("rowBytes", (bitmap.width * 4).toLong()) + if (BuildConfig.DEBUG) { + put("source1010102", if (source1010102) 1L else 0L) + put("converted1010102", 0L) + } + } } catch (e: Throwable) { NativeBuffer.free(pointer) throw e diff --git a/mobile/integration_test/native_core_test.dart b/mobile/integration_test/native_core_test.dart index f69e7cafc7..95e642e8be 100644 --- a/mobile/integration_test/native_core_test.dart +++ b/mobile/integration_test/native_core_test.dart @@ -1,13 +1,3 @@ -// Plumbing check: proves immich_native_core is usable from the real immich app on -// a real device — the build hook compiled the Rust for this target, the code asset -// bundled into the app, and the @Native symbols resolve at runtime. The payloads -// run against their device-verified ground truth: the EXIF rotate ported from -// native_image.c (#29337), the 10-bit convert matching Skia's Bitmap.copy (#29631), -// and the thumbhash decode. Calls the generated bindings directly — dart is the -// test harness here; the production callers are the platform decode pipelines. -// Self-contained: does NOT boot the immich app or need a server. -// -// Run: flutter test integration_test/native_core_test.dart -d import 'dart:convert'; import 'dart:ffi'; import 'dart:typed_data'; @@ -27,8 +17,8 @@ Uint8List? _withBuffers( int dstLen, bool Function(Pointer src, int dstLen, Pointer dst) call, ) { - final srcPtr = malloc(src.length); - final dstPtr = malloc(dstLen); + final srcPtr = calloc(src.length); + final dstPtr = calloc(dstLen); try { srcPtr.asTypedList(src.length).setAll(0, src); if (!call(srcPtr, dstLen, dstPtr)) { @@ -36,15 +26,15 @@ Uint8List? _withBuffers( } return Uint8List.fromList(dstPtr.asTypedList(dstLen)); } finally { - malloc.free(srcPtr); - malloc.free(dstPtr); + calloc.free(srcPtr); + calloc.free(dstPtr); } } void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); - test('native core loads: version roundtrips through the C string contract', () { + test('loads the native core', () { final ptr = immich_core_version(); expect(ptr, isNot(equals(nullptr))); final version = ptr.cast().toDartString(); @@ -52,7 +42,7 @@ void main() { expect(version, isNotEmpty); }); - test('orientation swaps dims exactly for the 90/270/transpose family', () { + test('reports swapped orientations', () { for (final o in [5, 6, 7, 8]) { expect(immich_core_orientation_swaps_dims(o), isTrue, reason: 'o=$o'); } @@ -61,16 +51,14 @@ void main() { } }); - test('exif rotate (the #29337 algorithm) rotates 180 on device', () { - // 2x1: red, green -> 180 -> green, red + test('rotates RGBA pixels', () { final src = Uint8List.fromList([255, 0, 0, 255, 0, 255, 0, 255]); final out = _withBuffers(src, 8, (s, len, d) => immich_core_rotate_rgba8888(s, src.length, 8, 2, 1, 3, d, len)); expect(out, [0, 255, 0, 255, 255, 0, 0, 255]); }); - test('10-bit convert (the #29631 algorithm) matches Skia ground truth on device', () { - // 179->45 and 111->28 pin round(v*255/1023) over >>2 (44/27) — the exact - // discriminating values probed on this hardware against Bitmap.copy. + test('converts RGBA_1010102 pixels', () { + // 179 and 111 distinguish rounded scaling from `>> 2`. final src = Uint8List.fromList([..._px1010102(1023, 0, 0, 3), ..._px1010102(179, 111, 0, 3)]); final out = _withBuffers( src, @@ -82,7 +70,7 @@ void main() { expect(out.sublist(4, 8), [45, 28, 0, 255]); }); - test('thumbhash decodes via the core into a malloc buffer', () { + test('decodes a thumbhash', () { final hash = base64Decode('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final hashPtr = malloc(hash.length); final info = malloc(3); @@ -100,7 +88,6 @@ void main() { expect(pixels.toSet().length, greaterThan(2)); malloc.free(ptr); - // malformed hash: null return, info untouched expect(immich_core_thumbhash_decode(hashPtr, 4, info), equals(nullptr)); } finally { malloc.free(hashPtr); diff --git a/mobile/integration_test/native_jni_test.dart b/mobile/integration_test/native_jni_test.dart index 5d4d903d4d..73fdbc8c30 100644 --- a/mobile/integration_test/native_jni_test.dart +++ b/mobile/integration_test/native_jni_test.dart @@ -1,27 +1,9 @@ -// JNI-layer check: proves the Rust core's Java_app_alextran_immich_* exports work -// under the real JVM by driving the production kotlin callers through pigeon — -// getThumbhash (NativeBuffer.allocate/wrap), requestImage preferEncoded -// (allocate/wrap + ByteBuffer.put), and a full 10-bit AVIF decode -// (toNativeBuffer -> NativeImage.convert1010102). Buffers come back as raw -// addresses and are freed from dart with malloc.free — the libc-heap handoff the -// whole design depends on. Android only: iOS has no JNI layer. -// -// The fixture is a 327-byte 64x64 solid-color 10-bit AVIF (yuv420p10le, bt709, -// limited range) that decodes to ~(45, 139, 107, 255). On API 33+ it decodes to -// RGBA_1010102 and exercises the rust convert; a device that decodes 10-bit -// straight to 8888 yields the same colors, so the assertions hold either way. -// Misreading 1010102 as rgba8888 (the #24906 bug) would give R~176 and A~218 — -// far outside the tolerance. -// -// Run: flutter test integration_test/native_jni_test.dart -d -// Teardown deletes the seeded fixture, which on API 30+ shows the system -// delete-consent dialog once. All assertions complete before teardown, so the -// pass is settled by then; a headless/CI run just has to confirm the dialog, e.g. -// adb shell uiautomator dump /sdcard/ui.xml # then tap the "Allow" node bounds import 'dart:convert'; import 'dart:ffi'; import 'dart:io'; +import 'dart:typed_data'; +import 'package:device_info_plus/device_info_plus.dart'; import 'package:ffi/ffi.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -37,10 +19,36 @@ const _fixture10BitAvifB64 = 'AAAMYXYxQ4EATAAAAAATY29scm5jbHgAAQACAAEAAAAAF2lwbWEAAAAAAAAAAQABBAECgwQAAAAu' 'bWRhdAoNAAAAAq//jV86AgQCCDIVEACLggAAAAAAgAAifC/LKY1kV6Bd'; +// 16x12 linear DNG with EXIF orientation 6; the pixel strip is appended below. +const _fixtureOrientedDngHeaderB64 = + 'SUkqAAgAAAAVAAABAwABAAAAEAAAAAEBAwABAAAADAAAAAIBAwADAAAACgEAAAMBAwABAAAAAQAAAAYBAwABAAAATIgAAAoB' + 'AwABAAAAAQAAABEBBAABAAAAvAEAABIBAwABAAAABgAAABUBAwABAAAAAwAAABYBAwABAAAADAAAABcBBAABAAAAgAQAABwB' + 'AwABAAAAAQAAACkBAwACAAAAAAABAD4BBQACAAAAEAEAAD8BBQAGAAAAIAEAABLGAQAEAAAAAQQAABPGAQAEAAAAAQEAABTG' + 'AgAMAAAAUAEAACHGCgAJAAAAXAEAACjGBQADAAAApAEAAFrGAwABAAAAFQAAAAAAAAAQABAAEAA3GqAAAAAAAiuHCgAAACAA' + 'hetRAAAAgADD9agAAAAAAs3MTAAAAAABzcxMAAAAgADNzEwAAAAAAo/C9QAAAAAQSW1taWNoIFRlc3QAAQAAAAEAAAAAAAAA' + 'AQAAAAAAAAABAAAAAAAAAAEAAAABAAAAAQAAAAAAAAABAAAAAAAAAAEAAAAAAAAAAQAAAAEAAAABAAAAAQAAAAEAAAABAAAA' + 'AQAAAAEAAAABAAAA'; + +Uint8List _orientedDng() { + final header = base64Decode(_fixtureOrientedDngHeaderB64); + final pixels = ByteData(16 * 12 * 6); + for (var y = 0; y < 12; y++) { + final r = (11 - y) * 65535 ~/ 11; + final b = y * 65535 ~/ 11; + for (var x = 0; x < 16; x++) { + final offset = (y * 16 + x) * 6; + pixels.setUint16(offset, r, Endian.little); + pixels.setUint16(offset + 2, 0, Endian.little); + pixels.setUint16(offset + 4, b, Endian.little); + } + } + return Uint8List(header.length + pixels.lengthInBytes) + ..setAll(0, header) + ..setAll(header.length, pixels.buffer.asUint8List()); +} + Uint8List _read(int address, int length) => Uint8List.fromList(Pointer.fromAddress(address).asTypedList(length)); -// The kotlin side allocates with libc malloc; freeing from dart via package:ffi -// hits the same process-global libc free. This IS the production ownership flow. void _free(int address) => malloc.free(Pointer.fromAddress(address)); void main() { @@ -52,6 +60,7 @@ void main() { final api = LocalImageApi(); final fixture = base64Decode(_fixture10BitAvifB64); String? assetId; + String? orientedAssetId; setUpAll(() async { await PhotoManager.setIgnorePermissionCheck(true); @@ -60,14 +69,15 @@ void main() { }); tearDownAll(() async { - if (assetId != null) { + final ids = [assetId, orientedAssetId].whereType().toList(); + if (ids.isNotEmpty) { try { - await PhotoManager.editor.deleteWithIds([assetId!]); + await PhotoManager.editor.deleteWithIds(ids); } catch (_) {} } }); - test('thumbhash decodes through NativeBuffer allocate+wrap and dart frees it', () async { + test('thumbhash JNI roundtrip', () async { final a = await api.getThumbhash('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final b = await api.getThumbhash('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final (w, h, rowBytes) = (a['width']!, a['height']!, a['rowBytes']!); @@ -80,13 +90,11 @@ void main() { final pixelsB = _read(b['pointer']!, rowBytes * h); _free(a['pointer']!); _free(b['pointer']!); - // Deterministic math into a correctly wrapped buffer: two runs byte-identical, - // and a real image decodes to more than one color. expect(pixelsA, pixelsB); expect(pixelsA.toSet().length, greaterThan(1)); }); - test('encoded request roundtrips the file bytes through the native buffer', () async { + test('encoded image buffer roundtrip', () async { final res = await api.requestImage( assetId!, requestId: 900001, @@ -102,20 +110,18 @@ void main() { expect(bytes, fixture); }); - test('10-bit decode lands correct colors through NativeImage.convert1010102', () async { + test('10-bit decode runs NativeImage.convert1010102', () async { final Map? res; try { res = await api.requestImage( assetId!, requestId: 900002, - width: 0, // unsized -> full-res decode, the load-original path + width: 0, height: 0, isVideo: false, preferEncoded: false, ); } on PlatformException catch (e) { - // Some devices cannot decode 10-bit AVIF at all (e.g. SM-X115 throws - // "getPixels failed" before toNativeBuffer runs) — nothing to assert there. markTestSkipped('device cannot decode the 10-bit AVIF fixture: ${e.message}'); return; } @@ -127,6 +133,18 @@ void main() { final pixels = _read(res['pointer']!, rowBytes * h); _free(res['pointer']!); + final source1010102 = res['source1010102']; + expect(source1010102, isNotNull, reason: 'decode result did not report its source format'); + if (source1010102 == 0) { + markTestSkipped('device decoded the 10-bit AVIF fixture without RGBA_1010102'); + return; + } + expect(source1010102, 1); + expect( + res['converted1010102'], + 1, + reason: 'RGBA_1010102 source fell back instead of running the native conversion', + ); for (final (x, y) in [(2, 2), (32, 32), (61, 61)]) { final o = (y * w + x) * 4; expect(pixels[o], closeTo(45, 12), reason: 'R at ($x,$y)'); @@ -135,4 +153,42 @@ void main() { expect(pixels[o + 3], 255, reason: 'A at ($x,$y)'); } }); + + test('raw EXIF orientation rotates through NativeImage.rotate', () async { + final sdkInt = (await DeviceInfoPlugin().androidInfo).version.sdkInt; + if (sdkInt < 29) { + markTestSkipped('raw image rotation needs Android 10 or newer'); + return; + } + final entity = await PhotoManager.editor.saveImage(_orientedDng(), filename: 'immich_jni_orientation.dng'); + orientedAssetId = entity.id; + expect(await entity.mimeTypeAsync, anyOf('image/dng', 'image/x-adobe-dng')); + expect(entity.orientation, 90); + expect((entity.width, entity.height), (16, 12)); + + final Map? res; + try { + res = await api.requestImage( + entity.id, + requestId: 900003, + width: 0, + height: 0, + isVideo: false, + preferEncoded: false, + ); + } on PlatformException catch (e) { + markTestSkipped('device cannot decode the DNG fixture: ${e.message}'); + return; + } + expect(res, isNotNull); + final (w, h, rowBytes) = (res!['width']!, res['height']!, res['rowBytes']!); + expect((w, h, rowBytes), (12, 16, 48)); + final pixels = _read(res['pointer']!, rowBytes * h); + _free(res['pointer']!); + for (final (x, r, b) in [(0, 0, 255), (11, 255, 0)]) { + final o = 8 * rowBytes + x * 4; + expect(pixels[o], closeTo(r, 12), reason: 'R at ($x,8)'); + expect(pixels[o + 2], closeTo(b, 12), reason: 'B at ($x,8)'); + } + }); } diff --git a/mobile/ios/Runner/Core/NativeCore.swift b/mobile/ios/Runner/Core/NativeCore.swift index aafd1f8a5b..0cee33656c 100644 --- a/mobile/ios/Runner/Core/NativeCore.swift +++ b/mobile/ios/Runner/Core/NativeCore.swift @@ -1,29 +1,57 @@ import Foundation +import OSLog -// Loads the shared Rust core (immich_core_ffi) — the Swift counterpart of -// Kotlin's `System.loadLibrary`. Flutter's native-assets build embeds the -// framework in the bundle without linking Runner against it, so symbols are -// resolved at runtime. Signatures mirror native/crates/immich_core_ffi/include/immich_core.h. +// Native assets embed the framework without linking Runner, so resolve its symbol at runtime. enum NativeCore { typealias ThumbhashDecode = @convention(c) ( UnsafePointer?, UInt, UnsafeMutablePointer? ) -> UnsafeMutablePointer? static let thumbhashDecode: ThumbhashDecode? = symbol("immich_core_thumbhash_decode") + private static let logger = Logger( + subsystem: Bundle.main.bundleIdentifier ?? "app.alextran.immich", + category: "NativeCore" + ) - private static let handle: UnsafeMutableRawPointer? = { + private static let handle: UnsafeMutableRawPointer? = load() + + private static func load() -> UnsafeMutableRawPointer? { if let frameworks = Bundle.main.privateFrameworksPath { let path = "\(frameworks)/immich_core_ffi.framework/immich_core_ffi" + dlerror() if let handle = dlopen(path, RTLD_NOW) { return handle } + let error = lastError() + logger.warning("dlopen failed for \(path, privacy: .public): \(error, privacy: .public)") } - // Fall back to the process scope (dart or a test host already loaded it). - return dlopen(nil, RTLD_NOW) - }() + + dlerror() + guard let handle = dlopen(nil, RTLD_NOW) else { + let error = lastError() + logger.error("dlopen failed for process scope: \(error, privacy: .public)") + return nil + } + return handle + } private static func symbol(_ name: String) -> T? { - guard let handle, let sym = dlsym(handle, name) else { return nil } + guard let handle else { + logger.error("native core is unavailable while loading \(name, privacy: .public)") + return nil + } + + dlerror() + guard let sym = dlsym(handle, name) else { + let error = lastError() + logger.error("dlsym failed for \(name, privacy: .public): \(error, privacy: .public)") + return nil + } return unsafeBitCast(sym, to: T.self) } + + private static func lastError() -> String { + guard let error = dlerror() else { return "unknown error" } + return String(cString: error) + } } diff --git a/mobile/ios/Runner/Images/LocalImagesImpl.swift b/mobile/ios/Runner/Images/LocalImagesImpl.swift index 912f2a25d2..5436f2dbe3 100644 --- a/mobile/ios/Runner/Images/LocalImagesImpl.swift +++ b/mobile/ios/Runner/Images/LocalImagesImpl.swift @@ -38,15 +38,20 @@ class LocalImageApiImpl: LocalImageApi { func getThumbhash(thumbhash: String, completion: @escaping (Result<[String : Int64], any Error>) -> Void) { ImageProcessing.queue.addOperation { - guard let data = Data(base64Encoded: thumbhash), let decode = NativeCore.thumbhashDecode - else { return completion(.failure(PigeonError(code: "", message: "Invalid base64 string: \(thumbhash)", details: nil)))} + guard let data = Data(base64Encoded: thumbhash) else { + return completion(.failure(PigeonError(code: "invalid-base64", message: "Invalid base64 thumbhash", details: nil))) + } + guard let decode = NativeCore.thumbhashDecode else { + return completion(.failure(PigeonError(code: "native-core-unavailable", message: "Native thumbhash decoder is unavailable", details: nil))) + } var info = [UInt32](repeating: 0, count: 3) let pointer = data.withUnsafeBytes { bytes in decode(bytes.bindMemory(to: UInt8.self).baseAddress, UInt(bytes.count), &info) } - guard let pointer - else { return completion(.failure(PigeonError(code: "", message: "Invalid thumbhash: \(thumbhash)", details: nil)))} + guard let pointer else { + return completion(.failure(PigeonError(code: "invalid-thumbhash", message: "Invalid thumbhash", details: nil))) + } completion(.success([ "pointer": Int64(Int(bitPattern: pointer)), diff --git a/mobile/ios/fastlane/Fastfile b/mobile/ios/fastlane/Fastfile index 7be1dd93d4..f23f2b3e19 100644 --- a/mobile/ios/fastlane/Fastfile +++ b/mobile/ios/fastlane/Fastfile @@ -34,9 +34,7 @@ platform :ios do ) end - # Xcode script phases don't inherit MISE_TRUSTED_CONFIG_PATHS, and the mise shim - # for rustup refuses untrusted configs, which kills the native assets build hook. - # `mise trust` persists trust to disk so it survives into the xcode environment. + # Xcode build phases need mise trust saved to disk. def trust_mise_configs return unless system("command -v mise > /dev/null 2>&1") sh("mise trust ../../../mise.toml") diff --git a/mobile/lib/utils/hooks/blurhash_hook.dart b/mobile/lib/utils/hooks/blurhash_hook.dart index 534c0ad8fb..fc5feafc71 100644 --- a/mobile/lib/utils/hooks/blurhash_hook.dart +++ b/mobile/lib/utils/hooks/blurhash_hook.dart @@ -1,16 +1,76 @@ import 'dart:convert'; +import 'dart:ffi'; import 'dart:typed_data'; +import 'package:ffi/ffi.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; -import 'package:thumbhash/thumbhash.dart' as thumbhash; +import 'package:immich_native_core/immich_native_core.dart'; ObjectRef useDriftBlurHashRef(RemoteAsset? asset) { - if (asset?.thumbHash == null) { - return useRef(null); + return useRef(decodeDriftThumbHash(asset?.thumbHash)); +} + +Uint8List? decodeDriftThumbHash(String? thumbHash) { + if (thumbHash == null || thumbHash.isEmpty) { + return null; } - final rbga = thumbhash.thumbHashToRGBA(base64Decode(asset!.thumbHash!)); + final Uint8List hash; + try { + hash = base64Decode(thumbHash); + } on FormatException { + return null; + } - return useRef(thumbhash.rgbaToBmp(rbga)); + final hashPtr = malloc(hash.length); + final info = malloc(3); + try { + hashPtr.asTypedList(hash.length).setAll(0, hash); + final rgba = immich_core_thumbhash_decode(hashPtr, hash.length, info); + if (rgba == nullptr) { + return null; + } + + try { + return _rgbaToBmp(rgba, info[0], info[1], info[2]); + } finally { + malloc.free(rgba); + } + } finally { + malloc.free(hashPtr); + malloc.free(info); + } +} + +Uint8List? _rgbaToBmp(Pointer rgba, int width, int height, int stride) { + if (width <= 0 || width > 32 || height <= 0 || height > 32 || stride != width * 4) { + return null; + } + + const headerSize = 54; + final imageSize = stride * height; + final data = ByteData(headerSize + imageSize); + + data + ..setUint16(0, 0x4d42, Endian.little) + ..setUint32(2, data.lengthInBytes, Endian.little) + ..setUint32(10, headerSize, Endian.little) + ..setUint32(14, 40, Endian.little) + ..setInt32(18, width, Endian.little) + ..setInt32(22, -height, Endian.little) + ..setUint16(26, 1, Endian.little) + ..setUint16(28, 32, Endian.little) + ..setUint32(34, imageSize, Endian.little); + + final pixels = rgba.asTypedList(imageSize); + for (var src = 0, dst = headerSize; src < imageSize; src += 4, dst += 4) { + data + ..setUint8(dst, pixels[src + 2]) + ..setUint8(dst + 1, pixels[src + 1]) + ..setUint8(dst + 2, pixels[src]) + ..setUint8(dst + 3, pixels[src + 3]); + } + + return data.buffer.asUint8List(); } diff --git a/mobile/mise.toml b/mobile/mise.toml index 431fa63001..0347fb5b89 100644 --- a/mobile/mise.toml +++ b/mobile/mise.toml @@ -1,10 +1,7 @@ [tools] "aqua:flutter/flutter" = "3.44.6" java = "21.0.2" -# immich_native_core builds from source via a dart build hook that drives rustup. -# mise bootstraps rustup + this toolchain; keep in sync with the crate's rust-toolchain.toml. -# targets are preinstalled here because mise exports RUSTUP_TOOLCHAIN, which makes -# rustup ignore the crate's rust-toolchain.toml (and its target list) at build time. +# RUSTUP_TOOLCHAIN makes build hooks ignore rust-toolchain.toml targets. rust = { version = "1.92.0", targets = "armv7-linux-androideabi,aarch64-linux-android,x86_64-linux-android,aarch64-apple-ios,aarch64-apple-ios-sim,x86_64-apple-ios" } [tools."github:CQLabs/homebrew-dcm"] diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index 3a208f4fd8..26cbb89ac3 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -1755,14 +1755,6 @@ packages: url: "https://pub.dev" source: hosted version: "0.7.11" - thumbhash: - dependency: "direct main" - description: - name: thumbhash - sha256: "5f6d31c5279ca0b5caa81ec10aae8dcaab098d82cb699ea66ada4ed09c794a37" - url: "https://pub.dev" - source: hosted - version: "0.1.0+1" timezone: dependency: "direct main" description: diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 09157b752a..30d163ccc7 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -72,7 +72,6 @@ dependencies: sqlite3: ^3.3.2 sqlite_async: 0.14.2 sqlite3_connection_pool: ^0.2.6 - thumbhash: 0.1.0+1 timezone: ^0.9.4 url_launcher: ^6.3.2 uuid: ^4.5.3 diff --git a/mobile/test/utils/hooks/blurhash_hook_test.dart b/mobile/test/utils/hooks/blurhash_hook_test.dart new file mode 100644 index 0000000000..a4b307a414 --- /dev/null +++ b/mobile/test/utils/hooks/blurhash_hook_test.dart @@ -0,0 +1,56 @@ +import 'dart:typed_data'; +import 'dart:ui' as ui; + +import 'package:flutter/material.dart'; +import 'package:flutter_hooks/flutter_hooks.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/utils/hooks/blurhash_hook.dart'; + +RemoteAsset _asset(String thumbHash) => RemoteAsset( + id: '1', + name: 'test.jpg', + ownerId: '1', + checksum: 'checksum', + type: AssetType.image, + createdAt: DateTime(2024), + updatedAt: DateTime(2024), + thumbHash: thumbHash, + isEdited: false, +); + +void main() { + test('decodes the native RGBA output into a BMP', () async { + final bmp = decodeDriftThumbHash('1QcSHQRnh493V4dIh4eXh1h4kJUI'); + + expect(bmp, isNotNull); + final data = ByteData.sublistView(bmp!); + expect(data.getUint16(0, Endian.little), 0x4d42); + expect(data.getUint32(2, Endian.little), bmp.length); + expect(data.getInt32(18, Endian.little), 23); + expect(data.getInt32(22, Endian.little), -32); + expect(data.getUint16(28, Endian.little), 32); + + final codec = await ui.instantiateImageCodec(bmp); + final frame = await codec.getNextFrame(); + expect((frame.image.width, frame.image.height), (23, 32)); + frame.image.dispose(); + codec.dispose(); + }); + + testWidgets('bad hashes fall back without throwing during build', (tester) async { + for (final hash in ['not base64!', 'AQIDBA==']) { + await tester.pumpWidget( + MaterialApp( + home: HookBuilder( + key: UniqueKey(), + builder: (context) => Text(useDriftBlurHashRef(_asset(hash)).value == null ? 'fallback' : 'decoded'), + ), + ), + ); + + expect(find.text('fallback'), findsOneWidget); + expect(tester.takeException(), isNull); + } + }); +} diff --git a/native/.gitignore b/native/.gitignore index 4ae97b8ece..3c9dd55cff 100644 --- a/native/.gitignore +++ b/native/.gitignore @@ -1,5 +1,2 @@ /target smoke/*.node -# generated + committed (regen via `mise run codegen`): -# crates/immich_core_ffi/include/immich_core.h (cbindgen) -# immich_native_core/lib/src/ffi/bindings.g.dart (ffigen) diff --git a/native/Cargo.toml b/native/Cargo.toml index 0d749cd3c0..f4e2759be1 100644 --- a/native/Cargo.toml +++ b/native/Cargo.toml @@ -6,23 +6,11 @@ members = [ "crates/immich_core_napi", ] -# shared logic lives in immich_core (no binding deps). each binding crate is a -# thin wrapper that picks its own crate-type: immich_core_ffi -> cdylib/staticlib, -# the C ABI consumed by dart (ffigen), swift (C interop) and kotlin (JNI shim); -# immich_core_napi -> cdylib (.node) for the node server. -# capabilities (image, ...) are cargo features on immich_core so both -# bindings opt into the same set. crate-type can't be feature-gated, which is why -# the bindings are separate crates rather than one crate with feature flags. - [workspace.package] version = "0.1.0" edition = "2021" license = "AGPL-3.0-only" -# single source of truth for all external dep versions. inner crates reference -# these with `{ workspace = true }` and never hardcode a version. -# default-features = false MUST live here (workspace level) — cargo ignores it if -# set only on the inner crate. inner crates then add the minimal features they need. [workspace.dependencies] tokio = { version = "1", default-features = false, features = [ "rt-multi-thread", @@ -35,14 +23,10 @@ napi-derive = "3" napi-build = "2" cbindgen = { version = "0.29", default-features = false } -# enforced by `mise run lint` (clippy -D warnings); the boundary crate also -# #![deny]s unwrap/expect. [workspace.lints.clippy] undocumented_unsafe_blocks = "deny" -# NB: no `panic = "abort"` — the FFI boundary relies on catch_unwind, which is a -# no-op under abort. default unwind is what lets a boundary panic become a null -# return instead of taking down the host (Flutter app / node server). +# FFI panic guards require the default unwind strategy. [profile.release] opt-level = 3 lto = true diff --git a/native/README.md b/native/README.md index bb8492a4fe..1b505a4af3 100644 --- a/native/README.md +++ b/native/README.md @@ -1,73 +1,29 @@ -# immich_native_core +# Native core -Shared Rust core for the pixel work the platforms can't do without native code, -built from source into the app via Flutter native assets. It replaces the android -C layer (`native_buffer.c`/`native_image.c`) and the per-platform thumbhash ports -(`ThumbHash.java`/`Thumbhash.swift`), so the same logic exists once, tested: -- **EXIF-orientation rotate** — from `native_image.c` (#29337; fixes #24796, - sideways RAW photos). Byte-for-byte the same affine + tiled copy, plus bounds - checks the raw C can't have. -- **RGBA_1010102 → RGBA8888 convert** — the 10-bit HEIC/AVIF color fix (#29631, - fixes #24906). Same `round(v*255/1023)` LUT + packing as the C, proven on-device - against Skia's `Bitmap.copy(ARGB_8888)`. -- **ThumbHash decode** — one bounds-checked implementation instead of the two - platform ports, which disagreed on rounding and crashed on truncated hashes. - Both platforms now render identical placeholders. - -The image ops fill a caller-owned output buffer (no allocation at the boundary) -because the production callers hold JNI-locked bitmaps; the thumbhash decode -returns a libc allocation the consumer frees with plain `free`. +Shared Rust code used by the Immich mobile app. Flutter builds it from source +through the `immich_native_core` native-assets hook. ## Layout ``` crates/ - immich_core pure logic, no binding deps. capabilities = cargo features - (image, thumbhash). - immich_core_ffi the hand-written C ABI + cbindgen header — consumed by dart - (ffigen), swift (C interop) and kotlin (JNI shims in src/android/) - immich_core_napi cdylib (.node) via napi-rs (server, unwired) -immich_native_core/ the Flutter package mobile depends on. build hook + ffigen @Native bindings. -smoke/ host dart + node roundtrip scripts (no device) -``` -Bindings are separate crates (Cargo can't gate `crate-type` by feature). - -## How the native lib is built (Flutter native assets — no prebuilt, no CI) -`immich_native_core/hook/build.dart` (`native_toolchain_rust`) compiles -`crates/immich_core_ffi` **from source on every app build** via rustup and bundles -it as a Flutter *code asset*. The Dart side uses ffigen `@Native` externals bound to -that asset — no `DynamicLibrary`, no prebuilt artifacts, no fetch/publish/separate-repo. - -Native assets is on by default on Flutter stable (3.38+), so a stock `flutter build` -runs the hook. Each builder needs **rustup** (the hook auto-installs the pinned -toolchain + targets from `crates/immich_core_ffi/rust-toolchain.toml`). - -## Dev commands (mise) -``` -mise run build cargo build --workspace -mise run test cargo test --workspace (host Rust tests, incl. FFI-boundary) -mise run lint clippy -D warnings (fmt: mise run fmt) -mise run codegen regen cbindgen header + ffigen @Native bindings — commit the result -mise run test:flutter HOST FFI roundtrip through the real build hook (no device) -mise run smoke Rust tests + host dart:ffi + host napi roundtrips + immich_core shared image and thumbhash code + immich_core_ffi C ABI and Android JNI exports + immich_core_napi Node binding +immich_native_core/ Flutter package and build hook +smoke/ host Dart and Node checks ``` -## Add a capability (end to end) -1. add the logic to `crates/immich_core` (behind a cargo feature if it pulls a dep). -2. expose a C entry in `crates/immich_core_ffi/src/capi/` — `#[no_mangle] pub extern "C"`, - wrap the body in `guard(...)`/`catch_unwind` (panic at the boundary → sentinel, never - unwind into the host), validate pointers, and either fill a caller-owned buffer or - return Rust-owned memory freed via `immich_core_free_string`. -3. `mise run codegen` — regenerates the committed cbindgen header + ffigen `@Native` bindings. -4. `mise run test:flutter` (host) + a case in `immich_native_core/test/` and in - `mobile/integration_test/native_core_test.dart` (device). The dart surface is the - generated bindings; add a hand-written dart wrapper only when a dart feature consumes it. -5. platform callers: kotlin via the existing JNI shim pattern (`NativeImage.kt`), swift - reads the same header natively. +## Commands -## Consume from immich/mobile -`immich_native_core: { path: ../native/immich_native_core }` in `mobile/pubspec.yaml`, -then `dart pub get`. No app-level Gradle/Podfile edits — the hook builds + bundles the -lib. Builders need rustup. See the package README for the iOS App-Extension caveat. +Building the mobile app requires `rustup`. The toolchain and targets are pinned +in `crates/immich_core_ffi/rust-toolchain.toml`. -`/native/` is codeowned by @santoshakil + @mertalev. License: reuses the immich -repo-root AGPL-3.0 (no separate license file). +``` +mise run build +mise run test +mise run lint +mise run fmt +mise run codegen +mise run test:flutter +mise run smoke +``` diff --git a/native/crates/immich_core/Cargo.toml b/native/crates/immich_core/Cargo.toml index 3ff36def53..e6516e06ba 100644 --- a/native/crates/immich_core/Cargo.toml +++ b/native/crates/immich_core/Cargo.toml @@ -6,8 +6,8 @@ license.workspace = true [features] default = ["image", "thumbhash"] -image = [] # pure pixel math, no deps -thumbhash = [] # placeholder decode, no deps +image = [] +thumbhash = [] [lints] workspace = true diff --git a/native/crates/immich_core/src/image.rs b/native/crates/immich_core/src/image.rs index 590524dc08..218de494e3 100644 --- a/native/crates/immich_core/src/image.rs +++ b/native/crates/immich_core/src/image.rs @@ -1,10 +1,6 @@ -//! EXIF-orientation rotation of RGBA8888 pixel buffers, ported from the Android -//! native_image.c (immich PR #29337). Lives here so the perf-critical pixel math -//! exists once, tested, callable from any platform's decode pipeline (Android RAW -//! today; the algorithm is platform-agnostic). The platform side keeps the bitmap -//! lock + output allocation and calls this to fill the destination buffer. +//! RGBA8888 EXIF rotation and RGBA_1010102 conversion. -// EXIF orientation values (androidx ExifInterface.ORIENTATION_*). +// androidx ExifInterface orientation values. const FLIP_HORIZONTAL: i32 = 2; const ROTATE_180: i32 = 3; const FLIP_VERTICAL: i32 = 4; @@ -13,17 +9,14 @@ const ROTATE_90: i32 = 6; const TRANSVERSE: i32 = 7; const ROTATE_270: i32 = 8; -// 32x32 u32 tile = 4KB, L1-resident so a 90/270 transpose's scattered writes stay hot. +// Keep transpose writes inside a 4 KB tile. const TILE: usize = 32; -/// Whether the orientation swaps width and height (the 90/270 + transpose family). pub fn swaps_dims(orientation: i32) -> bool { matches!(orientation, ROTATE_90 | ROTATE_270 | TRANSPOSE | TRANSVERSE) } -// (base, step_x, step_y): src pixel (sx,sy) maps to dst pixel index -// base + sx*step_x + sy*step_y for a destination of width `dw`. Mirrors -// native_image.c affine_for byte-for-byte. i64 so the math stays correct on 32-bit. +// src(sx, sy) maps to base + sx*step_x + sy*step_y in dst. fn affine_for(o: i32, sw: i64, sh: i64, dw: i64) -> (i64, i64, i64) { match o { ROTATE_90 => (sh - 1, dw, -1), @@ -37,12 +30,8 @@ fn affine_for(o: i32, sw: i64, sh: i64, dw: i64) -> (i64, i64, i64) { } } -/// Rotate `src` (RGBA8888, `sh` rows of `src_stride` bytes, `sw` pixels per row) into -/// `dst` (densely packed, `dw*dh*4` bytes) for the given EXIF orientation, where -/// (dw,dh) swap for the 90/270/transpose family. Returns `false` without touching -/// out-of-range memory if the sizes are inconsistent, so the caller can fall back. -/// Indexing is bounds-checked: a bad input fails safe (panic caught at the FFI -/// boundary / false here), never an out-of-bounds write like the raw C. +/// Rotates RGBA8888 pixels for an EXIF orientation. +/// Returns false when the dimensions or buffers are invalid. pub fn rotate_rgba8888( src: &[u8], src_stride: usize, @@ -51,12 +40,21 @@ pub fn rotate_rgba8888( orientation: i32, dst: &mut [u8], ) -> bool { - if sw == 0 || sh == 0 || src_stride < sw * 4 { + let Some(src_row_len) = sw.checked_mul(4) else { + return false; + }; + if sw == 0 || sh == 0 || src_stride < src_row_len { return false; } let dw = if swaps_dims(orientation) { sh } else { sw }; let dh = if swaps_dims(orientation) { sw } else { sh }; - if src.len() < src_stride * sh || dst.len() < dw * dh * 4 { + let Some(src_len) = src_stride.checked_mul(sh) else { + return false; + }; + let Some(dst_len) = dw.checked_mul(dh).and_then(|len| len.checked_mul(4)) else { + return false; + }; + if src.len() < src_len || dst.len() < dst_len { return false; } let (base, step_x, step_y) = affine_for(orientation, sw as i64, sh as i64, dw as i64); @@ -79,29 +77,13 @@ pub fn rotate_rgba8888( true } -// 10-bit -> 8-bit, matching Skia's Bitmap.copy(ARGB_8888): round(v * 255 / 1023). -// Compile-time LUT so it's one lookup per channel, not a mul+div per pixel. The -// integer form equals round-half-up for all 1024 inputs and v*255/1023 never lands -// on x.5, so it's exact for every value (verified on-device against Skia). -const SCALE10: [u8; 1024] = { - let mut lut = [0u8; 1024]; - let mut v = 0usize; - while v < 1024 { - lut[v] = ((v as u32 * 255 + 511) / 1023) as u8; - v += 1; - } - lut -}; +#[inline] +fn scale10(v: u32) -> u32 { + (v * 16336 + 32768) >> 16 +} -// 2-bit alpha -> 8-bit (a * 85). Photos decode opaque (a == 3 -> 255). -const ALPHA2: [u8; 4] = [0, 85, 170, 255]; - -/// Convert an Android RGBA_1010102 buffer (what a 10-bit HEIC/AVIF decodes to on -/// API 33+) to RGBA8888, byte-for-byte with Skia's `Bitmap.copy(ARGB_8888)`. Each -/// src pixel is a little-endian u32 with R in bits 0-9, G in 10-19, B in 20-29, -/// A in 30-31 (standard RGB10_A2 packing). `src` is `h` rows of `src_stride` bytes, -/// `dst` the caller's densely packed `w*h*4`. Returns false on inconsistent sizes -/// so the caller can fall back — same contract as [`rotate_rgba8888`], no alloc. +/// Converts little-endian RGBA_1010102 pixels to RGBA8888. +/// Returns false when the dimensions or buffers are invalid. pub fn rgba1010102_to_rgba8888( src: &[u8], src_stride: usize, @@ -109,24 +91,36 @@ pub fn rgba1010102_to_rgba8888( h: usize, dst: &mut [u8], ) -> bool { - if w == 0 || h == 0 || src_stride < w * 4 { + let Some(row_len) = w.checked_mul(4) else { + return false; + }; + if w == 0 || h == 0 || src_stride < row_len { return false; } - if src.len() < src_stride * h || dst.len() < w * h * 4 { + let Some(src_len) = src_stride.checked_mul(h) else { + return false; + }; + let Some(dst_len) = w.checked_mul(h).and_then(|len| len.checked_mul(4)) else { + return false; + }; + if src.len() < src_len || dst.len() < dst_len { return false; } + // Plain arithmetic auto-vectorizes to NEON and measured faster than a LUT on-device (#29631). for y in 0..h { let s_row = y * src_stride; let d_row = y * w * 4; for x in 0..w { let s = s_row + x * 4; - // explicit little-endian — dodges an unaligned *const u32 read on non-x86 + // Avoid unaligned u32 reads. let px = u32::from_le_bytes([src[s], src[s + 1], src[s + 2], src[s + 3]]); let d = d_row + x * 4; - dst[d] = SCALE10[(px & 0x3FF) as usize]; - dst[d + 1] = SCALE10[((px >> 10) & 0x3FF) as usize]; - dst[d + 2] = SCALE10[((px >> 20) & 0x3FF) as usize]; - dst[d + 3] = ALPHA2[((px >> 30) & 0x3) as usize]; + let r = scale10(px & 0x3FF); + let g = scale10((px >> 10) & 0x3FF); + let b = scale10((px >> 20) & 0x3FF); + let a = ((px >> 30) & 0x3) * 85; + let rgba = r | (g << 8) | (b << 16) | (a << 24); + dst[d..d + 4].copy_from_slice(&rgba.to_le_bytes()); } } true @@ -136,8 +130,7 @@ pub fn rgba1010102_to_rgba8888( mod tests { use super::*; - // Independent textbook EXIF transform: src(sx,sy) -> dst(dx,dy). Verifies the - // affine port against orientation *semantics*, not against itself. + // Independent EXIF mapping for the affine tests. fn ref_dst_xy(o: i32, sx: usize, sy: usize, sw: usize, sh: usize) -> (usize, usize) { match o { FLIP_HORIZONTAL => (sw - 1 - sx, sy), @@ -203,7 +196,7 @@ mod tests { #[test] fn identity_for_normal_orientation() { - let src: Vec = (0..24u8).collect(); // 2x3 RGBA + let src: Vec = (0..24u8).collect(); let mut dst = vec![0u8; 24]; assert!(rotate_rgba8888(&src, 8, 2, 3, 1, &mut dst)); assert_eq!(src, dst); @@ -211,7 +204,7 @@ mod tests { #[test] fn respects_src_stride_padding() { - let (sw, sh, stride) = (2usize, 2usize, 12usize); // 4 bytes row padding + let (sw, sh, stride) = (2usize, 2usize, 12usize); let mut src = vec![0u8; stride * sh]; for sy in 0..sh { for sx in 0..sw { @@ -222,7 +215,7 @@ mod tests { let mut dst = vec![0u8; sw * sh * 4]; assert!(rotate_rgba8888(&src, stride, sw, sh, ROTATE_180, &mut dst)); for i in 0..4 { - assert_eq!(&dst[i * 4..i * 4 + 4], &pixel(3 - i)); // 180: i -> N-1-i + assert_eq!(&dst[i * 4..i * 4 + 4], &pixel(3 - i)); } } @@ -230,11 +223,27 @@ mod tests { fn rejects_bad_sizes() { let src = vec![0u8; 16]; let mut small = vec![0u8; 4]; - assert!(!rotate_rgba8888(&src, 8, 2, 2, ROTATE_90, &mut small)); // dst too small - assert!(!rotate_rgba8888(&src, 4, 2, 2, 1, &mut small)); // stride < sw*4 + assert!(!rotate_rgba8888(&src, 8, 2, 2, ROTATE_90, &mut small)); + assert!(!rotate_rgba8888(&src, 4, 2, 2, 1, &mut small)); + assert!(!rotate_rgba8888( + &src, + usize::MAX, + usize::MAX, + 1, + 1, + &mut small + )); + assert!(!rotate_rgba8888(&src, usize::MAX, 1, 2, 1, &mut small)); } - // On-device (Pixel 9a) vs Skia's Bitmap.copy(ARGB_8888). 179/111 rule out `>> 2`. + #[test] + fn scale10_matches_exact_mapping() { + for v in 0..1024 { + assert_eq!(scale10(v), (v * 255 + 511) / 1023, "v={v}"); + } + } + + // 179 and 111 distinguish rounded scaling from `>> 2`. #[test] fn scale10_matches_skia() { for (v, want) in [ @@ -245,29 +254,30 @@ mod tests { (304, 76), (1023, 255), ] { - assert_eq!(SCALE10[v], want, "SCALE10[{v}]"); + assert_eq!(scale10(v), want, "v={v}"); + } + for (a, want) in [0, 85, 170, 255].into_iter().enumerate() { + assert_eq!(a as u32 * 85, want); } - assert_eq!(ALPHA2, [0, 85, 170, 255]); } - // px = little-endian u32; R low 10 bits, A top 2. #[test] fn convert_packing() { - let red = 0x0000_03FFu32.to_le_bytes(); // R=1023, rest 0 - let alpha = 0xC000_0000u32.to_le_bytes(); // A=3, rest 0 + let red = 0x0000_03FFu32.to_le_bytes(); + let alpha = 0xC000_0000u32.to_le_bytes(); let mut src = Vec::new(); src.extend_from_slice(&red); src.extend_from_slice(&alpha); let mut dst = vec![0u8; 8]; assert!(rgba1010102_to_rgba8888(&src, 8, 2, 1, &mut dst)); - assert_eq!(&dst[0..4], &[255, 0, 0, 0]); // opaque-less red - assert_eq!(&dst[4..8], &[0, 0, 0, 255]); // opaque black + assert_eq!(&dst[0..4], &[255, 0, 0, 0]); + assert_eq!(&dst[4..8], &[0, 0, 0, 255]); } #[test] fn convert_respects_src_stride_padding() { - let (w, h, stride) = (2usize, 2usize, 12usize); // 4 bytes row padding - let px = |v: u32| (v | 0xC000_0000).to_le_bytes(); // R=v, opaque + let (w, h, stride) = (2usize, 2usize, 12usize); + let px = |v: u32| (v | 0xC000_0000).to_le_bytes(); let mut src = vec![0u8; stride * h]; for (i, v) in [0u32, 179, 111, 1023].iter().enumerate() { let (x, y) = (i % w, i / w); @@ -285,8 +295,16 @@ mod tests { fn convert_rejects_bad_sizes() { let src = vec![0u8; 16]; let mut small = vec![0u8; 4]; - assert!(!rgba1010102_to_rgba8888(&src, 0, 0, 0, &mut small)); // zero dims - assert!(!rgba1010102_to_rgba8888(&src, 4, 2, 2, &mut small)); // stride < w*4 - assert!(!rgba1010102_to_rgba8888(&src, 8, 2, 2, &mut small)); // dst too small + assert!(!rgba1010102_to_rgba8888(&src, 0, 0, 0, &mut small)); + assert!(!rgba1010102_to_rgba8888(&src, 4, 2, 2, &mut small)); + assert!(!rgba1010102_to_rgba8888(&src, 8, 2, 2, &mut small)); + assert!(!rgba1010102_to_rgba8888( + &src, + usize::MAX, + usize::MAX, + 1, + &mut small, + )); + assert!(!rgba1010102_to_rgba8888(&src, usize::MAX, 1, 2, &mut small)); } } diff --git a/native/crates/immich_core/src/lib.rs b/native/crates/immich_core/src/lib.rs index c9dcc83107..102428fe1f 100644 --- a/native/crates/immich_core/src/lib.rs +++ b/native/crates/immich_core/src/lib.rs @@ -1,8 +1,4 @@ -//! immich_core — shared Rust core for the immich server (napi) and mobile (dart:ffi). -//! -//! Pure logic only: no binding or platform deps live here. Each binding crate -//! (`immich_core_ffi`, `immich_core_napi`) is a thin wrapper. Capabilities are -//! cargo features (`image`, ...) so every binding opts into the same set. +//! Shared native logic for Immich. #[cfg(feature = "image")] pub mod image; @@ -10,7 +6,6 @@ pub mod image; #[cfg(feature = "thumbhash")] pub mod thumbhash; -/// Version of the native core. Smoke-test entrypoint exercised by every binding. pub fn core_version() -> &'static str { env!("CARGO_PKG_VERSION") } diff --git a/native/crates/immich_core/src/thumbhash.rs b/native/crates/immich_core/src/thumbhash.rs index 6160a085e5..21f379497d 100644 --- a/native/crates/immich_core/src/thumbhash.rs +++ b/native/crates/immich_core/src/thumbhash.rs @@ -1,8 +1,4 @@ -//! ThumbHash placeholder decoding (https://evanw.me/blog/thumbhash), replacing the -//! per-platform ports (ThumbHash.java, Thumbhash.swift) with one implementation. -//! Those two disagreed on output rounding (java rounds, swift truncates) and -//! crashed on truncated hashes; this one is bounds-checked and rounds like the -//! java/android behavior, so both platforms now render identical placeholders. +//! ThumbHash placeholder decoding. // // Ported from Evan Wallace's reference implementation: // Copyright (c) 2023 Evan Wallace @@ -27,7 +23,6 @@ struct Header { h: usize, } -// Coefficient count for one channel — mirrors the reference iteration order. fn ac_len(nx: usize, ny: usize) -> usize { let mut n = 0; for cy in 0..ny { @@ -40,9 +35,6 @@ fn ac_len(nx: usize, ny: usize) -> usize { n } -// Parses and validates the header, including that every AC nibble the channels -// will read actually exists — a truncated hash decodes to None instead of the -// out-of-bounds crash of the old java/swift ports. fn parse(hash: &[u8]) -> Option
{ if hash.len() < 5 { return None; @@ -70,8 +62,7 @@ fn parse(hash: &[u8]) -> Option
{ } else { 7 }; - // The rendered size comes from the *unclamped* aspect ratio (reference quirk); - // a zero component would make a zero-sized image, so reject it as malformed. + // The format computes size before clamping the component counts. if lx_raw == 0 || ly_raw == 0 { return None; } @@ -113,8 +104,6 @@ fn parse(hash: &[u8]) -> Option
{ }) } -// Reads one channel's coefficients from the shared nibble stream -// (boost saturation by 1.25x for P/Q is applied by the caller via `scale`). fn decode_channel( hash: &[u8], ac_start: usize, @@ -137,15 +126,12 @@ fn decode_channel( ac } -/// Decoded placeholder size for `hash` — `None` if the hash is malformed. +/// Returns the decoded size, or None for a malformed hash. pub fn dims(hash: &[u8]) -> Option<(u32, u32)> { parse(hash).map(|hdr| (hdr.w as u32, hdr.h as u32)) } -/// Render `hash` as RGBA8888 (not premultiplied) into the caller's `dst`, which -/// must hold at least `w*h*4` bytes for the size reported by [`dims`]. Returns -/// false without touching `dst` on a malformed hash or short buffer — same -/// caller-owns-dst contract as the `image` module. +/// Decodes a hash into a caller-owned RGBA8888 buffer. pub fn to_rgba(hash: &[u8], dst: &mut [u8]) -> bool { let Some(hdr) = parse(hash) else { return false; @@ -184,7 +170,6 @@ pub fn to_rgba(hash: &[u8], dst: &mut [u8]) -> bool { let mut q = hdr.q_dc; let mut a = hdr.a_dc; - // DCT coefficients in f64 then narrowed, like the android port. for (cx, f) in fx.iter_mut().enumerate() { *f = (std::f64::consts::PI / hdr.w as f64 * (x as f64 + 0.5) * cx as f64).cos() as f32; @@ -250,9 +235,7 @@ fn to_u8(v: f32) -> u8 { mod tests { use super::*; - // Ground truth captured from the shipping Thumbhash.swift on 2026-07-10 (see - // the PR notes): swift truncates the final float->u8 step while this port - // rounds like the java one, so vectors match within 1 per channel. + // Captured from the old Swift decoder; channel values may differ by one. const OPAQUE_B64: &str = "1QcSHQRnh493V4dIh4eXh1h4kJUI"; const ALPHA_B64: &str = "1QeSHQR6Z4ePd1eHSIeHl4dYeJCVCIQ8WuEpd7M="; const OPAQUE_RGBA_HEX: &str = "404d71ff424f72ff475375ff4d5878ff545d7cff5b6480ff626984ff686e87ff6d7389ff71758bff72778bff72778bff\ @@ -405,7 +388,6 @@ mod tests { 000803ff010904ff020a04ff030b05ff"; fn b64(s: &str) -> Vec { - // tiny standalone base64 (test-only, avoids a dep) const T: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; let mut out = Vec::new(); let mut buf = 0u32; @@ -469,9 +451,6 @@ mod tests { #[test] fn rejects_truncated_hashes_at_every_length() { - // Every prefix must either parse AND decode in-bounds, or be cleanly - // rejected — the old java/swift ports crashed here. The transition must - // be a single boundary: rejected below it, accepted from it on. let hash = b64(ALPHA_B64); let mut first_ok = None; for len in 0..=hash.len() { @@ -490,7 +469,6 @@ mod tests { } } let first_ok = first_ok.expect("full hash must parse"); - // the header alone (alpha flag set, no AC data) can never be enough assert!(first_ok > 6, "accepted a bare header at {first_ok}"); } diff --git a/native/crates/immich_core_ffi/Cargo.toml b/native/crates/immich_core_ffi/Cargo.toml index 9604bc4640..d3f4cd6345 100644 --- a/native/crates/immich_core_ffi/Cargo.toml +++ b/native/crates/immich_core_ffi/Cargo.toml @@ -4,18 +4,12 @@ version.workspace = true edition.workspace = true license.workspace = true -# native_toolchain_rust requires cdylib (the bundled lib) + staticlib (iOS). It -# derives the artifact name from [package].name, so no [lib] name override here. -# "lib" additionally lets tests/ link the crate as a normal rust dependency. +# The build hook uses cdylib, iOS uses staticlib, and tests use lib. [lib] crate-type = ["lib", "cdylib", "staticlib"] -# features pinned explicitly so the cbindgen header + ffigen bindings always -# match the exported symbols regardless of default-feature drift. [dependencies] immich_core = { path = "../immich_core", default-features = false, features = ["image", "thumbhash"] } -# libc everywhere: returned buffers live on the libc heap so every consumer frees -# them with plain free (dart malloc.free / kotlin NativeBuffer.free / swift free). libc = { workspace = true } tokio = { workspace = true } diff --git a/native/crates/immich_core_ffi/build.rs b/native/crates/immich_core_ffi/build.rs index 80899f01c2..7faee5f358 100644 --- a/native/crates/immich_core_ffi/build.rs +++ b/native/crates/immich_core_ffi/build.rs @@ -1,15 +1,18 @@ use std::path::Path; fn main() { - println!("cargo:rerun-if-changed=src"); + // Directory entries in Cargo's dep-info make Flutter rerun the hook every build. + // Add new files here when they define exported items. + println!("cargo:rerun-if-changed=src/lib.rs"); + println!("cargo:rerun-if-changed=src/capi/mod.rs"); + println!("cargo:rerun-if-changed=src/capi/image.rs"); + println!("cargo:rerun-if-changed=src/capi/thumbhash.rs"); println!("cargo:rerun-if-changed=cbindgen.toml"); let crate_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); let out = Path::new(&crate_dir).join("include").join("immich_core.h"); std::fs::create_dir_all(out.parent().unwrap()).ok(); - // Hard-fail, not a warning: the header is committed and consumed by ffigen + - // Swift, so a silent codegen failure would leave a stale header in the tree. match cbindgen::generate(&crate_dir) { Ok(bindings) => { bindings.write_to_file(&out); diff --git a/native/crates/immich_core_ffi/cbindgen.toml b/native/crates/immich_core_ffi/cbindgen.toml index 4ceba6deb3..97a42a299c 100644 --- a/native/crates/immich_core_ffi/cbindgen.toml +++ b/native/crates/immich_core_ffi/cbindgen.toml @@ -1,3 +1,3 @@ language = "C" pragma_once = true -autogen_warning = "// Generated by cbindgen — do not edit." +autogen_warning = "// Generated by cbindgen. Do not edit." diff --git a/native/crates/immich_core_ffi/include/immich_core.h b/native/crates/immich_core_ffi/include/immich_core.h index 2e6fe184bc..7be3e21c64 100644 --- a/native/crates/immich_core_ffi/include/immich_core.h +++ b/native/crates/immich_core_ffi/include/immich_core.h @@ -1,6 +1,6 @@ #pragma once -// Generated by cbindgen — do not edit. +// Generated by cbindgen. Do not edit. #include #include @@ -8,8 +8,7 @@ #include /** - * Native core version as a NUL-terminated UTF-8 string. - * Free the result with [`immich_core_free_string`]. + * Returns the version as a C string. Free it with [`immich_core_free_string`]. */ char *immich_core_version(void); @@ -22,21 +21,15 @@ char *immich_core_version(void); void immich_core_free_string(char *ptr); /** - * Whether the EXIF `orientation` swaps width and height (the 90/270/transpose - * family) — callers use it to size and report the rotated output dims. + * Returns whether an EXIF orientation swaps width and height. */ bool immich_core_orientation_swaps_dims(int32_t orientation); /** - * Rotate an RGBA8888 image to the given EXIF `orientation`. `src` is `sh` rows of - * `src_stride` bytes; `dst` is the caller's densely-packed `dw*dh*4` output (dims - * swap for 90/270/transpose). Returns false (a safe no-op) on null pointers or - * inconsistent sizes so the caller can fall back. The platform side owns the - * bitmap lock + the dst allocation; this only fills dst. + * Rotates RGBA8888 buffers. Invalid input or a failed operation returns false; discard `dst`. * * # Safety - * `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, - * and the two ranges must not overlap. + * `src` and `dst` must be valid for their lengths, initialized, and must not overlap. */ bool immich_core_rotate_rgba8888(const uint8_t *src, uintptr_t src_len, @@ -48,15 +41,10 @@ bool immich_core_rotate_rgba8888(const uint8_t *src, uintptr_t dst_len); /** - * Convert an RGBA_1010102 image (`src`, `sh` rows of `src_stride` bytes) to - * RGBA8888 in the caller's densely-packed `w*h*4` `dst`, matching Skia's - * `Bitmap.copy(ARGB_8888)`. Returns false (a safe no-op) on null pointers or - * inconsistent sizes so the caller can fall back. The platform side owns the - * bitmap lock + the dst allocation; this only fills dst. + * Converts RGBA_1010102 to RGBA8888. Invalid input or a failed operation returns false; discard `dst`. * * # Safety - * `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, - * and the two ranges must not overlap. + * `src` and `dst` must be valid for their lengths, initialized, and must not overlap. */ bool immich_core_rgba1010102_to_rgba8888(const uint8_t *src, uintptr_t src_len, @@ -67,13 +55,10 @@ bool immich_core_rgba1010102_to_rgba8888(const uint8_t *src, uintptr_t dst_len); /** - * Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied - * by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns - * the buffer and releases it with `free`. Returns null on a malformed hash, - * leaving `out_info` untouched. + * Decodes a ThumbHash into a libc buffer and writes width, height, and row bytes to `out_info`. + * Free the buffer with `free`; malformed hashes return null. * * # Safety - * `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes - * of three u32 values. + * `hash` must be valid for `hash_len` bytes and `out_info` for three u32 writes. */ uint8_t *immich_core_thumbhash_decode(const uint8_t *hash, uintptr_t hash_len, uint32_t *out_info); diff --git a/native/crates/immich_core_ffi/rust-toolchain.toml b/native/crates/immich_core_ffi/rust-toolchain.toml index 201cf390b5..57a856c43d 100644 --- a/native/crates/immich_core_ffi/rust-toolchain.toml +++ b/native/crates/immich_core_ffi/rust-toolchain.toml @@ -1,28 +1,17 @@ -# The build hook (native_toolchain_rust) drives cargo via rustup and auto-installs -# this toolchain + targets. Pin a version (never bare stable/beta) for reproducible -# builds. Keep the channel in sync with mise.toml's rust pin. -# -# Full default target set from native_toolchain_rust: the hook validates the HOST -# triple too (any dart/flutter command builds a host code asset), so linux + windows -# must stay in for CI runners and contributor machines, not just the app targets. +# Keep this version in sync with native/mise.toml. [toolchain] channel = "1.92.0" targets = [ - # Android "armv7-linux-androideabi", "aarch64-linux-android", "x86_64-linux-android", - # iOS (device + simulator) "aarch64-apple-ios", "aarch64-apple-ios-sim", "x86_64-apple-ios", - # Windows "aarch64-pc-windows-msvc", "x86_64-pc-windows-msvc", - # Linux "aarch64-unknown-linux-gnu", "x86_64-unknown-linux-gnu", - # macOS "aarch64-apple-darwin", "x86_64-apple-darwin", ] diff --git a/native/crates/immich_core_ffi/src/android/buffer.rs b/native/crates/immich_core_ffi/src/android/buffer.rs index 175542cd77..edcdd8b0d7 100644 --- a/native/crates/immich_core_ffi/src/android/buffer.rs +++ b/native/crates/immich_core_ffi/src/android/buffer.rs @@ -1,16 +1,3 @@ -//! `NativeBuffer` — the libc-heap allocator Kotlin and Dart share. Ports of the -//! former native_buffer.c, preserving its exact contracts. -//! -//! `jint` sizes/offsets sign-extend through `as usize` exactly like C's -//! `int` → `size_t` conversion, so negative inputs stay huge-and-failing (malloc -//! returns NULL) rather than becoming new behavior. -//! -//! The env-using calls run inside `EnvUnowned::with_env`, which upgrades the -//! FFI-safe native-method env to a real `Env` and wraps the closure in -//! `catch_unwind` — so a JNI error or a panic maps to the sentinel (0/null) and -//! the caller falls back, never unwinding into the JVM. The pure allocator calls -//! don't touch the env and contain no panicking code, so they run directly. - use jni::objects::{JClass, JObject}; use jni::sys::{jint, jlong, jobject}; use jni::{EnvUnowned, Outcome}; @@ -21,8 +8,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_allocate<'local>( _class: JClass<'local>, size: jint, ) -> jlong { - // SAFETY: plain libc allocation; a negative size sign-extends huge and malloc - // returns NULL, which flows back to Kotlin as 0 — same as the C it replaces. + // SAFETY: malloc accepts the converted size and returns null on failure. unsafe { libc::malloc(size as usize) as jlong } } @@ -32,8 +18,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_free<'local>( _class: JClass<'local>, address: jlong, ) { - // SAFETY: `address` came from allocate/realloc above (libc heap), or is 0, - // which libc free accepts as a no-op. + // SAFETY: callers pass a libc allocation from this module, or null. unsafe { libc::free(address as usize as *mut libc::c_void) } } @@ -44,9 +29,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_realloc<'local>( address: jlong, size: jint, ) -> jlong { - // SAFETY: exact libc realloc semantics — NULL acts as malloc, OOM returns NULL - // without freeing the original. The grown pointer is later freed by Dart, so it - // must stay on the libc heap. + // SAFETY: callers pass a libc allocation from this module, or null. unsafe { libc::realloc(address as usize as *mut libc::c_void, size as usize) as jlong } } @@ -60,8 +43,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_wrap<'local>( crate::log::ensure_panic_hook(); let outcome = env .with_env(|env| -> jni::errors::Result { - // SAFETY: `address`/`capacity` describe a live allocation from allocate or - // realloc; the ByteBuffer only borrows it and Kotlin controls the lifetime. + // SAFETY: Kotlin keeps this allocation live while the buffer is used. let buffer = unsafe { env.new_direct_byte_buffer(address as usize as *mut u8, capacity as usize) }?; @@ -90,9 +72,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_createGlobalRef<'lo if obj.is_null() { return Ok(0); } - // The caller owns the reference for the process lifetime (it backs a - // never-released singleton), so hand out the raw ref and leak the wrapper - // via into_raw — dropping the Global would delete the ref under Kotlin. + // This reference backs a process-wide singleton. Ok(env.new_global_ref(&obj)?.into_raw() as jlong) }) .into_outcome(); diff --git a/native/crates/immich_core_ffi/src/android/image.rs b/native/crates/immich_core_ffi/src/android/image.rs index 31a1c1c44b..f0ef88de8a 100644 --- a/native/crates/immich_core_ffi/src/android/image.rs +++ b/native/crates/immich_core_ffi/src/android/image.rs @@ -1,7 +1,3 @@ -//! `NativeImage` — the pixel exports Kotlin calls: the bitmap ops ported from the -//! former native_image.c (lock the bitmap, run the shared `immich_core::image` -//! math into a fresh libc buffer, hand it back) and the thumbhash decode. - use std::panic::{catch_unwind, AssertUnwindSafe}; use jni::objects::{JByteArray, JClass, JIntArray, JObject}; @@ -14,15 +10,12 @@ use super::jnigraphics::{ ANDROID_BITMAP_RESULT_SUCCESS, }; -/// Locks the bitmap, runs `work` on its pixels into a fresh libc buffer of -/// `dst_len`, and hands the buffer to Kotlin via `out_info` {width, height, rowBytes}. -/// Returns 0 on any failure so the caller takes its existing Skia fallback. fn with_bitmap_into_buffer( env: &mut Env, bitmap: &JObject, out_info: &JIntArray, expected_format: i32, - out_dims: impl FnOnce(&AndroidBitmapInfo) -> (i32, i32), + out_dims: impl FnOnce(&AndroidBitmapInfo) -> (u32, u32), work: impl FnOnce(&AndroidBitmapInfo, &[u8], &mut [u8]) -> bool, ) -> jlong { let raw_env = env.get_raw(); @@ -44,10 +37,33 @@ fn with_bitmap_into_buffer( } let (dw, dh) = out_dims(&info); - let dst_len = info.width as usize * info.height as usize * 4; - // SAFETY: libc heap — this exact address is later freed via NativeBuffer.free - // (Kotlin) or malloc.free (Dart). - let dst = unsafe { libc::malloc(dst_len) } as *mut u8; + let Some(dst_len) = (info.width as usize) + .checked_mul(info.height as usize) + .and_then(|len| len.checked_mul(4)) + else { + return 0; + }; + let Some(src_len) = (info.stride as usize).checked_mul(info.height as usize) else { + return 0; + }; + let Ok(dw) = i32::try_from(dw) else { + return 0; + }; + let Ok(dh) = i32::try_from(dh) else { + return 0; + }; + let Some(row_bytes) = dw.checked_mul(4) else { + return 0; + }; + if dst_len == 0 + || src_len == 0 + || dst_len > isize::MAX as usize + || src_len > isize::MAX as usize + { + return 0; + } + // SAFETY: calloc returns dst_len zeroed bytes; callers free them through the libc heap. + let dst = unsafe { libc::calloc(dst_len, 1) } as *mut u8; if dst.is_null() { return 0; } @@ -58,18 +74,16 @@ fn with_bitmap_into_buffer( != ANDROID_BITMAP_RESULT_SUCCESS || src_pixels.is_null() { - // SAFETY: dst was just malloc'd above and never escaped. + // SAFETY: dst was just allocated above and never escaped. unsafe { libc::free(dst as *mut libc::c_void) }; return 0; } - let src_len = info.stride as usize * info.height as usize; - // AssertUnwindSafe: catching here keeps the unlock + free below on the panic - // path — otherwise an unwind would leak dst and leave the bitmap locked. + // Keep cleanup below reachable if the pixel operation panics. let ok = catch_unwind(AssertUnwindSafe(|| { // SAFETY: the locked bitmap is valid for `stride * height` bytes. let src = unsafe { std::slice::from_raw_parts(src_pixels as *const u8, src_len) }; - // SAFETY: dst was allocated with dst_len bytes above and never escaped. + // SAFETY: dst points to dst_len initialized bytes and has not escaped. let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, dst_len) }; work(&info, src, dst_slice) })) @@ -82,9 +96,8 @@ fn with_bitmap_into_buffer( return 0; } - let dims = [dw, dh, dw * 4]; + let dims = [dw, dh, row_bytes]; if out_info.set_region(env, 0, &dims).is_err() || env.exception_check() { - // Keep ownership here if Kotlin can never receive the address. // SAFETY: dst never escaped. unsafe { libc::free(dst as *mut libc::c_void) }; return 0; @@ -110,9 +123,9 @@ pub extern "system" fn Java_app_alextran_immich_NativeImage_rotate<'local>( ANDROID_BITMAP_FORMAT_RGBA_8888, |info| { if immich_core::image::swaps_dims(orientation) { - (info.height as i32, info.width as i32) + (info.height, info.width) } else { - (info.width as i32, info.height as i32) + (info.width, info.height) } }, |info, src, dst| { @@ -153,7 +166,7 @@ pub extern "system" fn Java_app_alextran_immich_NativeImage_convert1010102<'loca &bitmap, &out_info, ANDROID_BITMAP_FORMAT_RGBA_1010102, - |info| (info.width as i32, info.height as i32), + |info| (info.width, info.height), |info, src, dst| { immich_core::image::rgba1010102_to_rgba8888( src, diff --git a/native/crates/immich_core_ffi/src/android/jnigraphics.rs b/native/crates/immich_core_ffi/src/android/jnigraphics.rs index ddd3468642..7373c453d7 100644 --- a/native/crates/immich_core_ffi/src/android/jnigraphics.rs +++ b/native/crates/immich_core_ffi/src/android/jnigraphics.rs @@ -1,6 +1,3 @@ -//! The three stable-ABI bitmap calls from libjnigraphics.so (ships in every NDK -//! sysroot). Hand-declared instead of pulling the ndk crate for three functions. - use jni::sys::jobject; #[repr(C)] diff --git a/native/crates/immich_core_ffi/src/android/log.rs b/native/crates/immich_core_ffi/src/android/log.rs index 8f1ea59178..ef0bbc8fdc 100644 --- a/native/crates/immich_core_ffi/src/android/log.rs +++ b/native/crates/immich_core_ffi/src/android/log.rs @@ -1,6 +1,3 @@ -//! Logcat sink — `__android_log_write` from liblog (an NDK system library, like -//! jnigraphics). Errors show up as `E/immich_core` in logcat. - use std::ffi::CString; const ANDROID_LOG_ERROR: i32 = 6; diff --git a/native/crates/immich_core_ffi/src/android/mod.rs b/native/crates/immich_core_ffi/src/android/mod.rs index 4d0fd6e7e3..16bcd145e2 100644 --- a/native/crates/immich_core_ffi/src/android/mod.rs +++ b/native/crates/immich_core_ffi/src/android/mod.rs @@ -1,19 +1,4 @@ -//! Android JNI exports — the `NativeBuffer` and `NativeImage` Kotlin objects load -//! this library directly (`System.loadLibrary("immich_core_ffi")`), so the whole -//! native layer is this one Rust cdylib. Split by the Kotlin object each group -//! backs: [`buffer`] (NativeBuffer, the libc-heap allocator bridge) and [`image`] -//! (NativeImage, the bitmap pixel ops), with [`jnigraphics`] holding the -//! libjnigraphics declarations the image ops lock bitmaps through. -//! -//! Buffer memory MUST live on the libc heap: Dart frees these exact addresses via -//! `package:ffi` `malloc.free` (process-global libc `free`), and Kotlin grows them -//! with `realloc`. Rust's own allocator is never allowed to touch them. -//! -//! Panics never unwind into the JVM: the env-using methods run their body inside -//! `EnvUnowned::with_env`, which wraps it in `catch_unwind` and maps a panic (or a -//! JNI error) to the sentinel return; the pure allocator methods have no panicking -//! code at all. - +// Native buffers must stay on libc's heap: Dart frees them with malloc.free and Kotlin uses realloc. mod buffer; mod image; mod jnigraphics; diff --git a/native/crates/immich_core_ffi/src/capi/image.rs b/native/crates/immich_core_ffi/src/capi/image.rs index 597513147a..d2458aa78c 100644 --- a/native/crates/immich_core_ffi/src/capi/image.rs +++ b/native/crates/immich_core_ffi/src/capi/image.rs @@ -1,14 +1,7 @@ -//! C-ABI image ops. The platform side owns the bitmap lock + the `dst` allocation -//! and calls these to fill `dst` from `src`; both are caller-owned buffers. - -/// Shared boundary for the caller-owned-`dst` pixel ops: null-check both pointers, -/// view them as slices, and run `op` under catch_unwind — a panic mid-write only -/// leaves `dst` partially filled, and the `false` return tells the caller to -/// discard it. Nothing outside `src_len`/`dst_len` is touched. +/// Runs a pixel operation on caller-owned buffers. Invalid input or a failed operation returns false; discard `dst`. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, -/// and the two ranges must not overlap. +/// `src` and `dst` must be valid for their lengths, initialized, and must not overlap. pub(super) unsafe fn fill_dst( src: *const u8, src_len: usize, @@ -20,32 +13,24 @@ pub(super) unsafe fn fill_dst( if src.is_null() || dst.is_null() { return false; } - // SAFETY: caller guarantees `src` is valid for reads of `src_len` bytes (see # Safety). + // SAFETY: guaranteed by the caller. let src_slice = unsafe { std::slice::from_raw_parts(src, src_len) }; - // SAFETY: caller guarantees `dst` is valid for writes of `dst_len` bytes (see # Safety). + // SAFETY: guaranteed by the caller. let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, dst_len) }; - // AssertUnwindSafe: the closure writes through `&mut dst_slice`, which isn't - // UnwindSafe, but a partial write is discarded on the `false` path. std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| op(src_slice, dst_slice))) .unwrap_or(false) } -/// Whether the EXIF `orientation` swaps width and height (the 90/270/transpose -/// family) — callers use it to size and report the rotated output dims. +/// Returns whether an EXIF orientation swaps width and height. #[no_mangle] pub extern "C" fn immich_core_orientation_swaps_dims(orientation: i32) -> bool { super::guard(false, || immich_core::image::swaps_dims(orientation)) } -/// Rotate an RGBA8888 image to the given EXIF `orientation`. `src` is `sh` rows of -/// `src_stride` bytes; `dst` is the caller's densely-packed `dw*dh*4` output (dims -/// swap for 90/270/transpose). Returns false (a safe no-op) on null pointers or -/// inconsistent sizes so the caller can fall back. The platform side owns the -/// bitmap lock + the dst allocation; this only fills dst. +/// Rotates RGBA8888 buffers. Invalid input or a failed operation returns false; discard `dst`. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, -/// and the two ranges must not overlap. +/// `src` and `dst` must be valid for their lengths, initialized, and must not overlap. #[no_mangle] pub unsafe extern "C" fn immich_core_rotate_rgba8888( src: *const u8, @@ -57,7 +42,7 @@ pub unsafe extern "C" fn immich_core_rotate_rgba8888( dst: *mut u8, dst_len: usize, ) -> bool { - // SAFETY: pointers/lengths forwarded verbatim to fill_dst (see # Safety). + // SAFETY: guaranteed by this function's caller. unsafe { fill_dst(src, src_len, dst, dst_len, |s, d| { immich_core::image::rotate_rgba8888( @@ -72,15 +57,10 @@ pub unsafe extern "C" fn immich_core_rotate_rgba8888( } } -/// Convert an RGBA_1010102 image (`src`, `sh` rows of `src_stride` bytes) to -/// RGBA8888 in the caller's densely-packed `w*h*4` `dst`, matching Skia's -/// `Bitmap.copy(ARGB_8888)`. Returns false (a safe no-op) on null pointers or -/// inconsistent sizes so the caller can fall back. The platform side owns the -/// bitmap lock + the dst allocation; this only fills dst. +/// Converts RGBA_1010102 to RGBA8888. Invalid input or a failed operation returns false; discard `dst`. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, -/// and the two ranges must not overlap. +/// `src` and `dst` must be valid for their lengths, initialized, and must not overlap. #[no_mangle] pub unsafe extern "C" fn immich_core_rgba1010102_to_rgba8888( src: *const u8, @@ -91,7 +71,7 @@ pub unsafe extern "C" fn immich_core_rgba1010102_to_rgba8888( dst: *mut u8, dst_len: usize, ) -> bool { - // SAFETY: pointers/lengths forwarded verbatim to fill_dst (see # Safety). + // SAFETY: guaranteed by this function's caller. unsafe { fill_dst(src, src_len, dst, dst_len, |s, d| { immich_core::image::rgba1010102_to_rgba8888( diff --git a/native/crates/immich_core_ffi/src/capi/mod.rs b/native/crates/immich_core_ffi/src/capi/mod.rs index 10efe0f7d9..437b113fa0 100644 --- a/native/crates/immich_core_ffi/src/capi/mod.rs +++ b/native/crates/immich_core_ffi/src/capi/mod.rs @@ -1,16 +1,10 @@ -//! The portable C ABI — the `extern "C"` surface cbindgen turns into -//! `include/immich_core.h`, consumed by Dart (`@Native`) and Swift (the header). -//! [`image`] holds the pixel ops; this file holds the version/string lifecycle and -//! the shared panic guard. - pub mod image; pub mod thumbhash; use std::ffi::{c_char, CString}; use std::ptr; -/// Native core version as a NUL-terminated UTF-8 string. -/// Free the result with [`immich_core_free_string`]. +/// Returns the version as a C string. Free it with [`immich_core_free_string`]. #[no_mangle] pub extern "C" fn immich_core_version() -> *mut c_char { guard(ptr::null_mut(), || { @@ -28,16 +22,12 @@ pub unsafe extern "C" fn immich_core_free_string(ptr: *mut c_char) { return; } guard((), || { - // SAFETY: `ptr` came from this library's `CString::into_raw` (see # Safety). + // SAFETY: guaranteed by the caller. let s = unsafe { CString::from_raw(ptr) }; drop(s); }); } -/// Run `f` at the FFI boundary, turning a panic into `sentinel` rather than -/// unwinding across `extern "C"` into the host. Guards panics only — a bad `len` -/// or a double/foreign free is caller-contract UB that stays the caller's -/// `# Safety` obligation, not something this can catch. fn guard(sentinel: T, f: impl FnOnce() -> T + std::panic::UnwindSafe) -> T { crate::log::ensure_panic_hook(); std::panic::catch_unwind(f).unwrap_or(sentinel) @@ -60,16 +50,16 @@ mod tests { fn version_roundtrips_and_frees() { let p = immich_core_version(); assert!(!p.is_null()); - // SAFETY: `p` is a non-null NUL-terminated string from this library. + // SAFETY: p is a C string returned by this library. let s = unsafe { CStr::from_ptr(p) }.to_str().unwrap(); assert!(!s.is_empty()); - // SAFETY: `p` was returned by this library and is freed exactly once. + // SAFETY: p was returned by this library and is freed once. unsafe { immich_core_free_string(p) }; } #[test] fn free_null_is_noop() { - // SAFETY: free_string explicitly accepts null. + // SAFETY: null is allowed by the function contract. unsafe { immich_core_free_string(ptr::null_mut()) }; } } diff --git a/native/crates/immich_core_ffi/src/capi/thumbhash.rs b/native/crates/immich_core_ffi/src/capi/thumbhash.rs index d8ab9d0018..a4231f31b2 100644 --- a/native/crates/immich_core_ffi/src/capi/thumbhash.rs +++ b/native/crates/immich_core_ffi/src/capi/thumbhash.rs @@ -1,27 +1,15 @@ -//! C-ABI thumbhash decode. One call: parse, malloc, fill. Every consumer (dart, -//! swift, and kotlin via the JNI wrapper) wants an allocated RGBA buffer, so -//! unlike the image ops there is no caller-owned dst here — the buffer comes back -//! on the libc heap and the caller releases it with plain `free` (dart: -//! `malloc.free`, kotlin: `NativeBuffer.free`). - use std::panic::{catch_unwind, AssertUnwindSafe}; -/// Decodes `hash` into a fresh libc allocation. Shared by the C export below and -/// the android JNI wrapper. Returns the buffer with its (width, height), or None -/// on a malformed hash — never leaking the allocation on any failure path. pub(crate) fn decode_malloc(hash: &[u8]) -> Option<(*mut u8, u32, u32)> { let (w, h) = immich_core::thumbhash::dims(hash)?; let len = w as usize * h as usize * 4; - // SAFETY: libc heap — this exact address is later freed by the consumer via - // free()/malloc.free/NativeBuffer.free. - let dst = unsafe { libc::malloc(len) } as *mut u8; + // SAFETY: calloc returns len zeroed bytes; callers free them through the libc heap. + let dst = unsafe { libc::calloc(len, 1) } as *mut u8; if dst.is_null() { return None; } - // SAFETY: dst was allocated with len bytes above and never escaped. + // SAFETY: dst points to len initialized bytes and has not escaped. let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, len) }; - // AssertUnwindSafe: a panic mid-fill only leaves dst partially written, and - // dst is freed right here on that path. let ok = catch_unwind(AssertUnwindSafe(|| { immich_core::thumbhash::to_rgba(hash, dst_slice) })) @@ -34,14 +22,11 @@ pub(crate) fn decode_malloc(hash: &[u8]) -> Option<(*mut u8, u32, u32)> { Some((dst, w, h)) } -/// Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied -/// by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns -/// the buffer and releases it with `free`. Returns null on a malformed hash, -/// leaving `out_info` untouched. +/// Decodes a ThumbHash into a libc buffer and writes width, height, and row bytes to `out_info`. +/// Free the buffer with `free`; malformed hashes return null. /// /// # Safety -/// `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes -/// of three u32 values. +/// `hash` must be valid for `hash_len` bytes and `out_info` for three u32 writes. #[no_mangle] pub unsafe extern "C" fn immich_core_thumbhash_decode( hash: *const u8, @@ -52,11 +37,11 @@ pub unsafe extern "C" fn immich_core_thumbhash_decode( if hash.is_null() || out_info.is_null() { return std::ptr::null_mut(); } - // SAFETY: caller guarantees `hash` is valid for reads of `hash_len` bytes (see # Safety). + // SAFETY: guaranteed by the caller. let hash_slice = unsafe { std::slice::from_raw_parts(hash, hash_len) }; match catch_unwind(|| decode_malloc(hash_slice)) { Ok(Some((dst, w, h))) => { - // SAFETY: caller guarantees `out_info` is valid for three u32 writes (see # Safety). + // SAFETY: guaranteed by the caller. unsafe { *out_info = w; *out_info.add(1) = h; diff --git a/native/crates/immich_core_ffi/src/ios/log.rs b/native/crates/immich_core_ffi/src/ios/log.rs index 4a869b36d1..02721cec6e 100644 --- a/native/crates/immich_core_ffi/src/ios/log.rs +++ b/native/crates/immich_core_ffi/src/ios/log.rs @@ -1,7 +1,3 @@ -//! Unified-log sink — NSLog through hand-declared Foundation/CoreFoundation -//! externs (stable C ABI, no crate deps; same pattern as the android jnigraphics -//! declarations). Errors show up in Xcode's console and Console.app. - use std::ffi::{c_char, c_void, CString}; const K_CF_STRING_ENCODING_UTF8: u32 = 0x0800_0100; @@ -25,9 +21,7 @@ pub(crate) fn log_error(msg: &str) { let Ok(text) = CString::new(format!("immich_core: {}", msg.replace('\0', " "))) else { return; }; - // SAFETY: the format literal and message are live NUL-terminated strings; the - // "%@" format takes exactly one object argument, so no format injection from - // the message is possible. Both CFStrings are released after the call. + // SAFETY: both CFStrings stay live through NSLog and are released below. unsafe { let format = CFStringCreateWithCString(std::ptr::null(), c"%@".as_ptr(), K_CF_STRING_ENCODING_UTF8); diff --git a/native/crates/immich_core_ffi/src/ios/mod.rs b/native/crates/immich_core_ffi/src/ios/mod.rs index c6bcb31a2b..fc03ceac82 100644 --- a/native/crates/immich_core_ffi/src/ios/mod.rs +++ b/native/crates/immich_core_ffi/src/ios/mod.rs @@ -1,9 +1,3 @@ -//! iOS platform integration. Swift needs no binding layer here — it calls the -//! portable C ABI in [`crate::capi`] directly (via the cbindgen header and the -//! app's `NativeCore.swift` loader), which is why this module is small next to -//! [`crate::android`]: Kotlin's VM needs JNI shims, Swift does not. What lives -//! here is the code that must talk *to* the platform, like the unified-log sink. - mod log; pub(crate) use log::log_error; diff --git a/native/crates/immich_core_ffi/src/lib.rs b/native/crates/immich_core_ffi/src/lib.rs index 0a9f45e397..429777ae3f 100644 --- a/native/crates/immich_core_ffi/src/lib.rs +++ b/native/crates/immich_core_ffi/src/lib.rs @@ -1,12 +1,4 @@ -//! FFI wrapper around `immich_core` for the mobile app. One cdylib, two ABI -//! surfaces: [`capi`] is the portable `extern "C"` layer (Dart `@Native` + the -//! Swift header cbindgen emits into `include/immich_core.h`); [`android`] is the -//! JNI layer Kotlin loads via `System.loadLibrary` — [`ios`] stays small because -//! Swift calls [`capi`] directly. [`runtime`] is the shared tokio runtime for -//! async work; [`log`] makes boundary failures visible in logcat / Console.app. -//! -//! C strings returned here are heap-allocated; the caller frees them with -//! `immich_core_free_string`. +//! Mobile FFI bindings for `immich_core`. #![deny(clippy::unwrap_used, clippy::expect_used)] mod capi; @@ -21,9 +13,6 @@ mod android; #[cfg(target_os = "ios")] mod ios; -// Re-export the C-ABI surface at the crate root so Rust consumers (the c_abi -// integration test) reach it as `immich_core_ffi::immich_core_*`; the exported -// symbols themselves are name-based (`#[no_mangle]`), independent of this path. pub use capi::image::{ immich_core_orientation_swaps_dims, immich_core_rgba1010102_to_rgba8888, immich_core_rotate_rgba8888, diff --git a/native/crates/immich_core_ffi/src/log.rs b/native/crates/immich_core_ffi/src/log.rs index c4a33853e1..5f31ba6dde 100644 --- a/native/crates/immich_core_ffi/src/log.rs +++ b/native/crates/immich_core_ffi/src/log.rs @@ -1,9 +1,3 @@ -//! Failure visibility. Every FFI boundary converts a panic into a sentinel the -//! caller silently falls back on — right for the caller, but it would bury the -//! failure. [`ensure_panic_hook`] installs a process-wide hook (once) that writes -//! the panic message and location to the platform log first: logcat on Android, -//! the unified log (Console.app) on iOS, stderr elsewhere. - use std::sync::Once; #[cfg(target_os = "android")] diff --git a/native/crates/immich_core_ffi/src/runtime.rs b/native/crates/immich_core_ffi/src/runtime.rs index f2013f5cda..f1a83ec31f 100644 --- a/native/crates/immich_core_ffi/src/runtime.rs +++ b/native/crates/immich_core_ffi/src/runtime.rs @@ -1,8 +1,4 @@ -//! The shared tokio runtime for every async task the core ever runs. One static -//! multi-thread runtime, created lazily on first use and reused for the process -//! lifetime — FFI entry points must never build per-call or scoped runtimes. -//! When the first async capability gets an FFI surface, this can graduate to an -//! explicit init/shutdown lifecycle. +//! Shared Tokio runtime for FFI work. use std::sync::LazyLock; use tokio::runtime::{Builder, Runtime}; @@ -16,8 +12,7 @@ static RUNTIME: LazyLock = LazyLock::new(|| { .unwrap_or_else(|e| panic!("immich-core runtime failed to start: {e}")) }); -/// The process-wide runtime. Spawn long-lived work with `runtime().spawn(...)`; -/// FFI callers that must wait use `runtime().block_on(...)` off the UI thread. +/// Returns the process-wide runtime. pub fn runtime() -> &'static Runtime { &RUNTIME } @@ -36,7 +31,6 @@ mod tests { let first = runtime().block_on(async { 21 * 2 }); assert_eq!(first, 42); - // spawned work runs on the same shared runtime and can be awaited again let handle = runtime().spawn(async { tokio::time::sleep(std::time::Duration::from_millis(5)).await; 7 diff --git a/native/crates/immich_core_ffi/tests/c_abi.rs b/native/crates/immich_core_ffi/tests/c_abi.rs index 75d8b6e88e..4742579f79 100644 --- a/native/crates/immich_core_ffi/tests/c_abi.rs +++ b/native/crates/immich_core_ffi/tests/c_abi.rs @@ -1,9 +1,3 @@ -//! Exercises the extern "C" surface exactly as a foreign caller (dart/swift/kotlin) -//! would: raw pointers in, sentinel returns on bad input, caller-owned buffers. The -//! android JNI module is excluded — those functions need a live JVM and are covered -//! by mobile/integration_test/native_jni_test.dart on device. -// unsafe-without-SAFETY-comments allowed here: every call simulates a raw foreign -// caller, including deliberately invalid inputs the guards must reject. #![allow(clippy::unwrap_used, clippy::undocumented_unsafe_blocks)] use std::ffi::CStr; @@ -15,7 +9,6 @@ use immich_core_ffi::{ immich_core_version, }; -// "1QcSHQRnh493V4dIh4eXh1h4kJUI" decoded — the classic 23x32 opaque sample. const THUMBHASH: [u8; 21] = [ 0xd5, 0x07, 0x12, 0x1d, 0x04, 0x67, 0x87, 0x8f, 0x77, 0x57, 0x87, 0x48, 0x87, 0x87, 0x97, 0x87, 0x58, 0x78, 0x90, 0x95, 0x08, @@ -48,7 +41,6 @@ fn swaps_dims_matches_the_exif_family() { #[test] fn rotate_180_matches_expected_bytes() { - // 2x1: red, green -> 180 -> green, red let src: [u8; 8] = [255, 0, 0, 255, 0, 255, 0, 255]; let mut dst = [0u8; 8]; let ok = unsafe { @@ -60,7 +52,6 @@ fn rotate_180_matches_expected_bytes() { #[test] fn rotate_90_swaps_output_dims() { - // 2x1 -> 90 -> 1x2: first output row is the right-hand src pixel let src: [u8; 8] = [255, 0, 0, 255, 0, 255, 0, 255]; let mut dst = [0u8; 8]; let ok = unsafe { @@ -75,7 +66,6 @@ fn rotate_rejects_bad_input_without_touching_dst() { let src = [0u8; 16]; let mut dst = [0xAAu8; 16]; unsafe { - // null src / null dst assert!(!immich_core_rotate_rgba8888( ptr::null(), 16, @@ -96,7 +86,6 @@ fn rotate_rejects_bad_input_without_touching_dst() { ptr::null_mut(), 16 )); - // src_len shorter than stride*h, dst_len shorter than w*h*4 assert!(!immich_core_rotate_rgba8888( src.as_ptr(), 8, @@ -123,7 +112,7 @@ fn rotate_rejects_bad_input_without_touching_dst() { #[test] fn convert_matches_skia_ground_truth() { - // (1023 -> 255) and the round-vs-shift discriminators (179 -> 45, 111 -> 28). + // 179 and 111 distinguish rounded scaling from `>> 2`. let px = |r: u32, g: u32, b: u32, a: u32| -> [u8; 4] { ((r & 0x3FF) | ((g & 0x3FF) << 10) | ((b & 0x3FF) << 20) | ((a & 0x3) << 30)).to_le_bytes() }; @@ -195,7 +184,6 @@ fn thumbhash_decodes_into_a_malloc_buffer() { let len = (info[0] * info[1] * 4) as usize; let pixels = unsafe { std::slice::from_raw_parts(ptr, len) }; - // opaque hash: every pixel fully opaque, image not a single flat color assert!(pixels.chunks_exact(4).all(|px| px[3] == 255)); assert!(pixels.chunks_exact(4).any(|px| px[0] != pixels[0])); unsafe { libc::free(ptr as *mut libc::c_void) }; @@ -212,7 +200,6 @@ fn thumbhash_rejects_bad_input() { .is_null() ); } - // failed decodes never touched the out params assert_eq!(info, [7, 7, 7]); } diff --git a/native/crates/immich_core_napi/src/lib.rs b/native/crates/immich_core_napi/src/lib.rs index b006a28178..f5edad46fb 100644 --- a/native/crates/immich_core_napi/src/lib.rs +++ b/native/crates/immich_core_napi/src/lib.rs @@ -1,11 +1,7 @@ -//! napi-rs binding for immich_core (node server). -//! -//! Built as a cdylib loaded as a `.node` addon — the same shape as the server's -//! existing native deps (sharp, bcrypt). +//! Node binding for `immich_core`. use napi_derive::napi; -/// Native core version. JS: `core.coreVersion()`. #[napi] pub fn core_version() -> String { immich_core::core_version().to_owned() diff --git a/native/immich_native_core/.gitignore b/native/immich_native_core/.gitignore index b9d7f25b91..d72cd90995 100644 --- a/native/immich_native_core/.gitignore +++ b/native/immich_native_core/.gitignore @@ -1,4 +1,3 @@ -# Miscellaneous *.class *.log *.pyc @@ -12,19 +11,11 @@ .swiftpm/ migrate_working_dir/ -# IntelliJ related *.iml *.ipr *.iws .idea/ -# The .vscode folder contains launch configuration and tasks you configure in -# VS Code which you may wish to be included in version control, so this line -# is commented out by default. -#.vscode/ - -# Flutter/Dart/Pub related -# Libraries should not include pubspec.lock, per https://dart.dev/guides/libraries/private-files#pubspeclock. /pubspec.lock **/doc/api/ .dart_tool/ diff --git a/native/immich_native_core/README.md b/native/immich_native_core/README.md index d91a40284e..0d797aea05 100644 --- a/native/immich_native_core/README.md +++ b/native/immich_native_core/README.md @@ -1,43 +1,9 @@ # immich_native_core (Flutter package) -dart:ffi bindings to the `immich_native_core` Rust core. The native code is **built -from source on every app build** via a Dart build hook (Flutter native assets) — no -prebuilt binaries, no `DynamicLibrary`, no platform plugin glue. +Build hook and Dart FFI bindings for the Immich native core. -## Use it from immich/mobile +The hook builds `immich_core_ffi` from source and bundles it as a Flutter code +asset. Builders need `rustup`; the Rust version and targets are pinned in the +FFI crate's `rust-toolchain.toml`. -```yaml -# mobile/pubspec.yaml -dependencies: - immich_native_core: - path: ../native/immich_native_core -``` - -`dart pub get`. The dart surface is exactly the ffigen output of the C header — -the current capabilities (EXIF rotate, 10-bit convert, thumbhash decode) are called -by the platform decode pipelines, not by dart, so there are no hand-written dart -wrappers. Add one only when a dart feature actually consumes a function. - -No app-level Gradle/Podfile edits. `hook/build.dart` compiles the Rust crate and -Flutter bundles it as a code asset; the `@Native` bindings resolve against it. -**Requirement:** every machine that builds the app needs [rustup](https://rustup.rs) -— the hook auto-installs the pinned toolchain + targets from the crate's -`rust-toolchain.toml`. - -## Layout - -- `hook/build.dart` — builds `../crates/immich_core_ffi` via `native_toolchain_rust`. -- `lib/immich_native_core.dart` — barrel; re-exports the generated bindings. -- `lib/src/ffi/bindings.g.dart` — ffigen `@Native` output (committed; do not edit). -- `ffigen.yaml` — ffi-native mode; asset-id must match the hook's `assetName`. -- `test/` — host FFI roundtrip (`flutter test`); device runs via `mobile/integration_test`. - -## ⚠ iOS App Extensions - -Code assets are bundled into the app's **Runner** target. immich ships a Share -Extension and a Widget Extension — if the core is ever called from one of those, -verify the symbols resolve there (same family as the embed-into-Runner-only gotcha). -Not an issue while only the main app calls it. - -The Rust workspace, the codegen/build/test commands, and the "add a function" loop -live in [`../README.md`](../README.md). +Workspace commands are in [`../README.md`](../README.md). diff --git a/native/immich_native_core/analysis_options.yaml b/native/immich_native_core/analysis_options.yaml index a5744c1cfb..f9b303465f 100644 --- a/native/immich_native_core/analysis_options.yaml +++ b/native/immich_native_core/analysis_options.yaml @@ -1,4 +1 @@ include: package:flutter_lints/flutter.yaml - -# Additional information about this file can be found at -# https://dart.dev/guides/language/analysis-options diff --git a/native/immich_native_core/ffigen.yaml b/native/immich_native_core/ffigen.yaml index 868e67ab1f..8d3bfc6cfe 100644 --- a/native/immich_native_core/ffigen.yaml +++ b/native/immich_native_core/ffigen.yaml @@ -1,11 +1,8 @@ -# Regenerate: `mise run codegen` (cbindgen header -> ffigen @Native bindings). -# ffi-native mode emits top-level @Native externals + a library @DefaultAsset -# pointing at the code asset hook/build.dart produces — no DynamicLibrary loader. -# asset-id MUST equal the generated file's package URI (and the hook's assetName). +# Keep asset-id in sync with the build hook. name: ImmichNativeCoreBindings ffi-native: asset-id: 'package:immich_native_core/src/ffi/bindings.g.dart' -description: 'FFI bindings to immich_native_core — generated, do not edit.' +description: 'Generated FFI bindings to immich_native_core.' output: 'lib/src/ffi/bindings.g.dart' headers: entry-points: diff --git a/native/immich_native_core/hook/build.dart b/native/immich_native_core/hook/build.dart index 2824e17996..f1bcc8ffd7 100644 --- a/native/immich_native_core/hook/build.dart +++ b/native/immich_native_core/hook/build.dart @@ -4,33 +4,49 @@ import 'package:code_assets/code_assets.dart'; import 'package:hooks/hooks.dart'; import 'package:native_toolchain_rust/native_toolchain_rust.dart'; -// Builds crates/immich_core_ffi from source on every app build and bundles it as -// a code asset. assetName must match the ffigen output (its package URI is the -// @Native DefaultAsset id). The crate is a sibling, so point cratePath at it. +const _buildDependencies = [ + '../Cargo.toml', + '../Cargo.lock', + '../crates/immich_core/Cargo.toml', + '../crates/immich_core_ffi/Cargo.toml', + '../crates/immich_core_ffi/rust-toolchain.toml', + '../crates/immich_core_ffi/build.rs', + '../crates/immich_core_ffi/cbindgen.toml', +]; + void main(List args) async { await build(args, (input, output) async { + if (!input.config.buildCodeAssets) return; + output.dependencies.addAll( + _buildDependencies.map(input.packageRoot.resolve), + ); await _ensureRustTarget(input); + final rustFlags = _rustFlags(input); await RustBuilder( assetName: 'src/ffi/bindings.g.dart', cratePath: '../crates/immich_core_ffi', - // Android requires 16 KB-aligned load segments for Play (apps targeting - // Android 15+); the NDK linker still defaults to 4 KB, so force the page - // size for the .so. iOS/macOS/host use their own alignment and must not - // get this ELF-only flag. Set via env (not .cargo/config.toml) because the - // hook runs cargo from Flutter's cwd, where a crate-local config wouldn't - // be discovered. - extraCargoEnvironmentVariables: { - if (input.config.code.targetOS == OS.android) - 'RUSTFLAGS': '-C link-arg=-Wl,-z,max-page-size=16384', - }, + // Keep 16 KB alignment and override native_toolchain_rust's API 35 target. + extraCargoEnvironmentVariables: {'RUSTFLAGS': ?rustFlags}, ).run(input: input, output: output); }); } -// rustup only auto-installs the rust-toolchain.toml targets when that file drives -// toolchain selection; with RUSTUP_TOOLCHAIN exported (mise does on CI) the file is -// bypassed and cross targets never install. Add the build target explicitly — a -// fast no-op when it's already present. +String? _rustFlags(BuildInput input) { + final code = input.config.code; + if (code.targetOS != OS.android) return null; + final triple = switch (code.targetArchitecture) { + Architecture.arm => 'armv7a-linux-androideabi', + Architecture.arm64 => 'aarch64-linux-android', + Architecture.x64 => 'x86_64-linux-android', + _ => throw UnsupportedError( + 'Unsupported Android architecture: ${code.targetArchitecture}', + ), + }; + final api = code.android.targetNdkApi; + return '-C link-arg=-Wl,-z,max-page-size=16384 ' + '-C link-arg=--target=$triple$api'; +} + Future _ensureRustTarget(BuildInput input) async { final code = input.config.code; final triple = switch (code.targetOS) { @@ -51,6 +67,11 @@ Future _ensureRustTarget(BuildInput input) async { Architecture.x64 => 'x86_64-apple-darwin', _ => null, }, + OS.windows => switch (code.targetArchitecture) { + Architecture.arm64 => 'aarch64-pc-windows-msvc', + Architecture.x64 => 'x86_64-pc-windows-msvc', + _ => null, + }, OS.linux => switch (code.targetArchitecture) { Architecture.arm64 => 'aarch64-unknown-linux-gnu', Architecture.x64 => 'x86_64-unknown-linux-gnu', @@ -59,13 +80,27 @@ Future _ensureRustTarget(BuildInput input) async { _ => null, }; if (triple == null) return; - final crate = input.packageRoot - .resolve('../crates/immich_core_ffi/') - .toFilePath(); - // best effort — if rustup itself is broken the build below reports it properly - await Process.run('rustup', [ - 'target', - 'add', - triple, - ], workingDirectory: crate); + final crate = input.packageRoot.resolve('../crates/immich_core_ffi/'); + final toolchainFile = crate.resolve('rust-toolchain.toml'); + final source = await File.fromUri(toolchainFile).readAsString(); + final channel = RegExp( + r'^\s*channel\s*=\s*"([^"]+)"\s*$', + multiLine: true, + ).firstMatch(source)?.group(1); + if (channel == null) { + throw FormatException('Missing toolchain channel in $toolchainFile'); + } + final args = ['target', 'add', '--toolchain', channel, triple]; + final result = await Process.run( + 'rustup', + args, + workingDirectory: crate.toFilePath(), + ); + final out = '${result.stdout}'; + final err = '${result.stderr}'; + if (out.isNotEmpty) stdout.write(out); + if (err.isNotEmpty) stderr.write(err); + if (result.exitCode != 0) { + throw ProcessException('rustup', args, err, result.exitCode); + } } diff --git a/native/immich_native_core/lib/immich_native_core.dart b/native/immich_native_core/lib/immich_native_core.dart index 9fb5d098e2..cd1b65c98f 100644 --- a/native/immich_native_core/lib/immich_native_core.dart +++ b/native/immich_native_core/lib/immich_native_core.dart @@ -1,8 +1,3 @@ -/// dart:ffi bindings to the immich_native_core Rust core, built from source and -/// bundled via the Dart build hook. The dart surface is exactly the ffigen output -/// of the C header — the production callers of the current capabilities are the -/// platform decode pipelines (Kotlin/JNI, Swift), so there are no hand-written -/// dart wrappers; add one only when a dart feature actually consumes a function. library; export 'src/ffi/bindings.g.dart'; diff --git a/native/immich_native_core/lib/src/ffi/bindings.g.dart b/native/immich_native_core/lib/src/ffi/bindings.g.dart index 747b73665a..783d0d9bc1 100644 --- a/native/immich_native_core/lib/src/ffi/bindings.g.dart +++ b/native/immich_native_core/lib/src/ffi/bindings.g.dart @@ -7,8 +7,7 @@ library; import 'dart:ffi' as ffi; -/// Native core version as a NUL-terminated UTF-8 string. -/// Free the result with [`immich_core_free_string`]. +/// Returns the version as a C string. Free it with [`immich_core_free_string`]. @ffi.Native Function()>() external ffi.Pointer immich_core_version(); @@ -19,20 +18,14 @@ external ffi.Pointer immich_core_version(); @ffi.Native)>() external void immich_core_free_string(ffi.Pointer ptr); -/// Whether the EXIF `orientation` swaps width and height (the 90/270/transpose -/// family) — callers use it to size and report the rotated output dims. +/// Returns whether an EXIF orientation swaps width and height. @ffi.Native() external bool immich_core_orientation_swaps_dims(int orientation); -/// Rotate an RGBA8888 image to the given EXIF `orientation`. `src` is `sh` rows of -/// `src_stride` bytes; `dst` is the caller's densely-packed `dw*dh*4` output (dims -/// swap for 90/270/transpose). Returns false (a safe no-op) on null pointers or -/// inconsistent sizes so the caller can fall back. The platform side owns the -/// bitmap lock + the dst allocation; this only fills dst. +/// Rotates RGBA8888 buffers. Invalid input or a failed operation returns false; discard `dst`. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, -/// and the two ranges must not overlap. +/// `src` and `dst` must be valid for their lengths, initialized, and must not overlap. @ffi.Native< ffi.Bool Function( ffi.Pointer, @@ -56,15 +49,10 @@ external bool immich_core_rotate_rgba8888( int dst_len, ); -/// Convert an RGBA_1010102 image (`src`, `sh` rows of `src_stride` bytes) to -/// RGBA8888 in the caller's densely-packed `w*h*4` `dst`, matching Skia's -/// `Bitmap.copy(ARGB_8888)`. Returns false (a safe no-op) on null pointers or -/// inconsistent sizes so the caller can fall back. The platform side owns the -/// bitmap lock + the dst allocation; this only fills dst. +/// Converts RGBA_1010102 to RGBA8888. Invalid input or a failed operation returns false; discard `dst`. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, -/// and the two ranges must not overlap. +/// `src` and `dst` must be valid for their lengths, initialized, and must not overlap. @ffi.Native< ffi.Bool Function( ffi.Pointer, @@ -86,14 +74,11 @@ external bool immich_core_rgba1010102_to_rgba8888( int dst_len, ); -/// Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied -/// by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns -/// the buffer and releases it with `free`. Returns null on a malformed hash, -/// leaving `out_info` untouched. +/// Decodes a ThumbHash into a libc buffer and writes width, height, and row bytes to `out_info`. +/// Free the buffer with `free`; malformed hashes return null. /// /// # Safety -/// `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes -/// of three u32 values. +/// `hash` must be valid for `hash_len` bytes and `out_info` for three u32 writes. @ffi.Native< ffi.Pointer Function( ffi.Pointer, diff --git a/native/immich_native_core/pubspec.yaml b/native/immich_native_core/pubspec.yaml index dc41a30ec0..ad99e48c4b 100644 --- a/native/immich_native_core/pubspec.yaml +++ b/native/immich_native_core/pubspec.yaml @@ -1,26 +1,23 @@ name: immich_native_core -description: "dart:ffi bindings to the immich_native_core Rust core, built from source via Dart build hooks." +description: "Dart FFI bindings to the Immich native core." version: 0.1.0 homepage: https://github.com/immich-app/immich publish_to: none environment: sdk: '>=3.11.0 <4.0.0' - flutter: '>=3.3.0' + flutter: '>=3.38.0' -# Not a platform plugin: the native lib is built + bundled by hook/build.dart as a -# code asset (Flutter native assets), so there is no ffiPlugin / android / ios dir. dependencies: flutter: sdk: flutter - # build-hook deps — run at build time to compile the Rust crate (need rustup). code_assets: ^1.2.1 hooks: ^2.0.2 native_toolchain_rust: ^1.0.4 dev_dependencies: - ffi: ^2.2.0 # tests only — the lib surface is the generated bindings (dart:ffi) - ffigen: 20.1.1 # pinned exact — a caret bump can re-emit bindings + ffi: ^2.2.0 + ffigen: 20.1.1 # Keep generated bindings stable. flutter_test: sdk: flutter flutter_lints: ^6.0.0 diff --git a/native/immich_native_core/test/native_core_test.dart b/native/immich_native_core/test/native_core_test.dart index f94cb5ad9e..f8eaf6f286 100644 --- a/native/immich_native_core/test/native_core_test.dart +++ b/native/immich_native_core/test/native_core_test.dart @@ -1,6 +1,3 @@ -// Host FFI roundtrip — `flutter test` builds the hook for the host platform and -// resolves the @Native symbols, no device needed. Calls the generated bindings -// directly (the package's actual surface); device runs: mobile/integration_test. import 'dart:convert'; import 'dart:ffi'; import 'dart:typed_data'; @@ -21,10 +18,9 @@ Uint8List _px1010102(int r, int g, int b, int a) { typedef _ImageFn = bool Function(Pointer src, int dstLen, Pointer dst); -// malloc src+dst, run the native call, return the dst bytes (or null if it declined). Uint8List? _withBuffers(Uint8List src, int dstLen, _ImageFn call) { - final srcPtr = malloc(src.length); - final dstPtr = malloc(dstLen); + final srcPtr = calloc(src.length); + final dstPtr = calloc(dstLen); try { srcPtr.asTypedList(src.length).setAll(0, src); if (!call(srcPtr, dstLen, dstPtr)) { @@ -32,13 +28,13 @@ Uint8List? _withBuffers(Uint8List src, int dstLen, _ImageFn call) { } return Uint8List.fromList(dstPtr.asTypedList(dstLen)); } finally { - malloc.free(srcPtr); - malloc.free(dstPtr); + calloc.free(srcPtr); + calloc.free(dstPtr); } } void main() { - test('core loads: version roundtrips through the C string contract', () { + test('loads the core', () { final ptr = immich_core_version(); expect(ptr, isNot(equals(nullptr))); final version = ptr.cast().toDartString(); @@ -46,7 +42,7 @@ void main() { expect(version, isNotEmpty); }); - test('orientation swaps dims exactly for the 90/270/transpose family', () { + test('reports swapped orientations', () { for (final o in [5, 6, 7, 8]) { expect(immich_core_orientation_swaps_dims(o), isTrue, reason: 'o=$o'); } @@ -55,8 +51,7 @@ void main() { } }); - test('exif rotate: 180 reverses pixels, 90 swaps dims', () { - // 2x1 image: red, green (RGBA). + test('rotates RGBA pixels', () { final src = Uint8List.fromList([255, 0, 0, 255, 0, 255, 0, 255]); final r180 = _withBuffers( src, @@ -64,7 +59,7 @@ void main() { (s, len, d) => immich_core_rotate_rgba8888(s, src.length, 8, 2, 1, 3, d, len), ); - expect(r180, [0, 255, 0, 255, 255, 0, 0, 255]); // green, red + expect(r180, [0, 255, 0, 255, 255, 0, 0, 255]); final r90 = _withBuffers( src, @@ -72,7 +67,7 @@ void main() { (s, len, d) => immich_core_rotate_rgba8888(s, src.length, 8, 2, 1, 6, d, len), ); - expect(r90, isNotNull); // 90 -> 1x2, same byte count + expect(r90, isNotNull); }); test('rotate declines bad sizes instead of writing', () { @@ -86,8 +81,8 @@ void main() { expect(tooSmall, isNull); }); - test('10-bit convert matches the on-device Skia ground truth', () { - // 179->45 and 111->28 pin round(v*255/1023); a plain >>2 would give 44/27. + test('converts RGBA_1010102 pixels', () { + // 179 and 111 distinguish rounded scaling from `>> 2`. final src = Uint8List.fromList([ ..._px1010102(1023, 0, 0, 3), ..._px1010102(179, 111, 0, 3), @@ -114,7 +109,7 @@ void main() { expect(badStride, isNull); }); - test('thumbhash decodes via the core into a malloc buffer', () { + test('decodes a thumbhash', () { final hash = base64Decode('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final hashPtr = malloc(hash.length); final info = malloc(3); @@ -132,7 +127,6 @@ void main() { expect(pixels.toSet().length, greaterThan(2)); malloc.free(ptr); - // malformed hash: null return, info untouched expect(immich_core_thumbhash_decode(hashPtr, 4, info), equals(nullptr)); } finally { malloc.free(hashPtr); diff --git a/native/mise.toml b/native/mise.toml index 3b95d6e198..27ee33b04b 100644 --- a/native/mise.toml +++ b/native/mise.toml @@ -1,4 +1,5 @@ [tools] +"aqua:flutter/flutter" = "3.44.6" # keep in sync with ../mobile/mise.toml rust = "1.92.0" # keep in sync with rust-toolchain.toml (the build hook uses rustup) [tasks.build] @@ -17,11 +18,16 @@ run = "cargo fmt --all" description = "Clippy (warnings = errors)" run = "cargo clippy --workspace --all-targets -- -D warnings" -# Regen the committed cbindgen header + ffigen @Native bindings. [tasks."codegen:ffigen"] alias = "codegen" description = "Generate the C header (cbindgen) + Dart @Native bindings (ffigen)" sources = [ + "Cargo.toml", + "Cargo.lock", + "crates/immich_core/Cargo.toml", + "crates/immich_core_ffi/Cargo.toml", + "crates/immich_core_ffi/rust-toolchain.toml", + "crates/immich_core_ffi/build.rs", "crates/immich_core_ffi/src/**/*.rs", "crates/immich_core_ffi/cbindgen.toml", "immich_native_core/ffigen.yaml", @@ -31,12 +37,10 @@ outputs = [ "immich_native_core/lib/src/ffi/bindings.g.dart", ] run = [ - "cargo build -p immich_core_ffi", + "cargo build --locked -p immich_core_ffi", "cd immich_native_core && dart run ffigen --config ffigen.yaml && dart format lib/src/ffi/bindings.g.dart", ] -# Host FFI roundtrip through the real build hook — no device. Builds the Rust crate -# via rustup + resolves the @Native code asset. [tasks."test:flutter"] description = "Host FFI roundtrip via the build hook (flutter test)" dir = "immich_native_core" @@ -56,7 +60,7 @@ run = [ [tasks."smoke:dart"] description = "Host dart:ffi ABI roundtrip (raw DynamicLibrary on the cdylib)" depends = ["build:dart"] -run = "dart run smoke/dart_smoke.dart target/debug/libimmich_core_ffi.dylib" +run = "dart run smoke/dart_smoke.dart" [tasks."smoke:node"] description = "Host napi roundtrip" diff --git a/native/scripts/build-linux.sh b/native/scripts/build-linux.sh index 2df9df7915..bd4e59a2ec 100755 --- a/native/scripts/build-linux.sh +++ b/native/scripts/build-linux.sh @@ -1,10 +1,13 @@ #!/usr/bin/env bash -# Cross-build the napi addon for Linux server (x86_64 + aarch64) via zigbuild -# (no Docker) and stage as .node under dist/server//. -# In CI you'd build these natively per-arch instead; this is local convenience. +# Requires cargo-zigbuild and Zig on PATH. set -euo pipefail cd "$(dirname "$0")/.." +if ! command -v cargo-zigbuild >/dev/null; then + echo "cargo-zigbuild is required" >&2 + exit 1 +fi + CRATE=immich_core_napi for t in x86_64-unknown-linux-gnu aarch64-unknown-linux-gnu; do diff --git a/native/smoke/dart_smoke.dart b/native/smoke/dart_smoke.dart index 5519f2edb7..cbaeda91f3 100644 --- a/native/smoke/dart_smoke.dart +++ b/native/smoke/dart_smoke.dart @@ -1,9 +1,5 @@ -// Mobile-side roundtrip: open the dart:ffi cdylib and call into the shared core. -// Standalone script (no package:ffi dep) — reads the returned C string by hand. -// -// dart run smoke/dart_smoke.dart target/debug/libimmich_core_ffi.dylib - import 'dart:ffi'; +import 'dart:io'; typedef _VersionNative = Pointer Function(); typedef _FreeNative = Void Function(Pointer); @@ -18,11 +14,24 @@ String _readCString(Pointer p) { } void main(List args) { - final libPath = args.isNotEmpty ? args.first : 'target/debug/libimmich_core_ffi.dylib'; + final name = Platform.isMacOS + ? 'libimmich_core_ffi.dylib' + : Platform.isLinux + ? 'libimmich_core_ffi.so' + : Platform.isWindows + ? 'immich_core_ffi.dll' + : throw UnsupportedError('Unsupported host: ${Platform.operatingSystem}'); + final libPath = args.isNotEmpty + ? args.first + : File.fromUri(Platform.script.resolve('../target/debug/$name')).path; final lib = DynamicLibrary.open(libPath); - final version = lib.lookupFunction<_VersionNative, _VersionNative>('immich_core_version'); - final free = lib.lookupFunction<_FreeNative, _FreeDart>('immich_core_free_string'); + final version = lib.lookupFunction<_VersionNative, _VersionNative>( + 'immich_core_version', + ); + final free = lib.lookupFunction<_FreeNative, _FreeDart>( + 'immich_core_free_string', + ); final ptr = version(); print('DART core_version = ${_readCString(ptr)}'); diff --git a/native/smoke/node_smoke.mjs b/native/smoke/node_smoke.mjs index ba98e991ef..18a132dcde 100644 --- a/native/smoke/node_smoke.mjs +++ b/native/smoke/node_smoke.mjs @@ -1,4 +1,3 @@ -// Server-side roundtrip: load the napi addon and call into the shared core. import { createRequire } from 'node:module'; const require = createRequire(import.meta.url);