Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c9ac36a5 authored by Selim Cinek's avatar Selim Cinek
Browse files

Don't allow physics based animation to overshoot multiple times

The oscillations led to some odditites, so we're preventing any
secondary overshoots from happening.
This also fixes a few details around our usage of the animator
that could lead to having to theoretical issues.

Bug: 403344126
Bug: 393581344
Fixes: 402747925
Fixes: 402088478
Test: atest SystemUITests
Flag: com.android.systemui.physical_notification_movement
Change-Id: I808dd044e23671864fbadab22b851b9a72d2a4f5
parent 69ad55f3
Loading
Loading
Loading
Loading
+50 −10
Original line number Diff line number Diff line
@@ -24,14 +24,19 @@ import com.android.internal.dynamicanimation.animation.SpringForce
import com.android.systemui.res.R
import com.android.systemui.statusbar.notification.PhysicsPropertyAnimator.Companion.createDefaultSpring
import com.android.systemui.statusbar.notification.stack.AnimationProperties
import kotlin.math.sign

/**
 * A physically animatable property of a view.
 *
 * @param tag the view tag to safe this property in
 * @param property the property to animate.
 * @param avoidDoubleOvershoot should this property avoid double overshoot when animated
 */
data class PhysicsProperty(val tag: Int, val property: Property<View, Float>) {
data class PhysicsProperty
@JvmOverloads constructor(
    val tag: Int, val property: Property<View, Float>, val avoidDoubleOvershoot: Boolean = true
) {
    val offsetProperty =
        object : FloatProperty<View>(property.name) {
            override fun get(view: View): Float {
@@ -61,6 +66,8 @@ data class PropertyData(
    var offset: Float = 0f,
    var animator: SpringAnimation? = null,
    var delayRunnable: Runnable? = null,
    var startOffset: Float = 0f,
    var doubleOvershootAvoidingListener: DynamicAnimation.OnAnimationUpdateListener? = null
)

/**
@@ -140,30 +147,63 @@ private fun startAnimation(
    if (animator == null) {
        animator = SpringAnimation(view, animatableProperty.offsetProperty)
        propertyData.animator = animator
        animator.setSpring(createDefaultSpring())
        val listener = properties?.getAnimationEndListener(animatableProperty.property)
        if (listener != null) {
            animator.addEndListener(listener)
            // We always notify things as started even if we have a delay
            properties.getAnimationStartListener(animatableProperty.property)?.accept(animator)
        }
        // We always notify things as started even if we have a delay
        properties?.getAnimationStartListener(animatableProperty.property)?.accept(animator)
        // remove the tag when the animation is finished
        animator.addEndListener { _, _, _, _ -> propertyData.animator = null }
    }
        animator.addEndListener { _, _, _, _ ->
            propertyData.animator = null
            propertyData.doubleOvershootAvoidingListener = null
        }
    }
    if (animatableProperty.avoidDoubleOvershoot
        && propertyData.doubleOvershootAvoidingListener == null) {
        propertyData.doubleOvershootAvoidingListener =
            DynamicAnimation.OnAnimationUpdateListener { _, offset: Float, velocity: Float ->
                val isOscillatingBackwards = velocity.sign == propertyData.startOffset.sign
                val didAlreadyRemoveBounciness =
                    animator.spring.dampingRatio == SpringForce.DAMPING_RATIO_NO_BOUNCY
                val isOvershooting = offset.sign != propertyData.startOffset.sign
                if (isOvershooting && isOscillatingBackwards && !didAlreadyRemoveBounciness) {
                    // our offset is starting to decrease, let's remove all overshoot
                    animator.spring.setDampingRatio(SpringForce.DAMPING_RATIO_NO_BOUNCY)
                } else if (!isOvershooting
                    && (didAlreadyRemoveBounciness || isOscillatingBackwards)) {
                    // we already did overshoot, let's skip to the end to avoid oscillations.
                    // Usually we shouldn't hit this as setting the damping ratio avoid overshoots
                    // but it may still happen if we see jank
                    animator.skipToEnd();
                }
            }
        animator.addUpdateListener(propertyData.doubleOvershootAvoidingListener)
    } else if (!animatableProperty.avoidDoubleOvershoot
        && propertyData.doubleOvershootAvoidingListener != null) {
        animator.removeUpdateListener(propertyData.doubleOvershootAvoidingListener)
    }
    // reset a new spring as it may have been modified
    animator.setSpring(createDefaultSpring().setFinalPosition(0f))
    // TODO(b/393581344): look at custom spring
    endListener?.let { animator.addEndListener(it) }
    val newOffset = previousEndValue - newEndValue + propertyData.offset

    // Immedialely set the new offset that compensates for the immediate end value change
    propertyData.offset = newOffset
    property.set(view, newEndValue + newOffset)
    val startOffset = previousEndValue - newEndValue + propertyData.offset
    // Immediately set the new offset that compensates for the immediate end value change
    propertyData.offset = startOffset
    propertyData.startOffset = startOffset
    property.set(view, newEndValue + startOffset)

    // cancel previous starters still pending
    view.removeCallbacks(propertyData.delayRunnable)
    animator.setStartValue(newOffset)
    animator.setStartValue(startOffset)
    val startRunnable = Runnable {
        animator.animateToFinalPosition(0f)
        propertyData.delayRunnable = null
        // When setting a new spring on a running animation it doesn't properly set the finish
        // conditions and will never actually end them only calling start explicitly does that,
        // so let's start them again!
        animator.start()
    }
    if (properties != null && properties.delay > 0 && !animator.isRunning) {
        propertyData.delayRunnable = startRunnable
+12 −7
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ import android.view.View;
import androidx.annotation.NonNull;

import com.android.app.animation.Interpolators;
import com.android.internal.dynamicanimation.animation.DynamicAnimation;
import com.android.systemui.res.R;
import com.android.systemui.statusbar.notification.PhysicsProperty;
import com.android.systemui.statusbar.notification.PhysicsPropertyAnimator;
@@ -199,15 +200,19 @@ public class ExpandableViewState extends ViewState {
                if (animateHeight) {
                    expandableView.setActualHeightAnimating(true);
                }
                PhysicsPropertyAnimator.setProperty(child, HEIGHT_PROPERTY, this.height, properties,
                        animateHeight,
                        (animation, canceled, value, velocity) -> {
                DynamicAnimation.OnAnimationEndListener endListener = null;
                if (!ViewState.isAnimating(expandableView, HEIGHT_PROPERTY)) {
                    // only Add the end listener if we haven't already
                    endListener = (animation, canceled, value, velocity) -> {
                        expandableView.setActualHeightAnimating(false);
                            if (!canceled && child instanceof ExpandableNotificationRow) {
                                ((ExpandableNotificationRow) child).setGroupExpansionChanging(
                                        false /* isExpansionChanging */);
                        if (!canceled && child instanceof ExpandableNotificationRow row) {
                            row.setGroupExpansionChanging(false /* isExpansionChanging */);
                        }
                        });
                    };
                }
                PhysicsPropertyAnimator.setProperty(child, HEIGHT_PROPERTY, this.height, properties,
                        animateHeight,
                        endListener);
            } else {
                startHeightAnimationInterpolator(expandableView, properties);
            }
+1 −1
Original line number Diff line number Diff line
@@ -405,7 +405,7 @@ public class StackStateAnimator {
            public void onAnimationEnd(DynamicAnimation animation, boolean canceled, float value,
                    float velocity) {
                mAnimatorSet.remove(animation);
                if (mAnimatorSet.isEmpty() && !canceled) {
                if (mAnimatorSet.isEmpty()) {
                    onAnimationFinished();
                }
                mAnimationEndPool.push(this);
+20 −7
Original line number Diff line number Diff line
@@ -398,6 +398,14 @@ public class ViewState implements Dumpable {
        return childTag != null;
    }

    public static boolean isAnimating(View view, PhysicsProperty property) {
        Object childTag = getChildTag(view, property.getTag());
        if (childTag instanceof PropertyData propertyData) {
            return propertyData.getAnimator() != null;
        }
        return childTag != null;
    }

    /**
     * Start an animation to this viewstate
     *
@@ -680,13 +688,18 @@ public class ViewState implements Dumpable {
    private void startYTranslationAnimation(final View child, AnimationProperties properties) {
        if (mUsePhysicsForMovement) {
            // Y Translation does some extra calls when it ends, so lets add a listener
            DynamicAnimation.OnAnimationEndListener endListener =
            DynamicAnimation.OnAnimationEndListener endListener = null;
            if (!isAnimatingY(child)) {
                // Only add a listener if we're not animating yet
                endListener =
                        (animation, canceled, value, velocity) -> {
                            if (!canceled) {
                            HeadsUpUtil.setNeedsHeadsUpDisappearAnimationAfterClick(child, false);
                                HeadsUpUtil.setNeedsHeadsUpDisappearAnimationAfterClick(child,
                                        false);
                                onYTranslationAnimationFinished(child);
                            }
                        };
            }
            PhysicsPropertyAnimator.setProperty(child, Y_TRANSLATION, this.mYTranslation,
                    properties, properties.getAnimationFilter().animateY, endListener);
        } else {