[android] stack screen transition lag on 3.30.1

lovegaoshi commented 2 months ago

Description

Hello!

I'm using @react-navigation/native-stack's createNativeStackNavigator to create stacks of screens. Since I upgraded to react-native-screens to 3.30.1, I noticed a (more) noticeable lag of the previous screen when navigating to the next screen on android; downgrading to 3.29.0 "fixes" this lag. This is quite vague and I have no clue what could be the reason, thanks a lot in advance if anyone wouldnt mind taking a look at it!

Steps to reproduce

build https://github.com/lovegaoshi/azusa-player-mobile/releases/tag/dev-build-2024-04-10 uses 3.30.1. https://github.com/lovegaoshi/azusa-player-mobile/releases/tag/dev-build-2024-04-11 uses 3.29.0. the left hamburger menu -> settings -> navigate to any screen shows it.

Snack or a link to a repository

https://github.com/lovegaoshi/azusa-player-mobile/tree/ee9c096a7bf56b97a415f3b1e363096ed0730163

Screens version

3.30.1/3.29.0

React Native version

0.73.6

Platforms

Android

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Paper (Old Architecture)

Build type

Release mode

Device

Real device

Device model

Samsung S21 (android 14)

Acknowledgements

Yes

kkafar commented 2 months ago

Hi @lovegaoshi, do you think you could provide us with recordings of how this looks? I'm looking now for differences but cant spot one.

lovegaoshi commented 2 months ago

Thank you so much for taking a look @kkafar and sorry for not posting them earlier, here is 3.30.1 with the lag https://github.com/software-mansion/react-native-screens/assets/106490582/6e7926a9-e30f-4c8b-b6fb-bd3b16814d60

And 3.29.0 without lag https://github.com/software-mansion/react-native-screens/assets/106490582/eee9556d-a882-42af-b43f-f685072cad68

There is no code difference other than the dep downgrade; device is pixel7 but also shows on samsung s21

tboba commented 2 months ago

This looks like an issue with screen transitioning and draw ordering - we may need to check if this commit may cause this 😕

lovegaoshi commented 2 months ago

perhaps its android:duration="450" />?

tboba commented 2 months ago

Yeah, most likely that's because of the transition duration. However, what's even stranger, I can see that on both versions of screens this lag exists (it's shorter on 3.29.0, though). Can you also spot the same artifacts @lovegaoshi?

3.29.0:

https://github.com/software-mansion/react-native-screens/assets/23281839/a181278e-f24f-47bc-bb3f-7b5dc51fce77

3.29.0 (detailed version):

https://github.com/software-mansion/react-native-screens/assets/23281839/aabacd05-6605-4ae2-a637-4a890dd8a8d8

3.30.1:

https://github.com/software-mansion/react-native-screens/assets/23281839/bf58d1ee-e3d6-4929-9cfc-ff51f72d85f7

lovegaoshi commented 2 months ago

thank you so much @tboba for the thorough investigation, yes that's what I see as well; now with the pointed out commit it makes sense to me that both have "lags," 29.0 has a smaller "lag" with a 80ms duration and 30.1 is much more noticeable with a 450ms duration. I'll try to patch-package 3.30.1 with a minimum transition duration on my end.

tboba commented 2 months ago

Hi @lovegaoshi! Could you check if installing screens version from this branch: https://github.com/software-mansion/react-native-screens/pull/2110 fixes this issue?

lovegaoshi commented 2 months ago

hi @tboba thank you so much for the fix! I tried to patch android:duration into a smaller value but it didnt seem to make a difference on an emulator, yet to try with a real device. Also I tried to install from branch #2110 and I don't think I got it right, it's missing React:

do I just clone, yarn, yarn prepare and cp/link the cloned repo to my node_modules?

 ERROR  TypeError: Cannot read property 'useState' of null

This error is located at:
    in DelayedFreeze
    in InnerScreen
    in Screen (created by MaybeScreen)
    in MaybeScreen (created by Drawer)
    in RNSScreenContainer
    in ScreenContainer (created by MaybeScreenContainer)
    in MaybeScreenContainer (created by Drawer)
    in RCTView (created by View)
    in View (created by Drawer)
    in RCTView (created by View)
    in View (created by AnimatedComponent(View))
    in AnimatedComponent(View)
    in Unknown (created by Drawer)
    in RCTView (created by View)
    in View (created by AnimatedComponent(View))
    in AnimatedComponent(View)
    in Unknown (created by PanGestureHandler)
    in PanGestureHandler (created by Drawer)
    in Drawer (created by DrawerViewBase)
    in DrawerViewBase (created by DrawerView)
    in RNGestureHandlerRootView (created by GestureHandlerRootView)
    in GestureHandlerRootView (created by DrawerView)
    in RCTView (created by View)
    in View (created by SafeAreaProviderCompat)
    in SafeAreaProviderCompat (created by DrawerView)
    in DrawerView (created by DrawerNavigator)
    in PreventRemoveProvider (created by NavigationContent)
    in NavigationContent
    in Unknown (created by DrawerNavigator)
    in DrawerNavigator (created by AzusaPlayer)
    in RCTView (created by View)
    in View (created by AzusaPlayer)
    in EnsureSingleNavigator
    in BaseNavigationContainer
    in ThemeProvider
    in NavigationContainerInner (created by AzusaPlayer)
    in ThemeProvider (created by PaperProvider)
    in RCTView (created by View)
    in View (created by Portal.Host)
    in Portal.Host (created by PaperProvider)
    in RCTView (created by View)
    in View (created by SafeAreaInsetsContext)
    in SafeAreaProviderCompat (created by PaperProvider)
    in PaperProvider (created by AzusaPlayer)
    in AzusaPlayer (created by App)
    in RNCSafeAreaProvider (created by SafeAreaProvider)
    in SafeAreaProvider (created by App)
    in RCTView (created by View)
    in View (created by MainBackground)
    in RCTView (created by View)
    in View (created by ImageBackground)
    in ImageBackground (created by MainBackground)
    in MainBackground (created by App)
    in RNGestureHandlerRootView (created by GestureHandlerRootView)
    in GestureHandlerRootView (created by App)
    in App
    in RCTView (created by View)
    in View (created by AppContainer)
    in RCTView (created by View)
    in View (created by AppContainer)
    in AppContainer
    in azusa-player-mobile(RootComponent), js engine: hermes

tboba commented 2 months ago

@lovegaoshi hmm, I believe you need to do yarn prepare inside the node_modules/react-native-screens, since it's not automated now, because of yarn v4 😕

lovegaoshi commented 2 months ago

my sincere apologies, I dont seem to get this at all:S

If I clone, yarn, yarn prepare, then cp to node_modules, build succeeds but I got the above (React is null); if I clone, cp to node_modules, yarn, yarn prepare, bob build will not succeed on Failed to build definition files. (tsc fail, below) package.json doesn't include adroid/src in the files entry so I can't do git+ in package.json;

~/Documents/GitHub/azusa-player-mobile/node_modules$ git clone https://github.com/software-mansion/react-native-screens.git
Cloning into 'react-native-screens'...
remote: Enumerating objects: 20613, done.
remote: Counting objects: 100% (8418/8418), done.
remote: Compressing objects: 100% (2318/2318), done.
remote: Total 20613 (delta 6620), reused 7069 (delta 5676), pack-reused 12195
Receiving objects: 100% (20613/20613), 37.38 MiB | 7.88 MiB/s, done.
Resolving deltas: 100% (13085/13085), done.
user@localhost:~/Documents/GitHub/azusa-player-mobile/node_modules/react-native-screens$ git switch @tboba/restore-android-animations
Branch '@tboba/restore-android-animations' set up to track remote branch '@tboba/restore-android-animations' from 'origin'.
Switched to a new branch '@tboba/restore-android-animations'
user@localhost:~/Documents/GitHub/azusa-player-mobile/node_modules/react-native-screens$ git pull
Already up to date.
user@localhost:~/Documents/GitHub/azusa-player-mobile/node_modules/react-native-screens$ yarn; yarn prepare
➤ YN0000: · Yarn 4.1.1
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Post-resolution validation
➤ YN0002: │ react-native-screens@workspace:. doesn't provide @react-native-community/masked-view (p440d0), requested by @react-navigation/stack.
➤ YN0086: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code.
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0s 732ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 4s 260ms
➤ YN0000: · Done with warnings in 5s 384ms
ℹ Building target commonjs
ℹ Cleaning up previous build at lib/commonjs
ℹ Compiling 61 files in src with babel
Browserslist: caniuse-lite is outdated. Please run:
  npx update-browserslist-db@latest
  Why you should do it regularly: https://github.com/browserslist/update-db#readme
✔ Wrote files to lib/commonjs
ℹ Building target module
ℹ Cleaning up previous build at lib/module
ℹ Compiling 61 files in src with babel
✔ Wrote files to lib/module
ℹ Building target typescript
ℹ Cleaning up previous build at lib/typescript
ℹ Generating type definitions with tsc
src/components/FullWindowOverlay.tsx:15:13 - error TS2786: 'View' cannot be used as a JSX component.
  Its instance type 'View' is not a valid JSX element.

15     return <View {...props} />;
               ~~~~

efstathiosntonas commented 2 months ago

@lovegaoshi use patch-package and apply this patch, please note it's for react-native-screens@3.31.1

react-native-screens+3.31.1.patch

Click me

```diff diff --git a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt index b31e378..acfe9ad 100644 --- a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt +++ b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt @@ -278,4 +278,9 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) { enum class WindowTraits { ORIENTATION, COLOR, STYLE, TRANSLUCENT, HIDDEN, ANIMATED, NAVIGATION_BAR_COLOR, NAVIGATION_BAR_HIDDEN } + companion object { + fun isSystemAnimation(stackAnimation: StackAnimation): Boolean { + return stackAnimation === StackAnimation.DEFAULT || stackAnimation === StackAnimation.FADE || stackAnimation === StackAnimation.NONE + } + } } diff --git a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.kt b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.kt index 3915dd6..2fc1ac4 100644 --- a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.kt +++ b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenFragment.kt @@ -272,7 +272,7 @@ open class ScreenFragment : Fragment, ScreenFragmentWrapper { // onViewAnimationStart/End is triggered from View#onAnimationStart/End method of the fragment's root // view. We override an appropriate method of the StackFragment's // root view in order to achieve this. - if (isResumed) { + if (isResumed || screen.container?.topScreen === screen) { // Android dispatches the animation start event for the fragment that is being added first // however we want the one being dismissed first to match iOS. It also makes more sense from // a navigation point of view to have the disappear event first. diff --git a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt index 458465c..e12136e 100644 --- a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt +++ b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt @@ -4,6 +4,7 @@ import android.content.Context import android.graphics.Canvas import android.os.Build import android.view.View +import androidx.fragment.app.FragmentTransaction import com.facebook.react.bridge.ReactContext import com.facebook.react.uimanager.UIManagerHelper import com.swmansion.rnscreens.Screen.StackAnimation @@ -139,9 +140,9 @@ class ScreenStack(context: Context?) : ScreenContainer(context) { if (stackAnimation != null) { if (shouldUseOpenAnimation) { when (stackAnimation) { - StackAnimation.DEFAULT -> it.setCustomAnimations(R.anim.rns_default_enter_in, R.anim.rns_default_enter_out) - StackAnimation.NONE -> it.setCustomAnimations(R.anim.rns_no_animation_20, R.anim.rns_no_animation_20) - StackAnimation.FADE -> it.setCustomAnimations(R.anim.rns_fade_in, R.anim.rns_fade_out) + StackAnimation.DEFAULT -> it.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_OPEN) + StackAnimation.NONE -> it.setTransition(FragmentTransaction.TRANSIT_NONE) + StackAnimation.FADE -> it.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_FADE) StackAnimation.SLIDE_FROM_RIGHT -> it.setCustomAnimations(R.anim.rns_slide_in_from_right, R.anim.rns_slide_out_to_left) StackAnimation.SLIDE_FROM_LEFT -> it.setCustomAnimations(R.anim.rns_slide_in_from_left, R.anim.rns_slide_out_to_right) StackAnimation.SLIDE_FROM_BOTTOM -> it.setCustomAnimations( @@ -152,9 +153,9 @@ class ScreenStack(context: Context?) : ScreenContainer(context) { } } else { when (stackAnimation) { - StackAnimation.DEFAULT -> it.setCustomAnimations(R.anim.rns_default_exit_in, R.anim.rns_default_exit_out) - StackAnimation.NONE -> it.setCustomAnimations(R.anim.rns_no_animation_20, R.anim.rns_no_animation_20) - StackAnimation.FADE -> it.setCustomAnimations(R.anim.rns_fade_in, R.anim.rns_fade_out) + StackAnimation.DEFAULT -> it.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_CLOSE) + StackAnimation.NONE -> it.setTransition(FragmentTransaction.TRANSIT_NONE) + StackAnimation.FADE -> it.setTransition(FragmentTransaction.TRANSIT_FRAGMENT_FADE) StackAnimation.SLIDE_FROM_RIGHT -> it.setCustomAnimations(R.anim.rns_slide_in_from_left, R.anim.rns_slide_out_to_right) StackAnimation.SLIDE_FROM_LEFT -> it.setCustomAnimations(R.anim.rns_slide_in_from_right, R.anim.rns_slide_out_to_left) StackAnimation.SLIDE_FROM_BOTTOM -> it.setCustomAnimations( diff --git a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt index 48ff684..cdf2896 100644 --- a/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt +++ b/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt @@ -1,5 +1,9 @@ package com.swmansion.rnscreens +import android.animation.Animator +import android.animation.AnimatorInflater +import android.animation.AnimatorListenerAdapter +import android.animation.AnimatorSet import android.annotation.SuppressLint import android.content.Context import android.graphics.Color @@ -16,6 +20,9 @@ import android.view.animation.Transformation import android.widget.LinearLayout import androidx.appcompat.widget.Toolbar import androidx.coordinatorlayout.widget.CoordinatorLayout +import androidx.fragment.R +import androidx.fragment.app.FragmentTransaction +import com.facebook.react.bridge.UiThreadUtil import com.facebook.react.uimanager.PixelUtil import com.google.android.material.appbar.AppBarLayout import com.google.android.material.appbar.AppBarLayout.ScrollingViewBehavior @@ -86,6 +93,53 @@ class ScreenStackFragment : ScreenFragment, ScreenStackFragmentWrapper { notifyViewAppearTransitionEnd() } + // Similarly to ScreensCoordinatorLayout, this method listens only for the phases of default Android + // transitions (default/none/fade), since `ScreensCoordinatorLayout#startAnimation` is being + // called only for custom animations. + override fun onCreateAnimator(transit: Int, enter: Boolean, nextAnim: Int): Animator? { + val listener = object : AnimatorListenerAdapter() { + override fun onAnimationStart(animation: Animator) { + onViewAnimationStart() + super.onAnimationStart(animation) + } + + override fun onAnimationEnd(animation: Animator) { + onViewAnimationEnd() + super.onAnimationEnd(animation) + } + } + + // If there's custom animation set, use default onCreateAnimator implementation, as event + // handling will be handled by ScreensCoordinatorLayout. + if (!Screen.isSystemAnimation(screen.stackAnimation)) { + return super.onCreateAnimator(transit, enter, nextAnim) + } + + // When fragment is being removed or there's no transition selected, we simply + // return AnimatorSet without any animation. + if (isRemoving || transit == FragmentTransaction.TRANSIT_NONE) { + return AnimatorSet().apply { + addListener(listener) + } + } + + var selectedNextAnim = nextAnim + if (nextAnim == 0) { + selectedNextAnim = when (transit) { + FragmentTransaction.TRANSIT_FRAGMENT_OPEN -> if (enter) R.animator.fragment_open_enter else R.animator.fragment_open_exit + FragmentTransaction.TRANSIT_FRAGMENT_CLOSE -> if (enter) R.animator.fragment_close_enter else R.animator.fragment_close_exit + FragmentTransaction.TRANSIT_FRAGMENT_FADE -> if (enter) R.animator.fragment_fade_enter else R.animator.fragment_fade_exit + else -> 0 + } + } + + val animator = AnimatorInflater.loadAnimator(context, selectedNextAnim).apply { + addListener(listener) + } + + return animator + } + private fun notifyViewAppearTransitionEnd() { val screenStack = view?.parent if (screenStack is ScreenStack) { diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_in.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_in.xml deleted file mode 100644 index 4398c7e..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_in.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_out.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_out.xml deleted file mode 100644 index 84c9175..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_enter_out.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_in.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_in.xml deleted file mode 100644 index 6d6fa02..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_in.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_out.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_out.xml deleted file mode 100644 index b20a184..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_default_exit_out.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_in.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_in.xml deleted file mode 100644 index c78ea61..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_in.xml +++ /dev/null @@ -1,7 +0,0 @@ - - - diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_to_bottom.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_to_bottom.xml index 2334521..3b7e7a1 100644 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_to_bottom.xml +++ b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_fade_to_bottom.xml @@ -2,14 +2,14 @@ + android:shareInterpolator="false"> + android:interpolator="@android:interpolator/linear" + android:startOffset="100" + android:duration="150"/> diff --git a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_no_animation_20.xml b/node_modules/react-native-screens/android/src/main/res/base/anim/rns_no_animation_20.xml deleted file mode 100644 index 5cc0d23..0000000 --- a/node_modules/react-native-screens/android/src/main/res/base/anim/rns_no_animation_20.xml +++ /dev/null @@ -1,6 +0,0 @@ - - diff --git a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_in.xml b/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_in.xml deleted file mode 100644 index 1767203..0000000 --- a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_in.xml +++ /dev/null @@ -1,37 +0,0 @@ - - - - - - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_out.xml b/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_out.xml deleted file mode 100644 index e7dd72b..0000000 --- a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_enter_out.xml +++ /dev/null @@ -1,38 +0,0 @@ - - - - - - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_in.xml b/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_in.xml deleted file mode 100644 index 949ebb7..0000000 --- a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_in.xml +++ /dev/null @@ -1,38 +0,0 @@ - - - - - - - - - - diff --git a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_out.xml b/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_out.xml deleted file mode 100644 index ba4d84d..0000000 --- a/node_modules/react-native-screens/android/src/main/res/v33/anim-v33/rns_default_exit_out.xml +++ /dev/null @@ -1,38 +0,0 @@ - - - - - - - - - - ```

@tboba looks good to me, tested on Android simulators from version 8 up to 14

lovegaoshi commented 2 months ago

thanks a lot! works on my end, I see the bring forward/backward animation I think I saw in android 12 instead of the sliding one from macs

lovegaoshi commented 2 weeks ago

saw the 3.32.0 update but this issue still lingers :(

software-mansion / react-native-screens