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.

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

[MaterialContainerTransform] Huge delay running transition on 1.3.0-alpha01 #1415

Closed
edenman opened this issue Jun 17, 2020 · 30 comments
Closed
Assignees
Labels

Comments

@edenman
Copy link

edenman commented Jun 17, 2020

Description: 1.3.0-alpha01 causes MaterialContainerTransform to take way longer to run, and skip way more frames

Tried to update from 1.2.0-alpha06 but perf is a disaster.

Source code:
Here's how I'm launching the transition:

  private fun startContainerTransform(from: View, to: View): Flow<Boolean> {
    val channel = Channel<Boolean>()
    val transform = MaterialContainerTransform().apply {
      startView = from
      endView = to
      setPathMotion(MaterialArcMotion())
      scrimColor = Color.TRANSPARENT
      duration = 300L
      addListener(object : Transition.TransitionListener {
        override fun onTransitionEnd(transition: Transition) {
          channel.offer(true)
          channel.close()
        }

        override fun onTransitionResume(transition: Transition) {
        }

        override fun onTransitionPause(transition: Transition) {
        }

        override fun onTransitionCancel(transition: Transition) {
          Timber.e("Transition cancelled?")
          channel.offer(false)
          channel.close()
        }

        override fun onTransitionStart(transition: Transition) {
        }
      })
    }
    TransitionManager.beginDelayedTransition(screenContainer, transform)
    return channel.receiveAsFlow()
  }

This shows up in logcat:

Skipped 249 frames!  The application may be doing too much work on its main thread.

Android API version: 29

Material Library version: 1.3.0-alpha01

Device: Emulator Pixel 3a on API 29

@edenman edenman added the bug label Jun 17, 2020
@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 17, 2020

Thanks for reporting the issue. I wonder if this is related to the elevation shadow support we added recently (1094b48). As a test, can you try setting isElevationShadowEnabled = false?

@edenman
Copy link
Author

edenman commented Jun 17, 2020

No luck. Still takes a crazy-long time. From small to large, 1.2s. From large back to small, 5.3s.

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 17, 2020

Can you create a GitHub repo with a minimal sample app that replicates the necessary parts of your setup and reproduces the issue when switching between 1.2.0-alpha06 and 1.3.0-alpha01? That way we can take a look and try to debug what is going on.

@edenman
Copy link
Author

edenman commented Jun 18, 2020

Just tried producing a minimal sample but it doesn't repro the issue. Maybe it's something about the complexity of the views I'm trying to animate between (they are very complex in my real app's case)? I dunno, but I've already spent too much time on this, I'm just gonna stay on the working version for now. The sample I was working on: https://github.com/edenman/kmpPlayground

FWIW: I added some logging to draw in my real app in the detail screen view. When using 1.2.0-alpha06, it calls draw <10 times when transitioning back from detail->home. In the 1.3.0-alpha01, it calls draw 881 times.

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 18, 2020

Hmm ok. Can you provide any more info about the complex views in your real app? If this is a real issue that could affect other developers it would be really helpful for us to figure it out while we're still in 1.2 beta, before we reach stable.

Also, as you may have noticed, another big recent change was that we switched our default transitions from using android.transition to androidx.transition. I'm guessing you tried updating your real app to use androidx.transition.TransitionManager like you did in your sample app (edenman/kmpPlayground@ffdc51c). As a test, in your real app where you can reproduce the problem, can you try keeping the framework android.transition.TransitionManager and any related imports, and switching the Material transition imports from com.google.android.material.transition to com.google.android.material.transition.platform?

@edenman
Copy link
Author

edenman commented Jun 18, 2020

  1. Nothing special afaik. Both screens have RecyclerViews with relatively complicated row views. I don't know enough about how transitions work to know what might be causing the "drawing 881 times" issue but if you can point to potential causes I can dig some more.
  2. There doesn't seem to be a com.google.android.material.transition.platform.MaterialContainerTransform

@edenman
Copy link
Author

edenman commented Jun 18, 2020

Got a tip from @ricknout to try setTarget(from). Sadly, this causes the animation to not run at all.

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 19, 2020

  1. Hmm ok.
  2. com.google.android.material.transition.platform.MaterialContainerTransform should be available in 1.2.0-beta01 and 1.3.0-alpha01.

@skydoves
Copy link

Hi, is the same issue still happening in 1.2.0-beta01?
It looks similar to this issule #1344

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 23, 2020

@edenman are you able to test with 1.2.0-beta01 and com.google.android.material.transition.platform.MaterialContainerTransform (vs com.google.android.material.transition.MaterialContainerTransform.

I'm curious to see if this is an issue with the Framework Transitions library vs the AndroidX Transitions library.

@dsn5ft dsn5ft self-assigned this Jun 23, 2020
@edenman
Copy link
Author

edenman commented Jun 23, 2020

1.2.0-beta01 + platform.* = no animations. Maybe there's a problem with the magical way the transition is kicked off?

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 23, 2020

Hmm that's strange because in 1.2.0-alpha06 our transitions were based on the Framework transition APIs, which seemed to be working for you before. When setting up 1.2.0-beta01 + platform.*, did you make sure that all of your related transition imports are using android.transition as opposed to androidx.transition?

@edenman
Copy link
Author

edenman commented Jun 23, 2020

yup
Screen Shot 2020-06-24 at 9 39 01 AM

@edenman
Copy link
Author

edenman commented Jun 23, 2020

fwiw I was seeing a bunch of bugs on the previous version too, sometimes the new view wouldn't fully show, leading to the old view still being partially visible. Not sure if that's relevant. Anyway I'm punting on container transform entirely and going back to good-old slide left/right for now.

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 25, 2020

Gotcha. Since we can't reproduce the problem in your sample app, another possibility is to use your actual app project and JitPack to bisect and find the MDC Android commit that causes the issue.

If you're interested in trying this, you can add the following to your allprojects repo config:

allprojects {
    repositories {
      ...
      maven { url 'https://jitpack.io' }
    }
}

And then instead of depending on com.google.android.material:material, you could add the following to start with the first commit in 1.2.0-beta:

implementation 'com.github.material-components:material-components-android:d029b94'

This commit should essentially be the same as using 1.2.0-alpha06, so you shouldn't see the draw 881 times problem. Then by using the list of commits included in 1.2.0-beta01, you could choose the mid-point commit, e.g., 3767f48, try that, and depending on whether the problem is there, choose earlier or later mid-points.

Obviously this will take some time so no worries if you are unable to do it, but I think it would be a definitive way to find the problematic commit, so I wanted to provide these instructions in case you are able to do it.

@AxelBlaz3
Copy link

AxelBlaz3 commented Jun 26, 2020

I can confirm that there are issues with the 1.3.0-alpha01. I was googling for 2 days on how to fix the lag and read articles on how to improve performance. However, I just downgraded from 1.3.0-alpha01 -> 1.2.0-alpha06 and container transform transitions seem fluid.

In 1.3.0-alpha01, transitions were janky for the first few times of navigating to detail screen and heading back. However, they became fluid later. This is not the case with 1.2.0-alpha06. Transitions seem fluid from very first use.

@AxelBlaz3
Copy link

AxelBlaz3 commented Jun 26, 2020

I've noticed another bug in 1.3.0-alpha06 just now with the following code:

enterTransition = MaterialContainerTransform().apply { startView = findViewById(...) endView = findViewById (...) }

Here the startView is an ExtendedFloatingActionButton and endView is MaterialCardView. I've noticed that the startView will be zoomed to the dimensions of the endView (Though the expected case is, startView will be invisible half way).

In my case, ExtendedFloatingActionButton will transition the whole screen during the transition.

@VincentJoshuaET
Copy link

I had this issue with FAB -> MaterialCardView. Setting shadows to off fixed it.

@AxelBlaz3
Copy link

I haven't tried that yet. Looks promising. For now, I'll stick with 1.2.0-alpha06

@VincentJoshuaET
Copy link

Spoke too soon, it is still sluggish.

@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 29, 2020

@AxelBlaz3 and @VincentJoshuaET, can you provide a minimal sample app that reproduces the lag using 1.3.0-alpha01 or 1.2.0-beta01?

@AxelBlaz3
Copy link

@dsn5ft I've created a sample repository as per your request. However, I can't reproduce the error precisely as it was on my personal project. But still, I can see a jank when I'm navigating from Home -> Detail screen for the first time (when using 1.3.0-alpha01). It's fluid when I use 1.2.0-alpha06 even for the first time transition from Home -> Detail

Also, note this, I've been using a low end device to perform such tests. Transitions run smoothly on a device with SDM 845 chipset. I have run these tests on a device having the following specs:

Qualcomm Snapdragon 430 MSM8937 Chipset with 3 GB RAM (Device - Redmi 3S Prime)

Here's the repo link: https://github.com/AxelBlaz3/Sample

AxelBlaz3 added a commit to nano-kernel-project/Nano-Updater that referenced this issue Jun 29, 2020
• Livedata (2.3.0-alpha04 -> 2.3.0-alpha05)
• Jetpack Navigation (2.3.0-rc01 -> 2.3.0)
• MDC (1.2.0-beta01 -> 1.2.0-alpha06)

Had to downgrade MDC due to some issues

Issue: material-components/material-components-android#1415
Signed-off-by: AxelBlaz3 <karthikgaddam4@gmail.com>
@dsn5ft
Copy link
Contributor

dsn5ft commented Jun 30, 2020

Thanks @AxelBlaz3! This should be very helpful.

@AxelBlaz3
Copy link

AxelBlaz3 commented Jun 30, 2020

@dsn5ft I couldn't reproduce the issue properly. I mean, there's no jank in the sample app. Maybe because the data is static and not from server? My personal project app really lags for first few times and then everything will be fluid. What could be the possible reason behind that?

Personal app uses the MVVM architecture with Repository pattern. Also, dagger 2 is used for DI. Views are similar to the sample app. But still, the sample app is fluid. However, I face lag in personal app only for first few times of navigation from Home -> Detail screen and then everything will be fluid.

AxelBlaz3 added a commit to nano-kernel-project/Nano-Updater that referenced this issue Jul 1, 2020
• Livedata (2.3.0-alpha04 -> 2.3.0-alpha05)
• Jetpack Navigation (2.3.0-rc01 -> 2.3.0)
• MDC (1.2.0-beta01 -> 1.2.0-alpha06)

Had to downgrade MDC due to some issues

Issue: material-components/material-components-android#1415
Signed-off-by: AxelBlaz3 <karthikgaddam4@gmail.com>
@melihaksoy
Copy link

For me it's also a bit slower in new version, but I noticed views are getting mixed up during transition with 1.3.0-alpha03. There's no problem in 1.2.0-alpha05. Here's side by side comparison:

Using 1.2.0-alpha05
Using 1.3.0-alpha03

@hunterstich
Copy link
Contributor

@melihaksoy What kind of transition are you performing? Fragment to Fragment? View to View?

@melihaksoy
Copy link

@hunterstich It's view to view transition

@hunterstich
Copy link
Contributor

Are you setting a target on your MaterialContainerTransform as mentioned in the docs? - https://github.com/material-components/material-components-android/blob/master/docs/theming/Motion.md#transition-between-views

@melihaksoy
Copy link

Sorry for late response, I didn't add addTarget(endView), I see it wasn't there when using 1.2.0 alpha 🤔 Will check tomorrow if it's fixing the issue 👍

@dsn5ft dsn5ft assigned hunterstich and unassigned dsn5ft Oct 6, 2021
@drchen
Copy link
Contributor

drchen commented Feb 18, 2022

I'll close this issue for now due to it's a bit old and we haven't got further reports regarding this. Please feel free to reopen it if this is still happening.

@drchen drchen closed this as completed Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants