Migrate ChromeAnimation to the CompositorAnimator |
|
Issue descriptionChromeAnimation API are duplicated in the CompositorAnimator. Proposed patch is migrating ChromeAnimation into CompositorAnimator: https://chromium-review.googlesource.com/c/chromium/src/+/1234239 Notes: 1. in the https://chromium-review.googlesource.com/c/chromium/src/+/1244799 there was discussion and info from mdjones@, that we don't want to create FloatProperty without getProperty - in this patch we create getProperty returning null (to avoid creating additional *Get code in methods) 2. name in the FloatProperty is optional (can be empty) - this patch follows this rule when we don't need to distinguish some FloatProperty from others (BTW, naming of FloatProperty is not consistent among files and dropped by mdjones@ https://chromium-review.googlesource.com/c/chromium/src/+/1244244 was also about it) and puts ClassName.PROPERTY_NAME in other cases 3. CompositorAnimator is adding various method over Animator and patch is adding addUpdateListenerWithPropertyName - it's part of ellegant solution for finding Animators with listeners created from known FloatProperty 4. there is much more to optimize, but after first notes to 1234239 there were excluded and will be maybe proposed later.
,
Sep 30
If point 3 is still valid I need clear understandable for me info, how should I implement it.
,
Oct 2
Hi, I know that you're busy, but I still try to understand, what mdjones@ is saying about point 3 and I don't understand, why we can't have information about changing some FloatProperty inside CompositorAnimator. Could you explain it please? If I will change name of method from "addUpdateListenerWithPropertyName", will be it better?
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b044398a3477e8594cefece70d2bdc8e69dd085 commit 2b044398a3477e8594cefece70d2bdc8e69dd085 Author: Marcin Wiacek <marcin@mwiacek.com> Date: Wed Dec 12 23:34:51 2018 Move default interpolators (migration to CompositorAnimator, part 1) This is part of changes which put as goal removing redundant ChromeAnimation API by moving everything into CompositorAnimator. Public doc with proposal: https://docs.google.com/document/d/1VL5ntE7vn267IFjE4Zke0GkhVVTHz1bEP672Apj_E2k/edit This concrete patch is moving default interpolators. BUG=890643 Change-Id: I162eb117a1989fe78ad400222922652520d5e292 Reviewed-on: https://chromium-review.googlesource.com/c/1359033 Reviewed-by: Changwan Ryu <changwan@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Commit-Queue: Marcin Wiącek <marcin@mwiacek.com> Cr-Commit-Position: refs/heads/master@{#616105} [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AnimatedProgressBar.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiDelegate.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/ui/BottomBarAnimations.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimator.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/ChromeAnimation.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/Stack.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/stack/StackAnimation.java [modify] https://crrev.com/2b044398a3477e8594cefece70d2bdc8e69dd085/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/layouts/ChromeAnimationTest.java
,
Jan 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a340744fd7fb7457c3a0b8516ecc8473082fe41e commit a340744fd7fb7457c3a0b8516ecc8473082fe41e Author: Marcin Wiacek <marcin@mwiacek.com> Date: Thu Jan 03 20:29:33 2019 Migrate StackLayoutBase (migration to CompositorAnimator, part 3) This is part of changes which put as goal removing redundant ChromeAnimation API by moving everything into CompositorAnimator. Public doc with proposal: https://docs.google.com/document/d/1VL5ntE7vn267IFjE4Zke0GkhVVTHz1bEP672Apj_E2k/edit This concrete patch is migrating StackLayoutBase. BUG=890643 Change-Id: I66668b9373c54fc9573de8d797990b6e291f2b54 Reviewed-on: https://chromium-review.googlesource.com/c/1376851 Reviewed-by: Changwan Ryu <changwan@chromium.org> Commit-Queue: Marcin Wiącek <marcin@mwiacek.com> Cr-Commit-Position: refs/heads/master@{#619739} [modify] https://crrev.com/a340744fd7fb7457c3a0b8516ecc8473082fe41e/chrome/android/java/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimator.java [modify] https://crrev.com/a340744fd7fb7457c3a0b8516ecc8473082fe41e/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java
,
Jan 8
Patch in #5 causes a bug where the animation bounces (see attached video). Repro: 1. Have normal tabs open. 2. Open incognito tab. 3. Enter tab switcher. 4. Close incognito tab and observe bouncing animation. I'm reverting the patch for now.
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34d390f23f630f8c5187e096c44b47656bcafde2 commit 34d390f23f630f8c5187e096c44b47656bcafde2 Author: Matthew Jones <mdjones@chromium.org> Date: Tue Jan 08 21:13:49 2019 Revert "Migrate StackLayoutBase (migration to CompositorAnimator, part 3)" This reverts commit a340744fd7fb7457c3a0b8516ecc8473082fe41e. Reason for revert: See https://bugs.chromium.org/p/chromium/issues/detail?id=890643#c6 Original change's description: > Migrate StackLayoutBase (migration to CompositorAnimator, part 3) > > This is part of changes which put as goal removing redundant ChromeAnimation API by moving everything into CompositorAnimator. > > Public doc with proposal: https://docs.google.com/document/d/1VL5ntE7vn267IFjE4Zke0GkhVVTHz1bEP672Apj_E2k/edit > > This concrete patch is migrating StackLayoutBase. > > BUG=890643 > > Change-Id: I66668b9373c54fc9573de8d797990b6e291f2b54 > Reviewed-on: https://chromium-review.googlesource.com/c/1376851 > Reviewed-by: Changwan Ryu <changwan@chromium.org> > Commit-Queue: Marcin Wiącek <marcin@mwiacek.com> > Cr-Commit-Position: refs/heads/master@{#619739} TBR=changwan@chromium.org,marcin@mwiacek.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 890643 Change-Id: I66bc88fc88e648180cb66df43404af0449dcdaf2 Reviewed-on: https://chromium-review.googlesource.com/c/1401384 Reviewed-by: Matthew Jones <mdjones@chromium.org> Commit-Queue: Matthew Jones <mdjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#620876} [modify] https://crrev.com/34d390f23f630f8c5187e096c44b47656bcafde2/chrome/android/java/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimator.java [modify] https://crrev.com/34d390f23f630f8c5187e096c44b47656bcafde2/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayoutBase.java |
|
►
Sign in to add a comment |
|
Comment 1 by mar...@mwiacek.com
, Sep 30