Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(MM-62375): OOM on gif #8573

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+45 −8
Open

fix(MM-62375): OOM on gif #8573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12b3757
fix(MM-62375): OOM on gif
rahimrahman Feb 8, 2025
ea591bf
add APNG4Android into github action script to download
rahimrahman Feb 8, 2025
24cf5bd
change to mattermost/APNG4Android
rahimrahman Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/actions/prepare-node-deps/action.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,14 @@ runs:
cp "$COMPASS_ICONS" "assets/fonts/"
cp "$COMPASS_ICONS" "android/app/src/main/assets/fonts"
echo "::endgroup::"

- name: ci/clone-APNG4Android
shell: bash
env:
APNG4Android: "node_modules/APNG4Android"
run: |
echo "::group::clone-APNG4Android"
if [ ! -d "$APNG4Android" ]; then
git clone -b "fix/MM-62375-OOM-on-gif-with-build-updates" --single-branch --depth 1 https://github.com/mattermost/APNG4Android.git "$APNG4Android";
fi
echo "::endgroup::"
4 changes: 4 additions & 0 deletions android/settings.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ include ':reactnativenotifications'
project(':reactnativenotifications').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-notifications/lib/android/app')
include ':watermelondb-jsi'
project(':watermelondb-jsi').projectDir = new File(rootProject.projectDir, '../node_modules/@nozbe/watermelondb/native/android-jsi')
include ':frameanimation'
project(':frameanimation').projectDir = new File('../node_modules/APNG4Android/frameanimation')
include ':gif'
project(':gif').projectDir = new File('../node_modules/APNG4Android/gif')
Comment on lines +11 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to get this as project in expo-image/android/build.gradle, need to declare frameanimation and gif here.

includeBuild('../node_modules/@react-native/gradle-plugin')
apply from: new File(["node", "--print", "require.resolve('expo/package.json')"].execute(null, rootDir).text.trim(), "../scripts/autolinking.gradle")
useExpoModules()
4 changes: 2 additions & 2 deletions ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ PODS:
- ExpoModulesCore
- ExpoFileSystem (18.0.6):
- ExpoModulesCore
- ExpoImage (2.0.3):
- ExpoImage (2.0.4):
- ExpoModulesCore
- SDWebImage (~> 5.19.1)
- SDWebImageSVGCoder (~> 1.7.0)
Expand Down Expand Up @@ -2466,7 +2466,7 @@ SPEC CHECKSUMS:
ExpoCrypto: 483fc758365923f89ddaab4327c21cae9534841d
ExpoDevice: 449822f2c45660c1ce83283dd51c67fe5404996c
ExpoFileSystem: a38e1bb58d77f41717293a7b73ebd4014d8cb8dd
ExpoImage: 2c1f4adee95d033894c53346c721361736583728
ExpoImage: 18ec1a3e5cd96ee6162d988be3085e18113e143f
ExpoLinearGradient: 18148bd38f98fa0229c1f9df23393b32ab6d2d32
ExpoModulesCore: c9cb309323aee3c048099bb40ad0226e3fcf0c56
ExpoStoreReview: f38ed71ee04dfc02dde63a05f925f001cebba5d8
Expand Down
9 changes: 4 additions & 5 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"expo-application": "6.0.1",
"expo-crypto": "14.0.1",
"expo-device": "7.0.1",
"expo-image": "2.0.3",
"expo-image": "2.0.4",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to bump this? Just curious

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a mistake when reading the changelog. And looking at the changes, I thought the addition of try..catch was going to help, but it didn't. Looking deeper, I noticed that it was already implemented in 2.0.3, not a new change in 2.0.4.

Changelog for 2.0.4:

## 2.0.4 — 2025-01-10

_This version does not introduce any user-facing changes._

So it's pretty moot.

"expo-linear-gradient": "14.0.1",
"expo-store-review": "8.0.0",
"expo-video-thumbnails": "9.0.2",
Expand Down
18 changes: 18 additions & 0 deletions patches/expo-image+2.0.3.patch → patches/expo-image+2.0.4.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
diff --git a/node_modules/expo-image/android/build.gradle b/node_modules/expo-image/android/build.gradle
index a85fb5e..ac568c1 100644
--- a/node_modules/expo-image/android/build.gradle
+++ b/node_modules/expo-image/android/build.gradle
@@ -44,7 +44,12 @@ dependencies {
kapt "com.github.bumptech.glide:compiler:${GLIDE_VERSION}"
api 'com.caverock:androidsvg-aar:1.4'

- implementation "com.github.penfeizhou.android.animation:glide-plugin:3.0.2"
+ implementation project(':frameanimation')
+ implementation project(':gif')
+ implementation("com.github.penfeizhou.android.animation:glide-plugin:3.0.2") {
+ exclude group: 'com.github.penfeizhou.android.animation', module: 'frameanimation'
+ exclude group: 'com.github.penfeizhou.android.animation', module: 'gif'
Comment on lines +13 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without exclusions, it will cause duplicate symbols error. Since glide-plugin will have frameanimation (required module) in other modules (apng, webp, etc).

+ }
Comment on lines +10 to +15
Copy link
Contributor Author

@rahimrahman rahimrahman Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixes are in frameanimation & gif.

Now in this PR (that will not get merged).

implementation "com.github.bumptech.glide:avif-integration:${GLIDE_VERSION}"

api 'com.github.bumptech.glide:okhttp3-integration:4.11.0'
diff --git a/node_modules/expo-image/android/src/main/java/expo/modules/image/okhttp/ExpoImageOkHttpClientGlideModule.kt b/node_modules/expo-image/android/src/main/java/expo/modules/image/okhttp/ExpoImageOkHttpClientGlideModule.kt
index 071907c..edf10c1 100644
--- a/node_modules/expo-image/android/src/main/java/expo/modules/image/okhttp/ExpoImageOkHttpClientGlideModule.kt
Expand Down
5 changes: 5 additions & 0 deletions scripts/postinstall.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,8 @@ else
mkdir -p "android/app/src/main/res/raw/"
cp $SOUNDS/* "android/app/src/main/res/raw/"
fi

APNG4Android="node_modules/APNG4Android"
if [ ! -z "$APNG4Android" ]; then
git clone -b "fix/MM-62375-OOM-on-gif-with-build-updates" --single-branch --depth 1 https://github.com/mattermost/APNG4Android.git "$APNG4Android";
fi
Loading