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

Commit 58ab072d authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Fix NotificationStack size calculation

There was a lot of duplicated code to calculate:
1. How many notifications should fit while on the lockscreen
2. The height of the notification stack
The two things are now in NotificationstackSizeCalculator.kt, allowing also some Unit testing, and removing some responsibilities from NotificationPanelViewController.

There was a bug in the calculation of the notification stack bottom padding that resulted in less notifications being displayed in the lockscreen: the padding value set was from the bottom of the screen, but the notification stack bottom was a bit higher. By removing this value from the padding, more space can be used.

In computeMaxKeyguardNotifications, it seems there was an additional minimumPadding applied (from mClockPositionAlgorithm.getMinStackScrollerPadding()). However, the stack already has the padding at the top.

+ Added some debug lines, to make debugging of this easier in the future.
+ Renamed some setters adding "forDebug" suffix, as it was confusing to understand if they were doing anything useful. It turned out some properties are set to NotificationStackLayout only to draw debugging lines.

Bug: 222482219
Test: atest NotificationStackSizeCalculator && atest com.android.systemui.statusbar.notification && manual QA request
Change-Id: I7d972398ea39602259d6b5efa17672daf8da03c4
parent cde55d30
Loading
Loading
Loading
Loading
+34 −52
Original line number Diff line number Diff line
@@ -217,6 +217,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
    private HashSet<View> mFromMoreCardAdditions = new HashSet<>();
    private ArrayList<AnimationEvent> mAnimationEvents = new ArrayList<>();
    private ArrayList<View> mSwipedOutViews = new ArrayList<>();
    private NotificationStackSizeCalculator mNotificationStackSizeCalculator;
    private final StackStateAnimator mStateAnimator = new StackStateAnimator(this);
    private boolean mAnimationsEnabled;
    private boolean mChangePositionInProgress;
@@ -415,6 +416,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
    private NotificationShelf mShelf;
    private int mMaxDisplayedNotifications = -1;
    private float mKeyguardBottomPadding = -1;
    private float mKeyguardNotificationAvailableSpace = -1;
    @VisibleForTesting int mStatusBarHeight;
    private int mMinInteractionHeight;
    private final Rect mClipRect = new Rect();
@@ -757,7 +759,10 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
        } else {
            mDebugTextUsedYPositions.clear();
        }
        int y = mTopPadding;
        int y = 0;
        drawDebugInfo(canvas, y, Color.RED, /* label= */ "y = " + y);

        y = mTopPadding;
        drawDebugInfo(canvas, y, Color.RED, /* label= */ "mTopPadding = " + y);

        y = getLayoutHeight();
@@ -790,6 +795,10 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
        y = (int) mAmbientState.getStackY() + mIntrinsicContentHeight;
        drawDebugInfo(canvas, y, Color.YELLOW,
                /* label= */ "mAmbientState.getStackY() + mIntrinsicContentHeight = " + y);

        y = (int) (mAmbientState.getStackY() + mKeyguardNotificationAvailableSpace);
        drawDebugInfo(canvas, y, Color.RED, /* label= */
                "mAmbientState.getStackY() + mKeyguardNotificationAvailableSpace = " + y);
    }

    private void drawDebugInfo(Canvas canvas, int y, int color, String label) {
@@ -956,13 +965,15 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
    }

    private void reinitView() {
        initView(getContext(), mSwipeHelper);
        initView(getContext(), mSwipeHelper, mNotificationStackSizeCalculator);
    }

    @ShadeViewRefactor(RefactorComponent.SHADE_VIEW)
    void initView(Context context, NotificationSwipeHelper swipeHelper) {
    void initView(Context context, NotificationSwipeHelper swipeHelper,
            NotificationStackSizeCalculator notificationStackSizeCalculator) {
        mScroller = new OverScroller(getContext());
        mSwipeHelper = swipeHelper;
        mNotificationStackSizeCalculator = notificationStackSizeCalculator;

        setDescendantFocusability(FOCUS_AFTER_DESCENDANTS);
        setClipChildren(false);
@@ -2248,48 +2259,10 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
    @ShadeViewRefactor(RefactorComponent.STATE_RESOLVER)
    private void updateContentHeight() {
        final float scrimTopPadding = mAmbientState.isOnKeyguard() ? 0 : mMinimumPaddings;
        int height = (int) scrimTopPadding;
        float previousPaddingRequest = mPaddingBetweenElements;
        int numShownItems = 0;
        int numShownNotifs = 0;
        boolean finish = false;
        int maxDisplayedNotifications = mMaxDisplayedNotifications;
        ExpandableView previousView = null;

        for (int i = 0; i < getChildCount(); i++) {
            ExpandableView expandableView = (ExpandableView) getChildAt(i);
            boolean footerViewOnLockScreen = expandableView == mFooterView && onKeyguard();

            if (expandableView.getVisibility() != View.GONE
                    && !expandableView.hasNoContentHeight() && !footerViewOnLockScreen) {

                boolean limitReached = maxDisplayedNotifications != -1
                        && numShownNotifs >= maxDisplayedNotifications;
                final float viewHeight;
                if (limitReached) {
                    viewHeight = mShelf.getIntrinsicHeight();
                    finish = true;
                } else {
                    viewHeight = expandableView.getIntrinsicHeight();
                }
                if (height != 0) {
                    height += mPaddingBetweenElements;
                }
                float gapHeight = calculateGapHeight(previousView, expandableView, numShownNotifs);
                height += gapHeight;
                height += viewHeight;

                numShownItems++;
                if (viewHeight > 0 || !(expandableView instanceof MediaContainerView)) {
                    // Only count the media as a notification if it has a positive height.
                    numShownNotifs++;
                }
                previousView = expandableView;
                if (finish) {
                    break;
                }
            }
        }
        final int height =
                (int) scrimTopPadding + (int) mNotificationStackSizeCalculator.computeHeight(
                        /* notificationStackScrollLayout= */ this, mMaxDisplayedNotifications,
                        mShelf != null ? mShelf.getIntrinsicHeight() : 0);
        mIntrinsicContentHeight = height;

        // The topPadding can be bigger than the regular padding when qs is expanded, in that
@@ -4932,6 +4905,15 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
        mKeyguardBottomPadding = keyguardBottomPadding;
    }

    /**
     * For debugging only. Enables to draw a line related to the available size for notifications in
     * keyguard.
     */
    public void setKeyguardAvailableSpaceForDebug(float keyguardNotificationAvailableSpace) {
        mKeyguardNotificationAvailableSpace = keyguardNotificationAvailableSpace;
    }


    @ShadeViewRefactor(RefactorComponent.SHADE_VIEW)
    public void setShouldShowShelfOnly(boolean shouldShowShelfOnly) {
        mShouldShowShelfOnly = shouldShowShelfOnly;
+20 −3
Original line number Diff line number Diff line
@@ -181,6 +181,7 @@ public class NotificationStackScrollLayoutController {
    private final SectionHeaderController mSilentHeaderController;
    private final LockscreenShadeTransitionController mLockscreenShadeTransitionController;
    private final InteractionJankMonitor mJankMonitor;
    private final NotificationStackSizeCalculator mNotificationStackSizeCalculator;
    private final StackStateLogger mStackStateLogger;
    private final NotificationStackScrollLogger mLogger;

@@ -307,6 +308,7 @@ public class NotificationStackScrollLayoutController {
                R.dimen.lockscreen_shade_notification_movement);
        mTotalDistanceForFullShadeTransition = mResources.getDimensionPixelSize(
                R.dimen.lockscreen_shade_qs_transition_distance);
        mNotificationStackSizeCalculator.updateResources();
    }

    private final StatusBarStateController.StateListener mStateListener =
@@ -662,7 +664,8 @@ public class NotificationStackScrollLayoutController {
            ShadeController shadeController,
            InteractionJankMonitor jankMonitor,
            StackStateLogger stackLogger,
            NotificationStackScrollLogger logger) {
            NotificationStackScrollLogger logger,
            NotificationStackSizeCalculator notificationStackSizeCalculator) {
        mStackStateLogger = stackLogger;
        mLogger = logger;
        mAllowLongPress = allowLongPress;
@@ -688,6 +691,7 @@ public class NotificationStackScrollLayoutController {
        mCentralSurfaces = centralSurfaces;
        mScrimController = scrimController;
        mJankMonitor = jankMonitor;
        mNotificationStackSizeCalculator = notificationStackSizeCalculator;
        groupManager.registerGroupExpansionChangeListener(
                (changedRow, expanded) -> mView.onGroupExpandChanged(changedRow, expanded));
        legacyGroupManager.registerGroupChangeListener(new OnGroupChangeListener() {
@@ -758,7 +762,7 @@ public class NotificationStackScrollLayoutController {
            });
        }

        mView.initView(mView.getContext(), mSwipeHelper);
        mView.initView(mView.getContext(), mSwipeHelper, mNotificationStackSizeCalculator);
        mView.setKeyguardBypassEnabled(mKeyguardBypassController.getBypassEnabled());
        mKeyguardBypassController
                .registerOnBypassStateChangedListener(mView::setKeyguardBypassEnabled);
@@ -907,6 +911,13 @@ public class NotificationStackScrollLayoutController {
        return mView.getTop();
    }

    /**
     * @return the bottom of the view.
     */
    public int getBottom() {
        return mView.getBottom();
    }

    public float getTranslationX() {
        return mView.getTranslationX();
    }
@@ -1296,10 +1307,16 @@ public class NotificationStackScrollLayoutController {
     * appear on the keyguard.
     * Setting a negative number will disable rendering this line.
     */
    public void setKeyguardBottomPadding(float keyguardBottomPadding) {
    public void setKeyguardBottomPaddingForDebug(float keyguardBottomPadding) {
        mView.setKeyguardBottomPadding(keyguardBottomPadding);
    }

    /** For debugging only. */
    public void mKeyguardNotificationAvailableSpaceForDebug(
            float keyguardNotificationAvailableSpace) {
        mView.setKeyguardAvailableSpaceForDebug(keyguardNotificationAvailableSpace);
    }

    public RemoteInputController.Delegate createDelegate() {
        return new RemoteInputController.Delegate() {
            public void setRemoteInputActive(NotificationEntry entry,
+261 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.statusbar.notification.stack

import android.content.res.Resources
import android.util.Log
import android.view.View.GONE
import com.android.systemui.R
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Main
import com.android.systemui.statusbar.NotificationLockscreenUserManager
import com.android.systemui.statusbar.StatusBarState.KEYGUARD
import com.android.systemui.statusbar.SysuiStatusBarStateController
import com.android.systemui.statusbar.notification.collection.legacy.NotificationGroupManagerLegacy
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
import com.android.systemui.statusbar.notification.row.ExpandableView
import com.android.systemui.util.children
import javax.inject.Inject
import kotlin.math.max
import kotlin.properties.Delegates.notNull

private const val TAG = "NotificationStackSizeCalculator"
private const val DEBUG = false

/** Calculates number of notifications to display and the height of the notification stack. */
@SysUISingleton
class NotificationStackSizeCalculator
@Inject
constructor(
    private val groupManager: NotificationGroupManagerLegacy,
    private val lockscreenUserManager: NotificationLockscreenUserManager,
    private val statusBarStateController: SysuiStatusBarStateController,
    @Main private val resources: Resources
) {

    /**
     * Maximum # notifications to show on Keyguard; extras will be collapsed in an overflow shelf.
     * If there are exactly 1 + mMaxKeyguardNotifications, and they fit in the available space
     * (considering the overflow shelf is not displayed in this case), then all notifications are
     * shown.
     */
    private var maxKeyguardNotifications by notNull<Int>()

    /**
     * Minimum space between two notifications. There might be more space, see [calculateGapHeight].
     */
    private var notificationPadding by notNull<Int>()

    init {
        updateResources()
    }

    /**
     * Given the [availableSpace] constraint, calculates how many notification to show.
     *
     * This number is only valid in keyguard.
     *
     * @param availableSpace space for notifications. This doesn't include the space for the shelf.
     */
    fun computeMaxKeyguardNotifications(
        stack: NotificationStackScrollLayout,
        availableSpace: Float,
        shelfHeight: Float
    ): Int {
        log {
            "computeMaxKeyguardNotifications(" +
                "availableSpace=$availableSpace shelfHeight=$shelfHeight)"
        }

        val children: Sequence<ExpandableView> = stack.childrenSequence
        var remainingSpace: Float = availableSpace
        var count = 0
        var previous: ExpandableView? = null
        val onLockscreen = true
        val showableRows = children.filter { it.isShowable(onLockscreen) }
        val showableRowsCount = showableRows.count()
        showableRows.forEachIndexed { i, current ->
            val spaceNeeded = current.spaceNeeded(count, previous, stack, onLockscreen)
            previous = current
            log { "\ti=$i spaceNeeded=$spaceNeeded remainingSpace=$remainingSpace" }

            if (remainingSpace - spaceNeeded >= 0 && count < maxKeyguardNotifications) {
                count += 1
                remainingSpace -= spaceNeeded
            } else if (remainingSpace - spaceNeeded > -shelfHeight && i == showableRowsCount - 1) {
                log { "Showing all notifications. Shelf is not be needed." }
                // If this is the last one, and it fits using the space shelf would use, then we can
                // display it, as the shelf will not be needed (as all notifications are shown).
                return count + 1
            } else {
                log {
                    "No more fit. Returning $count. Space used: ${availableSpace - remainingSpace}"
                }
                return count
            }
        }
        log { "All fit. Returning $count" }
        return count
    }

    /**
     * Given the [maxNotifications] constraint, calculates the height of the
     * [NotificationStackScrollLayout]. This might or might not be in keyguard.
     *
     * @param stack stack containing notifications as children.
     * @param maxNotifications Maximum number of notifications. When reached, the others will go
     * into the shelf.
     * @param shelfHeight height of the shelf. It might be zero.
     *
     * @return height of the stack, including shelf height, if needed.
     */
    fun computeHeight(
        stack: NotificationStackScrollLayout,
        maxNotifications: Int,
        shelfHeight: Float
    ): Float {
        val children: Sequence<ExpandableView> = stack.childrenSequence
        val maxNotificationsArg = infiniteIfNegative(maxNotifications)
        var height = 0f
        var previous: ExpandableView? = null
        var count = 0
        val onLockscreen = onLockscreen()

        log { "computeHeight(maxNotification=$maxNotifications, shelf=$shelfHeight" }
        children.filter { it.isShowable(onLockscreen) }.forEach { current ->
            if (count < maxNotificationsArg) {
                val spaceNeeded = current.spaceNeeded(count, previous, stack, onLockscreen)
                log { "\ti=$count spaceNeeded=$spaceNeeded" }
                height += spaceNeeded
                count += 1
            } else {
                height += shelfHeight
                log { "returning height with shelf -> $height" }
                return height
            }
            previous = current
        }
        log { "Returning height without shelf -> $height" }
        return height
    }

    fun updateResources() {
        maxKeyguardNotifications =
            infiniteIfNegative(resources.getInteger(R.integer.keyguard_max_notification_count))

        notificationPadding =
            max(1, resources.getDimensionPixelSize(R.dimen.notification_divider_height))
    }

    private val NotificationStackScrollLayout.childrenSequence: Sequence<ExpandableView>
        get() = children.map { it as ExpandableView }

    private fun onLockscreen() = statusBarStateController.state == KEYGUARD

    private fun ExpandableView.spaceNeeded(
        visibleIndex: Int,
        previousView: ExpandableView?,
        stack: NotificationStackScrollLayout,
        onLockscreen: Boolean
    ): Float {
        assert(isShowable(onLockscreen))
        var size =
            if (onLockscreen) {
                getMinHeight(/* ignoreTemporaryStates= */ true).toFloat()
            } else {
                intrinsicHeight.toFloat()
            }
        if (visibleIndex != 0) {
            size += notificationPadding
        }
        size += calculateGapHeight(stack, previousView, visibleIndex)
        return size
    }

    private fun ExpandableView.isShowable(onLockscreen: Boolean): Boolean {
        if (visibility == GONE || hasNoContentHeight()) return false
        if (onLockscreen) {
            when (this) {
                is ExpandableNotificationRow -> {
                    if (isSummaryOfSuppressedGroup() || !canShowViewOnLockscreen() || isRemoved) {
                        return false
                    }
                }
                is MediaContainerView -> if (intrinsicHeight == 0) return false
                else -> return false
            }
        }
        return true
    }

    private fun ExpandableView.calculateGapHeight(
        stack: NotificationStackScrollLayout,
        previous: ExpandableView?,
        visibleIndex: Int
    ) = stack.calculateGapHeight(previous, /* current= */ this, visibleIndex)

    private fun ExpandableNotificationRow.isSummaryOfSuppressedGroup() =
        groupManager.isSummaryOfSuppressedGroup(entry.sbn)

    /**
     * Can a view be shown on the lockscreen when calculating the number of allowed notifications to
     * show?
     *
     * @return `true` if it can be shown.
     */
    private fun ExpandableView.canShowViewOnLockscreen(): Boolean {
        if (hasNoContentHeight()) {
            return false
        }
        if (this is ExpandableNotificationRow && !canShowRowOnLockscreen()) {
            return false
        } else if (visibility == GONE) {
            return false
        }
        return true
    }

    /**
     * Can a row be shown on the lockscreen when calculating the number of allowed notifications to
     * show?
     *
     * @return true if it can be shown
     */
    private fun ExpandableNotificationRow.canShowRowOnLockscreen(): Boolean {
        if (isSummaryOfSuppressedGroup()) {
            return false
        }
        if (!lockscreenUserManager.shouldShowOnKeyguard(entry)) {
            return false
        }
        return !isRemoved
    }

    private fun log(s: () -> String) {
        if (DEBUG) {
            Log.d(TAG, s())
        }
    }

    /** Returns infinite when [v] is negative. Useful in case a resource doesn't limit when -1. */
    private fun infiniteIfNegative(v: Int): Int =
        if (v < 0) {
            Int.MAX_VALUE
        } else {
            v
        }
}
+29 −123

File changed.

Preview size limit exceeded, changes collapsed.

+3 −1
Original line number Diff line number Diff line
@@ -134,6 +134,7 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase {
    @Mock private InteractionJankMonitor mJankMonitor;
    @Mock private StackStateLogger mStackLogger;
    @Mock private NotificationStackScrollLogger mLogger;
    @Mock private NotificationStackSizeCalculator mNotificationStackSizeCalculator;

    @Captor
    private ArgumentCaptor<StatusBarStateController.StateListener> mStateListenerArgumentCaptor;
@@ -186,7 +187,8 @@ public class NotificationStackScrollLayoutControllerTest extends SysuiTestCase {
                mShadeController,
                mJankMonitor,
                mStackLogger,
                mLogger
                mLogger,
                mNotificationStackSizeCalculator
        );

        when(mNotificationStackScrollLayout.isAttachedToWindow()).thenReturn(true);
Loading