material-components / material-components-android

Modular and customizable Material Design UI components for Android
Apache License 2.0
16.25k stars 3.05k forks source link

[MaterialArcMotion] MaterialArcMotion not working properly #1344

Closed skydoves closed 4 years ago

skydoves commented 4 years ago

[Description]

The MaterialArcMotion not working properly in 1.2.0-beta01. (After transitions to be based on androidx and add transitions platform subpackage)

[AS-IS]

When we transform View A into View B using MaterialArcMotion as a PathMotion and MaterialContainerTransform, it is delaying, transition incompletely, move weirdly.

[TO-BE]

It should be work properly. In the Alpha05/Alpha06 are working well. However, as we can see below in the beta01 the transitions working not properly.

Alpha-06

Beta-01

Screenshot Index Issue Description Device Information API
first delayed when returning the transformation. Pixel 2, Samsung Galaxy s10 23, 29
second Transformed incompletely. Pixel 2, Samsung Galaxy s7, Nexus 10, Samsung Galaxy s10 23, 27, 29, 29
third Returning weirdly when using with RecyclerView.Adapter Pixel 2, Samsung Galaxy s10 23, 29

[Source code repository]

Material-alpha06 branch Material-beta01 branch / migrated commit

[Android API version]

23, 29

[Gradle Build Tool Version]

3.6.3

[Material Library version]

1.2.0-beta01

dsn5ft commented 4 years ago

Thanks @skydoves, awesome repo btw 😊

I see that when you migrated to beta01, you updated to the com.google.android.material.transition.platform version of the transitions. Out of curiosity, does the same issue happen when using beta01 and the androidx version of our transitions? (com.google.android.material.transition)

As another test, we could try using the snapshot release from right before the commit (https://github.com/material-components/material-components-android/commit/cd36c2f5e77461b26f4723a765e11e6bf16a2a86) where we added the androidx vs platform support:

https://github.com/material-components/material-components-android/packages/81484?version=1.2.0-dev-20200426

And then to confirm that commit is the issue, we could use the snapshot release from after it:

https://github.com/material-components/material-components-android/packages/81484?version=1.2.0-dev-20200428

skydoves commented 4 years ago

Hi @dsn5ft! Thank you for your kind and detail answer 😊

Unfortunately, we can't fully migrate using com.google.android.material.transition package. (used androidx.transition) When transitioning an Activity, we need to set sharedElementEnterTransition and sharedElementReturnTransition on the window.

The setSharedElementEnterTransition and setSharedElementReturnTransition methods are receiving only android.transition as a parameter. (we can't set using androidx.transition)

Window.class

public void setSharedElementEnterTransition(Transition transition) {
    // stubs
  }

public void setSharedElementReturnTransition(Transition transition) {
    // stubs
  }

Thank you for your work!

hunterstich commented 4 years ago

Hey @skydoves,

Dan and I had a chance to look into a few of the issues you're experiencing with 1.2.0-beta01.

There are two things happening


[1] When you perform a View->View container transform, as you're doing when transforming a FAB into a toolbar or a FAB into a card, you have to set the container transform's target view to the endView using addTarget(endView). This prevents the container transform from being run on both views that are changing and eliminates the visual glitches you're seeing. Running your project, I was able to fix the problems by updating TransformationLayout with:

  private fun getTransform(mStartView: View, mEndView: View): MaterialContainerTransform {
    return MaterialContainerTransform().apply {
      startView = mStartView
      endView = mEndView
      addTarget(endView)
      ...

Not sure if this will work in all cases where you're using the layout, but hopefully it points you in the right direction : ).


[2] The duplicate animation running when you return to the list of cards is being caused by commit/57a8ebdcdd4e6. We're working on a fix for this and will keep you updated!

Let me know if the suggestions in [1] work for solving some of the issues you're seeing.

Hunter

skydoves commented 4 years ago

Hi, @hunterstich ! I appreciate your response :) I just checked your suggestion [1] us working well by adding addTarget(endView) fixed the problem. I always appreciate your team's works.

Best regards,

dsn5ft commented 4 years ago

@skydoves so I added back a slight additional background fade duration which seems to fix the extra return transition issue (https://github.com/material-components/material-components-android/commit/661c6a112712079e57a26a210b3bacc5967832b1). It's available in 1.2.0-rc01 which was released yesterday in case you want to try it out here.

We're tracking the potential container transform performance issues in https://github.com/material-components/material-components-android/issues/1415, and I'm wondering if that's related to the fact that addTarget is now required, because without it there may be multiple container transforms running. Anyway I'm closing this issue because I believe all of the problems have been addressed, but feel free to reopen or file separate more specific issues.

mykola-dev commented 4 years ago

i've observed same weird glitches and frames drops on 1.3.0 alpha02 addTarget(endView) fixed it. Guys, please add it to the docs. I spent whole saturday to make it work :)